Introduce com.google.errorprone.annotations.Modifier

for use in `@IncompatibleModifiers` and `@RequiredModifiers`, as an alternative to `javax.lang.model.element.Modifier` which is not available at compile-time on Android. #2122 PiperOrigin-RevId: 375137408
google · May 24, 2021 · ab5a85d · ab5a85d
1 parent b2c3930
commit ab5a85d
Show file tree

Hide file tree

Showing 8 changed files with 148 additions and 28 deletions.
diff --git a/annotations/src/main/java/com/google/errorprone/annotations/ForOverride.java b/annotations/src/main/java/com/google/errorprone/annotations/ForOverride.java
@@ -18,10 +18,6 @@
 
 import static java.lang.annotation.ElementType.METHOD;
 import static java.lang.annotation.RetentionPolicy.CLASS;
-import static javax.lang.model.element.Modifier.FINAL;
-import static javax.lang.model.element.Modifier.PRIVATE;
-import static javax.lang.model.element.Modifier.PUBLIC;
-import static javax.lang.model.element.Modifier.STATIC;
 
 import java.lang.annotation.Documented;
 import java.lang.annotation.Retention;
@@ -39,7 +35,8 @@
  * protected or package-private visibility, although their effective visibility is actually "none".
  */
 @Documented
-@IncompatibleModifiers({PUBLIC, PRIVATE, STATIC, FINAL})
+@IncompatibleModifiers(
+    modifier = {Modifier.PUBLIC, Modifier.PRIVATE, Modifier.STATIC, Modifier.FINAL})
 @Retention(CLASS) // Parent source might not be available while compiling subclass
 @Target(METHOD)
 public @interface ForOverride {}
diff --git a/annotations/src/main/java/com/google/errorprone/annotations/IncompatibleModifiers.java b/annotations/src/main/java/com/google/errorprone/annotations/IncompatibleModifiers.java
@@ -21,14 +21,13 @@
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
-import javax.lang.model.element.Modifier;
 
 /**
  * Annotation declaring that the target annotation is incompatible with any one of the provided
  * modifiers. For example, an annotation declared as:
  *
  * <pre>
- * {@literal @}IncompatibleModifiers(Modifier.PUBLIC)
+ * {@literal @}IncompatibleModifiers(modifier = Modifier.PUBLIC)
  * {@literal @}interface MyAnnotation {}
  * </pre>
  *
@@ -44,12 +43,17 @@
 @Retention(RetentionPolicy.CLASS) // Element's source might not be available during analysis
 @Target(ElementType.ANNOTATION_TYPE)
 public @interface IncompatibleModifiers {
+
+  /** @deprecated use {@link #modifier} instead */
+  @Deprecated
+  javax.lang.model.element.Modifier[] value() default {};
+
   /**
    * The incompatible modifiers. The annotated element is illegal with the presence of any one or
    * more of these modifiers.
    *
    * <p>Empty array has the same effect as not applying this annotation at all; duplicates are
    * allowed but have no effect.
    */
-  Modifier[] value();
+  Modifier[] modifier() default {};
 }
