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 new file mode 100644 index 0000000000..874260702c --- /dev/null +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/IsInstanceLambdaUsage.java @@ -0,0 +1,53 @@ +package tech.picnic.errorprone.bugpatterns; + +import static com.google.errorprone.BugPattern.LinkType.CUSTOM; +import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION; +import static com.google.errorprone.BugPattern.StandardTags.SIMPLIFICATION; +import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL; + +import com.google.auto.service.AutoService; +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.sun.source.tree.InstanceOfTree; +import com.sun.source.tree.LambdaExpressionTree; +import com.sun.source.tree.Tree.Kind; +import tech.picnic.errorprone.bugpatterns.util.SourceCode; + +/** + * A {@link BugChecker} that flags lambda expressions that can be replaced with a method reference + * of the form {@code T.class::isInstance}. + * + * @see MethodReferenceUsage + */ +// XXX: Consider folding this logic into the `MethodReferenceUsage` check. +@AutoService(BugChecker.class) +@BugPattern( + summary = "Prefer `Class::isInstance` method reference over equivalent lambda expression", + link = BUG_PATTERNS_BASE_URL + "IsInstanceLambdaUsage", + linkType = CUSTOM, + severity = SUGGESTION, + tags = SIMPLIFICATION) +public final class IsInstanceLambdaUsage extends BugChecker implements LambdaExpressionTreeMatcher { + private static final long serialVersionUID = 1L; + + /** Instantiates a new {@link IsInstanceLambdaUsage} instance. */ + public IsInstanceLambdaUsage() {} + + @Override + public Description matchLambdaExpression(LambdaExpressionTree tree, VisitorState state) { + if (tree.getKind() != Kind.LAMBDA_EXPRESSION || tree.getBody().getKind() != Kind.INSTANCE_OF) { + return Description.NO_MATCH; + } + + return describeMatch( + tree, + SuggestedFix.replace( + tree, + SourceCode.treeToString(((InstanceOfTree) tree.getBody()).getType(), state) + + ".class::isInstance")); + } +} diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MethodReferenceUsage.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MethodReferenceUsage.java index bc3d2afc85..87d4b3efd0 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MethodReferenceUsage.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MethodReferenceUsage.java @@ -37,6 +37,8 @@ /** * A {@link BugChecker} that flags lambda expressions that can be replaced with method references. + * + * @see IsInstanceLambdaUsage */ // XXX: Other custom expressions we could rewrite: // - `a -> "str" + a` to `"str"::concat`. But only if `str` is provably non-null. @@ -50,6 +52,9 @@ // `not(some::reference)`. // XXX: This check is extremely inefficient due to its reliance on `SuggestedFixes.compilesWithFix`. // Palantir's `LambdaMethodReference` check seems to suffer a similar issue at this time. +// XXX: Expressions of the form `i -> SomeType.class.isInstance(i)` are not replaced; fix that using +// a suitable generalization. +// XXX: Consider folding the `IsInstanceLambdaUsage` check into this class. @AutoService(BugChecker.class) @BugPattern( summary = "Prefer method references over lambda expressions", 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 new file mode 100644 index 0000000000..205cebd270 --- /dev/null +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/IsInstanceLambdaUsageTest.java @@ -0,0 +1,54 @@ +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 IsInstanceLambdaUsageTest { + private final CompilationTestHelper compilationTestHelper = + CompilationTestHelper.newInstance(IsInstanceLambdaUsage.class, getClass()); + private final BugCheckerRefactoringTestHelper refactoringTestHelper = + BugCheckerRefactoringTestHelper.newInstance(IsInstanceLambdaUsage.class, getClass()); + + @Test + void identification() { + compilationTestHelper + .addSourceLines( + "A.java", + "import java.util.stream.Stream;", + "", + "class A {", + " void m() {", + " // BUG: Diagnostic contains:", + " Stream.of(1).filter(i -> i instanceof Integer);", + " Stream.of(2).filter(Integer.class::isInstance);", + " }", + "}") + .doTest(); + } + + @Test + void replacement() { + refactoringTestHelper + .addInputLines( + "A.java", + "import java.util.stream.Stream;", + "", + "class A {", + " void m() {", + " Stream.of(1).filter(i -> i instanceof Integer);", + " }", + "}") + .addOutputLines( + "A.java", + "import java.util.stream.Stream;", + "", + "class A {", + " void m() {", + " Stream.of(1).filter(Integer.class::isInstance);", + " }", + "}") + .doTest(TestMode.TEXT_MATCH); + } +}