Skip to content

Commit

Permalink
Use instanceof pattern matching
Browse files Browse the repository at this point in the history
  • Loading branch information
Stephan202 committed Apr 28, 2023
1 parent 42a6db5 commit c8fdc95
Show file tree
Hide file tree
Showing 19 changed files with 109 additions and 125 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
import com.sun.source.tree.AnnotationTree;
import com.sun.source.tree.ClassTree;
import com.sun.source.tree.MethodTree;
import com.sun.source.tree.Tree;
import com.sun.source.tree.Tree.Kind;
import com.sun.tools.javac.code.Symbol;
import java.util.Map;
import javax.lang.model.element.AnnotationValue;
Expand Down Expand Up @@ -46,7 +46,7 @@ public Description matchAnnotation(AnnotationTree tree, VisitorState state) {
}

ClassTree clazz = state.findEnclosing(ClassTree.class);
if (clazz == null || clazz.getKind() != Tree.Kind.ENUM) {
if (clazz == null || clazz.getKind() != Kind.ENUM) {
return Description.NO_MATCH;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import com.sun.source.tree.AssignmentTree;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.NewArrayTree;
import com.sun.source.tree.Tree.Kind;
import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
Expand Down Expand Up @@ -119,7 +118,7 @@ private static Optional<Fix> dropRedundantCurlies(AnnotationTree tree, VisitorSt
* the expression as a whole.
*/
ExpressionTree value =
(arg.getKind() == Kind.ASSIGNMENT) ? ((AssignmentTree) arg).getExpression() : arg;
(arg instanceof AssignmentTree assignment) ? assignment.getExpression() : arg;

/* Store a fix for each expression that was successfully simplified. */
simplifyAttributeValue(value, state)
Expand All @@ -130,13 +129,10 @@ private static Optional<Fix> dropRedundantCurlies(AnnotationTree tree, VisitorSt
}

private static Optional<String> simplifyAttributeValue(ExpressionTree expr, VisitorState state) {
if (expr.getKind() != Kind.NEW_ARRAY) {
/* There are no curly braces or commas to be dropped here. */
return Optional.empty();
}

NewArrayTree array = (NewArrayTree) expr;
return simplifySingletonArray(array, state).or(() -> dropTrailingComma(array, state));
/* Drop curly braces or commas if possible. */
return expr instanceof NewArrayTree newArray
? simplifySingletonArray(newArray, state).or(() -> dropTrailingComma(newArray, state))
: Optional.empty();
}

/** Returns the expression describing the array's sole element, if any. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,19 +98,17 @@ public Description matchBlock(BlockTree tree, VisitorState state) {
}

private static Optional<ExpressionTree> tryMatchAssignment(Symbol targetSymbol, Tree tree) {
if (tree instanceof ExpressionStatementTree) {
return tryMatchAssignment(targetSymbol, ((ExpressionStatementTree) tree).getExpression());
if (tree instanceof ExpressionStatementTree expressionStatement) {
return tryMatchAssignment(targetSymbol, expressionStatement.getExpression());
}

if (tree instanceof AssignmentTree) {
AssignmentTree assignment = (AssignmentTree) tree;
if (tree instanceof AssignmentTree assignment) {
return targetSymbol.equals(ASTHelpers.getSymbol(assignment.getVariable()))
? Optional.of(assignment.getExpression())
: Optional.empty();
}

if (tree instanceof VariableTree) {
VariableTree declaration = (VariableTree) tree;
if (tree instanceof VariableTree declaration) {
return declaration.getModifiers().getAnnotations().isEmpty()
&& targetSymbol.equals(ASTHelpers.getSymbol(declaration))
? Optional.ofNullable(declaration.getInitializer())
Expand Down Expand Up @@ -148,11 +146,11 @@ private static boolean isIdentifierSymbolReferencedInAssociatedFinallyBlock(
Streams.stream(state.getPath()).skip(1),
Streams.stream(state.getPath()),
(tree, child) -> {
if (!(tree instanceof TryTree)) {
if (!(tree instanceof TryTree tryTree)) {
return null;
}

BlockTree finallyBlock = ((TryTree) tree).getFinallyBlock();
BlockTree finallyBlock = tryTree.getFinallyBlock();
return !child.equals(finallyBlock) ? finallyBlock : null;
})
.anyMatch(finallyBlock -> referencesIdentifierSymbol(symbol, finallyBlock));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -240,8 +240,8 @@ private static class ReplacementArgumentsConstructor
}

private void appendExpression(Tree tree) {
if (tree instanceof LiteralTree) {
formatString.append(((LiteralTree) tree).getValue());
if (tree instanceof LiteralTree literal) {
formatString.append(literal.getValue());
} else {
formatString.append(formatSpecifier);
formatArguments.add(tree);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.InstanceOfTree;
import com.sun.source.tree.LambdaExpressionTree;
import com.sun.source.tree.Tree.Kind;
import com.sun.source.tree.VariableTree;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;

Expand All @@ -42,12 +41,12 @@ public IsInstanceLambdaUsage() {}

@Override
public Description matchLambdaExpression(LambdaExpressionTree tree, VisitorState state) {
if (tree.getParameters().size() != 1 || tree.getBody().getKind() != Kind.INSTANCE_OF) {
if (tree.getParameters().size() != 1
|| !(tree.getBody() instanceof InstanceOfTree instanceOf)) {
return Description.NO_MATCH;
}

VariableTree param = Iterables.getOnlyElement(tree.getParameters());
InstanceOfTree instanceOf = (InstanceOfTree) tree.getBody();
if (!ASTHelpers.getSymbol(param).equals(ASTHelpers.getSymbol(instanceOf.getExpression()))) {
return Description.NO_MATCH;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -257,8 +257,8 @@ private static Optional<String> tryExtractValueSourceAttributeValue(
arguments.stream()
.map(
arg ->
arg instanceof MethodInvocationTree
? Iterables.getOnlyElement(((MethodInvocationTree) arg).getArguments())
arg instanceof MethodInvocationTree methodInvocation
? Iterables.getOnlyElement(methodInvocation.getArguments())
: arg)
.map(argument -> SourceCode.treeToString(argument, state))
.collect(joining(", ")))
Expand All @@ -285,11 +285,10 @@ private static <T> Optional<T> getElementIfSingleton(Collection<T> collection) {
private static Matcher<ExpressionTree> isSingleDimensionArrayCreationWithAllElementsMatching(
Matcher<? super ExpressionTree> elementMatcher) {
return (tree, state) -> {
if (!(tree instanceof NewArrayTree)) {
if (!(tree instanceof NewArrayTree newArray)) {
return false;
}

NewArrayTree newArray = (NewArrayTree) tree;
return newArray.getDimensions().isEmpty()
&& !newArray.getInitializers().isEmpty()
&& newArray.getInitializers().stream()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
import com.sun.source.tree.NewArrayTree;
import com.sun.source.tree.PrimitiveTypeTree;
import com.sun.source.tree.Tree;
import com.sun.source.tree.Tree.Kind;
import com.sun.source.util.TreeScanner;
import com.sun.tools.javac.code.Symtab;
import com.sun.tools.javac.code.Type;
Expand Down Expand Up @@ -119,13 +118,9 @@ private Optional<Fix> sortArrayElements(AnnotationTree tree, VisitorState state)
}

private static Optional<NewArrayTree> extractArray(ExpressionTree expr) {
if (expr.getKind() == Kind.ASSIGNMENT) {
return extractArray(((AssignmentTree) expr).getExpression());
}

return Optional.of(expr)
.filter(e -> e.getKind() == Kind.NEW_ARRAY)
.map(NewArrayTree.class::cast);
return expr instanceof AssignmentTree assignment
? extractArray(assignment.getExpression())
: Optional.of(expr).filter(NewArrayTree.class::isInstance).map(NewArrayTree.class::cast);
}

private static Optional<SuggestedFix.Builder> suggestSorting(
Expand Down Expand Up @@ -197,8 +192,8 @@ private static ImmutableList<ImmutableList<String>> getStructure(ExpressionTree
public @Nullable Void visitLiteral(LiteralTree node, @Nullable Void unused) {
Object value = ASTHelpers.constValue(node);
nodes.add(
value instanceof String
? STRING_ARGUMENT_SPLITTER.splitToStream((String) value).collect(toImmutableList())
value instanceof String str
? STRING_ARGUMENT_SPLITTER.splitToStream(str).collect(toImmutableList())
: ImmutableList.of(String.valueOf(value)));

return super.visitLiteral(node, unused);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
import com.sun.source.tree.ParenthesizedTree;
import com.sun.source.tree.ReturnTree;
import com.sun.source.tree.Tree;
import com.sun.source.tree.Tree.Kind;
import com.sun.source.tree.VariableTree;
import com.sun.tools.javac.code.Symbol;
import com.sun.tools.javac.code.Type;
Expand Down Expand Up @@ -85,6 +84,7 @@ public Description matchLambdaExpression(LambdaExpressionTree tree, VisitorState
.orElse(Description.NO_MATCH);
}

// XXX: Use switch pattern matching once the targeted JDK supports this.
private static Optional<SuggestedFix.Builder> constructMethodRef(
LambdaExpressionTree lambdaExpr, Tree subTree) {
return switch (subTree.getKind()) {
Expand Down Expand Up @@ -114,34 +114,35 @@ private static Optional<SuggestedFix.Builder> constructMethodRef(
.flatMap(expectedInstance -> constructMethodRef(lambdaExpr, subTree, expectedInstance));
}

@SuppressWarnings(
"java:S1151" /* Extracting `IDENTIFIER` case block to separate method does not improve readability. */)
// XXX: Review whether to use switch pattern matching once the targeted JDK supports this.
private static Optional<SuggestedFix.Builder> constructMethodRef(
LambdaExpressionTree lambdaExpr,
MethodInvocationTree subTree,
Optional<Name> expectedInstance) {
ExpressionTree methodSelect = subTree.getMethodSelect();
return switch (methodSelect.getKind()) {
case IDENTIFIER -> {
if (expectedInstance.isPresent()) {
/* Direct method call; there is no matching "implicit parameter". */
yield Optional.empty();
}
Symbol sym = ASTHelpers.getSymbol(methodSelect);
yield ASTHelpers.isStatic(sym)
? constructFix(lambdaExpr, sym.owner, methodSelect)
: constructFix(lambdaExpr, "this", methodSelect);

if (methodSelect instanceof IdentifierTree) {
if (expectedInstance.isPresent()) {
/* Direct method call; there is no matching "implicit parameter". */
return Optional.empty();
}
case MEMBER_SELECT -> constructMethodRef(
lambdaExpr, (MemberSelectTree) methodSelect, expectedInstance);
default -> throw new VerifyException(
"Unexpected type of expression: " + methodSelect.getKind());
};

Symbol sym = ASTHelpers.getSymbol(methodSelect);
return ASTHelpers.isStatic(sym)
? constructFix(lambdaExpr, sym.owner, methodSelect)
: constructFix(lambdaExpr, "this", methodSelect);
}

if (methodSelect instanceof MemberSelectTree memberSelect) {
return constructMethodRef(lambdaExpr, memberSelect, expectedInstance);
}

throw new VerifyException("Unexpected type of expression: " + methodSelect.getKind());
}

private static Optional<SuggestedFix.Builder> constructMethodRef(
LambdaExpressionTree lambdaExpr, MemberSelectTree subTree, Optional<Name> expectedInstance) {
if (subTree.getExpression().getKind() != Kind.IDENTIFIER) {
if (!(subTree.getExpression() instanceof IdentifierTree identifier)) {
// XXX: Could be parenthesized. Handle. Also in other classes.
/*
* Only suggest a replacement if the method select's expression provably doesn't have
Expand All @@ -150,12 +151,12 @@ private static Optional<SuggestedFix.Builder> constructMethodRef(
return Optional.empty();
}

Name lhs = ((IdentifierTree) subTree.getExpression()).getName();
Name lhs = identifier.getName();
if (expectedInstance.isEmpty()) {
return constructFix(lambdaExpr, lhs, subTree.getIdentifier());
}

Type lhsType = ASTHelpers.getType(subTree.getExpression());
Type lhsType = ASTHelpers.getType(identifier);
if (lhsType == null || !expectedInstance.orElseThrow().equals(lhs)) {
return Optional.empty();
}
Expand All @@ -180,8 +181,8 @@ private static Optional<Optional<Name>> matchArguments(

for (int i = 0; i < args.size(); i++) {
ExpressionTree arg = args.get(i);
if (arg.getKind() != Kind.IDENTIFIER
|| !((IdentifierTree) arg).getName().equals(expectedArguments.get(i + diff))) {
if (!(arg instanceof IdentifierTree identifier)
|| !identifier.getName().equals(expectedArguments.get(i + diff))) {
return Optional.empty();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public MissingRefasterAnnotation() {}
public Description matchClass(ClassTree tree, VisitorState state) {
long methodTypes =
tree.getMembers().stream()
.filter(member -> member.getKind() == Tree.Kind.METHOD)
.filter(MethodTree.class::isInstance)
.map(MethodTree.class::cast)
.filter(method -> !ASTHelpers.isGeneratedConstructor(method))
.map(method -> REFASTER_ANNOTATION.matches(method, state))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState
return describeMatch(tree, SuggestedFixes.removeElement(arguments.get(0), arguments, state));
}

// XXX: Use switch pattern matching once the targeted JDK supports this.
private static boolean isTypeDerivableFromContext(MethodInvocationTree tree, VisitorState state) {
Tree parent = state.getPath().getParentPath().getLeaf();
return switch (parent.getKind()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.IdentifierTree;
import com.sun.source.tree.LambdaExpressionTree;
import com.sun.source.tree.MemberSelectTree;
import com.sun.source.tree.MethodInvocationTree;
Expand Down Expand Up @@ -146,39 +147,42 @@ private static String getPreferredMethod(Type cmpType, boolean isStatic, Visitor
return isStatic ? "comparing" : "thenComparing";
}

// XXX: Use switch pattern matching once the targeted JDK supports this.
private static Optional<Type> getPotentiallyBoxedReturnType(ExpressionTree tree) {
return switch (tree.getKind()) {
case LAMBDA_EXPRESSION -> {
/* Return the lambda expression's actual return type. */
yield Optional.ofNullable(ASTHelpers.getType(((LambdaExpressionTree) tree).getBody()));
}
case MEMBER_REFERENCE -> {
/* Return the method's declared return type. */
// XXX: Very fragile. Do better.
Type subType2 = ((JCMemberReference) tree).referentType;
yield Optional.of(subType2.getReturnType());
}
default -> {
/* This appears to be a genuine `{,ToInt,ToLong,ToDouble}Function`. */
yield Optional.empty();
}
};
if (tree instanceof LambdaExpressionTree lambdaExpression) {
/* Return the lambda expression's actual return type. */
return Optional.ofNullable(ASTHelpers.getType(lambdaExpression.getBody()));
}

// XXX: The match against a concrete type and reference to one of its fields is fragile. Do
// better.
if (tree instanceof JCMemberReference memberReference) {
/* Return the method's declared return type. */
Type subType = memberReference.referentType;
return Optional.of(subType.getReturnType());
}

/* This appears to be a genuine `{,ToInt,ToLong,ToDouble}Function`. */
return Optional.empty();
}

private static Fix suggestFix(
MethodInvocationTree tree, String preferredMethodName, VisitorState state) {
ExpressionTree expr = tree.getMethodSelect();
return switch (expr.getKind()) {
case IDENTIFIER -> SuggestedFix.builder()

if (expr instanceof IdentifierTree) {
return SuggestedFix.builder()
.addStaticImport(Comparator.class.getName() + '.' + preferredMethodName)
.replace(expr, preferredMethodName)
.build();
case MEMBER_SELECT -> {
MemberSelectTree ms = (MemberSelectTree) tree.getMethodSelect();
yield SuggestedFix.replace(
ms, SourceCode.treeToString(ms.getExpression(), state) + '.' + preferredMethodName);
}
default -> throw new VerifyException("Unexpected type of expression: " + expr.getKind());
};
}

if (expr instanceof MemberSelectTree memberSelect) {
return SuggestedFix.replace(
memberSelect,
SourceCode.treeToString(memberSelect.getExpression(), state) + '.' + preferredMethodName);
}

throw new VerifyException("Unexpected type of expression: " + expr.getKind());
}
}
Loading

0 comments on commit c8fdc95

Please sign in to comment.