slachiewicz opened a new issue, #1075:
URL: https://github.com/apache/maven-doxia/issues/1075

   ### Describe the bug
   
   `SinkEventAttributeSet.hashCode()` folds in the resolve parent:
   
   ```java
   public int hashCode() {
       final int parentHash = (resolveParent == null ? 0 : 
resolveParent.hashCode());
       return attribs.hashCode() + parentHash;
   }
   ```
   
   but `equals(Object)` delegates to `isEqual(AttributeSet)`, which compares 
only the attribute count and containment and never looks at the resolve parent. 
Two sets with the same own attributes but different resolve parents are 
therefore equal with different hash codes, which breaks the `equals`/`hashCode` 
contract and makes the class unsound as a key in any hash-based collection once 
`setResolveParent` has been used.
   
   Raised by a review bot on #1073 and left alone there on purpose, since that 
PR only moves the class and changing equality semantics inside a move would 
have hidden a behaviour change in a diff that otherwise has none.
   
   ### Impact
   
   Currently theoretical as far as I can see: `setResolveParent` is not called 
anywhere in Doxia itself, nor in the report plugins that use this class — only 
in the class's own unit test. So this is a latent trap rather than a live bug, 
which is also why it is worth fixing deliberately rather than in passing.
   
   ### Possible directions
   
   Whichever way it goes, the two methods need to agree:
   
   - make `hashCode()` ignore the resolve parent, matching the existing 
`equals`. Least disruptive, and arguably right given `AttributeSet.isEqual` is 
defined in terms of attributes; or
   - make `equals` consider the resolve parent, matching the existing 
`hashCode`. Stricter, but changes when two sets compare equal, so it is the 
riskier of the two.
   
   Note `equals` and `isEqual` are not required to mean the same thing — 
`isEqual` comes from the Swing `AttributeSet` contract — so redefining `equals` 
without touching `isEqual` is available if that is preferred.
   
   ### Affected version
   
   2.1.0, and every earlier 1.x/2.x release; the code is unchanged since the 
class was introduced.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to