Introduce @NullMarked aliasing#1304
Conversation
wmdietl
left a comment
There was a problem hiding this comment.
Still a draft, but some quick comments.
| } | ||
|
|
||
| /** | ||
| * This method is almost similar to {@link |
There was a problem hiding this comment.
It would be nice to find a solution without this duplication.
There was a problem hiding this comment.
With code refactoring in #1331, we can get rid of this code duplication.
EDIT: tested at aosen-xiong#13.
Co-authored-by: Werner Dietl <wdietl@gmail.com>
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds JSpecify @NullMarked support by treating it as opting code into the Nullness type system (equivalent to @AnnotatedFor("nullness")) and refactors @AnnotatedFor scope checks into AnnotatedTypeFactory with caching.
Changes:
- Alias JSpecify
@NullMarkedto both the Nullness default qualifier behavior and@AnnotatedFor("nullness"). - Centralize and cache “is this element annotated for this checker (or upstream checker)?” logic in
AnnotatedTypeFactory. - Update conservative-defaults and suppression logic to use the new centralized API; add/adjust tests and changelog entry.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| framework/src/main/java/org/checkerframework/framework/util/defaults/QualifierDefaults.java | Switches conservative-default decision to use AnnotatedTypeFactory’s new @AnnotatedFor query. |
| framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeFactory.java | Introduces cached element-level @AnnotatedFor resolution and renames checker-matching helper. |
| framework/src/main/java/org/checkerframework/framework/stub/AnnotationFileParser.java | Updates stub parsing to use renamed @AnnotatedFor applicability method. |
| framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java | Replaces local @AnnotatedFor logic with an abstract hook implemented by subclasses. |
| framework/src/main/java/org/checkerframework/framework/source/AggregateChecker.java | Implements new abstract SourceChecker hook (returns false). |
| framework/src/main/java/org/checkerframework/common/util/debug/SignaturePrinter.java | Implements new abstract SourceChecker hook for the debug checker (returns false). |
| framework/src/main/java/org/checkerframework/common/util/count/JavaCodeStatistics.java | Implements new abstract SourceChecker hook (returns false). |
| framework/src/main/java/org/checkerframework/common/util/count/AnnotationStatistics.java | Implements new abstract SourceChecker hook (returns false). |
| framework/src/main/java/org/checkerframework/common/basetype/BaseTypeChecker.java | Implements new abstract hook by delegating to the type factory. |
| docs/CHANGELOG.md | Documents @NullMarked support as an alias for nullness opt-in. |
| checker/tests/nullness-nullmarked/NullMarkedSuppressError.java | Adds regression test for onlyAnnotatedFor behavior with @NullMarked. |
| checker/src/test/java/org/checkerframework/checker/test/junit/NullnessNullMarkedTest.java | Runs null-marked tests with -AonlyAnnotatedFor. |
| checker/src/main/java/org/checkerframework/checker/nullness/NullnessNoInitAnnotatedTypeFactory.java | Adds alias mapping from @NullMarked to @AnnotatedFor("nullness"). |
Comments suppressed due to low confidence (1)
docs/CHANGELOG.md:1
- The release date contains a placeholder (
May ?, 2026). Replace?with a concrete date or remove the date entirely until known, to avoid publishing ambiguous release metadata.
Version 3.49.5-eisop2 (May ?, 2026)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| throw new BugInCF( | ||
| "Call of QualifierDefaults.isElementAnnotatedForThisChecker with null"); |
| /** Mapping from an Element to the source Tree of the declaration. */ | ||
| private final Map<Element, Tree> elementToTreeCache; | ||
|
|
||
| /** A mapping of Element → Whether or not that element is AnnotatedFor this type system. */ |
| 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; |
Fixes #1299.
First merge #1331.