Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce More{ASTHelpers,JUnitMatchers,Matchers} utility classes #335

Merged
merged 16 commits into from
Nov 22, 2022
Merged
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