Skip to content

[IfChainToSwitch] refactor InstanceOfIr to use Optional instead of @Nullable, like in the rest of the rest of this class #5480

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
copybara-service wants to merge 1 commit into
base: master
Choose a base branch
from
+18 −12
Open

[IfChainToSwitch] refactor InstanceOfIr to use Optional instead of @Nullable, like in the rest of the rest of this class #5480

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 18 additions & 12 deletions core/src/main/java/com/google/errorprone/bugpatterns/IfChainToSwitch.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,14 +273,14 @@ private static String prettyPrint(
} else if (caseIr.instanceOfOptional().isPresent()) {
InstanceOfIr instanceOfIr = caseIr.instanceOfOptional().get();
sb.append("case ");
if (instanceOfIr.patternVariable() != null) {
if (instanceOfIr.patternVariable().isPresent()) {
sb.append(
printRawTypesAsWildcards(
getType(instanceOfIr.patternVariable()), state, suggestedFixBuilder));
getType(instanceOfIr.patternVariable().get()), state, suggestedFixBuilder));

Symbol sym = ASTHelpers.getSymbol(instanceOfIr.patternVariable());
Symbol sym = ASTHelpers.getSymbol(instanceOfIr.patternVariable().get());
sb.append(" ").append(sym.getSimpleName()).append(" ");
} else if (instanceOfIr.type() != null && instanceOfIr.expression() != null) {
} else if (instanceOfIr.expression().isPresent()) {
sb.append(
printRawTypesAsWildcards(getType(instanceOfIr.type()), state, suggestedFixBuilder));
// It's possible that "unused" could conflict with an existing local variable name;
Expand Down Expand Up @@ -441,6 +441,7 @@ private Optional<List<CaseIr>> maybeFixDefaultNullAndUnconditional(

boolean hasDefault = cases.stream().anyMatch(CaseIr::hasDefault);
boolean hasCaseNull = cases.stream().anyMatch(CaseIr::hasCaseNull);
// NOMUTANTS -- this is a performance optimization
boolean recheckDominanceNeeded = false;

boolean switchOnNullWouldImplicitlyThrow = switchOnNullWouldImplicitlyThrow(subject, cases);
Expand Down Expand Up @@ -1062,7 +1063,9 @@ private static Optional<ExpressionTree> validateInstanceofForSubject(
/* hasDefault= */ false,
/* instanceOfOptional= */ Optional.of(
new InstanceOfIr(
instanceOfTree.getExpression(), bpt.getVariable(), instanceOfTree.getType())),
Optional.ofNullable(instanceOfTree.getExpression()),
Optional.ofNullable(bpt.getVariable()),
instanceOfTree.getType())),
/* guardOptional= */ Optional.empty(),
/* expressionsOptional= */ Optional.empty(),
/* arrowRhsOptional= */ arrowRhsOptional,
Expand Down Expand Up @@ -1099,7 +1102,10 @@ private static Optional<ExpressionTree> validateInstanceofForSubject(
/* hasCaseNull= */ false,
/* hasDefault= */ false,
/* instanceOfOptional= */ Optional.of(
new InstanceOfIr(instanceOfTree.getExpression(), null, instanceOfTree.getType())),
new InstanceOfIr(
Optional.ofNullable(instanceOfTree.getExpression()),
Optional.empty(),
instanceOfTree.getType())),
/* guardOptional= */ Optional.empty(),
/* expressionsOptional= */ Optional.empty(),
/* arrowRhsOptional= */ arrowRhsOptional,
Expand Down Expand Up @@ -1526,8 +1532,8 @@ public static boolean isDominatedBy(
// Cannot unbox LHS pattern, so RHS primitive constant should come before it
return true;
}
} else if (instanceOfIr.patternVariable() != null) {
VariableTree patternVariable = instanceOfIr.patternVariable();
} else if (instanceOfIr.patternVariable().isPresent()) {
VariableTree patternVariable = instanceOfIr.patternVariable().get();
Type patternType = getType(patternVariable);
if (isSubtype(getType(constantExpression), patternType, state)) {
// RHS constant can be assigned to LHS pattern
Expand Down Expand Up @@ -1604,12 +1610,12 @@ public static boolean isDominatedBy(
Type lhsType =
lhs.instanceOfOptional().get().type() != null
? getType(lhs.instanceOfOptional().get().type())
: getType(lhs.instanceOfOptional().get().patternVariable().getType());
: getType(lhs.instanceOfOptional().get().patternVariable().get().getType());
var rhsInstanceOf = rhs.instanceOfOptional().get();
Type rhsType =
rhsInstanceOf.type() != null
? getType(rhsInstanceOf.type())
: getType(rhsInstanceOf.patternVariable().getType());
: getType(rhsInstanceOf.patternVariable().get().getType());
if (isSubtype(rhsType, lhsType, state)) {
// The LHS type is a subtype of the RHS type, so the LHS dominates the RHS
return true;
Expand All @@ -1626,9 +1632,9 @@ public static boolean isDominatedBy(
*/
record InstanceOfIr(
// In the example above, the expression would be `y`.
@Nullable ExpressionTree expression,
Optional<ExpressionTree> expression,
// In the example above, the variable tree would be `Y y`.
@Nullable VariableTree patternVariable,
Optional<VariableTree> patternVariable,
// In the example above, the type would be `Y`.
Tree type) {

Expand Down
Loading