SONARJAVA-4907 Fix FP on S1481 with type patterns in switch case clau…

…ses (#4713)
SonarSource · Mar 15, 2024 · 16c9ac1 · 16c9ac1
1 parent 3a5ae0b
commit 16c9ac1
Show file tree

Hide file tree

Showing 2 changed files with 77 additions and 4 deletions.
diff --git a/java-checks-test-sources/default/src/main/java/checks/unused/UnusedLocalVariableCheck.java b/java-checks-test-sources/default/src/main/java/checks/unused/UnusedLocalVariableCheck.java
@@ -193,4 +193,41 @@ record Bar(int used) { } // Compliant
       System.out.println(new Bar(42).used);
     }
   }
+
+  sealed interface Shape permits Box, Circle {}
+  record Box() implements Shape { }
+  record Circle() implements Shape {}
+
+  static void switchOnSealedClass(Shape shape) {
+    switch (shape) {
+      case Box unused -> { } // compliant
+      case Circle circle -> circle.toString();
+    }
+  }
+
+  static void switchWithTypePattern(Object o) {
+    switch (o) {
+      case Number used -> used.longValue();
+      case Shape unused -> { } // compliant
+      default -> System.out.println();
+    }
+  }
+
+  record MyRecord(int x, int y) { }
+
+  static void switchRecordGuardedPattern(Object o) {
+    switch(o) {
+      case MyRecord(int x, int y) when x > 42 -> { } // Compliant
+      case MyRecord(int x, int y) when y < 42 -> { } // Compliant
+      case MyRecord m when m.x > 42 -> { }
+      case MyRecord m when o.toString().length() > 42 -> { } // Compliant
+      case MyRecord(int x, int y) -> { } // Compliant
+      case MyRecord m -> { } // Compliant
+      case Object object -> {
+        object.toString();
+        var x = 42; // Noncompliant
+        System.out.println();
+      }
+    }
+  }
 }
diff --git a/java-checks/src/main/java/org/sonar/java/checks/unused/UnusedLocalVariableCheck.java b/java-checks/src/main/java/org/sonar/java/checks/unused/UnusedLocalVariableCheck.java
@@ -21,8 +21,10 @@
 
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Optional;
+import java.util.Set;
 import org.sonar.check.Rule;
 import org.sonar.java.checks.helpers.QuickFixHelper;
 import org.sonar.java.checks.helpers.UnresolvedIdentifiersVisitor;
@@ -32,6 +34,7 @@
 import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
 import org.sonar.plugins.java.api.semantic.Symbol;
 import org.sonar.plugins.java.api.tree.AssignmentExpressionTree;
+import org.sonar.plugins.java.api.tree.CaseLabelTree;
 import org.sonar.plugins.java.api.tree.IdentifierTree;
 import org.sonar.plugins.java.api.tree.ListTree;
 import org.sonar.plugins.java.api.tree.SyntaxToken;
@@ -50,7 +53,8 @@ public class UnusedLocalVariableCheck extends IssuableSubscriptionVisitor {
 
   private static final String MESSAGE = "Remove this unused \"%s\" local variable.";
 
-  private static final UnresolvedIdentifiersVisitor UNRESOLVED_IDENTIFIERS_VISITOR = new UnresolvedIdentifiersVisitor();
+  private static final UnresolvedIdentifiersAndSwitchCaseVisitor UNRESOLVED_IDENTIFIERS_AND_SWITCH_CASE_VISITOR =
+    new UnresolvedIdentifiersAndSwitchCaseVisitor();
 
   @Override
   public List<Tree.Kind> nodesToVisit() {
@@ -60,7 +64,7 @@ public List<Tree.Kind> nodesToVisit() {
   @Override
   public void visitNode(Tree tree) {
     if (tree.is(Tree.Kind.COMPILATION_UNIT)) {
-      UNRESOLVED_IDENTIFIERS_VISITOR.check(tree);
+      UNRESOLVED_IDENTIFIERS_AND_SWITCH_CASE_VISITOR.check(tree);
     }
   }
 
@@ -69,7 +73,7 @@ public void leaveNode(Tree tree) {
     if (tree.is(Tree.Kind.VARIABLE)) {
       VariableTree variable = (VariableTree) tree;
       String name = variable.simpleName().name();
-      boolean unresolved = UNRESOLVED_IDENTIFIERS_VISITOR.isUnresolved(name);
+      boolean unresolved = UNRESOLVED_IDENTIFIERS_AND_SWITCH_CASE_VISITOR.isUnresolved(name);
       if (!unresolved && isProperLocalVariable(variable) && isUnused(variable.symbol())) {
         QuickFixHelper.newIssue(context)
           .forRule(this)
@@ -111,7 +115,8 @@ private static boolean isProperLocalVariable(VariableTree variable) {
     return symbol.isLocalVariable()
       && !symbol.isParameter()
       && !isDefinedInCatchClause(variable)
-      && !isTryResource(variable);
+      && !isTryResource(variable)
+      && !UNRESOLVED_IDENTIFIERS_AND_SWITCH_CASE_VISITOR.isSwitchPatternVariable(variable);
   }
 
   private static boolean isDefinedInCatchClause(VariableTree variable) {
@@ -172,4 +177,35 @@ private static Optional<AnalyzerMessage.TextSpan> getQuickFixTextSpan(VariableTr
   private static Optional<SyntaxToken> getPrecedingComma(VariableTree variable) {
     return QuickFixHelper.previousVariable(variable).map(VariableTree::lastToken);
   }
+
+  private static class UnresolvedIdentifiersAndSwitchCaseVisitor extends UnresolvedIdentifiersVisitor {
+    private final Set<VariableTree> switchPatternVariables = new HashSet<>();
+    private boolean withinCaseLabel = false;
+
+    @Override
+    public Set<String> check(Tree tree) {
+      switchPatternVariables.clear();
+      withinCaseLabel = false;
+      return super.check(tree);
+    }
+
+    public boolean isSwitchPatternVariable(VariableTree variable) {
+      return switchPatternVariables.contains(variable);
+    }
+
+    @Override
+    public void visitCaseLabel(CaseLabelTree tree) {
+      withinCaseLabel = true;
+      super.visitCaseLabel(tree);
+      withinCaseLabel = false;
+    }
+
+    @Override
+    public void visitVariable(VariableTree tree) {
+      if (withinCaseLabel) {
+        switchPatternVariables.add(tree);
+      }
+      super.visitVariable(tree);
+    }
+  }
 }