From a19713208a8d7b571bb25a2f4f57250190d941c7 Mon Sep 17 00:00:00 2001 From: Guillaume Toison <86775455+gtoison@users.noreply.github.com> Date: Sun, 21 Apr 2024 21:25:42 +0200 Subject: [PATCH] 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 --- CHANGELOG.md | 1 + .../umd/cs/findbugs/detect/Issue2782Test.java | 6 ++--- .../findbugs/detect/FindDeadLocalStores.java | 6 ++--- spotbugsTestCases/src/java21/Issue2782.java | 22 +++++++++++++++++++ 4 files changed, 29 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index decf1af52e5..087516e5e7e 100644 --- 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 index c882a180abb..dde496e4f31 100644 --- 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 index 474724538cd..ad01c4451be 100644 --- 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 index e7e8f2261a2..66769253c58 100644 --- 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"; + }; + } }