Findbugs warning: Equals method should not assume anything about the type of its argument

Question

When running FindBugs on my project, I got a few instances of the error described above.

Namely, my overriding versions of equals cast the RHS object into the same type as the object in which the overriding version is defined.

However, I'm not sure whether a better design is possible, since AFAIK Java does not allow variance in method parameters, so it is not possible to define any other type for the equals parameter.

Am I doing something very wrong, or is FindBugs too eager?

A different way to phrase this question is: what is the correct behavior if the object passed to equals is not the same type as an LHS: Is this a false, or should there be an exception?

For example:

public boolean equals(Object rhs)
{
    MyType rhsMyType = (MyType)rhs; // Should throw exception
    if(this.field1().equals(rhsMyType.field1())... // Or whatever
}

Answer 1

Typically, when implementing equals you can check to see whether the class of the argument is equal (or compatible) to the implementing class before casting it. Something like this:

if (getClass() != obj.getClass())
    return false;
MyObj myObj = (MyObj) obj;

Doing it this way will prevent the FindBugs warning.

A side note to address a comment:
Some people argue to use instanceof instead of getClass to check type safety. There is a big debate on that, which I was trying not to get into when I noted that you can check for class equality or compatibility, but I guess I can't escape it. It boils down to this - if you use instanceof you can support equality between instances of a class and instances of its subclass, but you risk breaking the symmetric contract of equals. Generally I would recommend not to use instanceof unless you know you need it and you know what you are doing. For more information see:

• http://www.artima.com/weblogs/viewpost.jsp?thread=4744
• http://stackoverflow.com/questions/27581/overriding-equals-and-hashcode-in-java
• http://www.macchiato.com/columns/Durable5.html
• http://commons.apache.org/lang/api-release/org/apache/commons/lang/builder/EqualsBuilder.html (Apache common's implementation helper)
• http://www.eclipsezone.com/eclipse/forums/t92613.rhtml (Eclipse's default equals generator)
• NetBeans generator also uses getClass()

Answer 2

You're probably doing something like this:

public class Foo {
  // some code

  public void equals(Object o) {
    Foo other = (Foo) o;
    // the real equals code
  }
}

In this example you are assuming something about the argument of equals(): You are assuming it's of type Foo. This needs not be the case! You can also get a String (in which case you should almost definitely return false).

So your code should look like this:

public void equals(Object o) {
  if (!(o instanceof Foo)) {
    return false;
  }
  Foo other = (Foo) o;
  // the real equals code
}

(or use the more stringent getClass() != o.getClass() mentioned by Dave L.

You could also look at it this way:

Integer i = new Integer(42);
String s = "fourtytwo";
boolean b = i.equals(s);

Is there any reason that this code should throw a ClassCastException instead of finishing normally and setting b to false?

Throwing a ClassCastException as a response to .equals() wouldn't be sensible. Because even if it is a stupid question ("Of course a String is never equal to a Foo!") it's still a valid one with a perfectly fine answer ("no" == false).

Answer 3

I start my equals(Object) implementations like this:

if ((object == null) || !(object instaceof ThisClass)) {
    return false;
}

This will also prevent the FindBugs warning but will not automatically return false when a subclass of ThisClass is being handed in. It might also be considered equal, especially if its equals(Object) method hasn’t been overridden.

Answer 4

I'd recommend to ignore said findbugs warning. In practice, if equals is called with an object of an unexpected class, it is almost certainly a bug, and you want to fail fast on bugs.

For example, if you have an 'ArrayList files' and call files.contains("MyFile.txt"), it would be nice if you got a ClassCastException. Instead, Java just returns false, and it likely takes a long time until you discover that bug.