Fix TypedKey equals/hashCode symmetry with Key#14025
Open
gunjanjaswal wants to merge 2 commits into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #13678.
TypedKeyImplis a record implementingKey, but it never overridesequals/hashCode, so Java generates them from both record components and only matches anotherTypedKeyImpl. Adventure'sKeyImpl.equalscompares anyKeyby namespace + value, so equality is one-directional:That breaks the symmetry clause of
Object.equals, and the two hash differently, so aTypedKeycan go missing in aHashSet<Key>built from plain keys, andcontains/removeturn order dependent when both types are mixed.This overrides
equals/hashCodeonTypedKeyImpl:TypedKey, compare underlying key and registry key (same as before, so keys from different registries stay distinct).Key, compare namespace + value to match adventure and stay symmetric.hashCodereturns the underlying key's hash so equal keys share a hash.One tradeoff: a
TypedKeynow equals its plainKey, so twoTypedKeys with the same key but different registries stay unequal while both equal that plain key, which can break transitivity in that mixed case. That is inherent toTypedKey extends Key(Effective Java item 10). This follows the approach suggested in the issue; if you'd rather have strict transitivity, I can make equality key-only instead.Changes
TypedKeyImpl: overrideequals/hashCode.TypedKeyEqualityTestcovering symmetry, hashCode consistency, and registry distinction.