Skip to content

Commit

Permalink
Introduce More{ASTHelpers,JUnitMatchers,Matchers} utility classes (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
eric-picnic authored Nov 22, 2022
1 parent 4fead24 commit 84d425e
Show file tree
Hide file tree
Showing 7 changed files with 518 additions and 44 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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. */
Expand Down Expand Up @@ -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<MethodTree, AnnotationTree> TEST_METHOD =
annotations(
AT_LEAST_ONE,
anyOf(
isType("org.junit.jupiter.api.Test"),
hasMetaAnnotation("org.junit.jupiter.api.TestTemplate")));
private static final MultiMatcher<MethodTree, AnnotationTree> 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() {}
Expand Down Expand Up @@ -142,7 +125,7 @@ private void reportMethodRenameBlocker(MethodTree tree, String reason, VisitorSt
* </ul>
*/
private static Optional<String> 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));
}
Expand All @@ -158,15 +141,6 @@ private static Optional<String> 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)
Expand All @@ -188,18 +162,4 @@ private static Optional<String> 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<AnnotationTree> 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);
}
}
Original file line number Diff line number Diff line change
@@ -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.
*
* <p>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<MethodTree> 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();
}
}
Original file line number Diff line number Diff line change
@@ -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.
*
* <p>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<MethodTree, AnnotationTree> 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<MethodTree, AnnotationTree> 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<MethodTree, AnnotationTree> 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<String> 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);
}
}
Original file line number Diff line number Diff line change
@@ -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.
*
* <p>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 <T> 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 <T extends AnnotationTree> Matcher<T> 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);
}
}
Original file line number Diff line number Diff line change
@@ -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<String, VisitorState, Object> 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();
}
}
}
Loading

0 comments on commit 84d425e

Please sign in to comment.