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

Check that equals() and hashCode() read the same fields #35

Closed
cushon opened this issue Oct 31, 2014 · 2 comments
Closed

Check that equals() and hashCode() read the same fields #35

cushon opened this issue Oct 31, 2014 · 2 comments

Comments

@cushon
Copy link
Collaborator

cushon commented Oct 31, 2014

Original issue created by cpovirk@google.com on 2012-08-06 at 08:48 PM


If I add a new field to a class, I can remember to update equals() but forget to update hashCode(). How would we detect this? In many cases, both methods should read all fields of a class, but that's probably too strong a check. (For example, a List.equals() implementation might read no fields directly, preferring to operation on the public size() and get() methods.) A weaker but probably still useful check is that the two methods read the same set of fields. False positives are still possible. For example, the check would flag a field containing a cached hash code, which would likely be read in hashCode() but not equals(). This particular example is avoidable by requiring the hashCode() looks at a subset of the fields that equals() looks at. I conjecture that it's more common for programmers to update equals() but forget to update hashCode() than vice versa, so this further weakened check would likely still catch most problems.

@cushon
Copy link
Collaborator Author

cushon commented Oct 31, 2014

Original comment posted by eaftan@google.com on 2013-02-11 at 08:16 PM


This will require us to do some interprocedural analysis, but it would be a good check.


Status: Accepted
Labels: -Priority-Medium, Priority-High

@cushon
Copy link
Collaborator Author

cushon commented Jan 23, 2015

This sounds worthwhile. Checking both methods in a class wouldn't be difficult. We may need to exempt classes that only implement one, unless #157 is implemented.

@ronshapiro ronshapiro mentioned this issue Sep 12, 2018
ronshapiro pushed a commit that referenced this issue Sep 12, 2018
This will have plenty of false negatives for classes which compare fields via complex getters, but should work fairly well for simple data classes.

Fixes #35

RELNOTES: [InconsistentHashCode] Add a check to flag inconsistent implementations of hashCode.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=211950941
@cushon cushon closed this as completed in 9da085c Sep 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant