From b51f38697e042716810aa799dd1fbcce5c9763c8 Mon Sep 17 00:00:00 2001 From: Alex Cook Date: Tue, 28 Apr 2026 12:32:05 -0400 Subject: [PATCH 1/2] refactor: move AnnotatedFor check to BaseTypeChecker --- .../common/basetype/BaseTypeChecker.java | 65 +++++++++++++------ .../util/count/AnnotationStatistics.java | 2 +- .../common/util/count/JavaCodeStatistics.java | 2 +- .../common/util/debug/SignaturePrinter.java | 2 +- .../framework/source/AggregateChecker.java | 2 +- .../framework/source/SourceChecker.java | 2 +- .../framework/type/AnnotatedTypeFactory.java | 58 ++++------------- .../util/defaults/QualifierDefaults.java | 13 +++- 8 files changed, 72 insertions(+), 74 deletions(-) diff --git a/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeChecker.java b/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeChecker.java index 0cf38ece0dcb..771d9e01649f 100644 --- a/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeChecker.java +++ b/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,10 +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 @@ -69,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 elementAnnotatedForThisCheckerOrUpstreamCache = + new IdentityHashMap<>(); + /** An array containing just {@code BaseTypeChecker.class}. */ protected static Class[] baseTypeCheckerClassArray = new Class[] {BaseTypeChecker.class}; @@ -315,26 +327,39 @@ public BaseTypeChecker getUltimateParentChecker() { } } - /** - * 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); + 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; } } diff --git a/framework/src/main/java/org/checkerframework/common/util/count/AnnotationStatistics.java b/framework/src/main/java/org/checkerframework/common/util/count/AnnotationStatistics.java index 8696e35b31fc..c2a8fec8db08 100644 --- a/framework/src/main/java/org/checkerframework/common/util/count/AnnotationStatistics.java +++ b/framework/src/main/java/org/checkerframework/common/util/count/AnnotationStatistics.java @@ -330,7 +330,7 @@ public AnnotationProvider getAnnotationProvider() { } @Override - protected boolean isElementAnnotatedForThisCheckerOrUpstreamChecker(Element elt) { + public boolean isElementAnnotatedForThisCheckerOrUpstreamChecker(Element elt) { throw new BugInCF("Unexpected call to determine whether this checker is annotated"); } } diff --git a/framework/src/main/java/org/checkerframework/common/util/count/JavaCodeStatistics.java b/framework/src/main/java/org/checkerframework/common/util/count/JavaCodeStatistics.java index 2366021b4086..536a0ea72abe 100644 --- a/framework/src/main/java/org/checkerframework/common/util/count/JavaCodeStatistics.java +++ b/framework/src/main/java/org/checkerframework/common/util/count/JavaCodeStatistics.java @@ -207,7 +207,7 @@ public AnnotationProvider getAnnotationProvider() { } @Override - protected boolean isElementAnnotatedForThisCheckerOrUpstreamChecker(Element elt) { + public boolean isElementAnnotatedForThisCheckerOrUpstreamChecker(Element elt) { throw new BugInCF("Unexpected call to determine whether this checker is annotated"); } } diff --git a/framework/src/main/java/org/checkerframework/common/util/debug/SignaturePrinter.java b/framework/src/main/java/org/checkerframework/common/util/debug/SignaturePrinter.java index dec3e9645c51..2e907ddb553f 100644 --- a/framework/src/main/java/org/checkerframework/common/util/debug/SignaturePrinter.java +++ b/framework/src/main/java/org/checkerframework/common/util/debug/SignaturePrinter.java @@ -102,7 +102,7 @@ private void init(ProcessingEnvironment env, @Nullable @BinaryName String checke new SourceChecker() { @Override - protected boolean isElementAnnotatedForThisCheckerOrUpstreamChecker( + public boolean isElementAnnotatedForThisCheckerOrUpstreamChecker( Element elt) { throw new BugInCF( "Unexpected call to determine whether this checker is annotated"); diff --git a/framework/src/main/java/org/checkerframework/framework/source/AggregateChecker.java b/framework/src/main/java/org/checkerframework/framework/source/AggregateChecker.java index 579a2e76266b..efb03916b8cc 100644 --- a/framework/src/main/java/org/checkerframework/framework/source/AggregateChecker.java +++ b/framework/src/main/java/org/checkerframework/framework/source/AggregateChecker.java @@ -61,7 +61,7 @@ protected final Set> getImmediateSubcheckerClasse * org.checkerframework.checker.initialization.InitializationChecker#getUpstreamCheckerNames()}. */ @Override - protected boolean isElementAnnotatedForThisCheckerOrUpstreamChecker(Element elt) { + public boolean isElementAnnotatedForThisCheckerOrUpstreamChecker(Element elt) { return false; } } diff --git a/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java b/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java index 27e7dcbcf5b4..d6c1624d4da7 100644 --- a/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java +++ b/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java @@ -2999,7 +2999,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 */ - protected abstract boolean isElementAnnotatedForThisCheckerOrUpstreamChecker(Element elt); + public abstract boolean isElementAnnotatedForThisCheckerOrUpstreamChecker(Element elt); /** * Returns a modifiable set of lower-case strings that are prefixes for SuppressWarnings diff --git a/framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeFactory.java b/framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeFactory.java index e74d4d9d2586..1f271bff22bf 100644 --- a/framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeFactory.java +++ b/framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeFactory.java @@ -536,9 +536,6 @@ 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 elementAnnotatedFors = new IdentityHashMap<>(); - /** Whether to ignore type arguments from raw types. */ public final boolean ignoreRawTypeArguments; @@ -4100,6 +4097,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: @@ -6051,49 +6060,6 @@ 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. * diff --git a/framework/src/main/java/org/checkerframework/framework/util/defaults/QualifierDefaults.java b/framework/src/main/java/org/checkerframework/framework/util/defaults/QualifierDefaults.java index ff8855d2eb2f..69e70cd80ec5 100644 --- a/framework/src/main/java/org/checkerframework/framework/util/defaults/QualifierDefaults.java +++ b/framework/src/main/java/org/checkerframework/framework/util/defaults/QualifierDefaults.java @@ -748,6 +748,10 @@ public boolean applyConservativeDefaults(Element annotationScope) { return false; } + if (atypeFactory.isParsingAnnotationFiles()) { + return false; + } + // TODO: I would expect this: // atypeFactory.isFromByteCode(annotationScope)) { // to work instead of the @@ -761,8 +765,9 @@ public boolean applyConservativeDefaults(Element annotationScope) { && !isFromStubFile; if (isBytecode) { return useConservativeDefaultsBytecode - && !atypeFactory.isElementAnnotatedForThisCheckerOrUpstreamChecker( - 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 @@ -771,7 +776,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 !atypeFactory.isElementAnnotatedForThisCheckerOrUpstreamChecker(annotationScope); + return !atypeFactory + .getChecker() + .isElementAnnotatedForThisCheckerOrUpstreamChecker(annotationScope); } return false; } From 839542d2bbbe373e9e7f10c063c7d4e76c0ce050 Mon Sep 17 00:00:00 2001 From: Alex Cook Date: Tue, 28 Apr 2026 13:27:03 -0400 Subject: [PATCH 2/2] chore: spotless --- .../checkerframework/framework/type/AnnotatedTypeFactory.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeFactory.java b/framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeFactory.java index 1f271bff22bf..6eccc26c0d7d 100644 --- a/framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeFactory.java +++ b/framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeFactory.java @@ -4098,8 +4098,8 @@ protected void parseAnnotationFiles() { } /** - * 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. + * 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 */