Suggestions

error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/ClassCast.java → error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/ClassCastLambdaUsage.java

@@ -17,36 +17,38 @@
 import com.sun.source.tree.LambdaExpressionTree;
 import com.sun.source.tree.TypeCastTree;
 import com.sun.source.tree.VariableTree;
+import com.sun.tools.javac.code.Type;
 import tech.picnic.errorprone.utils.SourceCode;
 
 /**
  * A {@link BugChecker} that flags lambda expressions that can be replaced with a method reference
- * of the form {@code T.class::cast}, if applicable.
+ * of the form {@code T.class::cast}.
  */
 // XXX: Consider folding this logic into the `MethodReferenceUsage` check of the
 // `error-prone-experimental` module.
+// XXX: This check and its tests are structurally nearly identical to `IsInstanceLambdaUsage`.
+// Unless folded into `MethodReferenceUsage`, consider merging the two.
 @AutoService(BugChecker.class)
 @BugPattern(
-    summary =
-        "Prefer `Class::cast` method reference over equivalent lambda expression, if applicable",
-    link = BUG_PATTERNS_BASE_URL + "ClassCast",
+    summary = "Prefer `Class::cast` method reference over equivalent lambda expression",
+    link = BUG_PATTERNS_BASE_URL + "ClassCastLambdaUsage",
     linkType = CUSTOM,
     severity = SUGGESTION,
     tags = SIMPLIFICATION)
-public final class ClassCast extends BugChecker implements LambdaExpressionTreeMatcher {
+public final class ClassCastLambdaUsage extends BugChecker implements LambdaExpressionTreeMatcher {
   private static final long serialVersionUID = 1L;
 
-  /** Instantiates a new {@link ClassCast} instance. */
-  public ClassCast() {}
+  /** Instantiates a new {@link ClassCastLambdaUsage} instance. */
+  public ClassCastLambdaUsage() {}
 
   @Override
   public Description matchLambdaExpression(LambdaExpressionTree tree, VisitorState state) {
     if (tree.getParameters().size() != 1 || !(tree.getBody() instanceof TypeCastTree typeCast)) {
       return Description.NO_MATCH;
     }
 
-    String classCast = SourceCode.treeToString(typeCast.getType(), state);
-    if (isGenericCastExpression(classCast)) {
+    Type type = ASTHelpers.getType(typeCast);
+    if (type == null || type.isParameterized()) {
       return Description.NO_MATCH;
     }
 
@@ -55,10 +57,9 @@ public Description matchLambdaExpression(LambdaExpressionTree tree, VisitorState
       return Description.NO_MATCH;
     }
 
-    return describeMatch(tree, SuggestedFix.replace(tree, classCast + ".class::cast"));
-  }
-
-  private static boolean isGenericCastExpression(String expression) {
-    return expression.contains("<") && expression.contains(">");
+    return describeMatch(
+        tree,
+        SuggestedFix.replace(
+            tree, SourceCode.treeToString(typeCast.getType(), state) + ".class::cast"));
   }
 }

error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/IsInstanceLambdaUsage.java

@@ -25,6 +25,8 @@
  */
 // XXX: Consider folding this logic into the `MethodReferenceUsage` check of the
 // `error-prone-experimental` module.
+// XXX: This check and its tests are structurally nearly identical to `ClassCastLambdaUsage`. Unless
+// folded into `MethodReferenceUsage`, consider merging the two.
 @AutoService(BugChecker.class)
 @BugPattern(
     summary = "Prefer `Class::isInstance` method reference over equivalent lambda expression",

error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/ClassCastLambdaUsageTest.java

@@ -0,0 +1,68 @@
+package tech.picnic.errorprone.bugpatterns;
+
+import com.google.errorprone.BugCheckerRefactoringTestHelper;
+import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode;
+import com.google.errorprone.CompilationTestHelper;
+import org.junit.jupiter.api.Test;
+
+final class ClassCastLambdaUsageTest {
+  @Test
+  void identification() {
+    CompilationTestHelper.newInstance(ClassCastLambdaUsage.class, getClass())
+        .addSourceLines(
+            "A.java",
+            "import com.google.common.collect.ImmutableSet;",
+            "import java.util.stream.Stream;",
+            "",
+            "class A {",
+            "  void m() {",
+            "    Number localVariable = 0;",
+            "",
+            "    Stream.of(0).map(i -> i);",
+            "    Stream.of(1).map(i -> i + 1);",
+            "    Stream.of(2).map(Integer.class::cast);",
+            "    Stream.of(3).map(i -> (Integer) 2);",
+            "    Stream.of(4).map(i -> (Integer) localVariable);",
+            "    // XXX: Ideally this case is also flagged. Pick this up in the context of merging the",
+            "    // `ClassCastLambdaUsage` and `MethodReferenceUsage` checks, or introduce a separate check that",
+            "    // simplifies unnecessary block lambda expressions.",
+            "    Stream.of(5)",
+            "        .map(",
+            "            i -> {",
+            "              return (Integer) i;",
+            "            });",
+            "    Stream.<ImmutableSet>of(ImmutableSet.of(5)).map(l -> (ImmutableSet<Number>) l);",
+            "    Stream.of(ImmutableSet.of(6)).map(l -> (ImmutableSet<?>) l);",
+            "    Stream.of(7).reduce((a, b) -> (Integer) a);",
+            "",
+            "    // BUG: Diagnostic contains:",
+            "    Stream.of(8).map(i -> (Integer) i);",
+            "  }",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  void replacement() {
+    BugCheckerRefactoringTestHelper.newInstance(ClassCastLambdaUsage.class, getClass())
+        .addInputLines(
+            "A.java",
+            "import java.util.stream.Stream;",
+            "",
+            "class A {",
+            "  void m() {",
+            "    Stream.of(1).map(i -> (Integer) i);",
+            "  }",
+            "}")
+        .addOutputLines(
+            "A.java",
+            "import java.util.stream.Stream;",
+            "",
+            "class A {",
+            "  void m() {",
+            "    Stream.of(1).map(Integer.class::cast);",
+            "  }",
+            "}")
+        .doTest(TestMode.TEXT_MATCH);
+  }
+}

error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/IsInstanceLambdaUsageTest.java

@@ -18,22 +18,23 @@ void identification() {
             "  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);",
+            "    Stream.of(0).map(i -> i);",
+            "    Stream.of(1).map(i -> i + 1);",
+            "    Stream.of(2).filter(Integer.class::isInstance);",
+            "    Stream.of(3).filter(i -> i.getClass() instanceof Class);",
+            "    Stream.of(4).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)",
+            "    Stream.of(5)",
             "        .filter(",
             "            i -> {",
-            "              return localVariable instanceof Integer;",
+            "              return i instanceof Integer;",
             "            });",
-            "    Flux.just(5, \"foo\").distinctUntilChanged(v -> v, (a, b) -> a instanceof Integer);",
+            "    Flux.just(6, \"foo\").distinctUntilChanged(v -> v, (a, b) -> a instanceof Integer);",
             "",
             "    // BUG: Diagnostic contains:",
-            "    Stream.of(6).filter(i -> i instanceof Integer);",
+            "    Stream.of(7).filter(i -> i instanceof Integer);",
             "  }",
             "}")
         .doTest();