From fff620a5c267d6bd37501ae10ada8bad7e667074 Mon Sep 17 00:00:00 2001 From: Rick Ossendrijver Date: Thu, 8 Dec 2022 21:27:25 +0100 Subject: [PATCH] Rebase and resolve conflicts --- .../bugpatterns/util/ConflictDetection.java | 4 +- .../bugpatterns/util/MoreASTHelpers.java | 8 ++-- .../bugpatterns/util/MoreJUnitMatchers.java | 45 ++++++++++++++++--- .../JUnitFactoryMethodDeclarationTest.java | 1 - 4 files changed, 45 insertions(+), 13 deletions(-) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/ConflictDetection.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/ConflictDetection.java index 4947d444f8a..dbb313d1ef7 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/ConflictDetection.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/ConflictDetection.java @@ -1,7 +1,7 @@ package tech.picnic.errorprone.bugpatterns.util; import static tech.picnic.errorprone.bugpatterns.util.JavaKeywords.isReservedKeyword; -import static tech.picnic.errorprone.bugpatterns.util.MoreASTHelpers.isMethodInEnclosingClass; +import static tech.picnic.errorprone.bugpatterns.util.MoreASTHelpers.methodExistsInEnclosingClass; import com.google.errorprone.VisitorState; import com.sun.source.tree.ImportTree; @@ -36,7 +36,7 @@ private ConflictDetection() {} * {@link Optional#empty()} if no blocker was found. */ public static Optional findMethodRenameBlocker(String methodName, VisitorState state) { - if (isMethodInEnclosingClass(methodName, state)) { + if (methodExistsInEnclosingClass(methodName, state)) { return Optional.of( String.format("a method named `%s` already exists in this class", methodName)); } 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 index 14b08a5c197..ccfd3353bbb 100644 --- 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 @@ -29,10 +29,10 @@ public static ImmutableList findMethods(CharSequence methodName, Vis 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()); + .filter(MethodTree.class::isInstance) + .map(MethodTree.class::cast) + .filter(method -> method.getName().contentEquals(methodName)) + .collect(toImmutableList()); } /** 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 index 732a9454535..52e647e0c40 100644 --- 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 @@ -1,12 +1,17 @@ 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.common.collect.Iterables; +import com.google.errorprone.matchers.AnnotationMatcherUtils; import com.google.errorprone.matchers.Matcher; import com.google.errorprone.matchers.MultiMatcher; import com.google.errorprone.util.ASTHelpers; @@ -14,14 +19,16 @@ import com.sun.source.tree.AssignmentTree; import com.sun.source.tree.ExpressionTree; import com.sun.source.tree.MethodTree; +import com.sun.source.tree.NewArrayTree; import com.sun.tools.javac.code.Type; import java.util.Optional; import javax.lang.model.type.TypeKind; +import org.jspecify.nullness.Nullable; /** - * A set of JUnit-specific helper methods and {@link Matcher Matchers}. + * A collection of JUnit-specific helper methods and {@link Matcher}s. * - *

These helper methods are additions to the ones from {@link + *

These constants and methods are additions to the ones found in {@link * com.google.errorprone.matchers.JUnitMatchers}. */ public final class MoreJUnitMatchers { @@ -32,8 +39,7 @@ public final class MoreJUnitMatchers { anyOf( isType("org.junit.jupiter.api.Test"), hasMetaAnnotation("org.junit.jupiter.api.TestTemplate"))); - - /** Matches JUnit setup and teardown methods. */ + /** Matches JUnit Jupiter setup and teardown methods. */ public static final MultiMatcher SETUP_OR_TEARDOWN_METHOD = annotations( AT_LEAST_ONE, @@ -42,15 +48,42 @@ public final class MoreJUnitMatchers { 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 Matcher HAS_METHOD_SOURCE = + 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); + } + /** * Extracts the name of the JUnit factory method from a {@link * org.junit.jupiter.params.provider.MethodSource} annotation. diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/JUnitFactoryMethodDeclarationTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/JUnitFactoryMethodDeclarationTest.java index 56f9eacbf2e..02729e57b22 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/JUnitFactoryMethodDeclarationTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/JUnitFactoryMethodDeclarationTest.java @@ -139,7 +139,6 @@ void replacement() { "A.java", "import static org.junit.jupiter.params.provider.Arguments.arguments;", "", - "import java.util.List;", "import java.util.stream.Stream;", "import org.junit.jupiter.params.ParameterizedTest;", "import org.junit.jupiter.params.provider.Arguments;",