Skip to content

Commit

Permalink
Simplify yet more instanceofs, notably:
Browse files Browse the repository at this point in the history
  * inverted conditions (especially with a definite return inside the `then`, which is common
  * casts within an expression, e.g. `foo instanceof Bar && ((Bar) foo).frob()`.

I did a pile of extra simplification. You end up with a lot of redundant variables.

PiperOrigin-RevId: 716254421
  • Loading branch information
graememorgan authored and Error Prone Team committed Jan 16, 2025
1 parent 82de9ad commit 249c359
Show file tree
Hide file tree
Showing 164 changed files with 389 additions and 530 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -246,10 +246,7 @@ public int hashCode() {

@Override
public boolean equals(Object o) {
if (!(o instanceof BugCheckerInfo)) {
return false;
}
return checker.equals(((BugCheckerInfo) o).checker);
return o instanceof BugCheckerInfo bugCheckerInfo && checker.equals(bugCheckerInfo.checker);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -600,11 +600,8 @@ public interface YieldTreeMatcher extends Suppressible {

@Override
public boolean equals(Object obj) {
if (!(obj instanceof BugChecker)) {
return false;
}
BugChecker that = (BugChecker) obj;
return this.canonicalName().equals(that.canonicalName())
return obj instanceof BugChecker that
&& this.canonicalName().equals(that.canonicalName())
&& this.defaultSeverity().equals(that.defaultSeverity())
&& this.supportsSuppressWarnings() == that.supportsSuppressWarnings()
&& this.customSuppressionAnnotations().equals(that.customSuppressionAnnotations());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ public ControlFlowGraph getControlFlowGraph() {
}

Tree method = enclosingMethodPath.getLeaf();
if (method instanceof MethodTree && ((MethodTree) method).getBody() == null) {
if (method instanceof MethodTree methodTree && methodTree.getBody() == null) {
// expressions can occur in abstract methods, for example {@code Map.Entry} in:
//
// abstract Set<Map.Entry<K, V>> entries();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -454,8 +454,8 @@ private void generateConstraintsForWrite(
} else if ((rVal instanceof LiteralTree)
|| (rVal instanceof NewClassTree)
|| (rVal instanceof NewArrayTree)
|| ((rVal instanceof IdentifierTree)
&& ((IdentifierTree) rVal).getName().contentEquals("this"))) {
|| ((rVal instanceof IdentifierTree identifierTree)
&& identifierTree.getName().contentEquals("this"))) {
qualifierConstraints.putEdge(
ProperInferenceVar.NONNULL, TypeArgInferenceVar.create(ImmutableList.of(), rVal));
qualifierConstraints.putEdge(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ public Builder merge(@Nullable SuggestedFix other) {
* synthetic constructs added to the AST early enough to be visible from Error Prone.
*/
private static void checkNotSyntheticConstructor(Tree tree) {
if (tree instanceof MethodTree && ASTHelpers.isGeneratedConstructor((MethodTree) tree)) {
if (tree instanceof MethodTree methodTree && ASTHelpers.isGeneratedConstructor(methodTree)) {
throw new IllegalArgumentException("Cannot edit synthetic AST nodes");
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -626,7 +626,8 @@ private static boolean definedInSourceFile(Tree member) {
if (sym == null) {
return false;
}
if (member instanceof MethodTree && ASTHelpers.isGeneratedConstructor((MethodTree) member)) {
if (member instanceof MethodTree methodTree
&& ASTHelpers.isGeneratedConstructor(methodTree)) {
return false;
}

Expand Down Expand Up @@ -1100,11 +1101,11 @@ private static List<? extends AnnotationTree> findAnnotationsTree(Tree tree) {
tree ->
tree instanceof MethodTree
// Anonymous classes can't be suppressed
|| (tree instanceof ClassTree
&& ((ClassTree) tree).getSimpleName().length() != 0)
|| (tree instanceof ClassTree classTree
&& classTree.getSimpleName().length() != 0)
// Lambda parameters can't be suppressed unless they have Type decls
|| (tree instanceof VariableTree
&& !hasImplicitType((VariableTree) tree, state)))
|| (tree instanceof VariableTree variableTree
&& !hasImplicitType(variableTree, state)))
.findFirst()
.orElse(null);
}
Expand Down Expand Up @@ -1635,11 +1636,10 @@ public static SuggestedFix replaceIncludingComments(
TreePath path, String replacement, VisitorState state) {
Tree tree = path.getLeaf();
Tree parent = path.getParentPath().getLeaf();
if (!(parent instanceof ClassTree)) {
if (!(parent instanceof ClassTree classTree)) {
return SuggestedFix.replace(tree, replacement);
}
Tree previousMember = null;
ClassTree classTree = (ClassTree) parent;
int startTokenization;
for (Tree member : classTree.getMembers()) {
if (member instanceof MethodTree methodTree
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,7 @@ public Asserts(Matcher<ExpressionTree> expressionMatcher) {

@Override
public boolean matches(StatementTree statementTree, VisitorState state) {
if (!(statementTree instanceof AssertTree)) {
return false;
}

return expressionMatcher.matches(((AssertTree) statementTree).getCondition(), state);
return statementTree instanceof AssertTree assertTree
&& expressionMatcher.matches(assertTree.getCondition(), state);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -366,11 +366,8 @@ public static Matcher<ExpressionTree> methodInvocation(
public static Matcher<ExpressionTree> methodInvocation(
Matcher<ExpressionTree> methodSelectMatcher) {
return (expressionTree, state) -> {
if (!(expressionTree instanceof MethodInvocationTree)) {
return false;
}
MethodInvocationTree tree = (MethodInvocationTree) expressionTree;
return methodSelectMatcher.matches(tree.getMethodSelect(), state);
return expressionTree instanceof MethodInvocationTree tree
&& methodSelectMatcher.matches(tree.getMethodSelect(), state);
};
}

Expand Down Expand Up @@ -954,7 +951,7 @@ public static Matcher<MethodTree> constructorOfClass(String className) {
public static Matcher<ClassTree> hasMethod(Matcher<MethodTree> methodMatcher) {
return (t, state) -> {
for (Tree member : t.getMembers()) {
if (member instanceof MethodTree && methodMatcher.matches((MethodTree) member, state)) {
if (member instanceof MethodTree methodTree && methodMatcher.matches(methodTree, state)) {
return true;
}
}
Expand Down Expand Up @@ -1087,8 +1084,8 @@ public static Matcher<StatementTree> continueStatement() {
/** Matches an {@link ExpressionStatementTree} based on its {@link ExpressionTree}. */
public static Matcher<StatementTree> expressionStatement(Matcher<ExpressionTree> matcher) {
return (statementTree, state) ->
statementTree instanceof ExpressionStatementTree
&& matcher.matches(((ExpressionStatementTree) statementTree).getExpression(), state);
statementTree instanceof ExpressionStatementTree expressionStatementTree
&& matcher.matches(expressionStatementTree.getExpression(), state);
}

static Matcher<Tree> isSymbol(java.lang.Class<? extends Symbol> symbolClass) {
Expand Down Expand Up @@ -1581,10 +1578,10 @@ public static Matcher<MethodTree> compareToMethodDeclaration() {
public static Matcher<StatementTree> matchExpressionReturn(
Matcher<ExpressionTree> expressionTreeMatcher) {
return (statement, state) -> {
if (!(statement instanceof ReturnTree)) {
if (!(statement instanceof ReturnTree returnTree)) {
return false;
}
ExpressionTree expression = ((ReturnTree) statement).getExpression();
ExpressionTree expression = returnTree.getExpression();
if (expression == null) {
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,8 @@ public MethodInvocation(

@Override
public boolean matches(ExpressionTree expressionTree, VisitorState state) {
if (!(expressionTree instanceof MethodInvocationTree)) {
return false;
}

MethodInvocationTree tree = (MethodInvocationTree) expressionTree;

return methodSelectMatcher.matches(tree.getMethodSelect(), state)
return expressionTree instanceof MethodInvocationTree tree
&& methodSelectMatcher.matches(tree.getMethodSelect(), state)
&& methodArgumentMatcher.matches(tree, state);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,7 @@ public Returns(Matcher<? super ExpressionTree> returnedMatcher) {

@Override
public boolean matches(StatementTree expressionTree, VisitorState state) {
if (!(expressionTree instanceof ReturnTree)) {
return false;
}

return returnedMatcher.matches(((ReturnTree) expressionTree).getExpression(), state);
return expressionTree instanceof ReturnTree returnTree
&& returnedMatcher.matches(returnTree.getExpression(), state);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,8 @@ public StringLiteral(Pattern pattern) {

@Override
public boolean matches(ExpressionTree expressionTree, VisitorState state) {
if (expressionTree instanceof LiteralTree literalTree) {
Object actualValue = literalTree.getValue();
return actualValue instanceof String && matcher.test((String) actualValue);
} else {
return false;
}
return expressionTree instanceof LiteralTree literalTree
&& literalTree.getValue() instanceof String string
&& matcher.test(string);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,7 @@ public Throws(Matcher<? super ExpressionTree> thrownMatcher) {

@Override
public boolean matches(StatementTree expressionTree, VisitorState state) {
if (!(expressionTree instanceof ThrowTree)) {
return false;
}

return thrownMatcher.matches(((ThrowTree) expressionTree).getExpression(), state);
return expressionTree instanceof ThrowTree throwTree
&& thrownMatcher.matches(throwTree.getExpression(), state);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ private static boolean implementsVoidMethod(ExpressionTree tree, VisitorState st
*/
public static boolean isReturnValueUnused(ExpressionTree tree, VisitorState state) {
Symbol sym = getSymbol(tree);
if (!(sym instanceof MethodSymbol) || isVoidMethod((MethodSymbol) sym)) {
if (!(sym instanceof MethodSymbol methodSymbol) || isVoidMethod(methodSymbol)) {
return false;
}
if (tree instanceof MemberReferenceTree) {
Expand Down Expand Up @@ -255,10 +255,9 @@ public static boolean expectedExceptionTest(VisitorState state) {
* doReturn(val).when(t)}.
*/
public static boolean mockitoInvocation(Tree tree, VisitorState state) {
if (!(tree instanceof JCMethodInvocation)) {
if (!(tree instanceof JCMethodInvocation invocation)) {
return false;
}
JCMethodInvocation invocation = (JCMethodInvocation) tree;
if (!(invocation.getMethodSelect() instanceof JCFieldAccess)) {
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ interface BaseMethodMatcher {
BaseMethodMatcher METHOD =
tree -> {
Symbol sym = ASTHelpers.getSymbol(tree);
if (!(sym instanceof MethodSymbol)) {
if (!(sym instanceof MethodSymbol methodSymbol)) {
return null;
}
if (tree instanceof NewClassTree) {
Expand All @@ -39,7 +39,7 @@ interface BaseMethodMatcher {
if (tree instanceof MethodInvocationTree methodInvocationTree) {
tree = methodInvocationTree.getMethodSelect();
}
return MethodMatchState.create(tree, (MethodSymbol) sym);
return MethodMatchState.create(tree, methodSymbol);
};

BaseMethodMatcher CONSTRUCTOR =
Expand All @@ -51,10 +51,9 @@ interface BaseMethodMatcher {
}
}
Symbol sym = ASTHelpers.getSymbol(tree);
if (!(sym instanceof MethodSymbol)) {
if (!(sym instanceof MethodSymbol method)) {
return null;
}
MethodSymbol method = (MethodSymbol) sym;
if (!method.isConstructor()) {
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,10 @@ static final class Context {

public static Optional<Context> create(ExpressionTree tree) {
Symbol sym = ASTHelpers.getSymbol(tree);
if (!(sym instanceof MethodSymbol)) {
if (!(sym instanceof MethodSymbol methodSymbol)) {
return Optional.empty();
}
return Optional.of(new Context((MethodSymbol) sym, tree));
return Optional.of(new Context(methodSymbol, tree));
}
}

Expand Down
Loading

0 comments on commit 249c359

Please sign in to comment.