Refactor cache to determine if an element is annotatedfor

framework/src/main/java/org/checkerframework/common/basetype/BaseTypeChecker.java

@@ -25,6 +25,7 @@
 import java.util.Set;
 
 import javax.lang.model.element.AnnotationMirror;
+import javax.lang.model.element.Element;
 
 /**
  * An abstract {@link SourceChecker} that provides a simple {@link
@@ -313,4 +314,27 @@ public BaseTypeChecker getUltimateParentChecker() {
                     causeMessage);
         }
     }
+
+    /**
+     * The implementation of this method resides in AnnotatedTypeFactory (atf), see {@link
+     * AnnotatedTypeFactory#isElementAnnotatedForThisCheckerOrUpstreamChecker(Element)} We want to
+     * implement it here to avoid duplication and call
+     * atypeFactory.getChecker().isElementAnnotatedForThisCheckerOrUpstreamChecker(elt) in
+     * QualifierDefaults, but implement it here causes type-system error. The reason is the
+     * implementation requires an atf to call {@link AnnotatedTypeFactory#getDeclAnnotation(Element,
+     * Class)}to get the relavent {@code @AnnotatedFor} annotation on the element. However, the
+     * method is called during the initialization of the atf when applying qualifier defaults. At
+     * that time, the atf and the visitor are not fully initialized, so calling getTypeFactory()
+     * will result in a type-system error. The initialization logic is as follows: 1. Create the
+     * checker. 2. Create the visitor, and the visitor's constructor initializes the atf. 3. During
+     * postInit() of the atf, the qualifier defaults are applied, which need to call
+     * isElementAnnotatedForThisCheckerOrUpstreamChecker().
+     *
+     * @param elt the source code element to check, or null
+     * @return true if the element is annotated for this checker or an upstream checker
+     */
+    @Override
+    protected boolean isElementAnnotatedForThisCheckerOrUpstreamChecker(Element elt) {
+        return getTypeFactory().isElementAnnotatedForThisCheckerOrUpstreamChecker(elt);
+    }
 }

framework/src/main/java/org/checkerframework/common/util/count/AnnotationStatistics.java

@@ -23,6 +23,7 @@
 import org.checkerframework.framework.source.SourceVisitor;
 import org.checkerframework.framework.source.SupportedOptions;
 import org.checkerframework.javacutil.AnnotationProvider;
+import org.checkerframework.javacutil.BugInCF;
 
 import java.util.HashMap;
 import java.util.Map;
@@ -31,6 +32,7 @@
 
 import javax.annotation.processing.SupportedSourceVersion;
 import javax.lang.model.SourceVersion;
+import javax.lang.model.element.Element;
 import javax.lang.model.element.Name;
 import javax.tools.Diagnostic;
 
@@ -326,4 +328,9 @@ public AnnotationProvider getAnnotationProvider() {
         throw new UnsupportedOperationException(
                 "getAnnotationProvider is not implemented for this class.");
     }
+
+    @Override
+    protected boolean isElementAnnotatedForThisCheckerOrUpstreamChecker(Element elt) {
+        throw new BugInCF("Unexpected call to determine whether this checker is annotated");
+    }
 }

framework/src/main/java/org/checkerframework/common/util/count/JavaCodeStatistics.java

@@ -17,13 +17,15 @@
 import org.checkerframework.framework.source.SourceVisitor;
 import org.checkerframework.javacutil.AnnotationProvider;
 import org.checkerframework.javacutil.AnnotationUtils;
+import org.checkerframework.javacutil.BugInCF;
 import org.checkerframework.javacutil.TreeUtils;
 
 import java.util.List;
 
 import javax.annotation.processing.SupportedSourceVersion;
 import javax.lang.model.SourceVersion;
 import javax.lang.model.element.AnnotationMirror;
