getBundledTypeQualifiers() doesn't include aliased type annotations#7504
Merged
smillst merged 13 commits intotypetools:masterfrom Feb 26, 2026
Merged
getBundledTypeQualifiers() doesn't include aliased type annotations#7504smillst merged 13 commits intotypetools:masterfrom
getBundledTypeQualifiers() doesn't include aliased type annotations#7504smillst merged 13 commits intotypetools:masterfrom
Conversation
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Contributor
📝 WalkthroughWalkthroughThis PR introduces a public API method to check if an annotation class is aliased and modifies the type annotation loading process to exclude aliased annotations from bundled qualifiers before adding explicitly listed ones. Documentation is updated to clarify timing relationships around alias handling in the initialization sequence. Possibly related PRs
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeFactory.java`:
- Around line 1183-1185: Update the JavaDoc for the loader method in
AnnotatedTypeFactory to explicitly state that alias annotations are filtered out
and therefore the returned collection contains only canonical (non-alias)
qualifiers; mention that bundled qualifiers plus explicitly listed annotations
are considered but aliases encountered on the loader path are removed, and note
where this behavior occurs (the loader/filtering step in the method that loads
bundled qualifiers) so checker authors know the returned set will not include
alias annotations.
- Around line 1201-1207: In checkSupportedQualsAreTypeQuals(), avoid calling
getAnnotation(Target.class).value() without verifying the annotation exists:
first obtain Target targetAnno = annotationClass.getAnnotation(Target.class) and
only if targetAnno != null read targetAnno.value() and proceed with the
ElementType checks; leave the existing behavior from
AnnotationClassLoader#isTypeQualifierAnnotation (which allows qualifiers without
`@Target`, e.g., via `@SubtypeOf` or `@InvisibleQualifier`) intact so the method skips
the ElementType checks for annotations lacking `@Target` instead of throwing an
NPE.
- Around line 1224-1229: The alias filter
(annotations.removeIf(this::isAliasedTypeAnnotation)) misses class-based aliases
because addAliasedTypeAnnotation(Class,...) calls getSupportedTypeQualifiers()
which initializes supportedQuals before the alias is registered; update
addAliasedTypeAnnotation to register the alias into aliases first (or
invalidate/prune the supportedQuals cache after adding an alias) so that
subsequent calls to isAliasedTypeAnnotation and removeIf see the new alias;
alternatively defer computing supportedQuals until after all
addAliasedTypeAnnotation calls by removing eager initialization in
getSupportedTypeQualifiers or by clearing/refreshing supportedQuals when aliases
are added.
In
`@framework/src/main/java/org/checkerframework/framework/type/AnnotationClassLoader.java`:
- Around line 818-849: Update the class-level Javadoc on AnnotationClassLoader
to reflect the expanded qualifier criteria implemented in
isTypeQualifierAnnotation: document that a type qualifier is accepted not only
when it has a `@Target` including ElementType.TYPE_USE (and optionally
TYPE_PARAMETER) but also when the annotation is annotated with
`@InvisibleQualifier` or `@SubtypeOf` even if it lacks a `@Target`(TYPE_USE); mention
these specific meta-annotations (InvisibleQualifier, SubtypeOf) and reference
that this behavior is enforced by the isTypeQualifierAnnotation method and
AnnotationUtils.hasTypeQualifierElementTypes.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
docs/CHANGELOG.mdframework/src/main/java/org/checkerframework/common/subtyping/SubtypingAnnotationClassLoader.javaframework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeFactory.javaframework/src/main/java/org/checkerframework/framework/type/AnnotationClassLoader.java
💤 Files with no reviewable changes (1)
- framework/src/main/java/org/checkerframework/common/subtyping/SubtypingAnnotationClassLoader.java
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/CHANGELOG.md`:
- Around line 14-19: Add a changelog bullet documenting that
AnnotationClassLoader's bundled/external qualifier discovery is now alias-aware:
update the CHANGELOG to mention that getBundledTypeQualifiers() (and any related
external discovery such as getExternalTypeQualifiers()) was fixed to recognize
aliased/aliased type annotations (bundled qualifiers) using the new
isTypeQualifierAnnotation() logic so aliased type annotations and bundled
qualifiers are discovered correctly; keep the note concise and consistent with
the existing entry about the method rename.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeFactory.java (2)
1201-1229:⚠️ Potential issue | 🟠 MajorAlias filtering can miss class-based aliases due to init order.
loadTypeAnnotationsFromQualDirassumes aliases are registered before it runs, butaddAliasedTypeAnnotation(Class, …)callsgetSupportedTypeQualifiers()and can initializesupportedQualsbefore the alias is registered. That makesremoveIf(this::isAliasedTypeAnnotation)ineffective for bundled aliases and can still trigger the “alias should not be in type hierarchy” exception. Consider deferring the supported-quals check or invalidating caches when an alias is added.💡 Possible fix (avoid premature init)
protected void addAliasedTypeAnnotation(Class<?> aliasClass, AnnotationMirror canonicalAnno) { - if (getSupportedTypeQualifiers().contains(aliasClass)) { + if (supportedQuals != null && supportedQuals.contains(aliasClass)) { throw new BugInCF( "AnnotatedTypeFactory: alias %s should not be in type hierarchy for %s", aliasClass, this.getClass().getSimpleName()); } addAliasedTypeAnnotation(aliasClass.getCanonicalName(), canonicalAnno); + if (supportedQuals != null) { + supportedQuals.remove(aliasClass); + supportedQualNames = null; // recompute on next access + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeFactory.java` around lines 1201 - 1229, The bundled alias removal in loadTypeAnnotationsFromQualDir can miss class-based aliases because addAliasedTypeAnnotation may initialize supportedQuals via getSupportedTypeQualifiers before the alias is registered; to fix this, ensure supportedQuals is recomputed or invalidated when an alias is added: modify addAliasedTypeAnnotation to clear or mark supportedQuals/any cached qualifier state so subsequent calls to getSupportedTypeQualifiers/isAliasedTypeAnnotation see the new alias (or defer initializing supportedQuals until after postInit), and update loadTypeAnnotationsFromQualDir to rely on the refreshed cache so removeIf(this::isAliasedTypeAnnotation) reliably removes bundled aliases.
1183-1185:⚠️ Potential issue | 🟡 MinorDocument alias filtering in bundled-qualifier Javadoc.
The description still implies all bundled + explicit annotations are returned, but aliases are now filtered out in the loader path. Please clarify that aliases are excluded.
✍️ Suggested doc tweak
- * Loads, via reflection, all bundled qualifiers. A bundled qualifier is one that is shipped with - * the Checker Framework, in the qual directory of a checker. Returns the bundled annotations and - * the annotations passed as arguments. + * Loads, via reflection, all bundled qualifiers. A bundled qualifier is one that is shipped with + * the Checker Framework, in the qual directory of a checker. Returns the bundled annotations and + * the annotations passed as arguments, excluding any annotations registered as aliases via + * addAliasedTypeAnnotation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeFactory.java` around lines 1183 - 1185, Update the Javadoc on AnnotatedTypeFactory for the method that "Loads, via reflection, all bundled qualifiers" to explicitly state that alias annotations are excluded from the returned set; change the description from implying "all bundled + explicit annotations are returned" to something like "returns bundled annotations and the annotations passed as arguments, with any alias annotations filtered out (aliases are excluded from the loader path/returned set)." Mention AnnotatedTypeFactory and the bundled-qualifier loader in the comment so readers know aliases are intentionally omitted.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeFactory.java`:
- Around line 1201-1229: The bundled alias removal in
loadTypeAnnotationsFromQualDir can miss class-based aliases because
addAliasedTypeAnnotation may initialize supportedQuals via
getSupportedTypeQualifiers before the alias is registered; to fix this, ensure
supportedQuals is recomputed or invalidated when an alias is added: modify
addAliasedTypeAnnotation to clear or mark supportedQuals/any cached qualifier
state so subsequent calls to getSupportedTypeQualifiers/isAliasedTypeAnnotation
see the new alias (or defer initializing supportedQuals until after postInit),
and update loadTypeAnnotationsFromQualDir to rely on the refreshed cache so
removeIf(this::isAliasedTypeAnnotation) reliably removes bundled aliases.
- Around line 1183-1185: Update the Javadoc on AnnotatedTypeFactory for the
method that "Loads, via reflection, all bundled qualifiers" to explicitly state
that alias annotations are excluded from the returned set; change the
description from implying "all bundled + explicit annotations are returned" to
something like "returns bundled annotations and the annotations passed as
arguments, with any alias annotations filtered out (aliases are excluded from
the loader path/returned set)." Mention AnnotatedTypeFactory and the
bundled-qualifier loader in the comment so readers know aliases are
intentionally omitted.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeFactory.java
smillst
previously approved these changes
Feb 25, 2026
smillst
approved these changes
Feb 25, 2026
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeFactory.java (1)
1229-1231:⚠️ Potential issue | 🟠 MajorAlias filtering still relies on an invalid initialization assumption.
Line 1229’s assumption is contradicted by Line 3419 and Line 3479: both class-based alias registration paths call
getSupportedTypeQualifiers()before inserting the alias, which can initializesupportedQualstoo early and let bundled aliases survive the Line 1231 filter.💡 Suggested fix
@@ - * <p>This method must be called before {`@link` `#postInit`}. + * <p>Prefer calling this method before {`@link` `#getSupportedTypeQualifiers`()} is first computed. @@ protected void addAliasedTypeAnnotation(Class<?> aliasClass, AnnotationMirror canonicalAnno) { - if (getSupportedTypeQualifiers().contains(aliasClass)) { + if (supportedQuals != null && supportedQuals.contains(aliasClass)) { throw new BugInCF( "AnnotatedTypeFactory: alias %s should not be in type hierarchy for %s", aliasClass, this.getClass().getSimpleName()); } addAliasedTypeAnnotation(aliasClass.getCanonicalName(), canonicalAnno); + if (supportedQuals != null) { + supportedQuals.remove(aliasClass); + supportedQualNames = null; + } } @@ protected void addAliasedTypeAnnotation( Class<?> aliasClass, Class<?> canonicalClass, boolean copyElements, String... ignorableElements) { - if (getSupportedTypeQualifiers().contains(aliasClass)) { + if (supportedQuals != null && supportedQuals.contains(aliasClass)) { throw new BugInCF( "AnnotatedTypeFactory: alias %s should not be in type hierarchy for %s", aliasClass, this.getClass().getSimpleName()); } addAliasedTypeAnnotation( aliasClass.getCanonicalName(), canonicalClass, copyElements, ignorableElements); + if (supportedQuals != null) { + supportedQuals.remove(aliasClass); + supportedQualNames = null; + } }Also applies to: 3413-3414, 3418-3425, 3479-3486
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeFactory.java` around lines 1229 - 1231, The alias-filtering relies on supportedQuals being uninitialized, but addAliasedTypeAnnotation and two alias registration paths call getSupportedTypeQualifiers() early and can initialize supportedQuals before the filter runs; to fix, ensure alias filtering does not depend on supportedQuals being uninitialized by changing loadTypeAnnotationsFromQualDir's removal logic to use the explicit alias registry (or a direct check) instead of isAliasedTypeAnnotation that reads supportedQuals, or defer/avoid initializing supportedQuals inside getSupportedTypeQualifiers so aliases are registered before supportedQuals is populated; update addAliasedTypeAnnotation, getSupportedTypeQualifiers, isAliasedTypeAnnotation and the removeIf call so alias checks consult the authoritative alias store (not supportedQuals initialization state) and bundled aliases are correctly filtered.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeFactory.java`:
- Around line 1229-1231: The alias-filtering relies on supportedQuals being
uninitialized, but addAliasedTypeAnnotation and two alias registration paths
call getSupportedTypeQualifiers() early and can initialize supportedQuals before
the filter runs; to fix, ensure alias filtering does not depend on
supportedQuals being uninitialized by changing loadTypeAnnotationsFromQualDir's
removal logic to use the explicit alias registry (or a direct check) instead of
isAliasedTypeAnnotation that reads supportedQuals, or defer/avoid initializing
supportedQuals inside getSupportedTypeQualifiers so aliases are registered
before supportedQuals is populated; update addAliasedTypeAnnotation,
getSupportedTypeQualifiers, isAliasedTypeAnnotation and the removeIf call so
alias checks consult the authoritative alias store (not supportedQuals
initialization state) and bundled aliases are correctly filtered.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeFactory.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Merge after #7495.