diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/ClassCast.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/ClassCast.java new file mode 100644 index 00000000000..353340c1e53 --- /dev/null +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/ClassCast.java @@ -0,0 +1,64 @@ +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 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. + */ +// XXX: Consider folding this logic into the `MethodReferenceUsage` check of the +// `error-prone-experimental` module. +@AutoService(BugChecker.class) +@BugPattern( + summary = + "Prefer `Class::cast` method reference over equivalent lambda expression, if applicable", + link = BUG_PATTERNS_BASE_URL + "ClassCast", + linkType = CUSTOM, + severity = SUGGESTION, + tags = SIMPLIFICATION) +public final class ClassCast extends BugChecker implements LambdaExpressionTreeMatcher { + private static final long serialVersionUID = 1L; + + /** Instantiates a new {@link ClassCast} instance. */ + public ClassCast() {} + + @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)) { + 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, classCast + ".class::cast")); + } + + private static boolean isGenericCastExpression(String expression) { + return expression.contains("<") && expression.contains(">"); + } +} 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 496b1f1fcbb..5b0d47803a7 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/ClassCastTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/ClassCastTest.java new file mode 100644 index 00000000000..6106d2faa80 --- /dev/null +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/ClassCastTest.java @@ -0,0 +1,62 @@ +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 ClassCastTest { + @Test + void identification() { + CompilationTestHelper.newInstance(ClassCast.class, getClass()) + .addSourceLines( + "A.java", + "import com.google.common.collect.ImmutableList;", + "import com.google.common.collect.ImmutableSet;", + "import java.util.Optional;", + "import java.util.Set;", + "", + "class A {", + " void m() {", + " Number foo = 0;", + " Number bar = 1;", + "", + " // BUG: Diagnostic contains:", + " Optional.of(foo).map(i -> (Integer) i);", + "", + " Optional.of(foo).map(i -> 2);", + " Optional.of(foo).map(i -> (Integer) 2);", + " Optional.of(foo).map(i -> bar);", + " Optional.of(foo).map(i -> (Integer) bar);", + "", + " ImmutableList.of(Set.of(foo)).stream().map(l -> (ImmutableSet) l);", + " ImmutableList.of(Set.of(foo)).stream().map(l -> (ImmutableSet) l);", + " }", + "}") + .doTest(); + } + + @Test + void replacement() { + BugCheckerRefactoringTestHelper.newInstance(ClassCast.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/resources/tech/picnic/errorprone/refasterrules/ClassRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/ClassRulesTestInput.java index 94f6db36f90..59897bc36be 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 2c4314157a8..fd205051bc4 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; }