+import javax.lang.model.element.Element;
 import javax.lang.model.element.ExecutableElement;
 
 /**
@@ -203,4 +205,9 @@ public AnnotationProvider getAnnotationProvider() {
         throw new UnsupportedOperationException(
                 "getAnnotationProvider is not implemented for this class.");
     }
+
+    @Override
+    protected boolean isElementAnnotatedForThisCheckerOrUpstreamChecker(Element elt) {
+        throw new BugInCF("Unexpected call to determine whether this checker is annotated");
+    }
 }

framework/src/main/java/org/checkerframework/common/util/debug/SignaturePrinter.java

@@ -15,6 +15,7 @@
 import org.checkerframework.framework.type.AnnotatedTypeMirror.AnnotatedTypeVariable;
 import org.checkerframework.javacutil.AbstractTypeProcessor;
 import org.checkerframework.javacutil.AnnotationProvider;
+import org.checkerframework.javacutil.BugInCF;
 import org.checkerframework.javacutil.ElementUtils;
 import org.checkerframework.javacutil.UserError;
 import org.plumelib.reflection.Signatures;
@@ -100,6 +101,13 @@ private void init(ProcessingEnvironment env, @Nullable @BinaryName String checke
             checker =
                     new SourceChecker() {
 
+                        @Override
+                        protected boolean isElementAnnotatedForThisCheckerOrUpstreamChecker(
+                                Element elt) {
+                            throw new BugInCF(
+                                    "Unexpected call to determine whether this checker is annotated");
+                        }
+
                         @Override
                         protected SourceVisitor<?, ?> createSourceVisitor() {
                             return null;

framework/src/main/java/org/checkerframework/framework/source/AggregateChecker.java

@@ -7,6 +7,7 @@
 import java.util.LinkedHashSet;
 import java.util.Set;
 
+import javax.lang.model.element.Element;
 import javax.tools.Diagnostic;
 
 /**
@@ -52,4 +53,15 @@ protected final Set<Class<? extends SourceChecker>> getImmediateSubcheckerClasse
             // the checkers in the aggregate checker do.
         };
     }
+
+    /**
+     * Always returns false. Whether an aggregate checker is annotated with {@code @AnnotatedFor}
+     * depends on its subcheckers. For checkers that have an upstream checker and want to handle
+     * {@code @AnnotatedFor} in both this and the upstream checker, see {@link
+     * org.checkerframework.checker.initialization.InitializationChecker#getUpstreamCheckerNames()}.
+     */
+    @Override
+    protected boolean isElementAnnotatedForThisCheckerOrUpstreamChecker(Element elt) {
+        return false;
+    }
 }

framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java

@@ -31,9 +31,7 @@
 import org.checkerframework.checker.signature.qual.FullyQualifiedName;
 import org.checkerframework.common.basetype.BaseTypeChecker;
 import org.checkerframework.common.reflection.MethodValChecker;
-import org.checkerframework.framework.qual.AnnotatedFor;
 import org.checkerframework.framework.type.AnnotatedTypeFactory;
-import org.checkerframework.framework.util.CheckerMain;
 import org.checkerframework.framework.util.OptionConfiguration;
 import org.checkerframework.framework.util.TreePathCacher;
 import org.checkerframework.javacutil.AbstractTypeProcessor;
@@ -152,7 +150,7 @@
     // only issue errors for code inside the scope of `@NullMarked` annotations.
     // See
     // https://github.com/uber/NullAway/wiki/Configuration#only-nullmarked-version-0123-and-after.
-    // org.checkerframework.framework.source.SourceChecker.isAnnotatedForThisCheckerOrUpstreamChecker
+    // org.checkerframework.framework.source.SourceChecker.isElementAnnotatedForThisCheckerOrUpstreamChecker
     "onlyAnnotatedFor",
 
     // Unsoundly assume all methods have no side effects, are deterministic, or both.
@@ -2771,9 +2769,7 @@ public boolean shouldSuppressWarnings(TreePath path, String errKey) {
                     return true;
                 }
 
-                if (isAnnotatedForThisCheckerOrUpstreamChecker(elt)) {
-                    // Return false immediately. Do NOT check for AnnotatedFor in the enclosing
-                    // elements as the closest AnnotatedFor is already found.
+                if (isElementAnnotatedForThisCheckerOrUpstreamChecker(elt)) {
                     return false;
                 }
             } else if (TreeUtils.classTreeKinds().contains(decl.getKind())) {
@@ -2783,19 +2779,16 @@ public boolean shouldSuppressWarnings(TreePath path, String errKey) {
                     return true;
                 }
 
-                if (isAnnotatedForThisCheckerOrUpstreamChecker(elt)) {
-                    // Return false immediately. Do NOT check for AnnotatedFor in the enclosing
-                    // elements as the closest AnnotatedFor is already found.
+                if (isElementAnnotatedForThisCheckerOrUpstreamChecker(elt)) {
                     return false;
                 }
                 Element packageElement = elt.getEnclosingElement();
                 if (packageElement != null && packageElement.getKind() == ElementKind.PACKAGE) {
                     if (shouldSuppressWarnings(packageElement, errKey)) {
                         return true;
                     }
-                    if (isAnnotatedForThisCheckerOrUpstreamChecker(packageElement)) {
-                        // Return false immediately. Do NOT check for AnnotatedFor in the enclosing
-                        // elements as the closest AnnotatedFor is already found.
+
+                    if (isElementAnnotatedForThisCheckerOrUpstreamChecker(packageElement)) {
                         return false;
                     }
                 }
@@ -2873,11 +2866,6 @@ public boolean shouldSuppressWarnings(Element elt, String errKey) {
                     return true;
                 }
             }
-            if (isAnnotatedForThisCheckerOrUpstreamChecker(elt)) {
-                // Return false immediately. Do NOT check for AnnotatedFor in the
-                // enclosing elements, because they may not have an @AnnotatedFor.
-                return false;
-            }
         }
         return false;
     }
