Skip to content

Commit 5bd9ced

Browse files
markhbradyError Prone Team
authored andcommitted
[IfChainToSwitch] refactor common switch logic into SwitchUtils library. No functional changes
PiperOrigin-RevId: 867639648
1 parent c7ae7e1 commit 5bd9ced

1 file changed

Lines changed: 9 additions & 31 deletions

File tree

core/src/main/java/com/google/errorprone/bugpatterns/IfChainToSwitch.java

Lines changed: 9 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@
2020
import static com.google.common.collect.ImmutableList.toImmutableList;
2121
import static com.google.common.collect.ImmutableSet.toImmutableSet;
2222
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
23+
import static com.google.errorprone.bugpatterns.SwitchUtils.COMPILE_TIME_CONSTANT_MATCHER;
24+
import static com.google.errorprone.bugpatterns.SwitchUtils.isEnumValue;
25+
import static com.google.errorprone.bugpatterns.SwitchUtils.renderComments;
2326
import static com.google.errorprone.matchers.Description.NO_MATCH;
2427
import static com.google.errorprone.util.ASTHelpers.constValue;
2528
import static com.google.errorprone.util.ASTHelpers.getStartPosition;
@@ -40,12 +43,11 @@
4043
import com.google.errorprone.ErrorProneFlags;
4144
import com.google.errorprone.VisitorState;
4245
import com.google.errorprone.bugpatterns.BugChecker.IfTreeMatcher;
46+
import com.google.errorprone.bugpatterns.SwitchUtils.Validity;
4347
import com.google.errorprone.bugpatterns.threadsafety.ConstantExpressions;
4448
import com.google.errorprone.fixes.SuggestedFix;
4549
import com.google.errorprone.fixes.SuggestedFixes;
46-
import com.google.errorprone.matchers.CompileTimeConstantExpressionMatcher;
4750
import com.google.errorprone.matchers.Description;
48-
import com.google.errorprone.matchers.Matcher;
4951
import com.google.errorprone.suppliers.Suppliers;
5052
import com.google.errorprone.util.ASTHelpers;
5153
import com.google.errorprone.util.ErrorProneComment;
@@ -88,18 +90,6 @@ public final class IfChainToSwitch extends BugChecker implements IfTreeMatcher {
8890
// it's either an ExpressionStatement or a Throw. Refer to JLS 14 §14.11.1
8991
private static final ImmutableSet<Kind> KINDS_CONVERTIBLE_WITHOUT_BRACES =
9092
ImmutableSet.of(THROW, EXPRESSION_STATEMENT);
91-
private static final Matcher<ExpressionTree> COMPILE_TIME_CONSTANT_MATCHER =
92-
CompileTimeConstantExpressionMatcher.instance();
93-
94-
/**
95-
* Tri-state of whether the if-chain is valid, invalid, or possibly valid for conversion to a
96-
* switch.
97-
*/
98-
enum Validity {
99-
MAYBE_VALID,
100-
INVALID,
101-
VALID
102-
}
10393

10494
private final boolean enableMain;
10595
private final boolean enableSafe;
@@ -349,14 +339,6 @@ private static Range<Integer> buildCommentRange(ErrorProneComment comment, int i
349339
return Range.closedOpen(comment.getPos() + ifTreeStart, comment.getEndPos() + ifTreeStart);
350340
}
351341

352-
/** Render the supplied comments, separated by newlines. */
353-
private static String renderComments(ImmutableList<ErrorProneComment> comments) {
354-
return comments.stream()
355-
.map(ErrorProneComment::getText)
356-
.filter(commentText -> !commentText.isEmpty())
357-
.collect(joining("\n"));
358-
}
359-
360342
/**
361343
* Renders Java source code representation of the supplied {@code Type} that is suitable for use
362344
* in fixes, where any raw types are replaced with wildcard types. For example, `List` becomes
@@ -468,7 +450,7 @@ && isSubtype(
468450
boolean hasPattern = cases.stream().anyMatch(x -> x.instanceOfOptional().isPresent());
469451

470452
boolean allEnumValuesPresent =
471-
isEnum(subject, state)
453+
isEnumValue(subject, state)
472454
&& handledEnumValues.containsAll(ASTHelpers.enumValues(switchType.asElement()));
473455

474456
if (hasDefault && hasUnconditional) {
@@ -869,10 +851,6 @@ private IfChainAnalysisState analyzeIfStatement(
869851
ImmutableSet.copyOf(handledEnumValues));
870852
}
871853

872-
private static boolean isEnum(ExpressionTree tree, VisitorState state) {
873-
return isSubtype(getType(tree), state.getSymtab().enumSym.type, state);
874-
}
875-
876854
/** Determines whether any yield or break statements are present in the tree. */
877855
private static boolean hasBreakOrYieldInTree(Tree tree) {
878856
Boolean result =
@@ -958,7 +936,7 @@ private Optional<ExpressionTree> validatePredicateForSubject(
958936
hasElseIf);
959937
} else {
960938
// Predicate is a binary tree, but neither side is a constant.
961-
if (isEnum(lhs, state) || isEnum(rhs, state)) {
939+
if (isEnumValue(lhs, state) || isEnumValue(rhs, state)) {
962940
return validateEnumPredicateForSubject(
963941
lhs,
964942
rhs,
@@ -1236,8 +1214,8 @@ private Optional<ExpressionTree> validateEnumPredicateForSubject(
12361214
int caseEndPosition,
12371215
boolean hasElse,
12381216
boolean hasElseIf) {
1239-
boolean lhsIsEnumConstant = isEnum(lhs, state) && ASTHelpers.isEnumConstant(lhs);
1240-
boolean rhsIsEnumConstant = isEnum(rhs, state) && ASTHelpers.isEnumConstant(rhs);
1217+
boolean lhsIsEnumConstant = isEnumValue(lhs, state) && ASTHelpers.isEnumConstant(lhs);
1218+
boolean rhsIsEnumConstant = isEnumValue(rhs, state) && ASTHelpers.isEnumConstant(rhs);
12411219

12421220
if (lhsIsEnumConstant && rhsIsEnumConstant) {
12431221
// Comparing enum const to enum const, cannot convert
@@ -1549,7 +1527,7 @@ public static boolean isDominatedBy(
15491527
continue;
15501528
}
15511529
}
1552-
boolean isEnum = isEnum(constantExpression, state);
1530+
boolean isEnum = isEnumValue(constantExpression, state);
15531531
if (isEnum) {
15541532
if (lhs.guardOptional().isPresent()) {
15551533
// Guarded patterns cannot dominate enum values

0 commit comments

Comments
 (0)