Refactor cache to determine if an element is @AnnotatedFor

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

@@ -4,6 +4,7 @@
 import org.checkerframework.checker.nullness.qual.Nullable;
 import org.checkerframework.checker.signature.qual.ClassGetName;
 import org.checkerframework.dataflow.cfg.visualize.CFGVisualizer;
+import org.checkerframework.framework.qual.AnnotatedFor;
 import org.checkerframework.framework.qual.SubtypeOf;
 import org.checkerframework.framework.source.SourceChecker;
 import org.checkerframework.framework.type.AnnotatedTypeFactory;
@@ -13,6 +14,7 @@
 import org.checkerframework.javacutil.AbstractTypeProcessor;
 import org.checkerframework.javacutil.AnnotationProvider;
 import org.checkerframework.javacutil.BugInCF;
+import org.checkerframework.javacutil.ElementUtils;
 import org.checkerframework.javacutil.TypeSystemError;
 import org.checkerframework.javacutil.UserError;
 import org.plumelib.util.CollectionsPlume;
@@ -22,9 +24,13 @@
 import java.lang.reflect.InvocationTargetException;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.IdentityHashMap;
 import java.util.Set;
 
 import javax.lang.model.element.AnnotationMirror;
+import javax.lang.model.element.Element;
+import javax.lang.model.element.ElementKind;
+import javax.lang.model.element.PackageElement;
 
 /**
  * An abstract {@link SourceChecker} that provides a simple {@link
@@ -68,6 +74,13 @@
  */
 public abstract class BaseTypeChecker extends SourceChecker {
 
+    /**
+     * A mapping from an element to whether it is in an {@code @AnnotatedFor} scope for this checker
+     * or an upstream checker.
+     */
+    private final IdentityHashMap<Element, Boolean> elementAnnotatedForThisCheckerOrUpstreamCache =
+            new IdentityHashMap<>();
+
     /** An array containing just {@code BaseTypeChecker.class}. */
     protected static Class<?>[] baseTypeCheckerClassArray = new Class<?>[] {BaseTypeChecker.class};
 
@@ -313,4 +326,40 @@ public BaseTypeChecker getUltimateParentChecker() {
                     causeMessage);
         }
     }
+
+    @Override
+    public boolean isElementAnnotatedForThisCheckerOrUpstreamChecker(@Nullable Element elt) {
+        if (elt == null) {
+            throw new BugInCF(
+                    "Call of BaseTypeChecker.isElementAnnotatedForThisCheckerOrUpstreamChecker with null");
+        }
+
+        if (elementAnnotatedForThisCheckerOrUpstreamCache.containsKey(elt)) {
+            return elementAnnotatedForThisCheckerOrUpstreamCache.get(elt);
+        }
+
+        AnnotatedTypeFactory atypeFactory = getTypeFactory();
+        AnnotationMirror annotatedFor = atypeFactory.getDeclAnnotation(elt, AnnotatedFor.class);
+        boolean elementAnnotatedForThisChecker =
+                annotatedFor != null
+                        && atypeFactory.doesAnnotatedForApplyToThisChecker(annotatedFor);
+
+        if (!elementAnnotatedForThisChecker) {
+            Element parent;
+            if (elt.getKind() == ElementKind.PACKAGE) {
+                parent =
+                        ElementUtils.parentPackage(
+                                (PackageElement) elt, atypeFactory.getElementUtils());
+            } else {
+                parent = elt.getEnclosingElement();
+            }
+
+            if (parent != null && isElementAnnotatedForThisCheckerOrUpstreamChecker(parent)) {
+                elementAnnotatedForThisChecker = true;
+            }
+        }
+
+        elementAnnotatedForThisCheckerOrUpstreamCache.put(elt, elementAnnotatedForThisChecker);
+        return elementAnnotatedForThisChecker;
+    }
 }

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

@@ -31,6 +31,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 +327,11 @@ public AnnotationProvider getAnnotationProvider() {
         throw new UnsupportedOperationException(
                 "getAnnotationProvider is not implemented for this class.");
     }
+
+    @Override
+    public boolean isElementAnnotatedForThisCheckerOrUpstreamChecker(Element elt) {
+        // This checker only counts annotations; it does not perform any type-checking. Returning
+        // false keeps callers (e.g., QualifierDefaults) on safe defaults rather than crashing.
+        return false;
+    }
 }

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

