Skip to content

Commit

Permalink
Suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
rickie committed Aug 11, 2023
1 parent f29d74f commit d071681
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
import static com.google.errorprone.matchers.Matchers.staticMethod;
import static com.google.errorprone.matchers.Matchers.toType;
import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;
import static tech.picnic.errorprone.bugpatterns.util.MoreMatchers.isClassAnnotatedWithLombokData;
import static tech.picnic.errorprone.bugpatterns.util.MoreMatchers.HAS_LOMBOK_DATA;

import com.google.auto.service.AutoService;
import com.google.common.collect.Streams;
Expand Down Expand Up @@ -62,8 +62,6 @@ public final class DirectReturn extends BugChecker implements BlockTreeMatcher {
allOf(
not(toType(MethodInvocationTree.class, argument(0, isSameType(Class.class.getName())))),
staticMethod().onClass("org.mockito.Mockito").namedAnyOf("mock", "spy"));
private static final Matcher<ClassTree> IS_CLASS_ANNOTATED_WITH_LOMBOK_DATA =
isClassAnnotatedWithLombokData();

/** Instantiates a new {@link DirectReturn} instance. */
public DirectReturn() {}
Expand All @@ -72,8 +70,7 @@ public DirectReturn() {}
public Description matchBlock(BlockTree tree, VisitorState state) {
List<? extends StatementTree> statements = tree.getStatements();
ClassTree enclosingNode = ASTHelpers.findEnclosingNode(state.getPath(), ClassTree.class);
if (statements.size() < 2
|| IS_CLASS_ANNOTATED_WITH_LOMBOK_DATA.matches(enclosingNode, state)) {
if (statements.size() < 2 || HAS_LOMBOK_DATA.matches(enclosingNode, state)) {

Check warning on line 73 in error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/DirectReturn.java

View workflow job for this annotation

GitHub Actions / pitest

A change can be made to line 73 without causing a test to fail

removed conditional - replaced equality check with false (2 tests run RemoveConditionalMutator_EQUAL_ELSE)
return Description.NO_MATCH;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,16 @@
import com.sun.source.tree.AnnotationTree;
import com.sun.source.tree.ClassTree;
import com.sun.tools.javac.code.Symbol;
import lombok.Data;

/**
* A collection of general-purpose {@link Matcher}s.
*
* <p>These methods are additions to the ones found in {@link Matchers}.
*/
public final class MoreMatchers {
/** Matches classes annotated with Lombok's `@Data` annotation. */
public static final Matcher<ClassTree> HAS_LOMBOK_DATA = Matchers.hasAnnotation("lombok.Data");

private MoreMatchers() {}

/**
Expand All @@ -34,21 +36,6 @@ public static <T extends AnnotationTree> Matcher<T> hasMetaAnnotation(String ann
};
}

/**
* Returns a {@link Matcher} that determines whether a given {@link ClassTree} is annotated with
* {@link Data}.
*
* @param <T> The type of tree to match against.
* @return A {@link Matcher} that matches trees with {@link Data} annotation.
*/
public static <T extends ClassTree> Matcher<T> isClassAnnotatedWithLombokData() {
TypePredicate typePredicate = hasAnnotation("lombok.Data");
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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -225,22 +225,24 @@ void replacement() {
}

@Test
void excludeClassesAnnotatedWithLombokData() {
void ignoreClassesAnnotatedWithLombokData() {
BugCheckerRefactoringTestHelper.newInstance(DirectReturn.class, getClass())
.addInputLines("Data.java", "package lombok;", "public @interface Data {}")
.expectUnchanged()
.addInputLines(
"A.java",
"import lombok.Data;",
"",
"@Data",
"public class A {",
"class A {",
" private String field;",
"}")
.addOutputLines(
"A.java",
"import lombok.Data;",
"",
"@Data",
"public class A {",
"class A {",
" private String field;",
"}")
.doTest(TestMode.TEXT_MATCH);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,21 +1,24 @@
package tech.picnic.errorprone.bugpatterns.util;

import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
import static tech.picnic.errorprone.bugpatterns.util.MoreMatchers.HAS_LOMBOK_DATA;

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.AnnotationTreeMatcher;
import com.google.errorprone.bugpatterns.BugChecker.ClassTreeMatcher;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.sun.source.tree.AnnotationTree;
import com.sun.source.tree.ClassTree;
import org.junit.jupiter.api.Test;

final class MoreMatchersTest {
@Test
void hasMetaAnnotation() {
CompilationTestHelper.newInstance(TestMatcher.class, getClass())
CompilationTestHelper.newInstance(TestTemplateMatcher.class, getClass())
.addSourceLines(
"A.java",
"import org.junit.jupiter.api.AfterAll;",
Expand Down Expand Up @@ -47,9 +50,33 @@ void hasMetaAnnotation() {
.doTest();
}

@Test
void hasLombokDataAnnotation() {
CompilationTestHelper.newInstance(LombokDataAnnotationMatcher.class, getClass())
.addSourceLines("Data.java", "package lombok;", "public @interface Data {}")
.addSourceLines(
"A.java",
"import lombok.Data;",
"",
"@Data",
"// BUG: Diagnostic contains:",
"public class A {",
" private String field;",
"",
" static class B { }",
"",
" @Data",
" // BUG: Diagnostic contains:",
" static class C { }",
"}")
.addSourceLines("D.java", "import lombok.Data;", "", "public class D { }")
.doTest();
}

/** 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 {
public static final class TestTemplateMatcher extends BugChecker
implements AnnotationTreeMatcher {
private static final long serialVersionUID = 1L;
private static final Matcher<AnnotationTree> DELEGATE =
MoreMatchers.hasMetaAnnotation("org.junit.jupiter.api.TestTemplate");
Expand All @@ -59,4 +86,16 @@ public Description matchAnnotation(AnnotationTree tree, VisitorState state) {
return DELEGATE.matches(tree, state) ? describeMatch(tree) : Description.NO_MATCH;
}
}

/** A {@link BugChecker} that delegates to {@link MoreMatchers#HAS_LOMBOK_DATA} . */
@BugPattern(summary = "Interacts with `MoreMatchers` for testing purposes", severity = ERROR)
public static final class LombokDataAnnotationMatcher extends BugChecker
implements ClassTreeMatcher {
private static final long serialVersionUID = 1L;

@Override
public Description matchClass(ClassTree tree, VisitorState state) {
return HAS_LOMBOK_DATA.matches(tree, state) ? describeMatch(tree) : Description.NO_MATCH;
}
}
}

0 comments on commit d071681

Please sign in to comment.