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 a4b699e3cf7..7fb0fa0bc62 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitch.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitch.java @@ -64,6 +64,7 @@ import com.sun.source.tree.IdentifierTree; import com.sun.source.tree.LabeledStatementTree; import com.sun.source.tree.MemberSelectTree; +import com.sun.source.tree.PatternCaseLabelTree; import com.sun.source.tree.ReturnTree; import com.sun.source.tree.StatementTree; import com.sun.source.tree.SwitchTree; @@ -299,15 +300,6 @@ private static AnalysisResult analyzeSwitchTree(SwitchTree switchTree, VisitorSt // One-pass scan through each case in switch for (int caseIndex = 0; caseIndex < cases.size(); caseIndex++) { CaseTree caseTree = cases.get(caseIndex); - boolean hasCasePattern = - caseTree.getLabels().stream() - .anyMatch( - caseLabelTree -> caseLabelTree.getKind().name().equals("PATTERN_CASE_LABEL")); - if (hasCasePattern) { - // Case patterns are not currently supported by the checker. - return DEFAULT_ANALYSIS_RESULT; - } - NullDefaultKind nullDefaultKind = analyzeCaseForNullAndDefault(caseTree); boolean isDefaultCase = nullDefaultKind.equals(NullDefaultKind.KIND_DEFAULT) @@ -1014,7 +1006,7 @@ private static SuggestedFix convertDirectlyToExpressionSwitch( } if (nullDefaultKind.equals(NullDefaultKind.KIND_NEITHER)) { - replacementCodeBuilder.append(printCaseExpressions(caseTree, state)); + replacementCodeBuilder.append(printCaseExpressionsOrPatternAndGuard(caseTree, state)); } Optional commentsAfterCaseOptional = extractCommentsAfterCase(switchTree, allSwitchComments, state, caseIndex); @@ -1159,7 +1151,7 @@ private static SuggestedFix convertToReturnSwitch( } if (nullDefaultKind.equals(NullDefaultKind.KIND_NEITHER)) { - replacementCodeBuilder.append(printCaseExpressions(caseTree, state)); + replacementCodeBuilder.append(printCaseExpressionsOrPatternAndGuard(caseTree, state)); } Optional commentsAfterCaseOptional = @@ -1395,7 +1387,7 @@ private static SuggestedFix convertToAssignmentSwitch( } if (nullDefaultKind.equals(NullDefaultKind.KIND_NEITHER)) { - replacementCodeBuilder.append(printCaseExpressions(caseTree, state)); + replacementCodeBuilder.append(printCaseExpressionsOrPatternAndGuard(caseTree, state)); } Optional commentsAfterCaseOptional = @@ -1722,9 +1714,29 @@ private static String removeFallThruLines(String comments) { } } - /** Prints source for all expressions in a given {@code case}, separated by commas. */ - private static String printCaseExpressions(CaseTree caseTree, VisitorState state) { - return caseTree.getExpressions().stream().map(state::getSourceForNode).collect(joining(", ")); + private static boolean hasCasePattern(CaseTree caseTree) { + return caseTree.getLabels().stream() + .anyMatch(caseLabelTree -> caseLabelTree instanceof PatternCaseLabelTree); + } + + /** + * Prints source for all expressions in a given {@code case}, separated by commas, or the pattern + * and guard (if present). + */ + private static String printCaseExpressionsOrPatternAndGuard( + CaseTree caseTree, VisitorState state) { + if (!hasCasePattern(caseTree)) { + return caseTree.getExpressions().stream().map(state::getSourceForNode).collect(joining(", ")); + } + // Currently, `case`s can only have a single pattern, however the compiler's class structure + // does not reflect this restriction. + StringBuilder sb = + new StringBuilder( + caseTree.getLabels().stream().map(state::getSourceForNode).collect(joining(", "))); + if (caseTree.getGuard() != null) { + sb.append(" when ").append(state.getSourceForNode(caseTree.getGuard())).append(" "); + } + return sb.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 789f757afc5..96ace3ec01d 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitchTest.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitchTest.java @@ -1205,6 +1205,52 @@ public static void main(String[] args) { .doTest(TEXT_MATCH); } + @Test + public void switchOnString_patterns_error() { + refactoringHelper + .addInputLines( + "Test.java", + """ + public class Test { + public static void main(String[] args) { + switch (args[0]) { + case String s + when s.startsWith("a sale"): + { + System.out.println("it all starts with a sale"); + break; + } + case "one": + System.out.println("one"); + break; + case "two", "three": + System.out.println("two or three"); + break; + case String s: + System.out.println("some other string"); + break; + } + } + } + """) + .addOutputLines( + "Test.java", + """ + public class Test { + public static void main(String[] args) { + switch (args[0]) { + case String s when s.startsWith("a sale") -> System.out.println("it all starts with a sale"); + case "one" -> System.out.println("one"); + case "two", "three" -> System.out.println("two or three"); + case String s -> System.out.println("some other string"); + } + } + } + """) + .setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion=true") + .doTest(TEXT_MATCH); + } + @Test public void unnecessaryBreaks() { refactoringHelper @@ -1343,6 +1389,105 @@ public int foo(Suit suit) { .doTest(); } + @Test + public void switchByEnum_casePatternAndGuard_error() { + + refactoringHelper + .addInputLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + switch (suit) { + case HEART: + case DIAMOND: + return 1; + case SPADE: + System.out.println("spade"); + throw new RuntimeException(); + case CLUB: + throw new NullPointerException(); + case Suit s + when s == Suit.HEART: + throw new NullPointerException(); + default: + throw new NullPointerException(); + } + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + return switch (suit) { + case HEART, DIAMOND -> 1; + case SPADE -> { + System.out.println("spade"); + throw new RuntimeException(); + } + case CLUB -> throw new NullPointerException(); + case Suit s when s == Suit.HEART -> throw new NullPointerException(); + default -> throw new NullPointerException(); + }; + } + } + """) + .setArgs( + "-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion", + "-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion=false") + .setFixChooser(FixChoosers.FIRST) + .doTest(); + + refactoringHelper2 + .addInputLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + switch (suit) { + case HEART: + case DIAMOND: + return 1; + case SPADE: + System.out.println("spade"); + throw new RuntimeException(); + case CLUB: + throw new NullPointerException(); + case Suit s + when s == Suit.HEART: + throw new NullPointerException(); + default: + throw new NullPointerException(); + } + } + } + """) + .addOutputLines( + "Test.java", + """ + class Test { + public int foo(Suit suit) { + return switch (suit) { + case HEART, DIAMOND -> 1; + case SPADE -> { + System.out.println("spade"); + throw new RuntimeException(); + } + case CLUB -> throw new NullPointerException(); + case Suit s when s == Suit.HEART -> throw new NullPointerException(); + }; + } + } + """) + .setArgs( + "-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion", + "-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion=false") + .setFixChooser(FixChoosers.SECOND) + .doTest(); + } + @Test public void switchByEnum_middleNullCase_noError() { // The HEART case cannot be grouped with the null case per Java syntax