diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/ClassCastLambdaUsage.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/ClassCastLambdaUsage.java new file mode 100644 index 0000000000..11af5823db --- /dev/null +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/ClassCastLambdaUsage.java @@ -0,0 +1,65 @@ +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.utils.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.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}. + */ +// 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", + link = BUG_PATTERNS_BASE_URL + "ClassCastLambdaUsage", + linkType = CUSTOM, + severity = SUGGESTION, + tags = SIMPLIFICATION) +public final class ClassCastLambdaUsage extends BugChecker implements LambdaExpressionTreeMatcher { + private static final long serialVersionUID = 1L; + + /** 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; + } + + Type type = ASTHelpers.getType(typeCast); + if (type == null || type.isParameterized()) { + return Description.NO_MATCH; + } + + VariableTree param = Iterables.getOnlyElement(tree.getParameters()); + if (!ASTHelpers.getSymbol(param).equals(ASTHelpers.getSymbol(typeCast.getExpression()))) { + return Description.NO_MATCH; + } + + return describeMatch( + tree, + SuggestedFix.replace( + tree, SourceCode.treeToString(typeCast.getType(), state) + ".class::cast")); + } +} 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 2fa02a75e8..62bd72da8c 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 @@ -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", diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ClassRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ClassRules.java index 496b1f1fcb..5b0d47803a 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ClassRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ClassRules.java @@ -73,24 +73,6 @@ Predicate after(Class clazz) { } } - /** - * Prefer {@link Class#cast(Object)} method references over lambda expressions that require naming - * a variable. - */ - // XXX: Once the `ClassReferenceCast` rule is dropped, rename this rule to just `ClassCast`. - static final class ClassLiteralCast { - @BeforeTemplate - @SuppressWarnings("unchecked") - Function before() { - return t -> (S) t; - } - - @AfterTemplate - Function after() { - return Refaster.clazz()::cast; - } - } - /** * Prefer {@link Class#cast(Object)} method references over lambda expressions that require naming * a variable. diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/ClassCastLambdaUsageTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/ClassCastLambdaUsageTest.java new file mode 100644 index 0000000000..5b445ecacf --- /dev/null +++ b/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.of(ImmutableSet.of(5)).map(l -> (ImmutableSet) 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); + } +} 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 a6204c5abf..e57f0fbd41 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 @@ -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(); diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ClassRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ClassRulesTestInput.java index 94f6db36f9..59897bc36b 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ClassRulesTestInput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ClassRulesTestInput.java @@ -24,10 +24,6 @@ Predicate testClassReferenceIsInstancePredicate() { return s -> clazz.isInstance(s); } - Function testClassLiteralCast() { - return i -> (Integer) i; - } - Function testClassReferenceCast() { return i -> Integer.class.cast(i); } diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ClassRulesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ClassRulesTestOutput.java index 2c4314157a..fd205051bc 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ClassRulesTestOutput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ClassRulesTestOutput.java @@ -24,10 +24,6 @@ Predicate testClassReferenceIsInstancePredicate() { return clazz::isInstance; } - Function testClassLiteralCast() { - return Integer.class::cast; - } - Function testClassReferenceCast() { return Integer.class::cast; }