From 0d556b3d75b53445bb7f6f24526e959895519047 Mon Sep 17 00:00:00 2001 From: ghm Date: Fri, 15 Nov 2024 06:51:53 -0800 Subject: [PATCH] Introduce `ASTHelpers.isSwitchDefault`, and use it to burn down uses of `CaseTree.getExpression`, which is deprecated (with good reason!) PiperOrigin-RevId: 696872604 --- .../google/errorprone/util/ASTHelpers.java | 25 ++++++++++--------- .../google/errorprone/util/Reachability.java | 3 ++- .../bugpatterns/MissingCasesInEnumSwitch.java | 3 ++- .../StatementSwitchToExpressionSwitch.java | 7 +++--- .../errorprone/bugpatterns/SwitchDefault.java | 5 ++-- .../UnnecessaryDefaultInEnumSwitch.java | 3 ++- .../refaster/ControlFlowVisitor.java | 3 ++- 7 files changed, 28 insertions(+), 21 deletions(-) diff --git a/check_api/src/main/java/com/google/errorprone/util/ASTHelpers.java b/check_api/src/main/java/com/google/errorprone/util/ASTHelpers.java index b99cb8cc36c..a6f7e327d0a 100644 --- a/check_api/src/main/java/com/google/errorprone/util/ASTHelpers.java +++ b/check_api/src/main/java/com/google/errorprone/util/ASTHelpers.java @@ -2714,18 +2714,19 @@ private static boolean hasMatchingMethods( } public static Optional getSwitchDefault(SwitchTree switchTree) { - return switchTree.getCases().stream() - .filter( - (CaseTree c) -> { - if (!c.getExpressions().isEmpty()) { - return false; - } - List labels = c.getLabels(); - return labels.isEmpty() - || (labels.size() == 1 - && getOnlyElement(labels).getKind().name().equals("DEFAULT_CASE_LABEL")); - }) - .findFirst(); + return switchTree.getCases().stream().filter(c -> isSwitchDefault(c)).findFirst(); + } + + /** Returns whether {@code caseTree} is the default case of a switch statement. */ + public static boolean isSwitchDefault(CaseTree caseTree) { + if (!caseTree.getExpressions().isEmpty()) { + return false; + } + List labels = caseTree.getLabels(); + return labels.isEmpty() + || (labels.size() == 1 + // DEFAULT_CASE_LABEL is in Java 21, so we're stuck stringifying for now. + && getOnlyElement(labels).getKind().name().equals("DEFAULT_CASE_LABEL")); } private static final Supplier NULL_MARKED_NAME = diff --git a/check_api/src/main/java/com/google/errorprone/util/Reachability.java b/check_api/src/main/java/com/google/errorprone/util/Reachability.java index 12e644fa8ac..f48ad5abf27 100644 --- a/check_api/src/main/java/com/google/errorprone/util/Reachability.java +++ b/check_api/src/main/java/com/google/errorprone/util/Reachability.java @@ -19,6 +19,7 @@ import static com.google.common.base.MoreObjects.firstNonNull; import static com.google.common.collect.Iterables.getLast; import static com.google.errorprone.util.ASTHelpers.getSymbol; +import static com.google.errorprone.util.ASTHelpers.isSwitchDefault; import static java.util.Objects.requireNonNull; import com.google.errorprone.annotations.CanIgnoreReturnValue; @@ -241,7 +242,7 @@ public Boolean visitSwitch(SwitchTree tree, Void unused) { return true; } // (4) - if (tree.getCases().stream().noneMatch(c -> c.getExpression() == null)) { + if (tree.getCases().stream().noneMatch(c -> isSwitchDefault(c))) { return true; } // (5) diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/MissingCasesInEnumSwitch.java b/core/src/main/java/com/google/errorprone/bugpatterns/MissingCasesInEnumSwitch.java index b15dbd8e8ef..b861bcf3780 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/MissingCasesInEnumSwitch.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/MissingCasesInEnumSwitch.java @@ -18,6 +18,7 @@ import static com.google.common.collect.ImmutableSet.toImmutableSet; import static com.google.errorprone.BugPattern.SeverityLevel.WARNING; +import static com.google.errorprone.util.ASTHelpers.isSwitchDefault; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; @@ -48,7 +49,7 @@ public Description matchSwitch(SwitchTree tree, VisitorState state) { return Description.NO_MATCH; } // default case is present - if (tree.getCases().stream().anyMatch(c -> c.getExpression() == null)) { + if (tree.getCases().stream().anyMatch(c -> isSwitchDefault(c))) { return Description.NO_MATCH; } ImmutableSet handled = 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 2c0cf618024..43f648e0c35 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitch.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitch.java @@ -24,6 +24,7 @@ import static com.google.errorprone.matchers.Description.NO_MATCH; import static com.google.errorprone.util.ASTHelpers.getStartPosition; import static com.google.errorprone.util.ASTHelpers.getSymbol; +import static com.google.errorprone.util.ASTHelpers.isSwitchDefault; import static com.sun.source.tree.Tree.Kind.BLOCK; import static com.sun.source.tree.Tree.Kind.BREAK; import static com.sun.source.tree.Tree.Kind.EXPRESSION_STATEMENT; @@ -536,7 +537,7 @@ private static SuggestedFix convertDirectlyToExpressionSwitch( boolean firstCaseInGroup = true; for (int caseIndex = 0; caseIndex < cases.size(); caseIndex++) { CaseTree caseTree = cases.get(caseIndex); - boolean isDefaultCase = caseTree.getExpression() == null; + boolean isDefaultCase = isSwitchDefault(caseTree); // For readability, filter out trailing unlabelled break statement because these become a // No-Op when used inside expression switches @@ -656,7 +657,7 @@ private static SuggestedFix convertToReturnSwitch( boolean firstCaseInGroup = true; for (int caseIndex = 0; caseIndex < cases.size(); caseIndex++) { CaseTree caseTree = cases.get(caseIndex); - boolean isDefaultCase = caseTree.getExpression() == null; + boolean isDefaultCase = isSwitchDefault(caseTree); String transformedBlockSource = transformReturnOrThrowBlock(caseTree, state, getStatements(caseTree)); @@ -825,7 +826,7 @@ private static SuggestedFix convertToAssignmentSwitch( boolean firstCaseInGroup = true; for (int caseIndex = 0; caseIndex < cases.size(); caseIndex++) { CaseTree caseTree = cases.get(caseIndex); - boolean isDefaultCase = caseTree.getExpression() == null; + boolean isDefaultCase = isSwitchDefault(caseTree); ImmutableList filteredStatements = filterOutRedundantBreak(caseTree); String transformedBlockSource = diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/SwitchDefault.java b/core/src/main/java/com/google/errorprone/bugpatterns/SwitchDefault.java index 4bd82b4c43b..a59e834257d 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/SwitchDefault.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/SwitchDefault.java @@ -21,6 +21,7 @@ import static com.google.errorprone.matchers.Description.NO_MATCH; import static com.google.errorprone.util.ASTHelpers.getStartPosition; import static com.google.errorprone.util.ASTHelpers.getSwitchDefault; +import static com.google.errorprone.util.ASTHelpers.isSwitchDefault; import com.google.errorprone.BugPattern; import com.google.errorprone.VisitorState; @@ -55,7 +56,7 @@ public Description matchSwitch(SwitchTree tree, VisitorState state) { while (it.hasNext()) { CaseTree caseTree = it.next(); defaultStatementGroup.add(caseTree); - if (caseTree.getExpression() == null) { + if (isSwitchDefault(caseTree)) { while (it.hasNext() && isNullOrEmpty(caseTree.getStatements())) { caseTree = it.next(); defaultStatementGroup.add(caseTree); @@ -96,7 +97,7 @@ public Description matchSwitch(SwitchTree tree, VisitorState state) { // If the last statement group falls out of the switch, add a `break;` before moving // the default to the end. CaseTree last = getLast(tree.getCases()); - if (last.getExpression() == null || Reachability.canCompleteNormally(last)) { + if (isSwitchDefault(last) || Reachability.canCompleteNormally(last)) { replacement = "break;\n" + replacement; } fix.replace(start, end, "").postfixWith(getLast(tree.getCases()), replacement); diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/UnnecessaryDefaultInEnumSwitch.java b/core/src/main/java/com/google/errorprone/bugpatterns/UnnecessaryDefaultInEnumSwitch.java index 9b4a1493650..cc4b5443944 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/UnnecessaryDefaultInEnumSwitch.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/UnnecessaryDefaultInEnumSwitch.java @@ -22,6 +22,7 @@ import static com.google.errorprone.BugPattern.SeverityLevel.WARNING; import static com.google.errorprone.matchers.Description.NO_MATCH; import static com.google.errorprone.util.ASTHelpers.getStartPosition; +import static com.google.errorprone.util.ASTHelpers.isSwitchDefault; import static com.google.errorprone.util.Reachability.canCompleteNormally; import com.google.common.collect.ImmutableSet; @@ -79,7 +80,7 @@ public Description matchSwitch(SwitchTree switchTree, VisitorState state) { CaseTree caseBeforeDefault = null; CaseTree defaultCase = null; for (CaseTree caseTree : switchTree.getCases()) { - if (caseTree.getExpression() == null) { + if (isSwitchDefault(caseTree)) { defaultCase = caseTree; break; } else { diff --git a/core/src/main/java/com/google/errorprone/refaster/ControlFlowVisitor.java b/core/src/main/java/com/google/errorprone/refaster/ControlFlowVisitor.java index 12bca95267d..f855e98c212 100644 --- a/core/src/main/java/com/google/errorprone/refaster/ControlFlowVisitor.java +++ b/core/src/main/java/com/google/errorprone/refaster/ControlFlowVisitor.java @@ -19,6 +19,7 @@ import static com.google.errorprone.refaster.ControlFlowVisitor.Result.ALWAYS_RETURNS; import static com.google.errorprone.refaster.ControlFlowVisitor.Result.MAY_BREAK_OR_RETURN; import static com.google.errorprone.refaster.ControlFlowVisitor.Result.NEVER_EXITS; +import static com.google.errorprone.util.ASTHelpers.isSwitchDefault; import com.google.errorprone.refaster.ControlFlowVisitor.BreakContext; import com.google.errorprone.refaster.ControlFlowVisitor.Result; @@ -221,7 +222,7 @@ public Result visitSwitch(SwitchTree node, BreakContext cxt) { cxt.loopDepth++; try { for (CaseTree caseTree : node.getCases()) { - if (caseTree.getExpression() == null) { + if (isSwitchDefault(caseTree)) { seenDefault = true; }