Improve IsInstanceLambdaUsage check (#401)

Fixes #399.
PicnicSupermarket · Dec 9, 2022 · 096acfb · 096acfb
1 parent 17bcdb6
commit 096acfb
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 6 deletions.
diff --git a/...prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/IsInstanceLambdaUsage.java b/...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/...e-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/IsInstanceLambdaUsageTest.java b/...e-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();