From dc4c426ef3efd8625e583d4a734e5417dfc768d3 Mon Sep 17 00:00:00 2001 From: Error Prone Team Date: Fri, 31 May 2024 07:35:49 -0700 Subject: [PATCH] Enable StatementSwitchToExpressionSwitch analysis phase PiperOrigin-RevId: 639030266 --- .../StatementSwitchToExpressionSwitch.java | 13 +-- .../scanner/BuiltInCheckerSuppliers.java | 2 +- ...StatementSwitchToExpressionSwitchTest.java | 87 ++++++++++++++++++- 3 files changed, 86 insertions(+), 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 index f8136a249bb0..7a56e2bdf07b 100644 --- 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 statementTrees, - String transformedBlockSource, - StatementTree firstStatement, - VisitorState state) { + ImmutableList 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 index 9b4454ba74ec..8929178147fd 100644 --- 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/core/src/test/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitchTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitchTest.java index 7a8eef5ac386..8bc5c26f00a6 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitchTest.java +++ b/core/src/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