diff --git a/annotations/src/main/java/com/google/errorprone/annotations/Modifier.java b/annotations/src/main/java/com/google/errorprone/annotations/Modifier.java
@@ -0,0 +1,42 @@
+/*
+ * Copyright 2021 The Error Prone Authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.errorprone.annotations;
+
+/**
+ * Modifiers in the Java language, as specified in:
+ *
+ * <ul>
+ *   <li>https://docs.oracle.com/javase/specs/jls/se11/html/jls-8.html#jls-8.1.1
+ *   <li>https://docs.oracle.com/javase/specs/jls/se11/html/jls-8.html#jls-8.3.1
+ *   <li>https://docs.oracle.com/javase/specs/jls/se11/html/jls-8.html#jls-8.4.3
+ *   <li>https://docs.oracle.com/javase/specs/jls/se11/html/jls-9.html#jls-9.4
+ * </ul>
+ */
+public enum Modifier {
+  PUBLIC,
+  PROTECTED,
+  PRIVATE,
+  ABSTRACT,
+  DEFAULT,
+  STATIC,
+  FINAL,
+  TRANSIENT,
+  VOLATILE,
+  SYNCHRONIZED,
+  NATIVE,
+  STRICTFP
+}
diff --git a/annotations/src/main/java/com/google/errorprone/annotations/RequiredModifiers.java b/annotations/src/main/java/com/google/errorprone/annotations/RequiredModifiers.java
@@ -21,14 +21,13 @@
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
-import javax.lang.model.element.Modifier;
 
 /**
  * Annotation declaring that the target annotation requires all the specified modifiers. For
  * example, an annotation declared as:
  *
  * <pre>
- * {@literal @}RequiredModifiers(Modifier.PUBLIC)
+ * {@literal @}RequiredModifiers(modifier = Modifier.PUBLIC)
  * {@literal @}interface MyAnnotation {}
  * </pre>
  *
@@ -44,12 +43,17 @@
 @Retention(RetentionPolicy.CLASS) // Element's source might not be available during analysis
 @Target(ElementType.ANNOTATION_TYPE)
 public @interface RequiredModifiers {
+
+  /** @deprecated use {@link #modifier} instead */
+  @Deprecated
+  javax.lang.model.element.Modifier[] value() default {};
+
   /**
    * The required modifiers. The annotated element is illegal if any one or more of these modifiers
    * are absent.
    *
    * <p>Empty array has the same effect as not applying this annotation at all; duplicates are
    * allowed but have no effect.
    */
-  Modifier[] value();
+  Modifier[] modifier() default {};
 }
diff --git a/annotations/src/main/java/com/google/errorprone/annotations/Var.java b/annotations/src/main/java/com/google/errorprone/annotations/Var.java
@@ -20,7 +20,6 @@
 import static java.lang.annotation.ElementType.LOCAL_VARIABLE;
 import static java.lang.annotation.ElementType.PARAMETER;
 import static java.lang.annotation.RetentionPolicy.RUNTIME;
-import static javax.lang.model.element.Modifier.FINAL;
 
 import java.lang.annotation.Retention;
 import java.lang.annotation.Target;
@@ -42,5 +41,5 @@
  */
 @Target({FIELD, PARAMETER, LOCAL_VARIABLE})
 @Retention(RUNTIME)
-@IncompatibleModifiers(FINAL)
+@IncompatibleModifiers(modifier = {Modifier.FINAL})
 public @interface Var {}
diff --git a/annotations/src/main/java/com/google/errorprone/annotations/concurrent/LazyInit.java b/annotations/src/main/java/com/google/errorprone/annotations/concurrent/LazyInit.java
@@ -15,9 +15,9 @@
  */
 package com.google.errorprone.annotations.concurrent;
 
-import static javax.lang.model.element.Modifier.FINAL;
 
 import com.google.errorprone.annotations.IncompatibleModifiers;
+import com.google.errorprone.annotations.Modifier;
 import java.lang.annotation.ElementType;
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
@@ -61,7 +61,7 @@
  * unless you really understand this <b>and</b> you really need the performance benefits of
  * introducing the data race, do not use this construct.
  */
-@IncompatibleModifiers({FINAL})
+@IncompatibleModifiers(modifier = {Modifier.FINAL})
 @Retention(RetentionPolicy.RUNTIME)
 @Target(ElementType.FIELD)
 public @interface LazyInit {}
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/IncompatibleModifiersChecker.java b/core/src/main/java/com/google/errorprone/bugpatterns/IncompatibleModifiersChecker.java
@@ -19,21 +19,28 @@
 import static com.google.errorprone.BugPattern.LinkType.NONE;
 import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
 import static com.google.errorprone.matchers.Description.NO_MATCH;
+import static com.google.errorprone.util.MoreAnnotations.getValue;
 
