Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CartesianSet.equals always returns false for other Set types #6780

Closed
Marcono1234 opened this issue Oct 14, 2023 · 2 comments
Closed

CartesianSet.equals always returns false for other Set types #6780

Marcono1234 opened this issue Oct 14, 2023 · 2 comments
Assignees

Comments

@Marcono1234
Copy link
Contributor

Version

32.1.3-jre

Description

The internal class CartesianSet returned by Sets.cartesianProduct implements equals in a way that it is never equal to any other Set type because it calls super.equals which is actually Object.equals (and therefore checks for object identity).
That does not seem intended.

Example

Object o1 = ImmutableSet.of(ImmutableList.of(1, 2));
Object o2 = Sets.cartesianProduct(ImmutableList.of(
    ImmutableSet.of(1), ImmutableSet.of(2)
));

System.out.println("o1.equals(o2): " + o1.equals(o2));
System.out.println("o2.equals(o1): " + o2.equals(o1));
@eamonnmcmanus
Copy link
Member

That definitely looks like a violation of the contract of Set.equals. It's been like that for 15 years, so probably it hasn't mattered in practice? Still, I think we could have fallback logic for this case. It could just check that the other set has the right size and that the CartesianSet contains all of the other set's elements.

As far as I can see, CartesianSet.hashCode() does follow the contract for Set.hashCode().

@eamonnmcmanus
Copy link
Member

Actually, not quite 15 years, but 11, since this commit changed the parent type from AbstractSet to ForwardingCollection.

@eamonnmcmanus eamonnmcmanus self-assigned this Oct 15, 2023
copybara-service bot pushed a commit to google/error-prone that referenced this issue Oct 16, 2023
… equivalent.

(inspired by Marcono1234's google/guava#6780)

PiperOrigin-RevId: 573814421
copybara-service bot pushed a commit to google/error-prone that referenced this issue Oct 16, 2023
… equivalent.

(inspired by Marcono1234's google/guava#6780)

PiperOrigin-RevId: 573931267
copybara-service bot pushed a commit that referenced this issue Oct 17, 2023
…r `Set`.

In 2012 the parent type of `CartesianSet` was changed from `AbstractSet` to `ForwardingCollection`, with the inadvertent effect that a `CartesianSet` can never compare equal to another `Set` unless that is also a `CartesianSet`.

Fixes #6780.

RELNOTES=n/a
PiperOrigin-RevId: 573517273
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants