Enable StatementSwitchToExpressionSwitch analysis phase

PiperOrigin-RevId: 639030266
google · Jun 5, 2024 · dc4c426 · dc4c426
1 parent 39609bf
commit dc4c426
Show file tree

Hide file tree

Showing 3 changed files with 86 additions and 16 deletions.
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitch.java b/core/src/main/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitch.java
@@ -580,8 +580,7 @@ private static SuggestedFix convertDirectlyToExpressionSwitch(
         }
 
         // To improve readability, don't use braces on the rhs if not needed
-        if (shouldTransformCaseWithoutBraces(
-            filteredStatements, transformedBlockSource, filteredStatements.get(0), state)) {
+        if (shouldTransformCaseWithoutBraces(filteredStatements)) {
           // Single statement with no comments - no braces needed
           replacementCodeBuilder.append(transformedBlockSource);
         } else {
@@ -929,10 +928,7 @@ private static int getBlockEnd(
    * arrow symbol without braces, incorporating both language and readabilitiy considerations.
    */
   private static boolean shouldTransformCaseWithoutBraces(
-      ImmutableList<StatementTree> statementTrees,
-      String transformedBlockSource,
-      StatementTree firstStatement,
-      VisitorState state) {
+      ImmutableList<StatementTree> statementTrees) {
 
     if (statementTrees.isEmpty()) {
       // Instead, express as "-> {}"
@@ -944,11 +940,6 @@ private static boolean shouldTransformCaseWithoutBraces(
       return false;
     }
 
-    // If code has comments, use braces for readability
-    if (!transformedBlockSource.trim().equals(state.getSourceForNode(firstStatement).trim())) {
-      return false;
-    }
-
     StatementTree onlyStatementTree = statementTrees.get(0);
     return KINDS_CONVERTIBLE_WITHOUT_BRACES.contains(onlyStatementTree.getKind());
   }

diff --git a/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java b/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java
@@ -1052,6 +1052,7 @@ public static ScannerSupplier warningChecks() {
           ScopeAnnotationOnInterfaceOrAbstractClass.class,
           SelfAlwaysReturnsThis.class,
           ShortCircuitBoolean.class,
+          StatementSwitchToExpressionSwitch.class,
           StaticAssignmentInConstructor.class,
           StaticAssignmentOfThrowable.class,
           StaticGuardedByInstance.class,
@@ -1205,7 +1206,6 @@ public static ScannerSupplier warningChecks() {
           ReturnsNullCollection.class,
           ScopeOnModule.class,
           ScopeOrQualifierAnnotationRetention.class,
-          StatementSwitchToExpressionSwitch.class,
           StaticOrDefaultInterfaceMethod.class,
           StaticQualifiedUsingExpression.class,
           StringFormatWithLiteral.class,

diff --git a/...rc/test/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitchTest.java b/...rc/test/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitchTest.java
@@ -350,10 +350,10 @@ public void switchByEnumCard2_removesRedundantBreaks_error() {
             "  public void foo(Side side) { ",
             "    // BUG: Diagnostic contains: [StatementSwitchToExpressionSwitch]",
             "    switch(side) {",
-            "       case HEART -> {",
+            "       case HEART -> ",
             "          System.out.println(\"heart\");",
             "          // Pre break comment",
-            "       }",
+            "       ",
             "       case DIAMOND -> {",
             "          // Diamond break comment",
             "          break;",
@@ -590,9 +590,9 @@ public void switchWithDefaultInMiddle_error() {
             "          System.out.println(\"diamond\");",
             "          return;",
             "       }",
-            "       default -> { /* comment: */",
+            "       default -> /* comment: */",
             "         System.out.println(\"club\");",
-            "       }",
+            "       ",
             "       case SPADE -> System.out.println(\"spade\");",
             "    }",
             "  }",
@@ -1147,6 +1147,85 @@ public void switchByEnum_exampleInDocumentation_error() {
         .doTest();
   }
 
+  @Test
+  public void switchByEnum_caseHasOnlyComments_error() {
+    // When a case is solely comments, we should still try to convert the switch using braceless
+    // syntax
+    assumeTrue(RuntimeVersion.isAtLeast14());
+    helper
+        .addSourceLines(
+            "Test.java",
+            "class Test {",
+            "  enum Suit {HEARTS, CLUBS, SPADES, DIAMONDS};",
+            "  public Test() {}",
+            "  private void foo(Suit suit) {",
+            "    // BUG: Diagnostic contains: [StatementSwitchToExpressionSwitch]",
+            "    switch(suit) {",
+            "      case HEARTS:",
+            "        // A comment here",
+            "        // more comments.",
+            "      case DIAMONDS:",
+            "        System.out.println(\"Red diamonds\");",
+            "        break;",
+            "      case SPADES:",
+            "        // Fall through",
+            "      case CLUBS:",
+            "        bar();",
+            "        System.out.println(\"Black suit\");",
+            "    }",
+            "  }",
+            "  private void bar() {}",
+            "}")
+        .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion")
+        .doTest();
+
+    refactoringHelper
+        .addInputLines(
+            "Test.java",
+            "class Test {",
+            "  enum Suit {HEARTS, CLUBS, SPADES, DIAMONDS};",
+            "  public Test() {}",
+            "  private void foo(Suit suit) {",
+            "    switch(suit) {",
+            "      case HEARTS:",
+            "        // A comment here",
+            "        // more comments.",
+            "      case DIAMONDS:",
+            "        System.out.println(\"Heart or diamond\");",
+            "        break;",
+            "      case SPADES:",
+            "        // Fall through",
+            "      case CLUBS:",
+            "        bar();",
+            "        System.out.println(\"Black suit\");",
+            "    }",
+            "  }",
+            "  private void bar() {}",
+            "}")
+        .addOutputLines(
+            "Test.java",
+            "class Test {",
+            "  enum Suit {HEARTS, CLUBS, SPADES, DIAMONDS};",
+            "  public Test() {}",
+            "  private void foo(Suit suit) {",
+            "    switch(suit) {",
+            "      case HEARTS, DIAMONDS -> ",
+            "        // A comment here",
+            "        // more comments.",
+            "        System.out.println(\"Heart or diamond\");",
+            "      ",
+            "      case SPADES, CLUBS -> {",
+            "        bar();",
+            "        System.out.println(\"Black suit\");",
+            "       }",
+            "    }",
+            "  }",
+            "  private void bar() {}",
+            "}")
+        .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion")
+        .doTest();
+  }
+
   /**********************************
    *
    * Return switch test cases