From 49b5d2f51fe59725e2dc9b7c6ae740a9547f8629 Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Sat, 16 Sep 2023 12:53:16 +0200 Subject: [PATCH] WIP --- .../CanonicalAnnotationSyntax.java | 20 ++++----- .../bugpatterns/util/SourceCode.java | 13 ++++++ .../bugpatterns/util/SourceCodeTest.java | 45 +++++++++++++++++++ pom.xml | 4 +- 4 files changed, 70 insertions(+), 12 deletions(-) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/CanonicalAnnotationSyntax.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/CanonicalAnnotationSyntax.java index 208d0c99..b6c9b92e 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/CanonicalAnnotationSyntax.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/CanonicalAnnotationSyntax.java @@ -51,10 +51,10 @@ public final class CanonicalAnnotationSyntax extends BugChecker implements Annot @Override public Description matchAnnotation(AnnotationTree tree, VisitorState state) { - if (!isSourceAccurateCodeAvailable(tree, state)) { - /* Without the source code there's not much we can do. */ - return Description.NO_MATCH; - } +// if (!isSourceAccurateCodeAvailable(tree, state)) { +// /* Without the source code there's not much we can do. */ +// return Description.NO_MATCH; +// } return FIX_FACTORIES.stream() .map(op -> op.apply(tree, state)) @@ -65,11 +65,11 @@ public final class CanonicalAnnotationSyntax extends BugChecker implements Annot } // XXX: Explain, link to Lombok issue. - private static boolean isSourceAccurateCodeAvailable(AnnotationTree tree, VisitorState state) { - String source = state.getSourceForNode(tree); - String parentSource = state.getSourceForNode(state.getPath().getParentPath().getLeaf()); - return source != null && parentSource != null && parentSource.contains(source); - } +// private static boolean isSourceAccurateCodeAvailable(AnnotationTree tree, VisitorState state) { +// String source = state.getSourceForNode(tree); +// String parentSource = state.getSourceForNode(state.getPath().getParentPath().getLeaf()); +// return source != null && parentSource != null && parentSource.contains(source); +// } private static Optional dropRedundantParentheses(AnnotationTree tree, VisitorState state) { if (!tree.getArguments().isEmpty()) { @@ -110,7 +110,7 @@ public final class CanonicalAnnotationSyntax extends BugChecker implements Annot } ExpressionTree expr = AnnotationMatcherUtils.getArgument(tree, "value"); - if (expr == null) { + if (expr == null || !SourceCode.isLikelyAccurateSourceAvailable(state)) { /* This is not an explicit assignment to the `value` attribute. */ return Optional.empty(); } diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/SourceCode.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/SourceCode.java index 1fd8a17f..c1169767 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/SourceCode.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/SourceCode.java @@ -6,6 +6,7 @@ import static java.util.stream.Collectors.joining; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.CharMatcher; +import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; import com.google.common.collect.Streams; import com.google.errorprone.VisitorState; @@ -14,9 +15,12 @@ import com.google.errorprone.util.ErrorProneToken; import com.google.errorprone.util.ErrorProneTokens; import com.sun.source.tree.MethodInvocationTree; import com.sun.source.tree.Tree; +import com.sun.source.util.TreePath; import com.sun.tools.javac.util.JCDiagnostic.DiagnosticPosition; import com.sun.tools.javac.util.Position; +import java.util.Objects; import java.util.Optional; +import java.util.stream.Stream; /** * A collection of Error Prone utility methods for dealing with the source code representation of @@ -28,6 +32,15 @@ public final class SourceCode { private SourceCode() {} + // XXX: Explain. + public static boolean isLikelyAccurateSourceAvailable(VisitorState state) { + return !Strings.isNullOrEmpty( + Stream.iterate(state.getPath(), Objects::nonNull, TreePath::getParentPath) + .map(p -> state.getSourceForNode(p.getLeaf())) +// .filter(Objects::nonNull) + .reduce("", (prev, next) -> prev != null && next.contains(prev) ? next : null)); + } + /** * Returns a string representation of the given {@link Tree}, preferring the original source code * (if available) over its prettified representation. diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/util/SourceCodeTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/util/SourceCodeTest.java index 131da0ba..8d18643d 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/util/SourceCodeTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/util/SourceCodeTest.java @@ -5,6 +5,7 @@ import static com.google.errorprone.BugPattern.SeverityLevel.ERROR; import com.google.errorprone.BugCheckerRefactoringTestHelper; import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode; 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; @@ -20,6 +21,31 @@ import javax.lang.model.element.Name; import org.junit.jupiter.api.Test; final class SourceCodeTest { + @Test + void isLikelyAccurateSourceAvailable() { + CompilationTestHelper.newInstance(IsLikelyAccurateSourceAvailableTestChecker.class, getClass()) + .setArgs("-processor", "lombok.launch.AnnotationProcessorHider$AnnotationProcessor") + .addSourceLines( + "A.java", + "import lombok.Data;", + "import com.fasterxml.jackson.annotation.JsonProperty;", + "", + "class A {", + "class WithoutLombok {", + " // BUG: Diagnostic contains:", + " @JsonProperty(\"custom_field_name\")", + " private String field;", + "}", + "", + "@Data", + "class WithLombok {", + " // BUG: Diagnostic contains:", + " @JsonProperty(\"custom_field_name\")", + " private String field;", + "}}") + .doTest(); + } + @Test void deleteWithTrailingWhitespaceAnnotations() { BugCheckerRefactoringTestHelper.newInstance( @@ -228,6 +254,25 @@ final class SourceCodeTest { .doTest(TestMode.TEXT_MATCH); } + /** + * A {@link BugChecker} that uses {@link SourceCode#isLikelyAccurateSourceAvailable(VisitorState)} + * to flag annotations, fields and types for which accurate source code does not appear to be + * available. + */ + // XXX: Update set of matches Tree types. + @BugPattern(severity = ERROR, summary = "Interacts with `SourceCode` for testing purposes") + public static final class IsLikelyAccurateSourceAvailableTestChecker extends BugChecker + implements AnnotationTreeMatcher { + private static final long serialVersionUID = 1L; + + @Override + public Description matchAnnotation(AnnotationTree tree, VisitorState state) { + return SourceCode.isLikelyAccurateSourceAvailable(state) + ? Description.NO_MATCH + : describeMatch(tree); + } + } + /** * A {@link BugChecker} that uses {@link SourceCode#deleteWithTrailingWhitespace(Tree, * VisitorState)} to suggest the deletion of annotations and methods with a name containing diff --git a/pom.xml b/pom.xml index 8f0e6eac..69f44409 100644 --- a/pom.xml +++ b/pom.xml @@ -201,8 +201,8 @@ 1.10.3 ${version.error-prone-orig} v${version.error-prone-orig}-picnic-1 - HEAD-SNAPSHOT - + + 2.21.1 0.1.20 1.0 11