Rule S2160: false positive when class has one parent with non final equals method

  • versions used (SonarQube 6.7, SonarAnalyzer (Java))

In the example below, the rule will complain on C.
The reason is that as soon as a parent with final equals method is found the rule should go out of the while loop. The current implementation continues to check the parents, even after having found a final equals.

see source code

public class A {

	@Override
	public boolean equals(Object obj) {
		return super.equals(obj);
	}
}

public class B extends A {

	@Override
	public final boolean equals(Object obj) {
		return super.equals(obj);
	}
}

public class C extends B {

	private String newField;
}

A possible fix could be

  private static boolean parentClassImplementsEquals(ClassTree tree) {
    TypeTree superClass = tree.superClass();
    if (superClass != null) {
      Type superClassType = superClass.symbolType();
      while (superClassType.symbol().isTypeSymbol() && !superClassType.is("java.lang.Object")) {
        Symbol.TypeSymbol superClassSymbol = superClassType.symbol();
        if (hasNotFinalEqualsMethod(superClassSymbol)) {
          return true;
        } else if (hasFinalEqualsMethod(superClassSymbol)) {
          return false;
        }
        superClassType = superClassSymbol.superClass();
      }
    }
    return false;
  }

  private static boolean hasFinalEqualsMethod(Symbol.TypeSymbol superClassSymbol) {
    return superClassSymbol.lookupSymbols("equals").stream().anyMatch(symbol -> EQUALS_MATCHER.matches(symbol) && symbol.isFinal());
  }

Feel free to rewrite this to make it clearer/more performant!

Pull request was created to fix this
https://github.com/SonarSource/sonar-java/pull/2341

Hello @troosan,

Sorry for the delay answering you, and thanks for your very precise feedback, and fix!

This is indeed a FP on rule S2160, which definitely needs to be fixed. I created the following ticket to handle it, as it would be nice to have it part of our release note: SONARJAVA-3004

I’m also going to review your PR and eventually merge it into our master.

Cheers,
Michael

This topic was automatically closed 7 days after the last reply. New replies are no longer allowed.