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 1ed0b7db810..3b182ddcac0 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitch.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitch.java @@ -112,7 +112,6 @@ public final class StatementSwitchToExpressionSwitch extends BugChecker AssignmentSwitchAnalysisResult.of( /* canConvertToAssignmentSwitch= */ false, /* canCombineWithPrecedingVariableDeclaration= */ false, - /* canRemoveDefault= */ false, /* assignmentTargetOptional= */ Optional.empty(), /* assignmentKindOptional= */ Optional.empty(), /* assignmentSourceCodeOptional= */ Optional.empty()); @@ -121,11 +120,14 @@ public final class StatementSwitchToExpressionSwitch extends BugChecker AnalysisResult.of( /* canConvertDirectlyToExpressionSwitch= */ false, /* canConvertToReturnSwitch= */ false, + /* canRemoveDefault= */ false, DEFAULT_ASSIGNMENT_SWITCH_ANALYSIS_RESULT, /* groupedWithNextCase= */ ImmutableList.of()); private static final String EQUALS_STRING = "="; private static final Matcher COMPILE_TIME_CONSTANT_MATCHER = CompileTimeConstantExpressionMatcher.instance(); + private static final String REMOVE_DEFAULT_CASE_SHORT_DESCRIPTION = + "Remove default case because all enum values handled"; // Tri-state to represent the fall-thru control flow of a particular case of a particular // statement switch @@ -171,21 +173,35 @@ public Description matchSwitch(SwitchTree switchTree, VisitorState state) { List suggestedFixes = new ArrayList<>(); if (enableReturnSwitchConversion && analysisResult.canConvertToReturnSwitch()) { - suggestedFixes.add(convertToReturnSwitch(switchTree, state, analysisResult)); + suggestedFixes.add( + convertToReturnSwitch(switchTree, state, analysisResult, /* removeDefault= */ false)); + + if (analysisResult.canRemoveDefault()) { + suggestedFixes.add( + convertToReturnSwitch(switchTree, state, analysisResult, /* removeDefault= */ true)); + } } if (enableAssignmentSwitchConversion && analysisResult.assignmentSwitchAnalysisResult().canConvertToAssignmentSwitch()) { suggestedFixes.add( convertToAssignmentSwitch(switchTree, state, analysisResult, /* removeDefault= */ false)); - if (analysisResult.assignmentSwitchAnalysisResult().canRemoveDefault()) { + if (analysisResult.canRemoveDefault()) { suggestedFixes.add( convertToAssignmentSwitch( switchTree, state, analysisResult, /* removeDefault= */ true)); } } if (enableDirectConversion && analysisResult.canConvertDirectlyToExpressionSwitch()) { - suggestedFixes.add(convertDirectlyToExpressionSwitch(switchTree, state, analysisResult)); + suggestedFixes.add( + convertDirectlyToExpressionSwitch( + switchTree, state, analysisResult, /* removeDefault= */ false)); + + if (analysisResult.canRemoveDefault()) { + suggestedFixes.add( + convertDirectlyToExpressionSwitch( + switchTree, state, analysisResult, /* removeDefault= */ true)); + } } return suggestedFixes.isEmpty() @@ -344,10 +360,10 @@ && isSwitchExhaustiveWithoutDefault( return AnalysisResult.of( /* canConvertDirectlyToExpressionSwitch= */ allCasesHaveDefiniteControlFlow, canConvertToReturnSwitch, + canRemoveDefault, AssignmentSwitchAnalysisResult.of( canConvertToAssignmentSwitch, canCombineWithPrecedingVariableDeclaration, - canRemoveDefault, assignmentSwitchAnalysisState.assignmentTargetOptional(), assignmentSwitchAnalysisState.assignmentExpressionKindOptional(), assignmentSwitchAnalysisState @@ -631,10 +647,14 @@ private static boolean areStatementsConvertibleToExpressionSwitch( /** * Transforms the supplied statement switch into an expression switch directly. In this * conversion, each nontrivial statement block is mapped one-to-one to a new {@code Expression} or - * {@code StatementBlock} on the right-hand side. Comments are preserved where possible. + * {@code StatementBlock} on the right-hand side (the `default:` case is removed if {@code + * removeDefault} is true). Comments are preserved where possible. */ private static SuggestedFix convertDirectlyToExpressionSwitch( - SwitchTree switchTree, VisitorState state, AnalysisResult analysisResult) { + SwitchTree switchTree, + VisitorState state, + AnalysisResult analysisResult, + boolean removeDefault) { List cases = switchTree.getCases(); ImmutableList allSwitchComments = @@ -654,6 +674,11 @@ private static SuggestedFix convertDirectlyToExpressionSwitch( CaseTree caseTree = cases.get(caseIndex); boolean isDefaultCase = isSwitchDefault(caseTree); + if (removeDefault && isDefaultCase) { + // Skip default case + continue; + } + // For readability, filter out trailing unlabelled break statement because these become a // No-Op when used inside expression switches ImmutableList filteredStatements = filterOutRedundantBreak(caseTree); @@ -748,7 +773,12 @@ private static SuggestedFix convertDirectlyToExpressionSwitch( // Close the switch statement replacementCodeBuilder.append("\n}"); - return SuggestedFix.builder().replace(switchTree, replacementCodeBuilder.toString()).build(); + SuggestedFix.Builder suggestedFixBuilder = SuggestedFix.builder(); + if (removeDefault) { + suggestedFixBuilder.setShortDescription(REMOVE_DEFAULT_CASE_SHORT_DESCRIPTION); + } + suggestedFixBuilder.replace(switchTree, replacementCodeBuilder.toString()); + return suggestedFixBuilder.build(); } /** @@ -759,7 +789,10 @@ private static SuggestedFix convertDirectlyToExpressionSwitch( * conversion is possible. */ private static SuggestedFix convertToReturnSwitch( - SwitchTree switchTree, VisitorState state, AnalysisResult analysisResult) { + SwitchTree switchTree, + VisitorState state, + AnalysisResult analysisResult, + boolean removeDefault) { List statementsToDelete = new ArrayList<>(); List cases = switchTree.getCases(); @@ -778,6 +811,10 @@ private static SuggestedFix convertToReturnSwitch( for (int caseIndex = 0; caseIndex < cases.size(); caseIndex++) { CaseTree caseTree = cases.get(caseIndex); boolean isDefaultCase = isSwitchDefault(caseTree); + if (removeDefault && isDefaultCase) { + // Skip default case + continue; + } String transformedBlockSource = transformReturnOrThrowBlock(caseTree, state, getStatements(caseTree)); @@ -849,8 +886,11 @@ private static SuggestedFix convertToReturnSwitch( // removed. statementsToDelete.addAll(followingStatementsInBlock(switchTree, state)); - SuggestedFix.Builder suggestedFixBuilder = - SuggestedFix.builder().replace(switchTree, replacementCodeBuilder.toString()); + SuggestedFix.Builder suggestedFixBuilder = SuggestedFix.builder(); + if (removeDefault) { + suggestedFixBuilder.setShortDescription(REMOVE_DEFAULT_CASE_SHORT_DESCRIPTION); + } + suggestedFixBuilder.replace(switchTree, replacementCodeBuilder.toString()); // Delete trailing statements, leaving comments where feasible statementsToDelete.forEach(deleteMe -> suggestedFixBuilder.replace(deleteMe, "")); return suggestedFixBuilder.build(); @@ -1125,8 +1165,7 @@ private static SuggestedFix convertToAssignmentSwitch( SuggestedFix.Builder suggestedFixBuilder = SuggestedFix.builder(); if (removeDefault) { - suggestedFixBuilder.setShortDescription( - "Remove default case because all enum values handled"); + suggestedFixBuilder.setShortDescription(REMOVE_DEFAULT_CASE_SHORT_DESCRIPTION); } suggestedFixBuilder.replace(switchTree, replacementCodeBuilder.toString()); statementsToDelete.forEach(deleteMe -> suggestedFixBuilder.replace(deleteMe, "")); @@ -1470,6 +1509,12 @@ abstract static class AnalysisResult { // Whether the statement switch can be converted to a return switch abstract boolean canConvertToReturnSwitch(); + /** + * Whether the assignment switch is exhaustive even in the absence of the default case that + * exists in the original switch statement + */ + abstract boolean canRemoveDefault(); + // Results of the analysis for conversion to an assignment switch abstract AssignmentSwitchAnalysisResult assignmentSwitchAnalysisResult(); @@ -1479,11 +1524,13 @@ abstract static class AnalysisResult { static AnalysisResult of( boolean canConvertDirectlyToExpressionSwitch, boolean canConvertToReturnSwitch, + boolean canRemoveDefault, AssignmentSwitchAnalysisResult assignmentSwitchAnalysisResult, ImmutableList groupedWithNextCase) { return new AutoValue_StatementSwitchToExpressionSwitch_AnalysisResult( canConvertDirectlyToExpressionSwitch, canConvertToReturnSwitch, + canRemoveDefault, assignmentSwitchAnalysisResult, groupedWithNextCase); } @@ -1498,10 +1545,6 @@ abstract static class AssignmentSwitchAnalysisResult { // declaration abstract boolean canCombineWithPrecedingVariableDeclaration(); - // Whether the assignment switch is exhaustive even in the absence of the default case that - // exists in the original switch statement - abstract boolean canRemoveDefault(); - // Target of the assignment switch, if any abstract Optional assignmentTargetOptional(); @@ -1514,14 +1557,12 @@ abstract static class AssignmentSwitchAnalysisResult { static AssignmentSwitchAnalysisResult of( boolean canConvertToAssignmentSwitch, boolean canCombineWithPrecedingVariableDeclaration, - boolean canRemoveDefault, Optional assignmentTargetOptional, Optional assignmentKindOptional, Optional assignmentSourceCodeOptional) { return new AutoValue_StatementSwitchToExpressionSwitch_AssignmentSwitchAnalysisResult( canConvertToAssignmentSwitch, canCombineWithPrecedingVariableDeclaration, - canRemoveDefault, assignmentTargetOptional, assignmentKindOptional, assignmentSourceCodeOptional); 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 b1649121cd4..4adf726cde9 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitchTest.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitchTest.java @@ -16,11 +16,13 @@ package com.google.errorprone.bugpatterns; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.Iterables; +import static com.google.common.truth.Truth.assertThat; + import com.google.errorprone.BugCheckerRefactoringTestHelper; import com.google.errorprone.BugCheckerRefactoringTestHelper.FixChoosers; import com.google.errorprone.CompilationTestHelper; +import com.google.errorprone.fixes.Fix; +import java.util.List; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -68,8 +70,7 @@ public void foo(Side side) { } } """) - .setArgs( - ImmutableList.of("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion") .doTest(); // Check correct generated code @@ -131,8 +132,8 @@ public void foo(Side side) { } } """) - .setArgs( - ImmutableList.of("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion") + .setFixChooser(StatementSwitchToExpressionSwitchTest::assertOneFixAndChoose) .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); } @@ -166,8 +167,7 @@ public void foo(Side side) { } } """) - .setArgs( - ImmutableList.of("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion") .doTest(); // Check correct generated code @@ -224,8 +224,169 @@ public void foo(Side side) { } } """) - .setArgs( - ImmutableList.of("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion") + .setFixChooser(StatementSwitchToExpressionSwitchTest::assertOneFixAndChoose) + .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); + } + + @Test + public void switchByEnumExhaustiveWithoutDefault_removesDefault_error() { + // The switch covers all enum values and also includes a default clause, so assert that a + // secondary fix is generated to remove the default clause. + helper + .addSourceLines( + "Test.java", + """ + class Test { + enum Side { + OBVERSE, + REVERSE + }; + + public Test(int foo) {} + + public void foo(Side side) { + // BUG: Diagnostic contains: [StatementSwitchToExpressionSwitch] + switch (side) { + // Comment before first case + case OBVERSE: + // Explanatory comment + System.out.println("this block cannot complete normally"); + { + throw new NullPointerException(); + } + case REVERSE: + System.out.println("reverse"); + break; + default: + System.out.println("default"); + } + } + } + """) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion") + .doTest(); + + // Check correct generated code + refactoringHelper + .addInputLines( + "Test.java", + """ + class Test { + enum Side { + OBVERSE, + REVERSE + }; + + public Test(int foo) {} + + public void foo(Side side) { + switch (side) { + // Comment before first case + case OBVERSE: + // Explanatory comment + System.out.println("this block cannot complete normally"); + { + throw new NullPointerException(); + } + case REVERSE: + System.out.println("reverse"); + break; + default: + System.out.println("default"); + } + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + enum Side { + OBVERSE, + REVERSE + }; + + public Test(int foo) {} + + public void foo(Side side) { + switch (side) { + case OBVERSE -> { + // Comment before first case + // Explanatory comment + System.out.println("this block cannot complete normally"); + { + throw new NullPointerException(); + } + } + case REVERSE -> System.out.println("reverse"); + default -> System.out.println("default"); + } + } + } + """) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion") + .setFixChooser(FixChoosers.FIRST) + .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); + + refactoringHelper2 + .addInputLines( + "Test.java", + """ + class Test { + enum Side { + OBVERSE, + REVERSE + }; + + public Test(int foo) {} + + public void foo(Side side) { + switch (side) { + // Comment before first case + case OBVERSE: + // Explanatory comment + System.out.println("this block cannot complete normally"); + { + throw new NullPointerException(); + } + case REVERSE: + System.out.println("reverse"); + break; + default: + System.out.println("default"); + } + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + enum Side { + OBVERSE, + REVERSE + }; + + public Test(int foo) {} + + public void foo(Side side) { + switch (side) { + case OBVERSE -> { + // Comment before first case + // Explanatory comment + System.out.println("this block cannot complete normally"); + { + throw new NullPointerException(); + } + } + case REVERSE -> System.out.println("reverse"); + } + } + } + """) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion") + .setFixChooser(FixChoosers.SECOND) .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); } @@ -264,8 +425,7 @@ public void foo(Side side) { } } """) - .setArgs( - ImmutableList.of("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion") .doTest(); // Check correct generated code @@ -330,8 +490,8 @@ public void foo(Side side) { } } """) - .setArgs( - ImmutableList.of("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion") + .setFixChooser(StatementSwitchToExpressionSwitchTest::assertOneFixAndChoose) .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); } @@ -369,8 +529,7 @@ public void foo(Side side) { } } """) - .setArgs( - ImmutableList.of("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion") .doTest(); // Check correct generated code @@ -433,8 +592,8 @@ public void foo(Side side) { } } """) - .setArgs( - ImmutableList.of("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion") + .setFixChooser(StatementSwitchToExpressionSwitchTest::assertOneFixAndChoose) .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); } @@ -472,8 +631,7 @@ public void foo(Side side) { } } """) - .setArgs( - ImmutableList.of("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion") .doTest(); // Check correct generated code @@ -537,8 +695,8 @@ public void foo(Side side) { } } """) - .setArgs( - ImmutableList.of("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion") + .setFixChooser(StatementSwitchToExpressionSwitchTest::assertOneFixAndChoose) .doTest(); } @@ -572,8 +730,7 @@ public void foo(Side side) { } } """) - .setArgs( - ImmutableList.of("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion") .doTest(); } @@ -611,8 +768,7 @@ public void foo(Side side) { } } """) - .setArgs( - ImmutableList.of("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion") .doTest(); } @@ -651,8 +807,7 @@ public void foo(Side side) { } } """) - .setArgs( - ImmutableList.of("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion") .doTest(); // This check does not attempt to re-order cases, for example to move the default to the end, as @@ -716,8 +871,8 @@ public void foo(Side side) { } } """) - .setArgs( - ImmutableList.of("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion") + .setFixChooser(StatementSwitchToExpressionSwitchTest::assertOneFixAndChoose) .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); } @@ -755,8 +910,7 @@ public void foo(Side side) { } } """) - .setArgs( - ImmutableList.of("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion") .doTest(); // Check correct generated code @@ -821,8 +975,8 @@ public void foo(Side side) { } } """) - .setArgs( - ImmutableList.of("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion") + .setFixChooser(StatementSwitchToExpressionSwitchTest::assertOneFixAndChoose) .doTest(); } @@ -855,8 +1009,7 @@ public void foo(Side side) { } } """) - .setArgs( - ImmutableList.of("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion") .doTest(); // Check correct generated code @@ -912,8 +1065,8 @@ public void foo(Side side) { } } """) - .setArgs( - ImmutableList.of("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion") + .setFixChooser(StatementSwitchToExpressionSwitchTest::assertOneFixAndChoose) .doTest(); } @@ -948,8 +1101,7 @@ public void foo(Side side) { } } """) - .setArgs( - ImmutableList.of("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion") .doTest(); } @@ -992,8 +1144,7 @@ public void foo(Side side) { } } """) - .setArgs( - ImmutableList.of("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion") .doTest(); } @@ -1031,8 +1182,7 @@ public void foo(Side side) { } } """) - .setArgs( - ImmutableList.of("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion") .doTest(); // Check correct generated code @@ -1089,8 +1239,8 @@ public void foo(Side side) { } } """) - .setArgs( - ImmutableList.of("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion") + .setFixChooser(StatementSwitchToExpressionSwitchTest::assertOneFixAndChoose) .doTest(); } @@ -1131,8 +1281,7 @@ public void foo(Side side) { } } """) - .setArgs( - ImmutableList.of("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion") .doTest(); } @@ -1168,8 +1317,7 @@ public void foo(Side side) { } } """) - .setArgs( - ImmutableList.of("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion") .doTest(); } @@ -1199,8 +1347,7 @@ public void switchByEnumWithLambda_noError() { " }", " }", "}") - .setArgs( - ImmutableList.of("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion") .doTest(); } @@ -1230,8 +1377,7 @@ public void foo(Side side) { } } """) - .setArgs( - ImmutableList.of("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion") .doTest(); } @@ -1289,8 +1435,7 @@ private static Throwable bar() { } } """) - .setArgs( - ImmutableList.of("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion") .doTest(); } @@ -1395,6 +1540,7 @@ private void bar() {} } """) .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion") + .setFixChooser(StatementSwitchToExpressionSwitchTest::assertOneFixAndChoose) .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); } @@ -1505,6 +1651,7 @@ private void bar() {} } """) .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion") + .setFixChooser(StatementSwitchToExpressionSwitchTest::assertOneFixAndChoose) .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); } @@ -1615,6 +1762,7 @@ private void bar() {} } """) .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion") + .setFixChooser(StatementSwitchToExpressionSwitchTest::assertOneFixAndChoose) .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); } @@ -1704,8 +1852,8 @@ public void foo(Side side) { } } """) - .setArgs( - ImmutableList.of("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion") + .setFixChooser(StatementSwitchToExpressionSwitchTest::assertOneFixAndChoose) .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); } @@ -1794,8 +1942,8 @@ public void foo(Side side) { } } """) - .setArgs( - ImmutableList.of("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion") + .setFixChooser(StatementSwitchToExpressionSwitchTest::assertOneFixAndChoose) .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); } @@ -1829,8 +1977,7 @@ public int foo(Suit suit) { } } """) - .setArgs( - ImmutableList.of("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion") .doTest(); refactoringHelper @@ -1893,6 +2040,91 @@ public int foo(Suit suit) { } """) .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion") + .setFixChooser(StatementSwitchToExpressionSwitchTest::assertOneFixAndChoose) + .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); + } + + @Test + public void i4222() { + refactoringHelper + .addInputLines( + "Test.java", + """ + public class Test { + public static void main(String[] args) { + switch (args.length) { + case 0: + { + System.out.println(0); + break; + } + case 1: + System.out.println(1); + break; + case 2: + System.out.println(2); + System.out.println(2); + break; + } + } + } + """) + .addOutputLines( + "Test.java", + """ + public class Test { + public static void main(String[] args) { + switch (args.length) { + case 0 -> System.out.println(0); + case 1 -> System.out.println(1); + case 2 -> { + System.out.println(2); + System.out.println(2); + } + } + } + } + """) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion=true") + .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); + } + + @Test + public void unnecessaryBreaks() { + 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); } @@ -1936,9 +2168,7 @@ public int foo(Side side) { } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion") .doTest(); // Check correct generated code @@ -1999,9 +2229,8 @@ public int foo(Side side) { } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion") + .setFixChooser(StatementSwitchToExpressionSwitchTest::assertOneFixAndChoose) .doTest(); } @@ -2072,9 +2301,8 @@ public int foo(Side side) { } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion") + .setFixChooser(StatementSwitchToExpressionSwitchTest::assertOneFixAndChoose) .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); } @@ -2151,14 +2379,13 @@ public int foo(Side side) { } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion") + .setFixChooser(StatementSwitchToExpressionSwitchTest::assertOneFixAndChoose) .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); } @Test - public void switchByEnum_switchInReturnSwitchWithShouldNeverHappen_error() { + public void switchByEnum_switchInReturnSwitchWithShouldNeverHappen_noError() { // No error because the inner switch is the only fixable one helper .addSourceLines( @@ -2203,9 +2430,7 @@ public int foo(Side side) { } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion") .doTest(); } @@ -2244,9 +2469,7 @@ public int foo(Side side) { } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion") .doTest(); refactoringHelper @@ -2309,9 +2532,8 @@ public int foo(Side side) { } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion") + .setFixChooser(StatementSwitchToExpressionSwitchTest::assertOneFixAndChoose) .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); } @@ -2346,21 +2568,205 @@ public int foo(Side side) { default: // Fall through } - return -2; + return -2; + } + } + """) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion") + .doTest(); + } + + @Test + public void switchByEnum_alwaysThrows_noError() { + // Every case throws, thus no type for return switch + helper + .addSourceLines( + "Test.java", + """ + class Test { + enum Side { + HEART, + SPADE, + DIAMOND, + CLUB + }; + + public Test(int foo) {} + + public int invoke() { + return 123; + } + + public int foo(Side side) { + switch (side) { + case HEART: + case DIAMOND: + throw new NullPointerException(); + case SPADE: + throw new RuntimeException(); + default: + throw new NullPointerException(); + } + } + } + """) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion") + .doTest(); + } + + @Test + public void switchByEnum_returnSwitchWithShouldNeverHappen_errorAndRemoveShouldNeverHappen() { + // The switch has a case for each enum and "should never happen" error handling + helper + .addSourceLines( + "Test.java", + """ + class Test { + enum Side { + HEART, + SPADE, + DIAMOND, + CLUB + }; + + public Test(int foo) {} + + public int invoke() { + return 123; + } + + public int foo(Side side) { + System.out.println("don't delete 0"); + if (invoke() > 0) { + System.out.println("don't delete 1"); + // Preceding comment + // BUG: Diagnostic contains: [StatementSwitchToExpressionSwitch] + switch (side) { + case HEART: + // Fall through + case DIAMOND: + return invoke(); + case SPADE: + throw new RuntimeException(); + case CLUB: + throw new NullPointerException(); + } + // Custom comment - should never happen + int z = invoke(/* block comment 0 */ ); + // Custom comment 2 + { + z++; + } + throw new RuntimeException("Switch was not exhaustive at runtime " + z); + } + System.out.println("don't delete 2"); + return 0; + } + } + """) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion") + .doTest(); + + refactoringHelper + .addInputLines( + "Test.java", + """ + class Test { + enum Side { + HEART, + SPADE, + DIAMOND, + CLUB + }; + + public Test(int foo) {} + + public int invoke() { + return 123; + } + + public int foo(Side side) { + System.out.println("don't delete 0"); + if (invoke() > 0) { + System.out.println("don't delete 1"); + // Preceding comment + switch (side) { + case HEART /* lhs comment */: // rhs comment + // Fall through + case DIAMOND: + return invoke(); + case SPADE: + throw new RuntimeException(); + case CLUB: + throw new NullPointerException(); + } + // Custom comment - should never happen + int z = invoke(/* block comment 0 */ ); + // Custom comment 2 + { + z++; + } + throw new RuntimeException("Switch was not exhaustive at runtime " + z); + } + System.out.println("don't delete 2"); + return 0; + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + enum Side { + HEART, + SPADE, + DIAMOND, + CLUB + }; + + public Test(int foo) {} + + public int invoke() { + return 123; + } + + public int foo(Side side) { + System.out.println("don't delete 0"); + if (invoke() > 0) { + System.out.println("don't delete 1"); + // Preceding comment + return switch (side) { + case HEART, DIAMOND -> + /* lhs comment */ + // rhs comment + invoke(); + case SPADE -> throw new RuntimeException(); + case CLUB -> throw new NullPointerException(); + }; + // Custom comment - should never happen + + // Custom comment 2 + + } + System.out.println("don't delete 2"); + return 0; } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion")) - .doTest(); + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion") + .setFixChooser(StatementSwitchToExpressionSwitchTest::assertOneFixAndChoose) + .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); } @Test - public void switchByEnum_alwaysThrows_noError() { - // Every case throws, thus no type for return switch - helper - .addSourceLines( + public void switchByEnum_returnSwitchWithAllEnumValuesAndDefault_errorRemoveDefault() { + // The return switch has a case for each enum value *and* a default case handler. This test + // asserts that a secondary fix is proposed to remove the default case. Note that the original + // code cannot have a "should never happen" (after the statement switch) because the compiler + // will deduce that such code is unreachable. + + refactoringHelper + .addInputLines( "Test.java", """ class Test { @@ -2378,29 +2784,30 @@ public int invoke() { } public int foo(Side side) { - switch (side) { - case HEART: - case DIAMOND: - throw new NullPointerException(); - case SPADE: - throw new RuntimeException(); - default: - throw new NullPointerException(); + System.out.println("don't delete 0"); + if (invoke() > 0) { + System.out.println("don't delete 1"); + // Preceding comment + switch (side) { + case HEART /* lhs comment */: // rhs comment + // Fall through + case DIAMOND: + return invoke(); + case SPADE: + throw new RuntimeException(); + case CLUB: + throw new NullPointerException(); + default: + throw new NullPointerException(); + } + // Unreachable - no "should never happen" code } + System.out.println("don't delete 2"); + return 0; } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion")) - .doTest(); - } - - @Test - public void switchByEnum_returnSwitchWithShouldNeverHappen_errorAndRemoveShouldNeverHappen() { - // The switch has a case for each enum and "should never happen" error handling - helper - .addSourceLines( + .addOutputLines( "Test.java", """ class Test { @@ -2422,36 +2829,27 @@ public int foo(Side side) { if (invoke() > 0) { System.out.println("don't delete 1"); // Preceding comment - // BUG: Diagnostic contains: [StatementSwitchToExpressionSwitch] - switch (side) { - case HEART: - // Fall through - case DIAMOND: - return invoke(); - case SPADE: - throw new RuntimeException(); - case CLUB: - throw new NullPointerException(); - } - // Custom comment - should never happen - int z = invoke(/* block comment 0 */ ); - // Custom comment 2 - { - z++; - } - throw new RuntimeException("Switch was not exhaustive at runtime " + z); + return switch (side) { + case HEART, DIAMOND -> + /* lhs comment */ + // rhs comment + invoke(); + case SPADE -> throw new RuntimeException(); + case CLUB -> throw new NullPointerException(); + default -> throw new NullPointerException(); + }; + // Unreachable - no "should never happen" code } System.out.println("don't delete 2"); return 0; } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion")) - .doTest(); + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion") + .setFixChooser(FixChoosers.FIRST) + .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); - refactoringHelper + refactoringHelper2 .addInputLines( "Test.java", """ @@ -2483,14 +2881,10 @@ public int foo(Side side) { throw new RuntimeException(); case CLUB: throw new NullPointerException(); + default: + throw new NullPointerException(); } - // Custom comment - should never happen - int z = invoke(/* block comment 0 */ ); - // Custom comment 2 - { - z++; - } - throw new RuntimeException("Switch was not exhaustive at runtime " + z); + // Unreachable - no "should never happen" code } System.out.println("don't delete 2"); return 0; @@ -2527,19 +2921,15 @@ public int foo(Side side) { case SPADE -> throw new RuntimeException(); case CLUB -> throw new NullPointerException(); }; - // Custom comment - should never happen - - // Custom comment 2 - + // Unreachable - no "should never happen" code } System.out.println("don't delete 2"); return 0; } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion") + .setFixChooser(FixChoosers.SECOND) .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); } @@ -2591,9 +2981,7 @@ public int foo(Side side) { } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion") .doTest(); refactoringHelper @@ -2679,9 +3067,8 @@ public int foo(Side side) { } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion") + .setFixChooser(StatementSwitchToExpressionSwitchTest::assertOneFixAndChoose) .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); } @@ -2736,9 +3123,7 @@ public int foo(Side side) { } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion") .doTest(); refactoringHelper @@ -2837,9 +3222,8 @@ public int foo(Side side) { } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion") + .setFixChooser(StatementSwitchToExpressionSwitchTest::assertOneFixAndChoose) .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); } @@ -2928,9 +3312,8 @@ public int foo(Side side) { } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion") + .setFixChooser(StatementSwitchToExpressionSwitchTest::assertOneFixAndChoose) .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); } @@ -2965,9 +3348,7 @@ public void foo(Side side) { } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion") .doTest(); } @@ -3005,9 +3386,7 @@ public int foo(Side side) { } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion") .doTest(); } @@ -3045,9 +3424,7 @@ public int foo(Side side) { } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion") .doTest(); } @@ -3086,9 +3463,7 @@ public int foo(Side side) { } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion") .doTest(); } @@ -3123,9 +3498,7 @@ public int foo(Side side) { } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion") .doTest(); } @@ -3201,10 +3574,8 @@ public int foo(Side side) { } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion")) - .setFixChooser(Iterables::getOnlyElement) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion") + .setFixChooser(StatementSwitchToExpressionSwitchTest::assertOneFixAndChoose) .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); } @@ -3331,10 +3702,8 @@ public int foo(Side side) { } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion")) - .setFixChooser(Iterables::getOnlyElement) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion") + .setFixChooser(StatementSwitchToExpressionSwitchTest::assertOneFixAndChoose) .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); } @@ -3401,10 +3770,8 @@ public int foo(Side side) { } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion")) - .setFixChooser(Iterables::getOnlyElement) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion") + .setFixChooser(StatementSwitchToExpressionSwitchTest::assertOneFixAndChoose) .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); } @@ -3473,10 +3840,8 @@ public int foo(Side side) { } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion")) - .setFixChooser(Iterables::getOnlyElement) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion") + .setFixChooser(StatementSwitchToExpressionSwitchTest::assertOneFixAndChoose) .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); } @@ -3552,10 +3917,8 @@ public int foo(Side side) { } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion")) - .setFixChooser(Iterables::getOnlyElement) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion") + .setFixChooser(StatementSwitchToExpressionSwitchTest::assertOneFixAndChoose) .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); } @@ -3631,10 +3994,8 @@ public int foo(Side side) { } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion")) - .setFixChooser(Iterables::getOnlyElement) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion") + .setFixChooser(StatementSwitchToExpressionSwitchTest::assertOneFixAndChoose) .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); } @@ -3679,9 +4040,7 @@ public int foo(Side side) { } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion") .doTest(); // Check correct generated code. @@ -3756,10 +4115,8 @@ public int foo(Side side) { } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion")) - .setFixChooser(Iterables::getOnlyElement) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion") + .setFixChooser(StatementSwitchToExpressionSwitchTest::assertOneFixAndChoose) .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); } @@ -3802,9 +4159,7 @@ public int foo(Side side) { } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion") .doTest(); } @@ -3847,9 +4202,7 @@ public int foo(Side side) { } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion") .doTest(); } @@ -3890,9 +4243,7 @@ public int[] foo(Side side) { } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion") .doTest(); // Check correct generated code @@ -3960,10 +4311,8 @@ public int[] foo(Side side) { } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion")) - .setFixChooser(Iterables::getOnlyElement) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion") + .setFixChooser(StatementSwitchToExpressionSwitchTest::assertOneFixAndChoose) .doTest(); } @@ -4008,9 +4357,7 @@ public int[] foo(Side side) { } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion") .doTest(); } @@ -4053,9 +4400,7 @@ public int[] foo(Side side) { } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion") .doTest(); } @@ -4098,9 +4443,7 @@ public int foo(Side side) { } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion") .doTest(); } @@ -4146,9 +4489,7 @@ public int foo(Side side) { } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion") .doTest(); } @@ -4195,9 +4536,7 @@ public int foo(Side side) { } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion") .doTest(); } @@ -4245,9 +4584,7 @@ public int foo(Side side) { } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion") .doTest(); } @@ -4293,9 +4630,7 @@ public int foo(Side side) { } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion") .doTest(); } @@ -4333,9 +4668,7 @@ public int foo(Side side) { } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion") .doTest(); } @@ -4381,9 +4714,7 @@ public int foo(Side side) { } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion") .doTest(); refactoringHelper @@ -4445,12 +4776,10 @@ public int foo(Side side) { } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion") // There should be no second fix that attempts to remove the default case because there is // no default case. - .setFixChooser(Iterables::getOnlyElement) + .setFixChooser(StatementSwitchToExpressionSwitchTest::assertOneFixAndChoose) .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); } @@ -4526,9 +4855,7 @@ public int foo(Side side) { } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion") .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); refactoringHelper2 @@ -4593,9 +4920,7 @@ public int foo(Side side) { } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion") .setFixChooser(FixChoosers.SECOND) .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); } @@ -4634,9 +4959,7 @@ public int foo(Side side) { } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion") .doTest(); refactoringHelper @@ -4694,10 +5017,8 @@ public int foo(Side side) { } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion")) - .setFixChooser(Iterables::getOnlyElement) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion") + .setFixChooser(StatementSwitchToExpressionSwitchTest::assertOneFixAndChoose) .doTest(); } @@ -4741,9 +5062,7 @@ public int foo(Side side) { } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion") .doTest(); refactoringHelper @@ -4821,10 +5140,8 @@ public int foo(Side side) { } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion")) - .setFixChooser(Iterables::getOnlyElement) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion") + .setFixChooser(StatementSwitchToExpressionSwitchTest::assertOneFixAndChoose) .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); } @@ -4926,7 +5243,7 @@ private void updateScore(Suit suit) { } """) .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion") - .setFixChooser(Iterables::getOnlyElement) + .setFixChooser(StatementSwitchToExpressionSwitchTest::assertOneFixAndChoose) .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); } @@ -4961,9 +5278,7 @@ public int foo(Side side) { } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion") .doTest(); refactoringHelper @@ -5017,10 +5332,8 @@ public int foo(Side side) { } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion")) - .setFixChooser(Iterables::getOnlyElement) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion") + .setFixChooser(StatementSwitchToExpressionSwitchTest::assertOneFixAndChoose) .doTest(); } @@ -5056,96 +5369,10 @@ public int foo(Side side) { } } """) - .setArgs( - ImmutableList.of( - "-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion")) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion") .doTest(); } - @Test - public void i4222() { - refactoringHelper - .addInputLines( - "Test.java", - """ - public class Test { - public static void main(String[] args) { - switch (args.length) { - case 0: - { - System.out.println(0); - break; - } - case 1: - System.out.println(1); - break; - case 2: - System.out.println(2); - System.out.println(2); - break; - } - } - } - """) - .addOutputLines( - "Test.java", - """ - public class Test { - public static void main(String[] args) { - switch (args.length) { - case 0 -> System.out.println(0); - case 1 -> System.out.println(1); - case 2 -> { - System.out.println(2); - System.out.println(2); - } - } - } - } - """) - .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion=true") - .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); - } - - @Test - public void unnecessaryBreaks() { - 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); - } - @Test public void mixedExpressionsAndYields() { refactoringHelper @@ -5237,4 +5464,14 @@ String f(int x) { "-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion=true") .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); } + + /** + * Asserts that there is exactly one suggested fix and returns it. + * + *

Similar to {@code FixChoosers.FIRST}, but also asserts that there is exactly one fix. + */ + public static Fix assertOneFixAndChoose(List fixes) { + assertThat(fixes).hasSize(1); + return fixes.get(0); + } }