From d0716817152f7c54ad7a7d9898d046569bd4cf24 Mon Sep 17 00:00:00 2001 From: Rick Ossendrijver Date: Fri, 11 Aug 2023 16:26:30 +0200 Subject: [PATCH] Suggestions --- .../errorprone/bugpatterns/DirectReturn.java | 7 +-- .../bugpatterns/util/MoreMatchers.java | 19 ++------ .../bugpatterns/DirectReturnTest.java | 8 ++-- .../bugpatterns/util/MoreMatchersTest.java | 43 ++++++++++++++++++- 4 files changed, 51 insertions(+), 26 deletions(-) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/DirectReturn.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/DirectReturn.java index 62900529f19..1f7dec3416c 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/DirectReturn.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/DirectReturn.java @@ -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; @@ -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 IS_CLASS_ANNOTATED_WITH_LOMBOK_DATA = - isClassAnnotatedWithLombokData(); /** Instantiates a new {@link DirectReturn} instance. */ public DirectReturn() {} @@ -72,8 +70,7 @@ public DirectReturn() {} public Description matchBlock(BlockTree tree, VisitorState state) { List 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)) { return Description.NO_MATCH; } diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/MoreMatchers.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/MoreMatchers.java index a71ca36b088..b8bcf8c439e 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/MoreMatchers.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/MoreMatchers.java @@ -7,7 +7,6 @@ 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. @@ -15,6 +14,9 @@ *

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 HAS_LOMBOK_DATA = Matchers.hasAnnotation("lombok.Data"); + private MoreMatchers() {} /** @@ -34,21 +36,6 @@ public static Matcher hasMetaAnnotation(String ann }; } - /** - * Returns a {@link Matcher} that determines whether a given {@link ClassTree} is annotated with - * {@link Data}. - * - * @param The type of tree to match against. - * @return A {@link Matcher} that matches trees with {@link Data} annotation. - */ - public static Matcher 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); diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/DirectReturnTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/DirectReturnTest.java index b51a7da3aa5..319481b241c 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/DirectReturnTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/DirectReturnTest.java @@ -225,14 +225,16 @@ 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( @@ -240,7 +242,7 @@ void excludeClassesAnnotatedWithLombokData() { "import lombok.Data;", "", "@Data", - "public class A {", + "class A {", " private String field;", "}") .doTest(TestMode.TEXT_MATCH); diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/util/MoreMatchersTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/util/MoreMatchersTest.java index 842fb57b6b0..f0b812ed719 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/util/MoreMatchersTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/util/MoreMatchersTest.java @@ -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;", @@ -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 DELEGATE = MoreMatchers.hasMetaAnnotation("org.junit.jupiter.api.TestTemplate"); @@ -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; + } + } }