- Status: open --> closed-rejected
The bug EQ_DOESNT_OVERRIDE_EQUALS is defined as:
This class extends a class that defines an equals method and adds fields, but doesn't define an equals method itself. Thus, equality on instances of this class will ignore the identity of the subclass and the added fields. Be sure this is what is intended, and that you don't need to override the equals method. Even if you don't need to override the equals method, consider overriding it anyway to document the fact that the equals method for the subclass just return the result of invoking super.equals(o).
In other words, if class A defines equals, and you declare B extends A, if B adds any fields, it should override equals. This is completely wrong. On the contrary, you should NOT override equals.
As perfectly explained in Effective Java's item 8, doing so is really harmful, since you are breaking the invariant of symmetry
Quoting the official documentation:
The equals method implements an equivalence relation on non-null object references:
It is symmetric: for any non-null reference values x and y, x.equals(y) should return true if and only if y.equals(x) returns true.
Source: Oracle JDK 8 Documentation
To be more precise, let's see this example (taken from Effective Java, 2nd edition itself):
public class Point {
private final int x;
private final int y;
public Point(int x, int y) {
this.x = x;
this.y = y;
}
@Override public boolean equals(Object o) {
if (!(o instanceof Point))
return false;
Point p = (Point)o;
return p.x == x && p.y == y;
}
...
// Remainder omitted
}
This is completely legal, valid, and good looking code.
Now, let's assume we extend it to add a color:
public class ColorPoint extends Point {
private final Color color;
public ColorPoint(int x, int y, Color color) {
super(x, y);
this.color = color;
}
...
// Remainder omitted
}
This class would get reported by Findbugs for violating EQ_DOESNT_OVERRIDE_EQUALS
Yet doing:
// Broken - violates symmetry!
@Override public boolean equals(Object o) {
if (!(o instanceof ColorPoint))
return false;
return super.equals(o) && ((ColorPoint) o).color == color;
}
Completely breaks symmetry, since:
new Point(2, 3).equals(new ColorPoint(2, 3, Color.BLACK)); // true!
new ColorPoint(2, 3, Color.BLACK).equals(new Point(2, 3)); // false!
On the contrary of what Findbugs reports, you would NEVER want to override equals if it's been defined outside of Object. The current reporting and recommendation is invalid and dangerous.
That's exactly what we did in our own Findbugs plugin at Monits.