diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/IsInstanceLambdaUsage.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/IsInstanceLambdaUsage.java index 874260702c..75c4e1b630 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/IsInstanceLambdaUsage.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/IsInstanceLambdaUsage.java @@ -6,15 +6,18 @@ import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL; import com.google.auto.service.AutoService; +import com.google.common.collect.Iterables; import com.google.errorprone.BugPattern; import com.google.errorprone.VisitorState; import com.google.errorprone.bugpatterns.BugChecker; import com.google.errorprone.bugpatterns.BugChecker.LambdaExpressionTreeMatcher; import com.google.errorprone.fixes.SuggestedFix; import com.google.errorprone.matchers.Description; +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; /** @@ -39,15 +42,19 @@ public IsInstanceLambdaUsage() {} @Override public Description matchLambdaExpression(LambdaExpressionTree tree, VisitorState state) { - if (tree.getKind() != Kind.LAMBDA_EXPRESSION || tree.getBody().getKind() != Kind.INSTANCE_OF) { + if (tree.getParameters().size() != 1 || tree.getBody().getKind() != Kind.INSTANCE_OF) { + 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; } return describeMatch( tree, SuggestedFix.replace( - tree, - SourceCode.treeToString(((InstanceOfTree) tree.getBody()).getType(), state) - + ".class::isInstance")); + tree, SourceCode.treeToString(instanceOf.getType(), state) + ".class::isInstance")); } } diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/IsInstanceLambdaUsageTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/IsInstanceLambdaUsageTest.java index 205cebd270..3249f5a464 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/IsInstanceLambdaUsageTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/IsInstanceLambdaUsageTest.java @@ -17,12 +17,28 @@ void identification() { .addSourceLines( "A.java", "import java.util.stream.Stream;", + "import reactor.core.publisher.Flux;", "", "class A {", " void m() {", + " Integer localVariable = 0;", + "", + " Stream.of(0).map(i -> i + 1);", + " Stream.of(1).filter(Integer.class::isInstance);", + " Stream.of(2).filter(i -> i.getClass() instanceof Class);", + " Stream.of(3).filter(i -> localVariable instanceof Integer);", + " // XXX: Ideally this case is also flagged. Pick this up in the context of merging the", + " // `IsInstanceLambdaUsage` and `MethodReferenceUsage` checks, or introduce a separate check that", + " // simplifies unnecessary block lambda expressions.", + " Stream.of(4)", + " .filter(", + " i -> {", + " return localVariable instanceof Integer;", + " });", + " Flux.just(5, \"foo\").distinctUntilChanged(v -> v, (a, b) -> a instanceof Integer);", + "", " // BUG: Diagnostic contains:", - " Stream.of(1).filter(i -> i instanceof Integer);", - " Stream.of(2).filter(Integer.class::isInstance);", + " Stream.of(6).filter(i -> i instanceof Integer);", " }", "}") .doTest();