SONARJAVA-4441 Fix FP on S1301 when mutiple case labels are used in…

… Java 14 `switch`. (#4970)
SonarSource · Dec 23, 2024 · 71f25f0 · 71f25f0
1 parent 32060f9
commit 71f25f0
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 11 deletions.
diff --git a/.../checks/SwitchAtLeastThreeCasesCheck.java → ...s/SwitchAtLeastThreeCasesCheckSample.java b/.../checks/SwitchAtLeastThreeCasesCheck.java → ...s/SwitchAtLeastThreeCasesCheckSample.java
@@ -1,16 +1,19 @@
-class A {
+package checks.tests;
 
-  static void recordSwitch1(MyRecord object) {
+public class SwitchAtLeastThreeCasesCheckSample {
+  record MyRecord(int x, int y) {}
+
+  static void recordSwitch1(Object object) {
     switch (object) { // Compliant
       case String x when x.length() > 42 -> { }
-      case Integer i -> { }
+      default -> { }
     }
   }
 
-  static void recordSwitch1(Object object) {
+  static void recordSwitch2(Object object) {
     switch (object) { // Compliant
       case MyRecord(int x, int y) -> { }
-      case String s -> { }
+      default -> { }
     }
   }
 
@@ -19,22 +22,25 @@ sealed interface Shape permits Box, Circle {}
     record Box() implements Shape { }
     record Circle() implements Shape {}
 
-    void foo(Shape shape) {
+    default void foo(Shape shape) {
       switch (shape) { // Compliant because of type pattern matching
         case Box ignored -> { }
         case Circle ignored -> System.out.println();
       }
     }
 
-    void goo(Shape shape) {
+    default void goo(Shape shape) {
       switch (shape) { // Compliant because of type pattern matching
-        default -> System.out.println();
         case Box ignored -> { }
+        default -> System.out.println();
       }
     }
   }
 
-  public void f() {
+  private static void doSomething() {}
+  private static void doSomethingElse() {}
+
+  public void f(int variable) {
     switch (variable) { // Noncompliant {{Replace this "switch" statement by "if" statements to increase readability.}}
 //  ^^^^^^
       case 0:
@@ -55,6 +61,15 @@ public void f() {
         break;
     }
 
+    switch (variable) {
+      case 0, 1:
+        doSomething();
+        break;
+      default:
+        doSomethingElse();
+        break;
+    }
+
     switch (variable) { // Noncompliant
     }
 

diff --git a/java-checks/src/main/java/org/sonar/java/checks/SwitchAtLeastThreeCasesCheck.java b/java-checks/src/main/java/org/sonar/java/checks/SwitchAtLeastThreeCasesCheck.java
@@ -19,6 +19,7 @@
 import org.sonar.check.Rule;
 import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
 import org.sonar.plugins.java.api.tree.CaseGroupTree;
+import org.sonar.plugins.java.api.tree.CaseLabelTree;
 import org.sonar.plugins.java.api.tree.SwitchStatementTree;
 import org.sonar.plugins.java.api.tree.Tree;
 
@@ -42,13 +43,32 @@ public void visitNode(Tree tree) {
       if (hasLabelWithAllowedPattern(caseGroup)) {
         return;
       }
-      count += caseGroup.labels().size();
+      count += totalLabelCount(caseGroup);
     }
     if (count < 3) {
       reportIssue(switchStatementTree.switchKeyword(), "Replace this \"switch\" statement by \"if\" statements to increase readability.");
     }
   }
 
+  /**
+   * Count labels, taking into account Java 14 multi-label switch.
+   * For example, here we have 4 labels:
+   * <pre>
+   *   case "Monday", "Tuesday":
+   *   case "Wednesday:
+   *   default: // considered 1 label
+   * </pre>
+   */
+  private static int totalLabelCount(CaseGroupTree caseGroup) {
+    int total = 0;
+    for (CaseLabelTree label: caseGroup.labels()) {
+      int sz = label.expressions().size();
+      // `default` does not have any expressions, but we consider it 1 label.
+      total += sz > 0 ? sz : 1;
+    }
+    return total;
+  }
+
   private static boolean hasLabelWithAllowedPattern(CaseGroupTree caseGroupTree) {
     return caseGroupTree.labels().stream()
       .flatMap(label -> label.expressions().stream())

diff --git a/java-checks/src/test/java/org/sonar/java/checks/SwitchAtLeastThreeCasesCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/SwitchAtLeastThreeCasesCheckTest.java
@@ -19,12 +19,14 @@
 import org.junit.jupiter.api.Test;
 import org.sonar.java.checks.verifier.CheckVerifier;
 
+import static org.sonar.java.checks.verifier.TestUtils.testCodeSourcesPath;
+
 class SwitchAtLeastThreeCasesCheckTest {
 
   @Test
   void test() {
     CheckVerifier.newVerifier()
-      .onFile("src/test/files/checks/SwitchAtLeastThreeCasesCheck.java")
+      .onFile(testCodeSourcesPath("checks/tests/SwitchAtLeastThreeCasesCheckSample.java"))
       .withCheck(new SwitchAtLeastThreeCasesCheck())
       .verifyIssues();
   }