@@ -3009,33 +2997,7 @@ protected boolean messageKeyMatches(
      * @param elt the source code element to check, or null
      * @return true if the element is annotated for this checker or an upstream checker
      */
-    private boolean isAnnotatedForThisCheckerOrUpstreamChecker(@Nullable Element elt) {
-        // Return false if elt is null, or if neither useConservativeDefaultsSource nor
-        // issueErrorsForOnlyAnnotatedForScope is set, since the @AnnotatedFor status is irrelevant
-        // in that case.
-        // TODO: Refactor SourceChecker and QualifierDefaults to use a cache for determining if an
-        // element is annotated for.
-        if (elt == null || (!useConservativeDefaultsSource && !onlyAnnotatedFor)) {
-            return false;
-        }
-
-        AnnotatedFor anno = elt.getAnnotation(AnnotatedFor.class);
-
-        String[] userAnnotatedFors = (anno == null ? null : anno.value());
-
-        if (userAnnotatedFors != null) {
-            List<@FullyQualifiedName String> upstreamCheckerNames = getUpstreamCheckerNames();
-
-            for (String userAnnotatedFor : userAnnotatedFors) {
-                if (CheckerMain.matchesCheckerOrSubcheckerFromList(
-                        userAnnotatedFor, upstreamCheckerNames)) {
-                    return true;
-                }
-            }
-        }
-
-        return false;
-    }
+    protected abstract boolean isElementAnnotatedForThisCheckerOrUpstreamChecker(Element elt);
 
     /**
      * Returns a modifiable set of lower-case strings that are prefixes for SuppressWarnings

framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeFactory.java

@@ -536,6 +536,9 @@ void checkRep(String aliasName) {
     /** Mapping from a Tree to its TreePath. Shared between all instances. */
     private final TreePathCacher treePathCache;
 
+    /** A mapping of Element → Whether or not that element is AnnotatedFor this type system. */
+    private final IdentityHashMap<Element, Boolean> elementAnnotatedFors = new IdentityHashMap<>();
+
     /** Whether to ignore type arguments from raw types. */
     public final boolean ignoreRawTypeArguments;
 
@@ -6034,6 +6037,49 @@ public boolean doesAnnotatedForApplyToThisChecker(AnnotationMirror annotatedForA
         return false;
     }
 
+    /**
+     * Return true if the element has an {@code @AnnotatedFor} annotation, for this checker or an
+     * upstream checker that called this one.
+     *
+     * @param elt the source code element to check, or null
+     * @return true if the element is annotated for this checker or an upstream checker
+     */
+    public boolean isElementAnnotatedForThisCheckerOrUpstreamChecker(@Nullable Element elt) {
+        boolean elementAnnotatedForThisChecker = false;
+
+        if (elt == null) {
+            throw new BugInCF(
+                    "Call of AnnotatedTypeFactory.isElementAnnotatedForThisCheckerOrUpstreamChecker with null");
+        }
+
+        if (elementAnnotatedFors.containsKey(elt)) {
+            return elementAnnotatedFors.get(elt);
+        }
+
+        AnnotationMirror annotatedFor = getDeclAnnotation(elt, AnnotatedFor.class);
+
+        if (annotatedFor != null) {
+            elementAnnotatedForThisChecker = doesAnnotatedForApplyToThisChecker(annotatedFor);
+        }
+
+        if (!elementAnnotatedForThisChecker) {
+            Element parent;
+            if (elt.getKind() == ElementKind.PACKAGE) {
+                parent = ElementUtils.parentPackage((PackageElement) elt, elements);
+            } else {
+                parent = elt.getEnclosingElement();
+            }
+
+            if (parent != null && isElementAnnotatedForThisCheckerOrUpstreamChecker(parent)) {
+                elementAnnotatedForThisChecker = true;
+            }
+        }
+
+        elementAnnotatedFors.put(elt, elementAnnotatedForThisChecker);
+
+        return elementAnnotatedForThisChecker;
+    }
+
     /**
      * Get the {@code expression} field/element of the given contract annotation.
      *

framework/src/main/java/org/checkerframework/framework/util/defaults/QualifierDefaults.java

@@ -14,7 +14,6 @@
 
 import org.checkerframework.checker.interning.qual.FindDistinct;
 import org.checkerframework.checker.nullness.qual.Nullable;
-import org.checkerframework.framework.qual.AnnotatedFor;
 import org.checkerframework.framework.qual.DefaultQualifier;
 import org.checkerframework.framework.qual.TypeUseLocation;
 import org.checkerframework.framework.type.AnnotatedTypeFactory;
@@ -121,9 +120,6 @@ public class QualifierDefaults {
      */
     private final IdentityHashMap<Element, DefaultSet> elementDefaults = new IdentityHashMap<>();
 
-    /** A mapping of Element &rarr; Whether or not that element is AnnotatedFor this type system. */
-    private final IdentityHashMap<Element, Boolean> elementAnnotatedFors = new IdentityHashMap<>();
-
     /** CLIMB locations whose standard default is top for a given type system. */
     public static final List<TypeUseLocation> STANDARD_CLIMB_DEFAULTS_TOP =
             Collections.unmodifiableList(
@@ -642,46 +638,6 @@ private void applyDefaults(Tree tree, AnnotatedTypeMirror type) {
         }
     }
 
-    private boolean isElementAnnotatedForThisChecker(Element elt) {
-        boolean elementAnnotatedForThisChecker = false;
-
-        if (elt == null) {
-            throw new BugInCF(
-                    "Call of QualifierDefaults.isElementAnnotatedForThisChecker with null");
-        }
-
-        if (elementAnnotatedFors.containsKey(elt)) {
-            return elementAnnotatedFors.get(elt);
-        }
-
-        AnnotationMirror annotatedFor = atypeFactory.getDeclAnnotation(elt, AnnotatedFor.class);
-
-        if (annotatedFor != null) {
-            elementAnnotatedForThisChecker =
-                    atypeFactory.doesAnnotatedForApplyToThisChecker(annotatedFor);
-        }
-
-        if (!elementAnnotatedForThisChecker) {
-            Element parent;
-            if (elt.getKind() == ElementKind.PACKAGE) {
-                // TODO: should AnnotatedFor apply to subpackages??
-                // elt.getEnclosingElement() on a package is null; therefore,
-                // use the dedicated method.
-                parent = ElementUtils.parentPackage((PackageElement) elt, elements);
-            } else {
-                parent = elt.getEnclosingElement();
-            }
-
-            if (parent != null && isElementAnnotatedForThisChecker(parent)) {
-                elementAnnotatedForThisChecker = true;
-            }
-        }
-
-        elementAnnotatedFors.put(elt, elementAnnotatedForThisChecker);
-
-        return elementAnnotatedForThisChecker;
-    }
-
     /**
      * Returns the defaults that apply to the given Element, considering defaults from enclosing
      * Elements.
@@ -810,7 +766,8 @@ public boolean applyConservativeDefaults(Element annotationScope) {
                         && !isFromStubFile;
         if (isBytecode) {
             return useConservativeDefaultsBytecode
-                    && !isElementAnnotatedForThisChecker(annotationScope);
+                    && !atypeFactory.isElementAnnotatedForThisCheckerOrUpstreamChecker(
+                            annotationScope);
         } else if (isFromStubFile) {
             // TODO: Types in stub files not annotated for a particular checker should be
             // treated as unchecked bytecode.  For now, all types in stub files are treated as
@@ -819,7 +776,7 @@ public boolean applyConservativeDefaults(Element annotationScope) {
             // be treated like unchecked code except for methods in the scope of an @AnnotatedFor.
             return false;
         } else if (useConservativeDefaultsSource) {
-            return !isElementAnnotatedForThisChecker(annotationScope);
+            return !atypeFactory.isElementAnnotatedForThisCheckerOrUpstreamChecker(annotationScope);
         }
         return false;
     }