diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/JUnitSingleArguments.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/JUnitSingleArguments.java new file mode 100644 index 0000000000..9100456858 --- /dev/null +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/JUnitSingleArguments.java @@ -0,0 +1,50 @@ +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 com.google.errorprone.matchers.method.MethodMatchers.staticMethod; +import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL; + +import com.google.auto.service.AutoService; +import com.google.errorprone.BugPattern; +import com.google.errorprone.VisitorState; +import com.google.errorprone.bugpatterns.BugChecker; +import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher; +import com.google.errorprone.matchers.Description; +import com.google.errorprone.matchers.Matcher; +import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.MethodInvocationTree; +import tech.picnic.errorprone.bugpatterns.util.SourceCode; + +/** + * A {@link BugChecker} that flags uses of {@link + * org.junit.jupiter.params.provider.Arguments#arguments(Object...)} with a single (or no) argument; + * in such cases the use of {@link org.junit.jupiter.params.provider.Arguments} is not required as a + * {@link java.util.stream.Stream} of objects can be directly provided to a {@link + * org.junit.jupiter.params.provider.MethodSource}. + */ +@AutoService(BugChecker.class) +@BugPattern( + summary = "JUnit arguments wrapping a single object are redundant", + link = BUG_PATTERNS_BASE_URL + "JUnitSingleArguments", + linkType = CUSTOM, + severity = SUGGESTION, + tags = SIMPLIFICATION) +public final class JUnitSingleArguments extends BugChecker implements MethodInvocationTreeMatcher { + private static final long serialVersionUID = 1L; + private static final Matcher ARGUMENTS_ARGUMENTS = + staticMethod().onClass("org.junit.jupiter.params.provider.Arguments").named("arguments"); + + /** Instantiates a new {@link JUnitSingleArguments} instance. */ + public JUnitSingleArguments() {} + + @Override + public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { + if (ARGUMENTS_ARGUMENTS.matches(tree, state) && tree.getArguments().size() <= 1) { + return describeMatch(tree, SourceCode.unwrapMethodInvocation(tree, state)); + } + + return Description.NO_MATCH; + } +} diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/JUnitRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/JUnitRules.java index 218213ab9a..e64e3adeb6 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/JUnitRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/JUnitRules.java @@ -23,6 +23,7 @@ Arguments before(@Repeated T objects) { } @AfterTemplate + @SuppressWarnings("JUnitSingleArguments" /* The bugpattern does not understand Repeated. */) @UseImportPolicy(STATIC_IMPORT_ALWAYS) Arguments after(@Repeated T objects) { return arguments(objects); diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/JUnitSingleArgumentsTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/JUnitSingleArgumentsTest.java new file mode 100644 index 0000000000..e4f0b800c9 --- /dev/null +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/JUnitSingleArgumentsTest.java @@ -0,0 +1,30 @@ +package tech.picnic.errorprone.bugpatterns; + +import com.google.errorprone.CompilationTestHelper; +import org.junit.jupiter.api.Test; + +final class JUnitSingleArgumentsTest { + @Test + void identification() { + CompilationTestHelper.newInstance(JUnitSingleArguments.class, getClass()) + .addSourceLines( + "A.java", + "import static java.util.Objects.requireNonNull;", + "import static java.util.function.Function.identity;", + "import static org.junit.jupiter.params.provider.Arguments.arguments;", + "", + "class A {", + " void m() {", + " // BUG: Diagnostic contains:", + " arguments();", + " // BUG: Diagnostic contains:", + " arguments(1);", + " arguments(1, 2);", + "", + " identity();", + " requireNonNull(null);", + " }", + "}") + .doTest(); + } +} diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/util/MethodMatcherFactoryTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/util/MethodMatcherFactoryTest.java index 2e06f291aa..bd762f6c4c 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/util/MethodMatcherFactoryTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/util/MethodMatcherFactoryTest.java @@ -2,7 +2,6 @@ import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION; import static org.assertj.core.api.Assertions.assertThatThrownBy; -import static org.junit.jupiter.params.provider.Arguments.arguments; import com.google.common.collect.ImmutableList; import com.google.errorprone.BugPattern; @@ -17,7 +16,6 @@ import java.util.stream.Stream; 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; final class MethodMatcherFactoryTest { @@ -29,13 +27,13 @@ final class MethodMatcherFactoryTest { "com.example.A#m2(java.lang.String)", "com.example.sub.B#m3(int,int)")); - private static Stream createWithMalformedSignaturesTestCases() { + private static Stream> createWithMalformedSignaturesTestCases() { /* { signatures } */ return Stream.of( - arguments(ImmutableList.of("foo.bar")), - arguments(ImmutableList.of("foo.bar#baz")), - arguments(ImmutableList.of("a", "foo.bar#baz()")), - arguments(ImmutableList.of("foo.bar#baz()", "a"))); + ImmutableList.of("foo.bar"), + ImmutableList.of("foo.bar#baz"), + ImmutableList.of("a", "foo.bar#baz()"), + ImmutableList.of("foo.bar#baz()", "a")); } @MethodSource("createWithMalformedSignaturesTestCases")