From 0dcb8c1b8b6a2d2fb7ecc8a6ab02407179ff8a6d Mon Sep 17 00:00:00 2001 From: Gijs de Jong Date: Mon, 22 May 2023 12:55:45 +0200 Subject: [PATCH] Move matcher to `refaster-support` --- .../bugpatterns/util/MoreMatchers.java | 34 ---------- .../bugpatterns/util/MoreMatchersTest.java | 50 +-------------- .../refaster/matchers/HasTypeArguments.java | 38 ++++++++++++ .../matchers/HasTypeArgumentsTest.java | 62 +++++++++++++++++++ 4 files changed, 102 insertions(+), 82 deletions(-) create mode 100644 refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/HasTypeArguments.java create mode 100644 refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/HasTypeArgumentsTest.java diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/MoreMatchers.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/MoreMatchers.java index dba39904d2..b62e761db0 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/MoreMatchers.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/MoreMatchers.java @@ -5,11 +5,6 @@ import com.google.errorprone.predicates.TypePredicate; import com.google.errorprone.util.ASTHelpers; import com.sun.source.tree.AnnotationTree; -import com.sun.source.tree.ExpressionTree; -import com.sun.source.tree.MethodInvocationTree; -import com.sun.source.tree.NewClassTree; -import com.sun.source.tree.ParameterizedTypeTree; -import com.sun.source.tree.Tree; import com.sun.tools.javac.code.Symbol; /** @@ -37,35 +32,6 @@ public static Matcher hasMetaAnnotation(String ann }; } - /** - * Returns a {@link Matcher} that determines whether a given {@link ExpressionTree} has type - * arguments. - * - * @param The type of tree to match against. - * @return A {@link Matcher} that matches trees with type arguments. - */ - public static Matcher hasTypeArguments() { - return (tree, state) -> { - switch (tree.getKind()) { - case METHOD_INVOCATION: - return !((MethodInvocationTree) tree).getTypeArguments().isEmpty(); - case NEW_CLASS: - NewClassTree classTree = (NewClassTree) tree; - if (!classTree.getTypeArguments().isEmpty()) { - return true; - } - - if (classTree.getIdentifier().getKind() != Tree.Kind.PARAMETERIZED_TYPE) { - return false; - } - - return !((ParameterizedTypeTree) classTree.getIdentifier()).getTypeArguments().isEmpty(); - default: - return false; - } - }; - } - // XXX: Consider moving to a `MoreTypePredicates` utility class. private static TypePredicate hasAnnotation(String annotationClassName) { return (type, state) -> ASTHelpers.hasAnnotation(type.tsym, annotationClassName, state); diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/util/MoreMatchersTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/util/MoreMatchersTest.java index 460e4ec15f..842fb57b6b 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/util/MoreMatchersTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/util/MoreMatchersTest.java @@ -7,20 +7,15 @@ import com.google.errorprone.VisitorState; import com.google.errorprone.bugpatterns.BugChecker; import com.google.errorprone.bugpatterns.BugChecker.AnnotationTreeMatcher; -import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher; -import com.google.errorprone.bugpatterns.BugChecker.NewClassTreeMatcher; import com.google.errorprone.matchers.Description; import com.google.errorprone.matchers.Matcher; import com.sun.source.tree.AnnotationTree; -import com.sun.source.tree.ExpressionTree; -import com.sun.source.tree.MethodInvocationTree; -import com.sun.source.tree.NewClassTree; import org.junit.jupiter.api.Test; final class MoreMatchersTest { @Test void hasMetaAnnotation() { - CompilationTestHelper.newInstance(MetaAnnotationTestMatcher.class, getClass()) + CompilationTestHelper.newInstance(TestMatcher.class, getClass()) .addSourceLines( "A.java", "import org.junit.jupiter.api.AfterAll;", @@ -54,8 +49,7 @@ void hasMetaAnnotation() { /** A {@link BugChecker} that delegates to {@link MoreMatchers#hasMetaAnnotation(String)} . */ @BugPattern(summary = "Interacts with `MoreMatchers` for testing purposes", severity = ERROR) - public static final class MetaAnnotationTestMatcher extends BugChecker - implements AnnotationTreeMatcher { + public static final class TestMatcher extends BugChecker implements AnnotationTreeMatcher { private static final long serialVersionUID = 1L; private static final Matcher DELEGATE = MoreMatchers.hasMetaAnnotation("org.junit.jupiter.api.TestTemplate"); @@ -65,44 +59,4 @@ public Description matchAnnotation(AnnotationTree tree, VisitorState state) { return DELEGATE.matches(tree, state) ? describeMatch(tree) : Description.NO_MATCH; } } - - @Test - void hasTypeArgumentsMatcher() { - CompilationTestHelper.newInstance(HasTypeArgumentsTestMatcher.class, getClass()) - .addSourceLines( - "A.java", - "import com.google.common.collect.ImmutableSet;", - "import java.util.ArrayList;", - "import java.util.List;", - "", - "class A {", - " void foo(E first) {", - " // BUG: Diagnostic contains:", - " ImmutableSet.builder().add(first).build();", - " // BUG: Diagnostic contains:", - " new ImmutableSet.Builder().add(first).build();", - "", - " ImmutableSet foo = new ImmutableSet.Builder().add(1).build();", - " List bar = new ArrayList<>();", - " }", - "}") - .doTest(); - } - /** A {@link BugChecker} that delegates to {@link MoreMatchers#hasTypeArguments()} . */ - @BugPattern(summary = "Interacts with `MoreMatchers` for testing purposes", severity = ERROR) - public static final class HasTypeArgumentsTestMatcher extends BugChecker - implements MethodInvocationTreeMatcher, NewClassTreeMatcher { - private static final long serialVersionUID = 1L; - private static final Matcher DELEGATE = MoreMatchers.hasTypeArguments(); - - @Override - public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { - return DELEGATE.matches(tree, state) ? describeMatch(tree) : Description.NO_MATCH; - } - - @Override - public Description matchNewClass(NewClassTree tree, VisitorState state) { - return DELEGATE.matches(tree, state) ? describeMatch(tree) : Description.NO_MATCH; - } - } } diff --git a/refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/HasTypeArguments.java b/refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/HasTypeArguments.java new file mode 100644 index 0000000000..71838d9829 --- /dev/null +++ b/refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/HasTypeArguments.java @@ -0,0 +1,38 @@ +package tech.picnic.errorprone.refaster.matchers; + +import com.google.errorprone.VisitorState; +import com.google.errorprone.matchers.Matcher; +import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.MethodInvocationTree; +import com.sun.source.tree.NewClassTree; +import com.sun.source.tree.ParameterizedTypeTree; +import com.sun.source.tree.Tree; + +/** A matcher of expressions with type arguments. */ +public final class HasTypeArguments implements Matcher { + private static final long serialVersionUID = 1L; + + /** Instantiates a new {@link HasTypeArguments} instance. */ + public HasTypeArguments() {} + + @Override + public boolean matches(ExpressionTree tree, VisitorState state) { + switch (tree.getKind()) { + case METHOD_INVOCATION: + return !((MethodInvocationTree) tree).getTypeArguments().isEmpty(); + case NEW_CLASS: + NewClassTree classTree = (NewClassTree) tree; + if (!classTree.getTypeArguments().isEmpty()) { + return true; + } + + if (classTree.getIdentifier().getKind() != Tree.Kind.PARAMETERIZED_TYPE) { + return false; + } + + return !((ParameterizedTypeTree) classTree.getIdentifier()).getTypeArguments().isEmpty(); + default: + return false; + } + } +} diff --git a/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/HasTypeArgumentsTest.java b/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/HasTypeArgumentsTest.java new file mode 100644 index 0000000000..608aed2767 --- /dev/null +++ b/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/HasTypeArgumentsTest.java @@ -0,0 +1,62 @@ +package tech.picnic.errorprone.refaster.matchers; + +import static com.google.errorprone.BugPattern.SeverityLevel.ERROR; + +import com.google.errorprone.BugPattern; +import com.google.errorprone.CompilationTestHelper; +import com.google.errorprone.bugpatterns.BugChecker; +import org.junit.jupiter.api.Test; + +final class HasTypeArgumentsTest { + @Test + void matches() { + CompilationTestHelper.newInstance(MatcherTestChecker.class, getClass()) + .addSourceLines( + "A.java", + "import com.google.common.collect.ImmutableSet;", + "import java.util.ArrayList;", + "import java.util.List;", + "", + "class A {", + " Object negative1() {", + " return alwaysNull();", + " }", + "", + " Object negative2() {", + " return new Object();", + " }", + "", + " List negative3() {", + " return new ArrayList<>();", + " }", + "", + " ImmutableSet positive1() {", + " // BUG: Diagnostic contains:", + " return ImmutableSet.builder().build();", + " }", + "", + " ImmutableSet positive2() {", + " // BUG: Diagnostic contains:", + " return new ImmutableSet.Builder().build();", + " }", + "", + " private static T alwaysNull() {", + " return null;", + " }", + "}") + .doTest(); + } + + /** A {@link BugChecker} that simply delegates to {@link HasTypeArguments}. */ + @BugPattern(summary = "Flags expressions matched by `HasTypeArguments`", severity = ERROR) + public static final class MatcherTestChecker extends AbstractMatcherTestChecker { + private static final long serialVersionUID = 1L; + + // XXX: This is a false positive reported by Checkstyle. See + // https://github.com/checkstyle/checkstyle/issues2/10161#issuecomment-1242732120. + @SuppressWarnings("RedundantModifier") + public MatcherTestChecker() { + super(new HasTypeArguments()); + } + } +}