From 11e28aaa800c3fb716dad46944e88dac68c951fa Mon Sep 17 00:00:00 2001 From: cushon Date: Tue, 5 Dec 2017 18:23:55 -0800 Subject: [PATCH] Suppress ImmutableAnnotationChecker for AutoAnnotation-generated annotations Which contain array fields, but make defensive copies to ensure the class is immutable. RELNOTES: N/A ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=178042508 --- .../google/errorprone/util/ASTHelpers.java | 54 +++++++++++++++++++ .../ImmutableAnnotationChecker.java | 10 ++++ .../ImmutableAnnotationCheckerTest.java | 38 +++++++++++++ 3 files changed, 102 insertions(+) diff --git a/check_api/src/main/java/com/google/errorprone/util/ASTHelpers.java b/check_api/src/main/java/com/google/errorprone/util/ASTHelpers.java index 6437f856f21..bf74ed6961f 100644 --- a/check_api/src/main/java/com/google/errorprone/util/ASTHelpers.java +++ b/check_api/src/main/java/com/google/errorprone/util/ASTHelpers.java @@ -16,11 +16,13 @@ package com.google.errorprone.util; +import static com.google.common.base.Preconditions.checkNotNull; import static com.google.errorprone.matchers.JUnitMatchers.JUNIT4_RUN_WITH_ANNOTATION; import static com.sun.tools.javac.code.Scope.LookupKind.NON_RECURSIVE; import com.google.common.base.CharMatcher; import com.google.common.base.Predicate; +import com.google.common.collect.ImmutableSet; import com.google.errorprone.VisitorState; import com.google.errorprone.dataflow.nullnesspropagation.Nullness; import com.google.errorprone.dataflow.nullnesspropagation.NullnessAnalysis; @@ -46,6 +48,8 @@ import com.sun.source.tree.TypeParameterTree; import com.sun.source.tree.VariableTree; import com.sun.source.util.TreePath; +import com.sun.tools.javac.code.Attribute; +import com.sun.tools.javac.code.Attribute.Compound; import com.sun.tools.javac.code.Flags; import com.sun.tools.javac.code.Scope; import com.sun.tools.javac.code.Symbol; @@ -96,8 +100,10 @@ import java.util.stream.Stream; import javax.annotation.Nullable; import javax.lang.model.element.AnnotationMirror; +import javax.lang.model.element.AnnotationValue; import javax.lang.model.element.ElementKind; import javax.lang.model.type.TypeKind; +import javax.lang.model.util.SimpleAnnotationValueVisitor8; /** This class contains utility methods to work with the javac AST. */ public class ASTHelpers { @@ -943,4 +949,52 @@ public static MethodSymbol resolveExistingMethod( log.popDiagnosticHandler(handler); } } + + /** + * Returns the values of the given symbol's {@code javax.annotation.Generated} or {@code + * javax.annotation.processing.Generated} annotation, if present. + */ + public static ImmutableSet getGeneratedBy(ClassSymbol symbol, VisitorState state) { + checkNotNull(symbol); + Optional c = + Stream.of("javax.annotation.Generated", "javax.annotation.processing.Generated") + .map(state::getSymbolFromString) + .filter(a -> a != null) + .map(symbol::attribute) + .filter(a -> a != null) + .findFirst(); + if (!c.isPresent()) { + return ImmutableSet.of(); + } + Optional values = + c.get() + .getElementValues() + .entrySet() + .stream() + .filter(e -> e.getKey().getSimpleName().contentEquals("value")) + .map(e -> e.getValue()) + .findAny(); + if (!values.isPresent()) { + return ImmutableSet.of(); + } + ImmutableSet.Builder suppressions = ImmutableSet.builder(); + values + .get() + .accept( + new SimpleAnnotationValueVisitor8() { + @Override + public Void visitString(String s, Void aVoid) { + suppressions.add(s); + return super.visitString(s, aVoid); + } + + @Override + public Void visitArray(List vals, Void aVoid) { + vals.stream().forEachOrdered(v -> v.accept(this, null)); + return super.visitArray(vals, aVoid); + } + }, + null); + return suppressions.build(); + } } diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/ImmutableAnnotationChecker.java b/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/ImmutableAnnotationChecker.java index 9b739a8b275..3d2b615bc76 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/ImmutableAnnotationChecker.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/ImmutableAnnotationChecker.java @@ -19,6 +19,7 @@ import static com.google.errorprone.BugPattern.Category.JDK; import static com.google.errorprone.BugPattern.SeverityLevel.WARNING; import static com.google.errorprone.matchers.Description.NO_MATCH; +import static com.google.errorprone.util.ASTHelpers.getGeneratedBy; import static com.google.errorprone.util.ASTHelpers.getSymbol; import static com.google.errorprone.util.ASTHelpers.getType; @@ -39,6 +40,7 @@ import com.sun.source.tree.ClassTree; import com.sun.source.tree.Tree; import com.sun.tools.javac.code.Symbol.ClassSymbol; +import java.util.Collections; import java.util.Optional; /** @author cushon@google.com (Liam Miller-Cushon) */ @@ -57,6 +59,11 @@ public class ImmutableAnnotationChecker extends BugChecker implements ClassTreeM "annotations are immutable by default; annotating them with" + " @com.google.errorprone.annotations.Immutable is unnecessary"; + private static final ImmutableSet PROCESSOR_BLACKLIST = + ImmutableSet.of( + "com.google.auto.value.processor.AutoAnnotationProcessor" + ); + private final WellKnownMutability wellKnownMutability; @Deprecated // Used reflectively, but you should pass in ErrorProneFlags to get custom mutability @@ -76,6 +83,9 @@ public Description matchClass(ClassTree tree, VisitorState state) { || !WellKnownMutability.isAnnotation(state, symbol.type)) { return NO_MATCH; } + if (!Collections.disjoint(getGeneratedBy(symbol, state), PROCESSOR_BLACKLIST)) { + return NO_MATCH; + } if (ASTHelpers.hasAnnotation(symbol, Immutable.class, state)) { AnnotationTree annotation = diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/threadsafety/ImmutableAnnotationCheckerTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/threadsafety/ImmutableAnnotationCheckerTest.java index b66baddb3fe..cb099985b5d 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/threadsafety/ImmutableAnnotationCheckerTest.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/threadsafety/ImmutableAnnotationCheckerTest.java @@ -17,6 +17,7 @@ package com.google.errorprone.bugpatterns.threadsafety; import com.google.errorprone.CompilationTestHelper; +import java.lang.reflect.Method; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -329,4 +330,41 @@ public void jucImmutable() { "}") .doTest(); } + + @Test + public void generated() { + compilationHelper + .addSourceLines( + "Test.java", + "import java.lang.annotation.Annotation;", + (isJdk8OrEarlier() + ? "import javax.annotation.Generated;" + : "import javax.annotation.processing.Generated;"), + "@Generated(\"com.google.auto.value.processor.AutoAnnotationProcessor\")", + "class Test implements Deprecated {", + " public Class annotationType() { return Deprecated.class; }", + " int x;", + " private Test(int x) {", + " this.x = x;", + " }", + " public boolean forRemoval() {", + " return false;", + " }", + " public String since() {", + " return \"\";", + " }", + "}") + .doTest(); + } + + static boolean isJdk8OrEarlier() { + try { + Method versionMethod = Runtime.class.getMethod("version"); + Object version = versionMethod.invoke(null); + int majorVersion = (int) version.getClass().getMethod("major").invoke(version); + return majorVersion <= 8; + } catch (ReflectiveOperationException e) { + return true; + } + } }