Skip to content

Commit cc1e3bf

Browse files
markhbradyError Prone Team
markhbrady
authored and
Error Prone Team
committed
[StatementSwitchToExpressionSwitch] add support for patterns and guards in old-style (colon) switches.
PiperOrigin-RevId: 833871473
1 parent 9360e40 commit cc1e3bf

File tree

2 files changed

+172
-15
lines changed

2 files changed

+172
-15
lines changed

‎core/src/main/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitch.java‎

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@
6464
import com.sun.source.tree.IdentifierTree;
6565
import com.sun.source.tree.LabeledStatementTree;
6666
import com.sun.source.tree.MemberSelectTree;
67+
import com.sun.source.tree.PatternCaseLabelTree;
6768
import com.sun.source.tree.ReturnTree;
6869
import com.sun.source.tree.StatementTree;
6970
import com.sun.source.tree.SwitchTree;
@@ -299,15 +300,6 @@ private static AnalysisResult analyzeSwitchTree(SwitchTree switchTree, VisitorSt
299300
// One-pass scan through each case in switch
300301
for (int caseIndex = 0; caseIndex < cases.size(); caseIndex++) {
301302
CaseTree caseTree = cases.get(caseIndex);
302-
boolean hasCasePattern =
303-
caseTree.getLabels().stream()
304-
.anyMatch(
305-
caseLabelTree -> caseLabelTree.getKind().name().equals("PATTERN_CASE_LABEL"));
306-
if (hasCasePattern) {
307-
// Case patterns are not currently supported by the checker.
308-
return DEFAULT_ANALYSIS_RESULT;
309-
}
310-
311303
NullDefaultKind nullDefaultKind = analyzeCaseForNullAndDefault(caseTree);
312304
boolean isDefaultCase =
313305
nullDefaultKind.equals(NullDefaultKind.KIND_DEFAULT)
@@ -1014,7 +1006,7 @@ private static SuggestedFix convertDirectlyToExpressionSwitch(
10141006
}
10151007

10161008
if (nullDefaultKind.equals(NullDefaultKind.KIND_NEITHER)) {
1017-
replacementCodeBuilder.append(printCaseExpressions(caseTree, state));
1009+
replacementCodeBuilder.append(printCaseExpressionsOrPatternAndGuard(caseTree, state));
10181010
}
10191011
Optional<String> commentsAfterCaseOptional =
10201012
extractCommentsAfterCase(switchTree, allSwitchComments, state, caseIndex);
@@ -1159,7 +1151,7 @@ private static SuggestedFix convertToReturnSwitch(
11591151
}
11601152

11611153
if (nullDefaultKind.equals(NullDefaultKind.KIND_NEITHER)) {
1162-
replacementCodeBuilder.append(printCaseExpressions(caseTree, state));
1154+
replacementCodeBuilder.append(printCaseExpressionsOrPatternAndGuard(caseTree, state));
11631155
}
11641156

11651157
Optional<String> commentsAfterCaseOptional =
@@ -1395,7 +1387,7 @@ private static SuggestedFix convertToAssignmentSwitch(
13951387
}
13961388

13971389
if (nullDefaultKind.equals(NullDefaultKind.KIND_NEITHER)) {
1398-
replacementCodeBuilder.append(printCaseExpressions(caseTree, state));
1390+
replacementCodeBuilder.append(printCaseExpressionsOrPatternAndGuard(caseTree, state));
13991391
}
14001392

14011393
Optional<String> commentsAfterCaseOptional =
@@ -1722,9 +1714,29 @@ private static String removeFallThruLines(String comments) {
17221714
}
17231715
}
17241716

1725-
/** Prints source for all expressions in a given {@code case}, separated by commas. */
1726-
private static String printCaseExpressions(CaseTree caseTree, VisitorState state) {
1727-
return caseTree.getExpressions().stream().map(state::getSourceForNode).collect(joining(", "));
1717+
private static boolean hasCasePattern(CaseTree caseTree) {
1718+
return caseTree.getLabels().stream()
1719+
.anyMatch(caseLabelTree -> caseLabelTree instanceof PatternCaseLabelTree);
1720+
}
1721+
1722+
/**
1723+
* Prints source for all expressions in a given {@code case}, separated by commas, or the pattern
1724+
* and guard (if present).
1725+
*/
1726+
private static String printCaseExpressionsOrPatternAndGuard(
1727+
CaseTree caseTree, VisitorState state) {
1728+
if (!hasCasePattern(caseTree)) {
1729+
return caseTree.getExpressions().stream().map(state::getSourceForNode).collect(joining(", "));
1730+
}
1731+
// Currently, `case`s can only have a single pattern, however the compiler's class structure
1732+
// does not reflect this restriction.
1733+
StringBuilder sb =
1734+
new StringBuilder(
1735+
caseTree.getLabels().stream().map(state::getSourceForNode).collect(joining(", ")));
1736+
if (caseTree.getGuard() != null) {
1737+
sb.append(" when ").append(state.getSourceForNode(caseTree.getGuard())).append(" ");
1738+
}
1739+
return sb.toString();
17281740
}
17291741

17301742
/**

‎core/src/test/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitchTest.java‎

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1205,6 +1205,52 @@ public static void main(String[] args) {
12051205
.doTest(TEXT_MATCH);
12061206
}
12071207

1208+
@Test
1209+
public void switchOnString_patterns_error() {
1210+
refactoringHelper
1211+
.addInputLines(
1212+
"Test.java",
1213+
"""
1214+
public class Test {
1215+
public static void main(String[] args) {
1216+
switch (args[0]) {
1217+
case String s
1218+
when s.startsWith("a sale"):
1219+
{
1220+
System.out.println("it all starts with a sale");
1221+
break;
1222+
}
1223+
case "one":
1224+
System.out.println("one");
1225+
break;
1226+
case "two", "three":
1227+
System.out.println("two or three");
1228+
break;
1229+
case String s:
1230+
System.out.println("some other string");
1231+
break;
1232+
}
1233+
}
1234+
}
1235+
""")
1236+
.addOutputLines(
1237+
"Test.java",
1238+
"""
1239+
public class Test {
1240+
public static void main(String[] args) {
1241+
switch (args[0]) {
1242+
case String s when s.startsWith("a sale") -> System.out.println("it all starts with a sale");
1243+
case "one" -> System.out.println("one");
1244+
case "two", "three" -> System.out.println("two or three");
1245+
case String s -> System.out.println("some other string");
1246+
}
1247+
}
1248+
}
1249+
""")
1250+
.setArgs("-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion=true")
1251+
.doTest(TEXT_MATCH);
1252+
}
1253+
12081254
@Test
12091255
public void unnecessaryBreaks() {
12101256
refactoringHelper
@@ -1343,6 +1389,105 @@ public int foo(Suit suit) {
13431389
.doTest();
13441390
}
13451391

1392+
@Test
1393+
public void switchByEnum_casePatternAndGuard_error() {
1394+
1395+
refactoringHelper
1396+
.addInputLines(
1397+
"Test.java",
1398+
"""
1399+
class Test {
1400+
public int foo(Suit suit) {
1401+
switch (suit) {
1402+
case HEART:
1403+
case DIAMOND:
1404+
return 1;
1405+
case SPADE:
1406+
System.out.println("spade");
1407+
throw new RuntimeException();
1408+
case CLUB:
1409+
throw new NullPointerException();
1410+
case Suit s
1411+
when s == Suit.HEART:
1412+
throw new NullPointerException();
1413+
default:
1414+
throw new NullPointerException();
1415+
}
1416+
}
1417+
}
1418+
""")
1419+
.addOutputLines(
1420+
"Test.java",
1421+
"""
1422+
class Test {
1423+
public int foo(Suit suit) {
1424+
return switch (suit) {
1425+
case HEART, DIAMOND -> 1;
1426+
case SPADE -> {
1427+
System.out.println("spade");
1428+
throw new RuntimeException();
1429+
}
1430+
case CLUB -> throw new NullPointerException();
1431+
case Suit s when s == Suit.HEART -> throw new NullPointerException();
1432+
default -> throw new NullPointerException();
1433+
};
1434+
}
1435+
}
1436+
""")
1437+
.setArgs(
1438+
"-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion",
1439+
"-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion=false")
1440+
.setFixChooser(FixChoosers.FIRST)
1441+
.doTest();
1442+
1443+
refactoringHelper2
1444+
.addInputLines(
1445+
"Test.java",
1446+
"""
1447+
class Test {
1448+
public int foo(Suit suit) {
1449+
switch (suit) {
1450+
case HEART:
1451+
case DIAMOND:
1452+
return 1;
1453+
case SPADE:
1454+
System.out.println("spade");
1455+
throw new RuntimeException();
1456+
case CLUB:
1457+
throw new NullPointerException();
1458+
case Suit s
1459+
when s == Suit.HEART:
1460+
throw new NullPointerException();
1461+
default:
1462+
throw new NullPointerException();
1463+
}
1464+
}
1465+
}
1466+
""")
1467+
.addOutputLines(
1468+
"Test.java",
1469+
"""
1470+
class Test {
1471+
public int foo(Suit suit) {
1472+
return switch (suit) {
1473+
case HEART, DIAMOND -> 1;
1474+
case SPADE -> {
1475+
System.out.println("spade");
1476+
throw new RuntimeException();
1477+
}
1478+
case CLUB -> throw new NullPointerException();
1479+
case Suit s when s == Suit.HEART -> throw new NullPointerException();
1480+
};
1481+
}
1482+
}
1483+
""")
1484+
.setArgs(
1485+
"-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion",
1486+
"-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion=false")
1487+
.setFixChooser(FixChoosers.SECOND)
1488+
.doTest();
1489+
}
1490+
13461491
@Test
13471492
public void switchByEnum_middleNullCase_noError() {
13481493
// The HEART case cannot be grouped with the null case per Java syntax

0 commit comments

Comments
 (0)