Fixed FindDeadLocalStores to handle LOOKUPSWITCH and TABLESWITCH (#2937)

* test: added more tests for false positives * fix: a switch instruction is a BCEL Select LOOKUPSWITCH and TABLESWITCH both extend Select, use the base class to detect the start of a switch in FindDeadLocalStores Updated the line numbers of expected bugs * fix: reverted .* import --------- Co-authored-by: Jeremy Landis <jeremylandis@hotmail.com>
spotbugs · Apr 21, 2024 · a197132 · a197132
1 parent 1342c0f
commit a197132
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 6 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,7 @@ Currently the versioning policy of this project follows [Semantic Versioning v2.
 ## Unreleased - 2024-??-??
 ### Fixed
 - Fix FP `SING_SINGLETON_GETTER_NOT_SYNCHRONIZED` with eager instances ([#2932]https://github.com/spotbugs/spotbugs/issues/2932)
+- Do not report DLS_DEAD_LOCAL_STORE for Java 21's type switches when switch instruction is TABLESWITCH([#2736](https://github.com/spotbugs/spotbugs/issues/2736))
 - Fix FP `SE_BAD_FIELD` for record fields ([#2935]https://github.com/spotbugs/spotbugs/issues/2935)
 
 ## 4.8.4 - 2024-04-07

diff --git a/spotbugs-tests/src/test/java/edu/umd/cs/findbugs/detect/Issue2782Test.java b/spotbugs-tests/src/test/java/edu/umd/cs/findbugs/detect/Issue2782Test.java
@@ -25,13 +25,13 @@ void testIssue() {
         assertBugCount("BC_UNCONFIRMED_CAST", 1);
         assertBugCount("BC_UNCONFIRMED_CAST_OF_RETURN_VALUE", 1);
 
-        assertBugAtLine("BC_UNCONFIRMED_CAST", 50);
-        assertBugAtLine("BC_UNCONFIRMED_CAST_OF_RETURN_VALUE", 49);
+        assertBugAtLine("BC_UNCONFIRMED_CAST", 52);
+        assertBugAtLine("BC_UNCONFIRMED_CAST_OF_RETURN_VALUE", 51);
 
         // Checks for issue 2736
         assertBugCount("DLS_DEAD_LOCAL_STORE", 1);
 
-        assertBugAtLine("DLS_DEAD_LOCAL_STORE", 59);
+        assertBugAtLine("DLS_DEAD_LOCAL_STORE", 61);
     }
 
     private void assertBugCount(String type, int expectedCount) {

diff --git a/spotbugs/src/main/java/edu/umd/cs/findbugs/detect/FindDeadLocalStores.java b/spotbugs/src/main/java/edu/umd/cs/findbugs/detect/FindDeadLocalStores.java
@@ -52,11 +52,11 @@
 import org.apache.bcel.generic.LRETURN;
 import org.apache.bcel.generic.LSTORE;
 import org.apache.bcel.generic.LoadInstruction;
-import org.apache.bcel.generic.LOOKUPSWITCH;
 import org.apache.bcel.generic.MULTIANEWARRAY;
 import org.apache.bcel.generic.MethodGen;
 import org.apache.bcel.generic.NEWARRAY;
 import org.apache.bcel.generic.ObjectType;
+import org.apache.bcel.generic.Select;
 import org.apache.bcel.generic.StoreInstruction;
 import org.apache.bcel.generic.Type;
 
@@ -276,8 +276,8 @@ private void analyzeMethod(ClassContext classContext, Method method) throws Data
                     switchHandler.sawInvokeDynamic(pc, invokeMethodName);
 
                     continue;
-                } else if (handle.getInstruction() instanceof LOOKUPSWITCH) {
-                    LOOKUPSWITCH switchInstruction = (LOOKUPSWITCH) handle.getInstruction();
+                } else if (handle.getInstruction() instanceof Select) {
+                    Select switchInstruction = (Select) handle.getInstruction();
                     int[] indices = switchInstruction.getIndices();
 
                     switchHandler.enterSwitch(switchInstruction.getOpcode(),

diff --git a/spotbugsTestCases/src/java21/Issue2782.java b/spotbugsTestCases/src/java21/Issue2782.java
@@ -1,3 +1,5 @@
+import java.util.Collection;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
@@ -62,4 +64,24 @@ public static int deadStore(int value) {
             	return 42;
         }
     }
+
+    public static int collectionSize(Object value) {
+        return switch (value) {
+            case Map<?, ?> map -> map.size();
+            case Set<?> set -> set.size();
+            case List<?> list -> list.size();
+            case Collection<?> collection ->
+            throw new UnsupportedOperationException("Error " + value.getClass());
+            case null, default -> 0;
+        };
+    }
+
+    public static String getValueType(Object value) {
+        return switch (value) {
+            case List<?> list -> "list";
+            case Map<?, ?> map -> "map";
+            case Number number -> "number";
+            case null, default -> "other";
+        };
+    }
 }