Conversation
`AnnotatedTypeFactory#getDeclAnnotations` already anticipated a `CompletionFailure` from reading an element's annotations, but the `try` block wrapped `results.add(annotation)` rather than the call that actually throws, `elements.getAllAnnotationMirrors(elt)`. That method walks the superclass chain to collect `@Inherited` annotations and asks each superclass `isErroneous()`, which recurses into the superclass's type arguments and completes them. So reading the annotations of a classpath class whose supertype has a type argument that is not on the classpath -- for example `TableRowJsonCoder extends AtomicCoder<TableRow>` in Apache Beam, compiled without `TableRow` -- throws. javac itself never completes such a symbol and reports nothing, so the crash appeared only under the Checker Framework, as one of four unrelated-looking messages depending on where the exception surfaced. Fall back to the annotations written on the element itself, losing only inherited ones, and warn so that the user knows the classpath is incomplete. The result is cached in `cacheDeclAnnos`, so the warning is issued once per element rather than once per call. This is half of issue typetools#8055: `checker/jtreg/missingclassfile` also exercises shapes that go on to crash in `AnnotatedTypeMirror.createType`, which rejects the `ERROR` type that javac leaves behind. That is a separate fix, so the jtreg test stays `@ignore`d for now. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XU4K6ckax3NwbgG9LTFEcX
When javac fails to complete a class -- because its class file is not on the classpath -- `ClassSymbol.complete` does "quiet error recovery": it sets the symbol's kind to ERR before rethrowing. From then on every `ClassType` naming that symbol reports `TypeKind.ERROR`, because `ClassType.getKind` consults the symbol rather than itself. `AnnotatedTypeMirror.createType` rejected such a type with "input is not compilable". That assertion no longer earns its keep. The Checker Framework does not run on a compilation unit that javac rejected, so an ERROR type reaching `createType` is the classpath-gap case rather than uncompilable source. `BoundsInitializer` already has a `visitError` that hands the type to `createType`, which until now could only throw. Treat an ERROR type as the declared type it is. With the previous commit this completes issue typetools#8055, so remove the test's `@ignore`. Its header comment is rewritten to describe both bugs in the past tense and to state the expected outcome; the reference to `ElementUtils#isElementFromByteCode` is corrected to `getSuperClass`, which is where the `CompletionFailure` catch actually is; and the prose that sat between `@run main DeleteMissingClassFile` and the next tag moves out of the tag block, because jtreg was passing it to `main` as command-line arguments. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XU4K6ckax3NwbgG9LTFEcX
`AnnotatedTypeMirror.createType` wrapped an ERROR type in an `AnnotatedDeclaredType` whose underlying type was the error type itself. Such a type names a symbol whose class file is not on the classpath, so its supertypes, members, and type arguments cannot be read, and any later query about it can fail. Build the `AnnotatedDeclaredType` from `java.lang.Object` instead, so that the rest of the framework has a type it can answer questions about. Verified with `taintingtest` (which includes `framework/tests/all-systems`) and `nullnesstest`, and by replaying `checker/jtreg/missingclassfile` by hand -- jtreg is not installed on this machine -- where all five compilations succeed with only the expected `supertype.not.completed` warning. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XU4K6ckax3NwbgG9LTFEcX
Three fixes to the CompletionFailure handler in getDeclAnnotations. Retry getAllAnnotationMirrors instead of falling back to elt.getAnnotationMirrors(). A completer runs at most once -- javac installs NULL_COMPLETER before running it -- so the second call does not throw, and it returns the annotations inherited from every superclass that could be read. The fallback returned only the annotations written directly on elt. This is belt and braces: for a type declaration, inheritOverriddenDeclAnnosFromTypeDecl already re-adds the @inherited annotations of every direct supertype, and for any other element kind javac's getAllAnnotationMirrors returns exactly elt.getAnnotationMirrors(), so neither path could observe the difference. The retry does cover the isParsing early return, which skips inheritOverriddenDeclAnnosFromTypeDecl. Issue the warning after fromEle has been established, and swallow a nested CompletionFailure. Reporting a warning resolves @SuppressWarnings and @AnnotatedFor on elt and on every enclosing element, which is the very thing that raises CompletionFailure in issues 309 and 348; on a classpath with more than one gap, handling one missing class could throw a fresh CompletionFailure out of getDeclAnnotations. The fallback prints through checker.message, because SourceChecker.reportUnsuppressed rejects a null source despite its javadoc. Rename the message key supertype.not.completed to class.not.completed. The catch also covers a failure to complete elt itself -- getAllAnnotationMirrors starts with sym.getAnnotationMirrors(), which completes elt -- so a message blaming "a supertype" could name the wrong class and send the user looking for a supertype that is fine. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XU4K6ckax3NwbgG9LTFEcX
Commit 14017cf built the AnnotatedDeclaredType for an ERROR type out of java.lang.Object, on the theory that a symbol whose class file is absent cannot answer questions about itself. No failure motivated it, and it broke two things. createType's documented postcondition -- "an AnnotatedTypeMirror whose underlying type is type" -- no longer held, so code that zips the underlying type's type arguments against the ATM's positionally paired unrelated types. For lib.SuperTypeArg extends Box<Missing>, createType(Box<Missing>) returned an ATM whose getUnderlyingType().getTypeArguments().get(0) was lib.Missing but whose getTypeArguments().get(0).getUnderlyingType() was java.lang.Object; SupertypeFinder.supertypesFromElement then handed the erroneous Missing symbol to addComputedTypeAnnotations together with an Object ATM. The Object type also has no type parameters, so BoundsInitializer.InitializerVisitor.visitDeclared built an ATM from Object and then set a non-empty type-argument list onto it. That path is reachable without visitError: a javac ClassType whose tsym.kind is ERR reports TypeKind.ERROR but still dispatches to visitDeclared. Fold the ERROR arm into the DECLARED arm instead. The cast is sound: javax.lang.model.type.ErrorType extends DeclaredType, javac's Type$ErrorType extends ClassType, and in this case the object is a ClassType already. Guard the one cast that the old BugInCF used to protect. getTypeArguments' declaration branch casts createType's result to AnnotatedTypeVariable, which now yields a bare ClassCastException rather than a message naming the type; throw a BugInCF that names the type argument and its kind. The other casts that the review flagged, in BoundsInitializer.visitTypeVariable and visitWildcard, cannot see an ERROR type: visitor dispatch is by TypeKind, so their argument always matches. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XU4K6ckax3NwbgG9LTFEcX
A bare @compile checks only the exit status, so the test passed whether the warning was issued once, five times, or not at all. Give every compilation a .goal file, following the convention of checker/jtreg/SymbolNotFoundErrors.java. -XDrawDiagnostics is required, not cosmetic: without it a diagnostic that has a source position prints the absolute path that jtreg passed to javac, which would make the golden files machine-specific. Add InheritedAnno.java, the one compilation that is expected to fail, together with lib.QualParam and lib.SubOfQualParam. Its error is the assertion: SubOfQualParam inherits @HasQualifierParameter from QualParam, whose own supertype is the one that cannot be read, and that annotation is what makes the assignment an error. Without it the file compiles cleanly. The review also asked the test to detect that SuperTypeArg's supertype had silently become Box<Object>. That is not observable from source: any expression that surfaces the type argument makes javac itself need the class file and report "error: cannot access Missing" with no processor involved. The property that makes the bug possible is the same one that hides the substitution. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XU4K6ckax3NwbgG9LTFEcX
Record the new class.not.completed warning, the change to AnnotatedTypeMirror.createType's treatment of an ERROR type, and the closed issue. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XU4K6ckax3NwbgG9LTFEcX
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughThe framework now reports a Suggested reviewers: Priority: ➖ Normal Change: Bug fix Merge Risk: ⚪ Minimal · up to The change handles the reported missing-class-file path and adds targeted regression coverage without an unresolved merge-blocking risk. 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
The comment justified the retry with "a completer runs at most once, so the second call does not throw". That is true of a single symbol -- Symbol#complete installs NULL_COMPLETER before running the completer -- but getAllAnnotationMirrors walks the whole superclass chain, and the first call aborts at the first symbol that fails. On its own, that reason leaves the retry free to walk further than the first call did and to throw on a symbol it never reached, which is why the try/catch around the second call is not redundant with the claim above it. What actually stops the retry in the same place is ClassSymbol#complete's quiet error recovery: it sets the symbol's type to an ErrorType before rethrowing, so the walk's sup.isErroneous() check breaks out exactly where the first walk threw. Verified on JDK 21 with a probe annotation processor that calls getAllAnnotationMirrors three times on one element, over a class directory with class files deleted as in checker/jtreg/missingclassfile: a missing supertype type argument, a missing annotation type, missing Class- and enum-valued annotation elements, and combinations of those at two depths in the chain. Every shape throws on the first call only. The shape that would defeat the retry -- a failure completing an annotation type, which would not make the supertype erroneous and so would let the walk continue -- does not arise, because ClassReader tolerates an absent annotation class file silently. Keep the inner catch: it is three lines, its unreachability rests on every completer preserving ClassSymbol#complete's error recovery, and a CompletionFailure escaping getDeclAnnotations is the issue 8055 crash. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VDgr6aZpKDFnqw43VfHbLL
mernst
requested changes
Sep 10, 2026
| ### User-visible changes | ||
|
|
||
| New warning `class.not.completed` is issued when a class file that is needed to | ||
| compute an element's declaration annotations cannot be read. Previously, the |
Member
There was a problem hiding this comment.
There is no need for the second sentence. Let's keep the changelog concise, so that people will actually read it.
|
|
||
| Renamed `AnnotatedTypes.innerMostType()` to `innermostComponentType()`. | ||
|
|
||
| `AnnotatedTypeMirror.createType()` no longer throws `BugInCF` for a type whose |
Member
There was a problem hiding this comment.
Ordinary bug fixes don't go in the changelog.
|
|
||
| ### Closed issues | ||
|
|
||
| \#8055. |
Member
There was a problem hiding this comment.
Closed issues are filled in later. When they appear in pull requests, they lead to a lot of unnecessary conflicts.
| // enclosing elements, which can fail to read a class file too. | ||
| checker.message( | ||
| Diagnostic.Kind.WARNING, | ||
| "Cannot read a class file that is needed to compute the declaration annotations of %s:" |
Member
There was a problem hiding this comment.
This seems to duplicate text in messages.properties.
Member
Author
There was a problem hiding this comment.
It does, but the comment above explains that it can't use the normal reporting method that will use messages.properties.
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.
Fixes #8055.