Introduce ASTHelpers.isSwitchDefault, and use it to burn down uses …

…of `CaseTree.getExpression`, which is deprecated (with good reason!) PiperOrigin-RevId: 696872604
google · Nov 15, 2024 · 0d556b3 · 0d556b3
1 parent 3b16d61
commit 0d556b3
Show file tree

Hide file tree

Showing 7 changed files with 28 additions and 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
@@ -2714,18 +2714,19 @@ private static boolean hasMatchingMethods(
   }
 
   public static Optional<? extends CaseTree> getSwitchDefault(SwitchTree switchTree) {
-    return switchTree.getCases().stream()
-        .filter(
-            (CaseTree c) -> {
-              if (!c.getExpressions().isEmpty()) {
-                return false;
-              }
-              List<? extends Tree> 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<? extends Tree> 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<Name> 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
@@ -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
@@ -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<String> 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
@@ -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<StatementTree> 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
@@ -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
@@ -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
@@ -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;
         }