Erroneous description for java:S2159

The description for java:S2159 (ver. 8.6) states: Comparisons of dissimilar types will always return false. (This is triggered by calling the .equals() method.)

This isn’t true in general, since one can always write an equals() method that takes a type other than its class. At most, it’s true for the libraries, but I don’t know them all well enough to say definitely.

So probably this needs to be reworded.

Hello @MisterPi

I understand the concern, however, I’m unsure we should add ambiguity in the description. I think it is fine to implicitly assume that it is “how it is expected to work”. The issue will highlight a suspicious code, the user will be the best to assess if this is a legitimate usage or not.

Do you have in mind a possible phrasing for the description that would not over-complexify it?

I guess it depends on exactly what the rule checker does. Does it check if the class actually has an equals() method defined for dissimilar classes?

Also, does this rule only apply to the use of the .equals() method? Because the description doesn’t explicitly say that. But the list of violations includes “comparing an object with null” which seems perfectly legitimate if you say something like “if (o == null)…”

Does it check if the class actually has an equals() method defined for dissimilar classes?

No. I’m not sure to see the point here, dissimilar classes should never be equals, with or without defining equals. Am I missing somethign?

does this rule only apply to the use of the .equals() method?

Yes, it only applies to .equals(). We may indeed want to explicitly say it in the description.

Well, since it is almost always necessary to override equals() (otherwise you end up with Object.equals() which is just plain old ==), you CAN code it to handle dissimilar types. (Whether one SHOULD do that is a different issue…) For instance:

public class testjava {
        private String str;
        private int tag;

        public testjava(String s, int t) {str=s; tag=t;}

        @Override
        public boolean equals(Object obj) {
                if (this == obj) return true;
                if (obj == null) return false;
                if (getClass() == obj.getClass()) {
                        testjava o = (testjava) obj;
                        return tag == o.tag && str.equals(o.str);
                }
                if (str.getClass() == obj.getClass()) {
                        String o = (String) obj;
                        return tag == 0 && str.equals(o);
                }
                return false;
        }

    public static void main(String[] args) {
        testjava tj = new testjava("foo", 0);
        System.out.println(tj.equals("foo"));
    }
}

This defines a “tagged” string, where the “normal” case is the tag is 0, so for convenience, the class defines a “normal” tagged string as being equal to the root string:

PS C:\temp> javac .\testjava.java
PS C:\temp> java testjava
true