-import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Sets;
 import com.google.errorprone.BugPattern;
 import com.google.errorprone.VisitorState;
-import com.google.errorprone.annotations.IncompatibleModifiers;
 import com.google.errorprone.bugpatterns.BugChecker.AnnotationTreeMatcher;
 import com.google.errorprone.fixes.SuggestedFixes;
 import com.google.errorprone.matchers.Description;
 import com.google.errorprone.util.ASTHelpers;
 import com.sun.source.tree.AnnotationTree;
 import com.sun.source.tree.ModifiersTree;
 import com.sun.source.tree.Tree;
+import com.sun.tools.javac.code.Attribute;
+import com.sun.tools.javac.code.Symbol;
+import java.util.Collection;
+import java.util.LinkedHashSet;
+import java.util.List;
 import java.util.Set;
+import javax.lang.model.element.AnnotationValue;
 import javax.lang.model.element.Modifier;
+import javax.lang.model.element.VariableElement;
+import javax.lang.model.util.SimpleAnnotationValueVisitor8;
 
 /** @author [email protected] (Steven Goldfeder) */
 @BugPattern(
@@ -45,15 +52,28 @@
     severity = ERROR)
 public class IncompatibleModifiersChecker extends BugChecker implements AnnotationTreeMatcher {
 
+  private static final String INCOMPATIBLE_MODIFIERS =
+      "com.google.errorprone.annotations.IncompatibleModifiers";
+
   @Override
   public Description matchAnnotation(AnnotationTree tree, VisitorState state) {
-    IncompatibleModifiers annotation = ASTHelpers.getAnnotation(tree, IncompatibleModifiers.class);
+    Symbol sym = ASTHelpers.getSymbol(tree);
+    if (sym == null) {
+      return NO_MATCH;
+    }
+    Attribute.Compound annotation =
+        sym.getRawAttributes().stream()
+            .filter(a -> a.type.tsym.getQualifiedName().contentEquals(INCOMPATIBLE_MODIFIERS))
+            .findAny()
+            .orElse(null);
     if (annotation == null) {
       return NO_MATCH;
     }
-    ImmutableSet<Modifier> incompatibleModifiers = ImmutableSet.copyOf(annotation.value());
+    Set<Modifier> incompatibleModifiers = new LinkedHashSet<>();
+    getValue(annotation, "value").ifPresent(a -> getModifiers(incompatibleModifiers, a));
+    getValue(annotation, "modifier").ifPresent(a -> getModifiers(incompatibleModifiers, a));
     if (incompatibleModifiers.isEmpty()) {
-      return Description.NO_MATCH;
+      return NO_MATCH;
     }
 
     Tree parent = state.getPath().getParentPath().getLeaf();
@@ -83,4 +103,21 @@ public Description matchAnnotation(AnnotationTree tree, VisitorState state) {
         .setMessage(message)
         .build();
   }
+
+  private static void getModifiers(Collection<Modifier> modifiers, Attribute attribute) {
+    class Visitor extends SimpleAnnotationValueVisitor8<Void, Void> {
+      @Override
+      public Void visitEnumConstant(VariableElement c, Void unused) {
+        modifiers.add(Modifier.valueOf(c.getSimpleName().toString()));
+        return null;
+      }
+
+      @Override
+      public Void visitArray(List<? extends AnnotationValue> vals, Void unused) {
+        vals.forEach(val -> val.accept(this, null));
+        return null;
+      }
+    }
+    attribute.accept(new Visitor(), null);
+  }
 }
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/RequiredModifiersChecker.java b/core/src/main/java/com/google/errorprone/bugpatterns/RequiredModifiersChecker.java
@@ -18,21 +18,29 @@
 
 import static com.google.errorprone.BugPattern.LinkType.NONE;
 import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
+import static com.google.errorprone.matchers.Description.NO_MATCH;
+import static com.google.errorprone.util.MoreAnnotations.getValue;
 
-import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Sets;
 import com.google.errorprone.BugPattern;
 import com.google.errorprone.VisitorState;
-import com.google.errorprone.annotations.RequiredModifiers;
 import com.google.errorprone.bugpatterns.BugChecker.AnnotationTreeMatcher;
 import com.google.errorprone.fixes.SuggestedFixes;
 import com.google.errorprone.matchers.Description;
 import com.google.errorprone.util.ASTHelpers;
 import com.sun.source.tree.AnnotationTree;
 import com.sun.source.tree.ModifiersTree;
 import com.sun.source.tree.Tree;
+import com.sun.tools.javac.code.Attribute;
+import com.sun.tools.javac.code.Symbol;
+import java.util.Collection;
+import java.util.LinkedHashSet;
+import java.util.List;
 import java.util.Set;
+import javax.lang.model.element.AnnotationValue;
 import javax.lang.model.element.Modifier;
+import javax.lang.model.element.VariableElement;
+import javax.lang.model.util.SimpleAnnotationValueVisitor8;
 
 /** @author [email protected] (Steven Goldfeder) */
 @BugPattern(
@@ -46,28 +54,40 @@ public class RequiredModifiersChecker extends BugChecker implements AnnotationTr
 
   private static final String MESSAGE_TEMPLATE =
       "%s has specified that it must be used together with the following modifiers: %s";
+  private static final String REQUIRED_MODIFIERS =
+      "com.google.errorprone.annotations.RequiredModifiers";
 
   @Override
   public Description matchAnnotation(AnnotationTree tree, VisitorState state) {
-    RequiredModifiers annotation = ASTHelpers.getAnnotation(tree, RequiredModifiers.class);
+    Symbol sym = ASTHelpers.getSymbol(tree);
+    if (sym == null) {
+      return NO_MATCH;
+    }
+    Attribute.Compound annotation =
+        sym.getRawAttributes().stream()
+            .filter(a -> a.type.tsym.getQualifiedName().contentEquals(REQUIRED_MODIFIERS))
+            .findAny()
+            .orElse(null);
     if (annotation == null) {
-      return Description.NO_MATCH;
+      return NO_MATCH;
     }
-    Set<Modifier> requiredModifiers = ImmutableSet.copyOf(annotation.value());
+    Set<Modifier> requiredModifiers = new LinkedHashSet<>();
+    getValue(annotation, "value").ifPresent(a -> getModifiers(requiredModifiers, a));
+    getValue(annotation, "modifier").ifPresent(a -> getModifiers(requiredModifiers, a));
     if (requiredModifiers.isEmpty()) {
-      return Description.NO_MATCH;
+      return NO_MATCH;
     }
 
     Tree parent = state.getPath().getParentPath().getLeaf();
     if (!(parent instanceof ModifiersTree)) {
       // e.g. An annotated package name
-      return Description.NO_MATCH;
+      return NO_MATCH;
     }
 
     Set<Modifier> missing = Sets.difference(requiredModifiers, ((ModifiersTree) parent).getFlags());
 
     if (missing.isEmpty()) {
-      return Description.NO_MATCH;
+      return NO_MATCH;
     }
 
     String annotationName = ASTHelpers.getAnnotationName(tree);
@@ -86,4 +106,21 @@ public Description matchAnnotation(AnnotationTree tree, VisitorState state) {
         .setMessage(customMessage)
         .build();
   }
+
+  private static void getModifiers(Collection<Modifier> modifiers, Attribute attribute) {
+    class Visitor extends SimpleAnnotationValueVisitor8<Void, Void> {
+      @Override
+      public Void visitEnumConstant(VariableElement c, Void unused) {
+        modifiers.add(Modifier.valueOf(c.getSimpleName().toString()));
+        return null;
+      }
+
+      @Override
+      public Void visitArray(List<? extends AnnotationValue> vals, Void unused) {
+        vals.forEach(val -> val.accept(this, null));
+        return null;
+      }
+    }
+    attribute.accept(new Visitor(), null);
+  }
 }