Java 17 - False positive SA_LOCAL_SELF_COMPARISON when using instanceof pattern matching

[SchubP]

Hi,

there seems to still be an issue with the problem stated in the title.
It was taken care of way back but is now (again?) relevant. Back then there were PRs to test/fix it: #1136 and #1435

We are using:

• Spotbugs 4.6.0 in Eclipse 4.23 and 4.5.3 in Maven-3.8.5-Build
• Java 17.0.2 by Adoptium
• With this code snippet we get the false positive: https://github.com/spotbugs/spotbugs/blob/master/spotbugsTestCases/src/java14/IncorrectSelfComparisonInstanceOfPatternMatching.java

There are serveral other similar issues and comments here mentioning this sitiuation:

• 50cb283#commitcomment-49080418
• Two False positives SA_LOCAL_SELF_COMPARISON + ES_COMPARING_STRINGS_WITH_EQ when using instanceof pattern matching (jdk16) #1617

Is it maybe possible, that this check/exclusion only works for Java 14 but not 15, 16, 17, ...?
For now we will probaby disable these checks.

Many thanks in advance,
Cheers, Paul

[bsutton]

I'm seeing the same issue with jdk 17:

{
	public List<VersionRange> _rangesFor(VersionConstraint constraint) throws ArgumentException
	{
		if (constraint.isEmpty())
			return new ArrayList<VersionRange>();
		if (constraint instanceof VersionUnion union) // this line generates the false positive.
			return union.ranges;
		throw new ArgumentException("Unknown VersionConstraint type $constraint.");
	}

VersionUnion.java:281 Self comparison of constraint with itself
VersionUnion._rangesFor(VersionConstraint) [Scariest(1), High confidence]

[famod]

I'm also seeing this, but only when using ECJ (Eclipse compiler) via maven-compiler-plugin.
There is no such issue when using maven-compiler-plugin with forceJavacCompilerUse.

[SchubP]

This issue is probably also related: #1944
What @famod pointed out seems interesting: all related to Eclipse/ECJ/Tycho ...

[Martin-Schwenk]

When compiling the following example

public static void main(String[] args) {
    Number n = Integer.valueOf("0");
    if (n instanceof Integer i) {
        System.out.println("int " + i);
    }
}

with ejc (and then decompiling with javap) I get

         0: ldc           #16                 // String 0
         2: invokestatic  #18                 // Method java/lang/Integer.valueOf:(Ljava/lang/String;)Ljava/lang/Integer;
         5: astore_1
         6: aload_1
         7: astore_3
         8: aload_3
         9: instanceof    #19                 // class java/lang/Integer
        12: ifeq          50
        15: aload_3
        16: checkcast     #19                 // class java/lang/Integer
        19: dup
        20: astore_2
        21: aload_3
        22: checkcast     #19                 // class java/lang/Integer
        25: if_acmpne     50
        28: getstatic     #24                 // Field java/lang/System.out:Ljava/io/PrintStream;
...
        50: return

which indeed has a reference check ("if_acmpne") that compares two reference that are always equal. The duplicate "checkcast"-pattern seeems like a bug in ejc to me.
If I compile with javac, I get

         0: ldc           #7                  // String 0
         2: invokestatic  #9                  // Method java/lang/Integer.valueOf:(Ljava/lang/String;)Ljava/lang/Integer;
         5: astore_1
         6: aload_1
         7: instanceof    #10                 // class java/lang/Integer
        10: ifeq          30
        13: aload_1
        14: checkcast     #10                 // class java/lang/Integer
        17: astore_2
        18: getstatic     #15                 // Field java/lang/System.out:Ljava/io/PrintStream;
...
        30: return

which is what I'd expect and which shouldn't trigger the spotbugs warning.
Tested with eclipse 4.24.0 and openjdk 17.0.3.

[trancexpress]

Another bug in ecj with this snippet, not sure if there already is a spotbugs issue for it:

public class InstanceOfPatternTest {
	static String STR = "2";
	public static void main(String[] args) {
		if ( switch(STR) {
				default -> "zero";
			  }
				instanceof String s) {
			System.out.println(s);
		}
	}
}

Bug: Return value of String.hashCode() ignored in test.InstanceOfPatternTest.main(String[])

(at if ( switch(STR) {)

[gtoison]

So this seems to be an issue with ECJ as described here: eclipse-jdt/eclipse.jdt.core#1406
There are improvements in eclipse-jdt/eclipse.jdt.core#2000 and eclipse-jdt/eclipse.jdt.core#300 and this should fix the issue
In case you still have the issue with another compiler (e.g. javac) please let us know