From d88730767480d5c9ac190c4960f783e79c22c457 Mon Sep 17 00:00:00 2001
From: Liam Miller-Cushon <cushon@google.com>
Date: Thu, 1 Aug 2024 16:30:08 -0700
Subject: [PATCH] Omit some unnecessary break statements when translating to
 `->` switches

PiperOrigin-RevId: 658578239
---
 .../StatementSwitchToExpressionSwitch.java    | 14 +++----
 ...StatementSwitchToExpressionSwitchTest.java | 38 ++++++++++++++++++-
 2 files changed, 42 insertions(+), 10 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
index cf3a3ca05c7..39b5b7585d0 100644
--- a/core/src/main/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitch.java
+++ b/core/src/main/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitch.java
@@ -36,7 +36,6 @@
 import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Iterables;
 import com.google.common.collect.Streams;
 import com.google.errorprone.BugPattern;
 import com.google.errorprone.ErrorProneFlags;
@@ -585,7 +584,7 @@ private static SuggestedFix convertDirectlyToExpressionSwitch(
                     groupedCaseCommentsAccumulator.length() == 0
                         ? null
                         : groupedCaseCommentsAccumulator.toString(),
-                    transformedBlockSource.isEmpty() ? null : transformedBlockSource,
+                    transformedBlockSource.isEmpty() ? null : transformedBlockSource.trim(),
                     commentsBeforeRemovedBreak.orElse(null),
                     commentsAfterCaseOptional.orElse(null));
       }
@@ -902,7 +901,7 @@ && getStatements(caseTree).size() > filteredStatements.size()) {
           state
               .getSourceCode()
               .subSequence(
-                  state.getEndPosition(Iterables.getLast(filteredStatements)),
+                  state.getEndPosition(getLast(filteredStatements)),
                   getStartPosition(getStatements(caseTree).get(getStatements(caseTree).size() - 1)))
               .toString()
               .trim();
@@ -952,11 +951,10 @@ private static String transformBlock(
 
     StringBuilder transformedBlockBuilder = new StringBuilder();
     int codeBlockStart = extractLhsComments(caseTree, state, transformedBlockBuilder);
-    int codeBlockEnd =
-        filteredStatements.isEmpty()
-            ? getBlockEnd(state, caseTree)
-            : state.getEndPosition(Streams.findLast(filteredStatements.stream()).get());
-    transformedBlockBuilder.append(state.getSourceCode(), codeBlockStart, codeBlockEnd);
+    if (!filteredStatements.isEmpty()) {
+      int codeBlockEnd = state.getEndPosition(getLast(filteredStatements));
+      transformedBlockBuilder.append(state.getSourceCode(), codeBlockStart, codeBlockEnd);
+    }
 
     return transformedBlockBuilder.toString();
   }
diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitchTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitchTest.java
index eab2d16735b..be63a77f336 100644
--- a/core/src/test/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitchTest.java
+++ b/core/src/test/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitchTest.java
@@ -361,7 +361,6 @@ public void switchByEnumCard2_removesRedundantBreaks_error() {
             "          // Post break comment",
             "       case DIAMOND -> {",
             "          // Diamond break comment",
-            "          break;",
             "       }",
             "       case SPADE, CLUB -> System.out.println(\"everything else\");",
             "    }",
@@ -1385,7 +1384,6 @@ public void switchByEnum_surroundingBracesCannotRemove_error() {
             "    switch(side) {",
             "      case OBVERSE -> {",
             "        // The quick brown fox, jumps over the lazy dog, etc.",
-            "        break;",
             "      }",
             "      default -> ",
             "        throw new RuntimeException(\"Invalid type.\");",
@@ -3623,4 +3621,40 @@ public void i4222() {
         .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion=true")
         .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH);
   }
+
+  @Test
+  public void unnecessaryBreaks() {
+    assumeTrue(RuntimeVersion.isAtLeast14());
+    refactoringHelper
+        .addInputLines(
+            "Test.java",
+            "public class Test {",
+            "  public static void main(String[] args) {",
+            "    switch (args.length) {",
+            "      case 0:",
+            "        System.out.println(0);",
+            "        break;",
+            "      default:",
+            "        // hello",
+            "        // world",
+            "        break;",
+            "    }",
+            "  }",
+            "}")
+        .addOutputLines(
+            "Test.java",
+            "public class Test {",
+            "  public static void main(String[] args) {",
+            "    switch (args.length) {",
+            "      case 0 -> System.out.println(0);",
+            "      default -> {",
+            "        // hello",
+            "        // world",
+            "      }",
+            "    }",
+            "  }",
+            "}")
+        .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion=true")
+        .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH);
+  }
 }