Skip to content

Commit

Permalink
Suggestions (2)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Stephan202 @rickie
Stephan202 authored and rickie committed Nov 18, 2022
1 parent 708e8b1 commit 36c710d
Show file tree
Hide file tree
Showing 3 changed files with 121 additions and 45 deletions.
22 changes: 15 additions & 7 deletions ...rone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/MoreJUnitMatchers.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@
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;
Expand All @@ -16,6 +18,7 @@
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.
Expand Down Expand Up @@ -43,7 +46,7 @@ public final class MoreJUnitMatchers {
/**
* Matches methods that have a {@link org.junit.jupiter.params.provider.MethodSource} annotation.
*/
public static final Matcher<MethodTree> HAS_METHOD_SOURCE =
public static final MultiMatcher<MethodTree, AnnotationTree> HAS_METHOD_SOURCE =
annotations(AT_LEAST_ONE, isType("org.junit.jupiter.params.provider.MethodSource"));

private MoreJUnitMatchers() {}
Expand All @@ -57,17 +60,22 @@ private MoreJUnitMatchers() {}
*/
static ImmutableSet<String> getMethodSourceFactoryNames(
AnnotationTree methodSourceAnnotation, MethodTree method) {
String methodName = method.getName().toString();
ExpressionTree value = AnnotationMatcherUtils.getArgument(methodSourceAnnotation, "value");

if (!(value instanceof NewArrayTree)) {
return ImmutableSet.of(ASTHelpers.constValue(value, String.class));
return ImmutableSet.of(toMethodSourceFactoryName(value, methodName));
}

NewArrayTree arrayTree = (NewArrayTree) value;
return arrayTree.getInitializers().isEmpty()
? ImmutableSet.of(method.getName().toString())
: arrayTree.getInitializers().stream()
.map(name -> ASTHelpers.constValue(name, String.class))
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);
}
}
129 changes: 99 additions & 30 deletions ...-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/util/MoreJUnitMatchersTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,23 +1,30 @@
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.ArrayList;
import java.util.List;
import java.util.Map;
import org.junit.jupiter.api.Test;

final class MoreJUnitMatchersTest {
@Test
void matches() {
CompilationTestHelper.newInstance(TestChecker.class, getClass())
void methodMatchers() {
CompilationTestHelper.newInstance(MethodMatchersTestChecker.class, getClass())
.addSourceLines(
"A.java",
"import static org.junit.jupiter.params.provider.Arguments.arguments;",
Expand All @@ -36,24 +43,24 @@ void matches() {
"class A {",
" @BeforeAll",
" // BUG: Diagnostic contains: SETUP_OR_TEARDOWN_METHOD",
" public void setUp1() {}",
" public void beforeAll() {}",
"",
" @BeforeEach",
" @Test",
" // BUG: Diagnostic contains: TEST_METHOD, SETUP_OR_TEARDOWN_METHOD",
" protected void setUp2() {}",
" protected void beforeEachAndTest() {}",
"",
" @AfterEach",
" // BUG: Diagnostic contains: SETUP_OR_TEARDOWN_METHOD",
" private void tearDown1() {}",
" private void afterEach() {}",
"",
" @AfterAll",
" // BUG: Diagnostic contains: SETUP_OR_TEARDOWN_METHOD",
" private void tearDown2() {}",
" private void afterAll() {}",
"",
" @Test",
" // BUG: Diagnostic contains: TEST_METHOD",
" void testFoo() {}",
" void test() {}",
"",
" private static Stream<Arguments> booleanArgs() {",
" return Stream.of(arguments(false), arguments(true));",
Expand All @@ -62,43 +69,105 @@ void matches() {
" @ParameterizedTest",
" @MethodSource(\"booleanArgs\")",
" // BUG: Diagnostic contains: TEST_METHOD, HAS_METHOD_SOURCE",
" void testBar(boolean b) {}",
" void parameterizedTest(boolean b) {}",
"",
" @RepeatedTest(2)",
" // BUG: Diagnostic contains: TEST_METHOD",
" private void qux() {}",
" private void repeatedTest() {}",
"",
" private void quux() {}",
" 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 delegates to {@link Matcher Matchers} from {@link MoreJUnitMatchers}.
* A {@link BugChecker} that flags methods matched by {@link Matcher}s of {@link MethodTree}s
* exposed by {@link MoreJUnitMatchers}.
*/
@BugPattern(
summary = "Flags methods matched by Matchers from `MoreJUnitMatchers`",
severity = ERROR)
public static final class TestChecker extends BugChecker implements MethodTreeMatcher {
@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<String, Matcher<MethodTree>> 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) {
List<String> diagnosticsMessages = new ArrayList<>();

if (MoreJUnitMatchers.TEST_METHOD.matches(tree, state)) {
diagnosticsMessages.add("TEST_METHOD");
}
if (MoreJUnitMatchers.HAS_METHOD_SOURCE.matches(tree, state)) {
diagnosticsMessages.add("HAS_METHOD_SOURCE");
}
if (MoreJUnitMatchers.SETUP_OR_TEARDOWN_METHOD.matches(tree, state)) {
diagnosticsMessages.add("SETUP_OR_TEARDOWN_METHOD");
}
ImmutableSet<String> matches =
METHOD_MATCHERS.entrySet().stream()
.filter(e -> e.getValue().matches(tree, state))
.map(Map.Entry::getKey)
.collect(toImmutableSet());

return diagnosticsMessages.isEmpty()
return matches.isEmpty()
? Description.NO_MATCH
: buildDescription(tree).setMessage(String.join(", ", diagnosticsMessages)).build();
: 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();
}
}
}
15 changes: 7 additions & 8 deletions ...prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/util/MoreMatchersTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,18 @@

final class MoreMatchersTest {
@Test
void matcher() {
void hasMetaAnnotation() {
CompilationTestHelper.newInstance(TestMatcher.class, getClass())
.addSourceLines(
"/A.java",
"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 {",
" private void negative1() {}",
" void negative1() {}",
"",
" @Test",
" void negative2() {}",
Expand All @@ -38,26 +38,25 @@ void matcher() {
"",
" // BUG: Diagnostic contains:",
" @ParameterizedTest",
" void testBar() {}",
" void positive1() {}",
"",
" // BUG: Diagnostic contains:",
" @RepeatedTest(2)",
" void testBaz() {}",
" void positive2() {}",
"}")
.doTest();
}

/** A {@link BugChecker} that delegates to `MoreMatchers#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 TestMatcher extends BugChecker implements AnnotationTreeMatcher {
private static final long serialVersionUID = 1L;

private static final Matcher<AnnotationTree> DELEGATE =
MoreMatchers.hasMetaAnnotation("org.junit.jupiter.api.TestTemplate");

@Override
public Description matchAnnotation(AnnotationTree tree, VisitorState state) {
return DELEGATE.matches(tree, state) ? buildDescription(tree).build() : Description.NO_MATCH;
return DELEGATE.matches(tree, state) ? describeMatch(tree) : Description.NO_MATCH;
}
}
}

0 comments on commit 36c710d

Please sign in to comment.