@@ -24,6 +24,7 @@
 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 +204,11 @@ public AnnotationProvider getAnnotationProvider() {
         throw new UnsupportedOperationException(
                 "getAnnotationProvider is not implemented for this class.");
     }
+
+    @Override
+    public boolean isElementAnnotatedForThisCheckerOrUpstreamChecker(Element elt) {
+        // This checker only counts code; it does not perform any type-checking. Returning false
+        // keeps callers (e.g., QualifierDefaults) on safe defaults rather than crashing.
+        return false;
+    }
 }

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

@@ -100,6 +100,15 @@ private void init(ProcessingEnvironment env, @Nullable @BinaryName String checke
             checker =
                     new SourceChecker() {
 
+                        @Override
+                        public boolean isElementAnnotatedForThisCheckerOrUpstreamChecker(
+                                Element elt) {
+                            // SignaturePrinter only prints signatures; it does not perform any
+                            // type-checking. Returning false keeps callers (e.g.,
+                            // QualifierDefaults) on safe defaults rather than crashing.
+                            return false;
+                        }
+
                         @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
+    public 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.
@@ -2773,9 +2771,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())) {
@@ -2785,21 +2781,17 @@ 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.
-                        return false;
-                    }
+                    // No need to call isElementAnnotatedForThisCheckerOrUpstreamChecker on the
+                    // package: the call above on the class element already walks enclosing
+                    // elements (including the package) recursively.
                 }
             } else {
                 throw new BugInCF("Unexpected declaration kind: " + decl.getKind() + " " + decl);
@@ -2875,11 +2867,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;
     }
@@ -3011,33 +2998,8 @@ 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;
-    }
+    public abstract boolean isElementAnnotatedForThisCheckerOrUpstreamChecker(
+            @Nullable 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

@@ -4118,6 +4118,18 @@ protected void parseAnnotationFiles() {
         ajavaTypes.parseAjavaFiles();
     }
 
+    /**
+     * Returns true if this type factory is currently parsing stub files or ajava files. While this
+     * is true, annotation-file-backed lookup may observe partially loaded annotation-file data.
+     *
+     * @return true if stub files or ajava files are currently being parsed
+     */
+    public boolean isParsingAnnotationFiles() {
+        return stubTypes.isParsing()
+                || ajavaTypes.isParsing()
+                || (currentFileAjavaTypes != null && currentFileAjavaTypes.isParsing());
+    }
+
     /**
      * Returns all of the declaration annotations whose name equals the passed annotation class (or
      * is an alias for it) including annotations:

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;
@@ -116,9 +115,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(
@@ -637,46 +633,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.
@@ -792,6 +748,22 @@ public boolean applyConservativeDefaults(Element annotationScope) {
             return false;
         }
 
+        // Skip the conservative-defaults check while annotation files are being parsed, to avoid
+        // an initialization cycle. During GenericAnnotatedTypeFactory.postInit(),
+        // parseAnnotationFiles()
+        // runs the stub/ajava parser, which asks the type factory for defaulted types. That reaches
+        // here and would call checker.isElementAnnotatedForThisCheckerOrUpstreamChecker(...), which
+        // routes through BaseTypeChecker.getTypeFactory() — but the visitor (and thus the type
+        // factory) is not yet installed on the checker, causing an NPE. Eagerly-parsed annotation
+        // files (checker @StubFiles, command-line stubs, ajava files, annotated-JDK
+        // package-info.java)
+        // are the risky cases; most JDK class stubs are only parsed lazily after init completes.
+        // Stub-file elements are still treated as checked code by the isFromStubFile branch below
+        // once parsing has finished.
+        if (atypeFactory.isParsingAnnotationFiles()) {
+            return false;
+        }
+
         // TODO: I would expect this:
         //   atypeFactory.isFromByteCode(annotationScope)) {
         // to work instead of the
@@ -805,7 +777,9 @@ public boolean applyConservativeDefaults(Element annotationScope) {
                         && !isFromStubFile;
         if (isBytecode) {
             return useConservativeDefaultsBytecode
-                    && !isElementAnnotatedForThisChecker(annotationScope);
+                    && !atypeFactory
+                            .getChecker()
+                            .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
@@ -814,7 +788,9 @@ 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
+                    .getChecker()
+                    .isElementAnnotatedForThisCheckerOrUpstreamChecker(annotationScope);
         }
         return false;
     }