From 3b16d6147af7b8ac0a682893daddf9b0055e5f89 Mon Sep 17 00:00:00 2001 From: ghm Date: Fri, 15 Nov 2024 04:02:58 -0800 Subject: [PATCH] Remove the reflective CaseTree methods from ASTHelpers. PiperOrigin-RevId: 696836584 --- .../google/errorprone/util/ASTHelpers.java | 114 +----------------- .../bugpatterns/MissingCasesInEnumSwitch.java | 2 +- .../StatementSwitchToExpressionSwitch.java | 7 +- .../TraditionalSwitchExpression.java | 7 +- .../bugpatterns/UnnecessaryBreakInSwitch.java | 7 +- .../UnnecessaryDefaultInEnumSwitch.java | 2 +- 6 files changed, 14 insertions(+), 125 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 0b762e849bb..b99cb8cc36c 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 @@ -2713,38 +2713,14 @@ private static boolean hasMatchingMethods( return false; } - private static final Method CASE_TREE_GET_LABELS = getCaseTreeGetLabelsMethod(); - - private static @Nullable Method getCaseTreeGetLabelsMethod() { - try { - return CaseTree.class.getMethod("getLabels"); - } catch (NoSuchMethodException e) { - return null; - } - } - - @SuppressWarnings("unchecked") // reflection - private static List getCaseLabels(CaseTree caseTree) { - if (CASE_TREE_GET_LABELS == null) { - return ImmutableList.of(); - } - try { - return (List) CASE_TREE_GET_LABELS.invoke(caseTree); - } catch (ReflectiveOperationException e) { - throw new LinkageError(e.getMessage(), e); - } - } - - // getExpression() is being used for compatibility with earlier JDK versions - @SuppressWarnings("deprecation") public static Optional getSwitchDefault(SwitchTree switchTree) { return switchTree.getCases().stream() .filter( (CaseTree c) -> { - if (c.getExpression() != null) { + if (!c.getExpressions().isEmpty()) { return false; } - List labels = getCaseLabels(c); + List labels = c.getLabels(); return labels.isEmpty() || (labels.size() == 1 && getOnlyElement(labels).getKind().name().equals("DEFAULT_CASE_LABEL")); @@ -2752,92 +2728,6 @@ public static Optional getSwitchDefault(SwitchTree switchTre .findFirst(); } - private static final Method CASE_TREE_GET_EXPRESSIONS = getCaseTreeGetExpressionsMethod(); - - private static @Nullable Method getCaseTreeGetExpressionsMethod() { - try { - return CaseTree.class.getMethod("getExpressions"); - } catch (NoSuchMethodException e) { - return null; - } - } - - /** - * Returns true if the given {@link CaseTree} is in the form: {@code case -> - * }. - */ - public static boolean isRuleKind(CaseTree caseTree) { - if (GET_CASE_KIND_METHOD == null) { - return false; - } - Enum kind; - try { - kind = (Enum) GET_CASE_KIND_METHOD.invoke(caseTree); - } catch (ReflectiveOperationException e) { - return false; - } - return kind.name().equals("RULE"); - } - - private static final Method GET_CASE_KIND_METHOD = getGetCaseKindMethod(); - - private static @Nullable Method getGetCaseKindMethod() { - try { - return CaseTree.class.getMethod("getCaseKind"); - } catch (NoSuchMethodException e) { - return null; - } - } - - /** - * Returns the statement or expression after the arrow for a {@link CaseTree} of the form: {@code - * case -> }. - */ - public static @Nullable Tree getCaseTreeBody(CaseTree caseTree) { - if (GET_CASE_BODY_METHOD == null) { - return null; - } - try { - return (Tree) GET_CASE_BODY_METHOD.invoke(caseTree); - } catch (ReflectiveOperationException e) { - throw new LinkageError(e.getMessage(), e); - } - } - - private static final @Nullable Method GET_CASE_BODY_METHOD = getGetCaseBodyMethod(); - - private static @Nullable Method getGetCaseBodyMethod() { - try { - return CaseTree.class.getMethod("getBody"); - } catch (NoSuchMethodException e) { - return null; - } - } - - /** - * Retrieves a stream containing all case expressions, in order, for a given {@code CaseTree}. - * This method acts as a facade to the {@code CaseTree.getExpressions()} API, falling back to - * legacy APIs when necessary. - */ - @SuppressWarnings({ - "deprecation", // getExpression() is being used for compatibility with earlier JDK versions - "unchecked", // reflection - }) - public static Stream getCaseExpressions(CaseTree caseTree) { - if (Runtime.version().feature() < 12) { - // "default" case gives an empty stream - return Stream.ofNullable(caseTree.getExpression()); - } - if (CASE_TREE_GET_EXPRESSIONS == null) { - return Stream.empty(); - } - try { - return ((List) CASE_TREE_GET_EXPRESSIONS.invoke(caseTree)).stream(); - } catch (ReflectiveOperationException e) { - throw new LinkageError(e.getMessage(), e); - } - } - private static final Supplier NULL_MARKED_NAME = memoize(state -> state.getName("org.jspecify.annotations.NullMarked")); 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 1d9ce7d08c2..b15dbd8e8ef 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/MissingCasesInEnumSwitch.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/MissingCasesInEnumSwitch.java @@ -53,7 +53,7 @@ public Description matchSwitch(SwitchTree tree, VisitorState state) { } ImmutableSet handled = tree.getCases().stream() - .flatMap(ASTHelpers::getCaseExpressions) + .flatMap(c -> c.getExpressions().stream()) .filter(IdentifierTree.class::isInstance) .map(e -> ((IdentifierTree) e).getName().toString()) .collect(toImmutableSet()); 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 d5b09870f15..2c0cf618024 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitch.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitch.java @@ -22,7 +22,6 @@ import static com.google.common.collect.Iterables.getOnlyElement; import static com.google.errorprone.BugPattern.SeverityLevel.WARNING; import static com.google.errorprone.matchers.Description.NO_MATCH; -import static com.google.errorprone.util.ASTHelpers.getCaseExpressions; import static com.google.errorprone.util.ASTHelpers.getStartPosition; import static com.google.errorprone.util.ASTHelpers.getSymbol; import static com.sun.source.tree.Tree.Kind.BLOCK; @@ -208,11 +207,11 @@ 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 isDefaultCase = (getCaseExpressions(caseTree).count() == 0); + boolean isDefaultCase = caseTree.getExpressions().isEmpty(); hasDefaultCase |= isDefaultCase; // Accumulate enum values included in this case handledEnumValues.addAll( - getCaseExpressions(caseTree) + caseTree.getExpressions().stream() .filter(IdentifierTree.class::isInstance) .map(expressionTree -> ((IdentifierTree) expressionTree).getName().toString()) .collect(toImmutableSet())); @@ -1123,7 +1122,7 @@ 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 getCaseExpressions(caseTree).map(state::getSourceForNode).collect(joining(", ")); + return caseTree.getExpressions().stream().map(state::getSourceForNode).collect(joining(", ")); } /** diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/TraditionalSwitchExpression.java b/core/src/main/java/com/google/errorprone/bugpatterns/TraditionalSwitchExpression.java index 090b40e0175..749ac7305f7 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/TraditionalSwitchExpression.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/TraditionalSwitchExpression.java @@ -18,13 +18,14 @@ import static com.google.errorprone.BugPattern.SeverityLevel.WARNING; import static com.google.errorprone.matchers.Description.NO_MATCH; -import static com.google.errorprone.util.ASTHelpers.isRuleKind; import com.google.errorprone.BugPattern; import com.google.errorprone.VisitorState; import com.google.errorprone.matchers.Description; import com.sun.source.tree.CaseTree; +import com.sun.source.tree.CaseTree.CaseKind; import com.sun.source.tree.Tree; +import com.sun.source.tree.Tree.Kind; /** A {@link BugChecker}; see the associated {@link BugPattern} annotation for details. */ @BugPattern(summary = "Prefer -> switches for switch expressions", severity = WARNING) @@ -32,11 +33,11 @@ public class TraditionalSwitchExpression extends BugChecker implements BugChecke @Override public Description matchCase(CaseTree tree, VisitorState state) { - if (isRuleKind(tree)) { + if (tree.getCaseKind().equals(CaseKind.RULE)) { return NO_MATCH; } Tree parent = state.getPath().getParentPath().getLeaf(); - if (!parent.getKind().name().equals("SWITCH_EXPRESSION")) { + if (!parent.getKind().equals(Kind.SWITCH_EXPRESSION)) { return NO_MATCH; } return describeMatch(parent); diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/UnnecessaryBreakInSwitch.java b/core/src/main/java/com/google/errorprone/bugpatterns/UnnecessaryBreakInSwitch.java index 0aeba268bf0..114125eaf3a 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/UnnecessaryBreakInSwitch.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/UnnecessaryBreakInSwitch.java @@ -19,17 +19,16 @@ import static com.google.common.collect.Iterables.getLast; import static com.google.errorprone.BugPattern.SeverityLevel.WARNING; import static com.google.errorprone.matchers.Description.NO_MATCH; -import static com.google.errorprone.util.ASTHelpers.isRuleKind; import com.google.common.collect.ImmutableList; import com.google.errorprone.BugPattern; import com.google.errorprone.VisitorState; import com.google.errorprone.fixes.SuggestedFix; import com.google.errorprone.matchers.Description; -import com.google.errorprone.util.ASTHelpers; import com.sun.source.tree.BlockTree; import com.sun.source.tree.BreakTree; import com.sun.source.tree.CaseTree; +import com.sun.source.tree.CaseTree.CaseKind; import com.sun.source.tree.IfTree; import com.sun.source.tree.Tree; import com.sun.source.util.SimpleTreeVisitor; @@ -41,10 +40,10 @@ public class UnnecessaryBreakInSwitch extends BugChecker implements BugChecker.CaseTreeMatcher { @Override public Description matchCase(CaseTree tree, VisitorState state) { - if (!isRuleKind(tree)) { + if (!tree.getCaseKind().equals(CaseKind.RULE)) { return NO_MATCH; } - Tree body = ASTHelpers.getCaseTreeBody(tree); + Tree body = tree.getBody(); ImmutableList unnecessaryBreaks = unnecessaryBreaks(body); if (unnecessaryBreaks.isEmpty()) { return NO_MATCH; 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 5f792aa73d9..9b4a1493650 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/UnnecessaryDefaultInEnumSwitch.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/UnnecessaryDefaultInEnumSwitch.java @@ -231,7 +231,7 @@ private static boolean trivialDefault(List defaultState private static SetView unhandledCases(SwitchTree tree, TypeSymbol switchType) { ImmutableSet handledCases = tree.getCases().stream() - .flatMap(ASTHelpers::getCaseExpressions) + .flatMap(e -> e.getExpressions().stream()) .filter(IdentifierTree.class::isInstance) .map(p -> ((IdentifierTree) p).getName().toString()) .collect(toImmutableSet());