JavaStubifier: improve unloadable-annotation failure DX#1878
Merged
Conversation
…pt-in skip Stubifying the JSpecify reference checker's annotated JDK hit a stray source file with JUnit annotations, which aborted the whole run with a message naming the annotation but not the source file being processed; finding it required grepping a 14k-file tree. MinimizerCallback.process is the layer that reliably knows the file (it is a SourceRoot.Callback parameter, unlike BinaryStubWriter, which operates on a bare CompilationUnit and is also called directly by tests with no file behind it), so it now wraps any RuntimeException that escapes BinaryStubWriter.process(cu) with that path, covering every "Serialization failure" message the writer can produce, not just the annotation-loading one. Also add a `--skipUnloadableAnnotations` command-line flag. When set, BinaryStubWriter drops an annotation whose @target it cannot read from the binary stub output instead of aborting, printing one warning line per annotation-and-file pair to stderr. This mirrors the existing isUnloadablePlatformAnnotation drop-and-continue path, but is opt-in and warned because -- unlike a platform annotation missing from the stubifier's own narrow JVM -- the annotation could be a real qualifier a checker's wider classpath would have resolved, so dropping it can silently change the annotated JDK's meaning. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
PR #1878's CI failed on every JDK except 21 (cftests-junit, cftests-junit-jdk21, cftests-nonjunit all red on 8/11/17/25/26/27). Reproduced with ORG_GRADLE_PROJECT_useJdkVersion=8: compileTestJava fails because PrintStream(OutputStream, boolean, Charset) and ByteArrayOutputStream.toString(Charset) were only added in Java 10, and this project's compileTestJava still targets Java 8 (sourceCompatibility/targetCompatibility = VERSION_1_8), for which those symbols do not exist on the real JDK 8 boot classpath -- unlike "--release 8" restrictions, a genuinely older JDK's classpath simply lacks them. useJdkVersion=11/17/25/26/27 apparently also fail in CI for the same reason, even though those JDKs do carry the Charset overloads; useJdkVersion=21 is the only local run that reused the host JVM directly (no toolchain), which is likely why it alone was unaffected. Switch to the String-charset-name overloads instead, matching the existing convention in CheckerMain.outputArgumentsToFile and CheckerMain's jar-path decoding: @SuppressWarnings("JdkObsolete") on the declaration, with an explicit Charset#name() argument so -Xep:DefaultCharset:ERROR (enabled for useJdkVersion 21-26) has no complaint either. Verified locally with ORG_GRADLE_PROJECT_useJdkVersion=8 (previously failing to compile) and =21 (must stay clean of errorprone) via ":framework:test --tests org.checkerframework.framework.stubifier.*". Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
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.
While stubifying the JSpecify reference checker's annotated JDK, one stray source file with a JUnit annotation aborted the whole run with
cannot load annotation org.junit.runners.Parameterized.Parameters ...— naming the annotation but not the file, which had to be found by grepping a 14k-file tree. And there was no way to proceed past it.Two changes:
File context in the failure.
JavaStubifier.MinimizerCallback.processnow wrapsbinaryStubWriter.process(cu)and rethrows with the absolute file path in the message. That layer is the one place that reliably knows the file (it's theSourceRoot.Callbackparameter), and the single wrap covers every "Serialization failure" the writer can produce —BinaryStubWriter.process(CompilationUnit)is also called directly from tests with no file behind it, so the context does not belong there.Opt-in
--skipUnloadableAnnotationsflag. When passed toJavaStubifier, an annotation whose@Targetcannot be read is omitted from the binary stub output — one deduped warning line to stderr naming annotation + file — and processing continues, mirroring the existingisUnloadablePlatformAnnotationdrop-and-continue path inrouteAnnotations. Default behavior (flag absent) is unchanged: hard fail, now with the file named. The flag's Javadoc spells out the trade-off: dropping a real qualifier silently changes checker behavior, so this is for known-irrelevant annotations (like a stray test file), not a default way to push through classpath problems.Tests: two new cases in the existing
JavaStubifierTestharness — default run fails with annotation and file path in the message; flagged run succeeds, warns, and the stub verifiably omits the annotation. Full:framework:test,assemble, andjavadocDoclintAllpass on JDK 21. Real-world check: the reference checker'sincludeJSpecifyJDKstubification run against this branch succeeds.🤖 Generated with Claude Code
https://claude.ai/code/session_01EaHhVqLoJukm4kKsE7UfY4