Skip to content

Commit

Permalink
Remove the reflective CaseTree methods from ASTHelpers.
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 696836584
  • Loading branch information
graememorgan authored and Error Prone Team committed Nov 15, 2024
1 parent f1308a0 commit 3b16d61
Show file tree
Hide file tree
Showing 6 changed files with 14 additions and 125 deletions.
114 changes: 2 additions & 112 deletions check_api/src/main/java/com/google/errorprone/util/ASTHelpers.java
Original file line number Diff line number Diff line change
Expand Up @@ -2713,131 +2713,21 @@ 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<? extends Tree> getCaseLabels(CaseTree caseTree) {
if (CASE_TREE_GET_LABELS == null) {
return ImmutableList.of();
}
try {
return (List<? extends Tree>) 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<? extends CaseTree> getSwitchDefault(SwitchTree switchTree) {
return switchTree.getCases().stream()
.filter(
(CaseTree c) -> {
if (c.getExpression() != null) {
if (!c.getExpressions().isEmpty()) {
return false;
}
List<? extends Tree> labels = getCaseLabels(c);
List<? extends Tree> labels = c.getLabels();
return labels.isEmpty()
|| (labels.size() == 1
&& getOnlyElement(labels).getKind().name().equals("DEFAULT_CASE_LABEL"));
})
.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 <expression> ->
* <expression>}.
*/
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 <expression> -> <body>}.
*/
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<? extends ExpressionTree> 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<? extends ExpressionTree>) CASE_TREE_GET_EXPRESSIONS.invoke(caseTree)).stream();
} catch (ReflectiveOperationException e) {
throw new LinkageError(e.getMessage(), e);
}
}

private static final Supplier<Name> NULL_MARKED_NAME =
memoize(state -> state.getName("org.jspecify.annotations.NullMarked"));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public Description matchSwitch(SwitchTree tree, VisitorState state) {
}
ImmutableSet<String> handled =
tree.getCases().stream()
.flatMap(ASTHelpers::getCaseExpressions)
.flatMap(c -> c.getExpressions().stream())
.filter(IdentifierTree.class::isInstance)
.map(e -> ((IdentifierTree) e).getName().toString())
.collect(toImmutableSet());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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()));
Expand Down Expand Up @@ -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(", "));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,25 +18,26 @@

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)
public class TraditionalSwitchExpression 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 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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<BreakTree> unnecessaryBreaks = unnecessaryBreaks(body);
if (unnecessaryBreaks.isEmpty()) {
return NO_MATCH;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ private static boolean trivialDefault(List<? extends StatementTree> defaultState
private static SetView<String> unhandledCases(SwitchTree tree, TypeSymbol switchType) {
ImmutableSet<String> handledCases =
tree.getCases().stream()
.flatMap(ASTHelpers::getCaseExpressions)
.flatMap(e -> e.getExpressions().stream())
.filter(IdentifierTree.class::isInstance)
.map(p -> ((IdentifierTree) p).getName().toString())
.collect(toImmutableSet());
Expand Down

0 comments on commit 3b16d61

Please sign in to comment.