Ignore checkcast in Java 21 type switch

.github/workflows/build.yml

@@ -9,7 +9,7 @@ jobs:
     strategy:
       matrix:
         os: [ubuntu-latest, windows-latest, macos-latest]
-        java: [17, 21]
+        java: [21]
       max-parallel: 6
     runs-on: ${{ matrix.os }}
     continue-on-error: true

.github/workflows/release.yml

@@ -42,10 +42,10 @@ jobs:
           commit_message: 'docs: update CHANGELOG.md for ${{ github.ref_name }}'
           file_pattern: CHANGELOG.md
 
-      - name: Set up JDK 17
+      - name: Set up JDK 21
         uses: actions/setup-java@v4
         with:
-          java-version: '17'
+          java-version: '21'
           distribution: temurin
           cache: gradle
       - uses: gradle/wrapper-validation-action@v1

CHANGELOG.md

@@ -14,9 +14,11 @@ Currently the versioning policy of this project follows [Semantic Versioning v2.
 - Do not report SE_NO_SERIALVERSIONID or other serialization issues for records ([#2793](https://github.com/spotbugs/spotbugs/issues/2793))
 - Added support for CONSTANT_Dynamic ([#2759](https://github.com/spotbugs/spotbugs/issues/2759))
 - Ignore generic variable types when looking for BC_UNCONFIRMED_CAST_OF_RETURN_VALUE ([#1219](https://github.com/spotbugs/spotbugs/issues/1219))
+- Do not report BC_UNCONFIRMED_CAST for Java 21's type switches ([#2813](https://github.com/spotbugs/spotbugs/pull/2813))
 
 ### Build
 - Fix sonar coverage for project ([#2796](https://github.com/spotbugs/spotbugs/issues/2796))
+- Upgraded the build to run on JDK 21 only now that we have bug samples using Java 21 language features ([#2813](https://github.com/spotbugs/spotbugs/pull/2813))
 
 ### Added
 - New detector `MultipleInstantiationsOfSingletons` and introduced new bug types:

README.md

@@ -17,6 +17,8 @@ More information at the [official website](https://spotbugs.github.io/). A lot o
 
 SpotBugs is built using [Gradle](https://gradle.org). The recommended way to obtain it is to simply run the `gradlew` (or `gradlew.bat`) wrapper, which will automatically download and run the correct version as needed (using the settings in `gradle/wrapper/gradle-wrapper.properties`).
 
+Building SpotBugs requires a JDK 21 installation (running SpotBugs requires a JDK 8 or above).
+
 To see a list of build options, run `gradle tasks` (or `gradlew tasks`). The `build` task will perform a full build and test.
 
 To build the SpotBugs plugin for Eclipse, you'll need to create the file `eclipsePlugin/local.properties`, containing a property `eclipseRoot.dir` that points to an Eclipse installation's root directory (see `.github/workflows/release.yml` for an example), then run the build.

spotbugs-tests/src/test/java/edu/umd/cs/findbugs/detect/Issue2782Test.java

@@ -0,0 +1,30 @@
+package edu.umd.cs.findbugs.detect;
+
+import edu.umd.cs.findbugs.AbstractIntegrationTest;
+import edu.umd.cs.findbugs.test.matcher.BugInstanceMatcher;
+import edu.umd.cs.findbugs.test.matcher.BugInstanceMatcherBuilder;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.condition.DisabledOnJre;
+import org.junit.jupiter.api.condition.JRE;
+
+import static edu.umd.cs.findbugs.test.CountMatcher.containsExactly;
+import static org.hamcrest.MatcherAssert.*;
+
+public class Issue2782Test extends AbstractIntegrationTest {
+
+    @Test
+    @DisabledOnJre({ JRE.JAVA_8, JRE.JAVA_11, JRE.JAVA_17 })
+    void testIssue() {
+        performAnalysis(
+                "../java21/Issue2782.class",
+                "../java21/Issue2782$MyInterface.class",
+                "../java21/Issue2782$Impl1.class",
+                "../java21/Issue2782$Impl2.class");
+
+        BugInstanceMatcher matcher = new BugInstanceMatcherBuilder()
+                .bugType("BC_UNCONFIRMED_CAST")
+                .build();
+
+        assertThat(getBugCollection(), containsExactly(0, matcher));
+    }
+}

spotbugs/src/main/java/edu/umd/cs/findbugs/SwitchHandler.java

@@ -20,7 +20,9 @@
 package edu.umd.cs.findbugs;
 
 import java.util.ArrayList;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 
 import javax.annotation.CheckForNull;
 
@@ -32,9 +34,14 @@
 
 public class SwitchHandler {
     private final List<SwitchDetails> switchOffsetStack;
+    /**
+     * The set of program counters for the 'switch' instruction of each of the type switches
+     */
+    private final Set<Integer> typeSwitchPC;
 
     public SwitchHandler() {
         switchOffsetStack = new ArrayList<>();
+        typeSwitchPC = new HashSet<>();
     }
 
     public int stackSize() {
@@ -55,11 +62,20 @@ int numEnumValues(@CheckForNull XClass c) {
     }
 
     public void enterSwitch(DismantleBytecode dbc, @CheckForNull XClass enumType) {
-
-        assert dbc.getOpcode() == Const.TABLESWITCH || dbc.getOpcode() == Const.LOOKUPSWITCH;
         int[] switchOffsets = dbc.getSwitchOffsets();
-        SwitchDetails details = new SwitchDetails(dbc.getPC(), switchOffsets, dbc.getDefaultSwitchOffset(), switchOffsets.length == numEnumValues(
-                enumType));
+        enterSwitch(dbc.getOpcode(), dbc.getPC(), switchOffsets, dbc.getDefaultSwitchOffset(), switchOffsets.length == numEnumValues(enumType));
+    }
+
+    /**
+     * @param opCode The op code of the switch, should be <code>TABLESWITCH</code> or <code>LOOKUPSWITCH</code>
+     * @param pc The PC of the switch instruction
+     * @param switchOffsets The PC offsets of the switch cases
+     * @param defaultSwitchOffset The PC of the default case
+     * @param exhaustive <code>true</code> if the switch is exhaustive
+     */
+    public void enterSwitch(int opCode, int pc, int[] switchOffsets, int defaultSwitchOffset, @CheckForNull boolean exhaustive) {
+        assert opCode == Const.TABLESWITCH || opCode == Const.LOOKUPSWITCH;
+        SwitchDetails details = new SwitchDetails(pc, switchOffsets, defaultSwitchOffset, exhaustive);
 
 
         int size = switchOffsetStack.size();
@@ -120,6 +136,61 @@ public SourceLineAnnotation getCurrentSwitchStatement(BytecodeScanningDetector d
                 detector.getClassContext(), detector, details.switchPC, details.switchPC + details.maxOffset - 1);
     }
 
+    /**
+     * For type switches introduced in Java 21 we are using the invocation of a bootstrap 'typeswitch()' method to
+     * detect that the switch operates on the class of the object.
+     *
+     * @param pc
+     * @param methodName
+     */
+    public void sawInvokeDynamic(int pc, String methodName) {
+        if ("typeSwitch".equals(methodName)) {
+            typeSwitchPC.add(pc + 5);
+        }
+    }
+
+    /**
+     * In type switches a <code>CHECKCAST</code> is inserted by the compiler for each case. This method checks if the
+     * instruction is one of these casts and then checks if the corresponding switch is a type switch.
+     *
+     * @param opCode The operation code
+     * @param pc The program counter
+     * @return <code>true</code>If this instruction is a cast for a type switch
+     */
+    public boolean isTypeSwitchCaseCheckCast(int opCode, int pc) {
+        if (opCode != Const.CHECKCAST) {
+            return false;
+        }
+
+        // For a type switch each case starts with an aload or an aload_<n> followed by a checkcast
+        // aload_<n> does not have an operand so we're trying the previous pc (i.e. pc - 1)
+        // aload takes and index operand so we're trying pc - 2
+        // Since we're on a checkcast look for a switch with a target on the previous PC
+        SwitchDetails switchDetails = findSwitchDetailsByPc(pc - 1, pc - 2);
+
+        return switchDetails != null && typeSwitchPC.contains(switchDetails.switchPC);
+    }
+
+    /**
+     * Finds a switch from the first PC of a case
+     *
+     * @param possiblePC The possible first PC of a switch case
+     * @return The <code>SwitchDetails</code> of the switch corresponding to the case or null if there was no case at this PC
+     */
+    private SwitchDetails findSwitchDetailsByPc(int... possiblePC) {
+        for (SwitchDetails switchDetails : switchOffsetStack) {
+            for (int offset : switchDetails.swOffsets) {
+                for (int pc : possiblePC) {
+                    if (pc == offset + switchDetails.switchPC) {
+                        return switchDetails;
+                    }
+                }
+            }
+        }
+
+        return null;
+    }
+
     public static class SwitchDetails {
         final int switchPC;
 

spotbugs/src/main/java/edu/umd/cs/findbugs/detect/FindBadCast2.java

@@ -25,6 +25,7 @@
 import org.apache.bcel.generic.LDC;
 import org.apache.bcel.generic.MethodGen;
 import org.apache.bcel.generic.ReferenceType;
+import org.apache.bcel.generic.LOOKUPSWITCH;
 import org.apache.bcel.generic.Type;
 import org.apache.bcel.generic.TypedInstruction;
 
@@ -38,6 +39,7 @@
 import edu.umd.cs.findbugs.MethodAnnotation;
 import edu.umd.cs.findbugs.Priorities;
 import edu.umd.cs.findbugs.SourceLineAnnotation;
+import edu.umd.cs.findbugs.SwitchHandler;
 import edu.umd.cs.findbugs.SystemProperties;
 import edu.umd.cs.findbugs.ba.CFG;
 import edu.umd.cs.findbugs.ba.CFGBuilderException;
@@ -196,6 +198,7 @@ private void analyzeMethod(ClassContext classContext, Method method) throws CFGB
         Map<BugAnnotation, String> instanceOfChecks = new HashMap<>();
         String constantClass = null;
         boolean methodInvocationWasGeneric = false;
+        SwitchHandler switchHandler = new SwitchHandler();
 
         int pcForConstantClass = -1;
         for (Iterator<Location> i = cfg.locationIterator(); i.hasNext();) {
@@ -207,8 +210,24 @@ private void analyzeMethod(ClassContext classContext, Method method) throws CFGB
 
             boolean wasMethodInvocationWasGeneric = methodInvocationWasGeneric;
             methodInvocationWasGeneric = false;
+
+            if (ins instanceof LOOKUPSWITCH) {
+                LOOKUPSWITCH switchInstruction = (LOOKUPSWITCH) ins;
+                int[] indices = switchInstruction.getIndices();
+
+                switchHandler.enterSwitch(ins.getOpcode(),
+                        pc,
+                        indices,
+                        0, // Not sure how to get the default offset from BCEL but it doesn't matter here
+                        false); // It shouldn't matter here if the switch was exhaustive or not
+            }
+
             if (ins instanceof InvokeInstruction) {
                 if (ins instanceof INVOKEDYNAMIC) {
+                    INVOKEDYNAMIC invokeDynamicInstruction = (INVOKEDYNAMIC) ins;
+                    String invokeMethodName = invokeDynamicInstruction.getMethodName(cpg);
+                    switchHandler.sawInvokeDynamic(pc, invokeMethodName);
+
                     continue;
                 }
                 InvokeInstruction iinv = (InvokeInstruction) ins;
@@ -325,6 +344,11 @@ private void analyzeMethod(ClassContext classContext, Method method) throws CFGB
                 continue;
             }
 
+            if (switchHandler.isTypeSwitchCaseCheckCast(ins.getOpcode(), pc)) {
+                // javac adds a checkcast for each case in a type switch, ignore them
+                continue;
+            }
+
             String refSig = refType.getSignature();
             String castSig2 = castSig;
             String refSig2 = refSig;

spotbugsTestCases/build.gradle

@@ -41,7 +41,9 @@ dependencies {
 tasks.withType(JavaCompile).configureEach {
   options.compilerArgs << '-Xlint:none'
   options.encoding = 'UTF-8'
-  if (it.name == 'classesJava17') {
+  if (it.name == 'classesJava21') {
+    options.release = 21
+  } else if (it.name == 'classesJava17') {
     options.release = 17
   } else if (it.name == 'classesJava11') {
     options.release = 11
@@ -81,10 +83,17 @@ def classesJava17 = tasks.register('classesJava17', JavaCompile) {
   source = file('src/java17')
 }
 
+def classesJava21 = tasks.register('classesJava21', JavaCompile) {
+  destinationDirectory.set(layout.buildDirectory.dir("classes/java/java21"))
+  classpath = sourceSets.main.compileClasspath
+  source = file('src/java21')
+}
+
 tasks.named('classes').configure {
   dependsOn classesJava8
   dependsOn classesJava11
   dependsOn classesJava17
+  dependsOn classesJava21
 }
 
 sonar {

spotbugsTestCases/src/java21/Issue2782.java

@@ -0,0 +1,44 @@
+
+public class Issue2782 {
+
+    public interface MyInterface<T> {
+    }
+
+    static class Impl1 implements MyInterface<String> {
+        public MyInterface<String> helloFromImpl1() {
+            return this;
+        }
+    }
+
+    static class Impl2 implements MyInterface<Boolean> {
+        public MyInterface<Boolean> helloFromImpl2() {
+            return this;
+        }
+    }
+
+    public static <T> void method(MyInterface<T> instance) {
+        Object bla = switch (instance) {
+            case Impl1 impl1 -> impl1.helloFromImpl1();
+            case Impl2 impl2 -> impl2.helloFromImpl2();
+            default -> throw new IllegalArgumentException("Blah");
+        };
+
+        System.out.println(bla);
+    }
+
+    public static <T> Object nestedSwitches(MyInterface<T> x1, MyInterface<T> x2, MyInterface<T> x3) {
+        return switch (x1) {
+            case Impl1 impl1 -> switch (x2) {
+                case Impl1 a -> switch (x3) {
+                    case Impl1 x -> x.helloFromImpl1();
+                    case Impl2 y -> y.helloFromImpl2();
+                    default -> "";
+                };
+                case Impl2 b -> b.helloFromImpl2();
+                default -> "";
+            };
+            case Impl2 impl2 -> impl2.helloFromImpl2();
+            default -> throw new IllegalArgumentException("Blah");
+        };
+    }
+}