From 84d425e4ca499df11998a641b15e87454ea78b67 Mon Sep 17 00:00:00 2001 From: Eric Staffas <114935344+eric-picnic@users.noreply.github.com> Date: Tue, 22 Nov 2022 13:35:03 +0100 Subject: [PATCH] Introduce `More{ASTHelpers,JUnitMatchers,Matchers}` utility classes (#335) --- .../bugpatterns/JUnitMethodDeclaration.java | 48 +---- .../bugpatterns/util/MoreASTHelpers.java | 49 +++++ .../bugpatterns/util/MoreJUnitMatchers.java | 81 ++++++++ .../bugpatterns/util/MoreMatchers.java | 39 ++++ .../bugpatterns/util/MoreASTHelpersTest.java | 110 +++++++++++ .../util/MoreJUnitMatchersTest.java | 173 ++++++++++++++++++ .../bugpatterns/util/MoreMatchersTest.java | 62 +++++++ 7 files changed, 518 insertions(+), 44 deletions(-) create mode 100644 error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/MoreASTHelpers.java create mode 100644 error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/MoreJUnitMatchers.java create mode 100644 error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/MoreMatchers.java create mode 100644 error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/util/MoreASTHelpersTest.java create mode 100644 error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/util/MoreJUnitMatchersTest.java create mode 100644 error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/util/MoreMatchersTest.java diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/JUnitMethodDeclaration.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/JUnitMethodDeclaration.java index 26373ba53e..5b9a67388e 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/JUnitMethodDeclaration.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/JUnitMethodDeclaration.java @@ -13,6 +13,8 @@ import static java.util.function.Predicate.not; import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL; import static tech.picnic.errorprone.bugpatterns.util.JavaKeywords.isReservedKeyword; +import static tech.picnic.errorprone.bugpatterns.util.MoreJUnitMatchers.SETUP_OR_TEARDOWN_METHOD; +import static tech.picnic.errorprone.bugpatterns.util.MoreJUnitMatchers.TEST_METHOD; import com.google.auto.service.AutoService; import com.google.common.collect.ImmutableSet; @@ -25,18 +27,13 @@ import com.google.errorprone.matchers.Description; import com.google.errorprone.matchers.Matcher; import com.google.errorprone.matchers.Matchers; -import com.google.errorprone.matchers.MultiMatcher; -import com.google.errorprone.predicates.TypePredicate; import com.google.errorprone.util.ASTHelpers; -import com.sun.source.tree.AnnotationTree; -import com.sun.source.tree.ClassTree; import com.sun.source.tree.ImportTree; import com.sun.source.tree.MethodTree; import com.sun.source.tree.Tree; -import com.sun.tools.javac.code.Symbol; import java.util.Optional; import javax.lang.model.element.Modifier; -import javax.lang.model.element.Name; +import tech.picnic.errorprone.bugpatterns.util.MoreASTHelpers; import tech.picnic.errorprone.bugpatterns.util.SourceCode; /** A {@link BugChecker} that flags non-canonical JUnit method declarations. */ @@ -64,20 +61,6 @@ public final class JUnitMethodDeclaration extends BugChecker implements MethodTr Matchers.not(hasModifier(Modifier.FINAL)), Matchers.not(hasModifier(Modifier.PRIVATE)), enclosingClass(hasModifier(Modifier.ABSTRACT)))); - private static final MultiMatcher TEST_METHOD = - annotations( - AT_LEAST_ONE, - anyOf( - isType("org.junit.jupiter.api.Test"), - hasMetaAnnotation("org.junit.jupiter.api.TestTemplate"))); - private static final MultiMatcher SETUP_OR_TEARDOWN_METHOD = - annotations( - AT_LEAST_ONE, - anyOf( - isType("org.junit.jupiter.api.AfterAll"), - isType("org.junit.jupiter.api.AfterEach"), - isType("org.junit.jupiter.api.BeforeAll"), - isType("org.junit.jupiter.api.BeforeEach"))); /** Instantiates a new {@link JUnitMethodDeclaration} instance. */ public JUnitMethodDeclaration() {} @@ -142,7 +125,7 @@ private void reportMethodRenameBlocker(MethodTree tree, String reason, VisitorSt * */ private static Optional findMethodRenameBlocker(String methodName, VisitorState state) { - if (isMethodInEnclosingClass(methodName, state)) { + if (MoreASTHelpers.methodExistsInEnclosingClass(methodName, state)) { return Optional.of( String.format("a method named `%s` already exists in this class", methodName)); } @@ -158,15 +141,6 @@ private static Optional findMethodRenameBlocker(String methodName, Visit return Optional.empty(); } - private static boolean isMethodInEnclosingClass(String methodName, VisitorState state) { - return state.findEnclosing(ClassTree.class).getMembers().stream() - .filter(MethodTree.class::isInstance) - .map(MethodTree.class::cast) - .map(MethodTree::getName) - .map(Name::toString) - .anyMatch(methodName::equals); - } - private static boolean isSimpleNameStaticallyImported(String simpleName, VisitorState state) { return state.getPath().getCompilationUnit().getImports().stream() .filter(ImportTree::isStatic) @@ -188,18 +162,4 @@ private static Optional tryCanonicalizeMethodName(MethodTree tree) { .map(name -> Character.toLowerCase(name.charAt(0)) + name.substring(1)) .filter(name -> !Character.isDigit(name.charAt(0))); } - - // XXX: Move to a `MoreMatchers` utility class. - private static Matcher hasMetaAnnotation(String annotationClassName) { - TypePredicate typePredicate = hasAnnotation(annotationClassName); - return (tree, state) -> { - Symbol sym = ASTHelpers.getSymbol(tree); - return sym != null && typePredicate.apply(sym.type, state); - }; - } - - // XXX: Move 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/main/java/tech/picnic/errorprone/bugpatterns/util/MoreASTHelpers.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/MoreASTHelpers.java new file mode 100644 index 0000000000..ccfd3353bb --- /dev/null +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/MoreASTHelpers.java @@ -0,0 +1,49 @@ +package tech.picnic.errorprone.bugpatterns.util; + +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.collect.ImmutableList.toImmutableList; + +import com.google.common.collect.ImmutableList; +import com.google.errorprone.VisitorState; +import com.sun.source.tree.ClassTree; +import com.sun.source.tree.MethodTree; + +/** + * A collection of helper methods for working with the AST. + * + *

These methods are additions to the ones found in {@link + * com.google.errorprone.util.ASTHelpers}. + */ +public final class MoreASTHelpers { + private MoreASTHelpers() {} + + /** + * Finds methods with the specified name in given the {@link VisitorState}'s current enclosing + * class. + * + * @param methodName The method name to search for. + * @param state The {@link VisitorState} from which to derive the enclosing class of interest. + * @return The {@link MethodTree}s of the methods with the given name in the enclosing class. + */ + public static ImmutableList findMethods(CharSequence methodName, VisitorState state) { + ClassTree clazz = state.findEnclosing(ClassTree.class); + checkArgument(clazz != null, "Visited node is not enclosed by a class"); + return clazz.getMembers().stream() + .filter(MethodTree.class::isInstance) + .map(MethodTree.class::cast) + .filter(method -> method.getName().contentEquals(methodName)) + .collect(toImmutableList()); + } + + /** + * Determines whether there are any methods with the specified name in given the {@link + * VisitorState}'s current enclosing class. + * + * @param methodName The method name to search for. + * @param state The {@link VisitorState} from which to derive the enclosing class of interest. + * @return Whether there are any methods with the given name in the enclosing class. + */ + public static boolean methodExistsInEnclosingClass(CharSequence methodName, VisitorState state) { + return !findMethods(methodName, state).isEmpty(); + } +} diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/MoreJUnitMatchers.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/MoreJUnitMatchers.java new file mode 100644 index 0000000000..59aacd1791 --- /dev/null +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/MoreJUnitMatchers.java @@ -0,0 +1,81 @@ +package tech.picnic.errorprone.bugpatterns.util; + +import static com.google.common.collect.ImmutableSet.toImmutableSet; +import static com.google.errorprone.matchers.ChildMultiMatcher.MatchType.AT_LEAST_ONE; +import static com.google.errorprone.matchers.Matchers.annotations; +import static com.google.errorprone.matchers.Matchers.anyOf; +import static com.google.errorprone.matchers.Matchers.isType; +import static java.util.Objects.requireNonNullElse; +import static tech.picnic.errorprone.bugpatterns.util.MoreMatchers.hasMetaAnnotation; + +import com.google.common.base.Strings; +import com.google.common.collect.ImmutableSet; +import com.google.errorprone.matchers.AnnotationMatcherUtils; +import com.google.errorprone.matchers.Matcher; +import com.google.errorprone.matchers.MultiMatcher; +import com.google.errorprone.util.ASTHelpers; +import com.sun.source.tree.AnnotationTree; +import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.MethodTree; +import com.sun.source.tree.NewArrayTree; +import org.jspecify.nullness.Nullable; + +/** + * A collection of JUnit-specific helper methods and {@link Matcher}s. + * + *

These constants and methods are additions to the ones found in {@link + * com.google.errorprone.matchers.JUnitMatchers}. + */ +public final class MoreJUnitMatchers { + /** Matches JUnit Jupiter test methods. */ + public static final MultiMatcher TEST_METHOD = + annotations( + AT_LEAST_ONE, + anyOf( + isType("org.junit.jupiter.api.Test"), + hasMetaAnnotation("org.junit.jupiter.api.TestTemplate"))); + /** Matches JUnit Jupiter setup and teardown methods. */ + public static final MultiMatcher SETUP_OR_TEARDOWN_METHOD = + annotations( + AT_LEAST_ONE, + anyOf( + isType("org.junit.jupiter.api.AfterAll"), + isType("org.junit.jupiter.api.AfterEach"), + isType("org.junit.jupiter.api.BeforeAll"), + isType("org.junit.jupiter.api.BeforeEach"))); + /** + * Matches methods that have a {@link org.junit.jupiter.params.provider.MethodSource} annotation. + */ + public static final MultiMatcher HAS_METHOD_SOURCE = + annotations(AT_LEAST_ONE, isType("org.junit.jupiter.params.provider.MethodSource")); + + private MoreJUnitMatchers() {} + + /** + * Returns the names of the JUnit value factory methods specified by the given {@link + * org.junit.jupiter.params.provider.MethodSource} annotation. + * + * @param methodSourceAnnotation The annotation from which to extract value factory method names. + * @return One or more value factory names. + */ + static ImmutableSet getMethodSourceFactoryNames( + AnnotationTree methodSourceAnnotation, MethodTree method) { + String methodName = method.getName().toString(); + ExpressionTree value = AnnotationMatcherUtils.getArgument(methodSourceAnnotation, "value"); + + if (!(value instanceof NewArrayTree)) { + return ImmutableSet.of(toMethodSourceFactoryName(value, methodName)); + } + + return ((NewArrayTree) value) + .getInitializers().stream() + .map(name -> toMethodSourceFactoryName(name, methodName)) + .collect(toImmutableSet()); + } + + private static String toMethodSourceFactoryName( + @Nullable ExpressionTree tree, String annotatedMethodName) { + return requireNonNullElse( + Strings.emptyToNull(ASTHelpers.constValue(tree, String.class)), annotatedMethodName); + } +} 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 new file mode 100644 index 0000000000..b62e761db0 --- /dev/null +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/MoreMatchers.java @@ -0,0 +1,39 @@ +package tech.picnic.errorprone.bugpatterns.util; + +import com.google.errorprone.matchers.Matcher; +import com.google.errorprone.matchers.Matchers; +import com.google.errorprone.predicates.TypePredicate; +import com.google.errorprone.util.ASTHelpers; +import com.sun.source.tree.AnnotationTree; +import com.sun.tools.javac.code.Symbol; + +/** + * A collection of general-purpose {@link Matcher}s. + * + *

These methods are additions to the ones found in {@link Matchers}. + */ +public final class MoreMatchers { + private MoreMatchers() {} + + /** + * Returns a {@link Matcher} that determines whether a given {@link AnnotationTree} has a + * meta-annotation of the specified type. + * + * @param The type of tree to match against. + * @param annotationType The binary type name of the annotation (e.g. + * "org.jspecify.annotations.Nullable", or "some.package.OuterClassName$InnerClassName") + * @return A {@link Matcher} that matches trees with the specified meta-annotation. + */ + public static Matcher hasMetaAnnotation(String annotationType) { + TypePredicate typePredicate = hasAnnotation(annotationType); + return (tree, state) -> { + Symbol sym = ASTHelpers.getSymbol(tree); + return sym != null && typePredicate.apply(sym.type, state); + }; + } + + // 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/MoreASTHelpersTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/util/MoreASTHelpersTest.java new file mode 100644 index 0000000000..44496738bf --- /dev/null +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/util/MoreASTHelpersTest.java @@ -0,0 +1,110 @@ +package tech.picnic.errorprone.bugpatterns.util; + +import static com.google.errorprone.BugPattern.SeverityLevel.ERROR; + +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Maps; +import com.google.errorprone.BugPattern; +import com.google.errorprone.CompilationTestHelper; +import com.google.errorprone.VisitorState; +import com.google.errorprone.bugpatterns.BugChecker; +import com.google.errorprone.bugpatterns.BugChecker.MethodTreeMatcher; +import com.google.errorprone.matchers.Description; +import com.sun.source.tree.MethodTree; +import java.util.function.BiFunction; +import org.junit.jupiter.api.Test; + +final class MoreASTHelpersTest { + @Test + void findMethods() { + CompilationTestHelper.newInstance(FindMethodsTestChecker.class, getClass()) + .addSourceLines( + "A.java", + "class A {", + " // BUG: Diagnostic contains: {foo=1, bar=2, baz=0}", + " void foo() {}", + "", + " // BUG: Diagnostic contains: {foo=1, bar=2, baz=0}", + " void bar() {}", + "", + " // BUG: Diagnostic contains: {foo=1, bar=2, baz=0}", + " void bar(int i) {}", + "", + " static class B {", + " // BUG: Diagnostic contains: {foo=0, bar=1, baz=1}", + " void bar() {}", + "", + " // BUG: Diagnostic contains: {foo=0, bar=1, baz=1}", + " void baz() {}", + " }", + "}") + .doTest(); + } + + @Test + void methodExistsInEnclosingClass() { + CompilationTestHelper.newInstance(MethodExistsTestChecker.class, getClass()) + .addSourceLines( + "A.java", + "class A {", + " // BUG: Diagnostic contains: {foo=true, bar=true, baz=false}", + " void foo() {}", + "", + " // BUG: Diagnostic contains: {foo=true, bar=true, baz=false}", + " void bar() {}", + "", + " // BUG: Diagnostic contains: {foo=true, bar=true, baz=false}", + " void bar(int i) {}", + "", + " static class B {", + " // BUG: Diagnostic contains: {foo=false, bar=true, baz=true}", + " void bar() {}", + "", + " // BUG: Diagnostic contains: {foo=false, bar=true, baz=true}", + " void baz() {}", + " }", + "}") + .doTest(); + } + + private static String createDiagnosticsMessage( + BiFunction valueFunction, VisitorState state) { + return Maps.toMap(ImmutableSet.of("foo", "bar", "baz"), key -> valueFunction.apply(key, state)) + .toString(); + } + + /** + * A {@link BugChecker} that delegates to {@link MoreASTHelpers#findMethods(CharSequence, + * VisitorState)}. + */ + @BugPattern(summary = "Interacts with `MoreASTHelpers` for testing purposes", severity = ERROR) + public static final class FindMethodsTestChecker extends BugChecker implements MethodTreeMatcher { + private static final long serialVersionUID = 1L; + + @Override + public Description matchMethod(MethodTree tree, VisitorState state) { + return buildDescription(tree) + .setMessage( + createDiagnosticsMessage( + (methodName, s) -> MoreASTHelpers.findMethods(methodName, s).size(), state)) + .build(); + } + } + + /** + * A {@link BugChecker} that delegates to {@link + * MoreASTHelpers#methodExistsInEnclosingClass(CharSequence, VisitorState)}. + */ + @BugPattern(summary = "Interacts with `MoreASTHelpers` for testing purposes", severity = ERROR) + public static final class MethodExistsTestChecker extends BugChecker + implements MethodTreeMatcher { + private static final long serialVersionUID = 1L; + + @Override + public Description matchMethod(MethodTree tree, VisitorState state) { + return buildDescription(tree) + .setMessage(createDiagnosticsMessage(MoreASTHelpers::methodExistsInEnclosingClass, state)) + .build(); + } + } +} diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/util/MoreJUnitMatchersTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/util/MoreJUnitMatchersTest.java new file mode 100644 index 0000000000..caab722793 --- /dev/null +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/util/MoreJUnitMatchersTest.java @@ -0,0 +1,173 @@ +package tech.picnic.errorprone.bugpatterns.util; + +import static com.google.common.collect.ImmutableSet.toImmutableSet; +import static com.google.errorprone.BugPattern.SeverityLevel.ERROR; +import static tech.picnic.errorprone.bugpatterns.util.MoreJUnitMatchers.HAS_METHOD_SOURCE; +import static tech.picnic.errorprone.bugpatterns.util.MoreJUnitMatchers.SETUP_OR_TEARDOWN_METHOD; +import static tech.picnic.errorprone.bugpatterns.util.MoreJUnitMatchers.TEST_METHOD; + +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; +import com.google.errorprone.BugPattern; +import com.google.errorprone.CompilationTestHelper; +import com.google.errorprone.VisitorState; +import com.google.errorprone.bugpatterns.BugChecker; +import com.google.errorprone.bugpatterns.BugChecker.MethodTreeMatcher; +import com.google.errorprone.matchers.Description; +import com.google.errorprone.matchers.Matcher; +import com.sun.source.tree.AnnotationTree; +import com.sun.source.tree.MethodTree; +import java.util.Map; +import org.junit.jupiter.api.Test; + +final class MoreJUnitMatchersTest { + @Test + void methodMatchers() { + CompilationTestHelper.newInstance(MethodMatchersTestChecker.class, getClass()) + .addSourceLines( + "A.java", + "import static org.junit.jupiter.params.provider.Arguments.arguments;", + "", + "import java.util.stream.Stream;", + "import org.junit.jupiter.api.AfterAll;", + "import org.junit.jupiter.api.AfterEach;", + "import org.junit.jupiter.api.BeforeAll;", + "import org.junit.jupiter.api.BeforeEach;", + "import org.junit.jupiter.api.RepeatedTest;", + "import org.junit.jupiter.api.Test;", + "import org.junit.jupiter.params.ParameterizedTest;", + "import org.junit.jupiter.params.provider.Arguments;", + "import org.junit.jupiter.params.provider.MethodSource;", + "", + "class A {", + " @BeforeAll", + " // BUG: Diagnostic contains: SETUP_OR_TEARDOWN_METHOD", + " public void beforeAll() {}", + "", + " @BeforeEach", + " @Test", + " // BUG: Diagnostic contains: TEST_METHOD, SETUP_OR_TEARDOWN_METHOD", + " protected void beforeEachAndTest() {}", + "", + " @AfterEach", + " // BUG: Diagnostic contains: SETUP_OR_TEARDOWN_METHOD", + " private void afterEach() {}", + "", + " @AfterAll", + " // BUG: Diagnostic contains: SETUP_OR_TEARDOWN_METHOD", + " private void afterAll() {}", + "", + " @Test", + " // BUG: Diagnostic contains: TEST_METHOD", + " void test() {}", + "", + " private static Stream booleanArgs() {", + " return Stream.of(arguments(false), arguments(true));", + " }", + "", + " @ParameterizedTest", + " @MethodSource(\"booleanArgs\")", + " // BUG: Diagnostic contains: TEST_METHOD, HAS_METHOD_SOURCE", + " void parameterizedTest(boolean b) {}", + "", + " @RepeatedTest(2)", + " // BUG: Diagnostic contains: TEST_METHOD", + " private void repeatedTest() {}", + "", + " private void unannotatedMethod() {}", + "}") + .doTest(); + } + + @Test + void getMethodSourceFactoryNames() { + CompilationTestHelper.newInstance(MethodSourceFactoryNamesTestChecker.class, getClass()) + .addSourceLines( + "A.java", + "import org.junit.jupiter.params.provider.MethodSource;", + "", + "class A {", + " @MethodSource", + " // BUG: Diagnostic contains: [matchingMethodSource]", + " void matchingMethodSource(boolean b) {}", + "", + " @MethodSource()", + " // BUG: Diagnostic contains: [matchingMethodSourceWithParens]", + " void matchingMethodSourceWithParens(boolean b) {}", + "", + " @MethodSource(\"\")", + " // BUG: Diagnostic contains: [matchingMethodSourceMadeExplicit]", + " void matchingMethodSourceMadeExplicit(boolean b) {}", + "", + " @MethodSource({\"\"})", + " // BUG: Diagnostic contains: [matchingMethodSourceMadeExplicitWithParens]", + " void matchingMethodSourceMadeExplicitWithParens(boolean b) {}", + "", + " @MethodSource({})", + " // BUG: Diagnostic contains: []", + " void noMethodSources(boolean b) {}", + "", + " @MethodSource(\"myValueFactory\")", + " // BUG: Diagnostic contains: [myValueFactory]", + " void singleCustomMethodSource(boolean b) {}", + "", + " @MethodSource({\"firstValueFactory\", \"secondValueFactory\"})", + " // BUG: Diagnostic contains: [firstValueFactory, secondValueFactory]", + " void twoCustomMethodSources(boolean b) {}", + "", + " @MethodSource({\"myValueFactory\", \"\"})", + " // BUG: Diagnostic contains: [myValueFactory, customAndMatchingMethodSources]", + " void customAndMatchingMethodSources(boolean b) {}", + "}") + .doTest(); + } + + /** + * A {@link BugChecker} that flags methods matched by {@link Matcher}s of {@link MethodTree}s + * exposed by {@link MoreJUnitMatchers}. + */ + @BugPattern(summary = "Interacts with `MoreJUnitMatchers` for testing purposes", severity = ERROR) + public static final class MethodMatchersTestChecker extends BugChecker + implements MethodTreeMatcher { + private static final long serialVersionUID = 1L; + private static final ImmutableMap> METHOD_MATCHERS = + ImmutableMap.of( + "TEST_METHOD", TEST_METHOD, + "HAS_METHOD_SOURCE", HAS_METHOD_SOURCE, + "SETUP_OR_TEARDOWN_METHOD", SETUP_OR_TEARDOWN_METHOD); + + @Override + public Description matchMethod(MethodTree tree, VisitorState state) { + ImmutableSet matches = + METHOD_MATCHERS.entrySet().stream() + .filter(e -> e.getValue().matches(tree, state)) + .map(Map.Entry::getKey) + .collect(toImmutableSet()); + + return matches.isEmpty() + ? Description.NO_MATCH + : buildDescription(tree).setMessage(String.join(", ", matches)).build(); + } + } + + /** + * A {@link BugChecker} that flags methods with a JUnit {@code @MethodSource} annotation by + * enumerating the associated value factory method names. + */ + @BugPattern(summary = "Interacts with `MoreJUnitMatchers` for testing purposes", severity = ERROR) + public static final class MethodSourceFactoryNamesTestChecker extends BugChecker + implements MethodTreeMatcher { + private static final long serialVersionUID = 1L; + + @Override + public Description matchMethod(MethodTree tree, VisitorState state) { + AnnotationTree annotation = + Iterables.getOnlyElement(HAS_METHOD_SOURCE.multiMatchResult(tree, state).matchingNodes()); + + return buildDescription(tree) + .setMessage(MoreJUnitMatchers.getMethodSourceFactoryNames(annotation, tree).toString()) + .build(); + } + } +} 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 new file mode 100644 index 0000000000..842fb57b6b --- /dev/null +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/util/MoreMatchersTest.java @@ -0,0 +1,62 @@ +package tech.picnic.errorprone.bugpatterns.util; + +import static com.google.errorprone.BugPattern.SeverityLevel.ERROR; + +import com.google.errorprone.BugPattern; +import com.google.errorprone.CompilationTestHelper; +import com.google.errorprone.VisitorState; +import com.google.errorprone.bugpatterns.BugChecker; +import com.google.errorprone.bugpatterns.BugChecker.AnnotationTreeMatcher; +import com.google.errorprone.matchers.Description; +import com.google.errorprone.matchers.Matcher; +import com.sun.source.tree.AnnotationTree; +import org.junit.jupiter.api.Test; + +final class MoreMatchersTest { + @Test + void hasMetaAnnotation() { + CompilationTestHelper.newInstance(TestMatcher.class, getClass()) + .addSourceLines( + "A.java", + "import org.junit.jupiter.api.AfterAll;", + "import org.junit.jupiter.api.RepeatedTest;", + "import org.junit.jupiter.api.Test;", + "import org.junit.jupiter.api.TestTemplate;", + "import org.junit.jupiter.params.ParameterizedTest;", + "", + "class A {", + " void negative1() {}", + "", + " @Test", + " void negative2() {}", + "", + " @AfterAll", + " void negative3() {}", + "", + " @TestTemplate", + " void negative4() {}", + "", + " // BUG: Diagnostic contains:", + " @ParameterizedTest", + " void positive1() {}", + "", + " // BUG: Diagnostic contains:", + " @RepeatedTest(2)", + " void positive2() {}", + "}") + .doTest(); + } + + /** A {@link BugChecker} that delegates to {@link MoreMatchers#hasMetaAnnotation(String)} . */ + @BugPattern(summary = "Interacts with `MoreMatchers` for testing purposes", severity = ERROR) + 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"); + + @Override + public Description matchAnnotation(AnnotationTree tree, VisitorState state) { + return DELEGATE.matches(tree, state) ? describeMatch(tree) : Description.NO_MATCH; + } + } +}