Skip to content

Guard BadImport's type-annotation move against unpositioned trees - #6083

Open
adityaanikam wants to merge 1 commit into
google:masterfrom
adityaanikam:fix-badimport-typeuse-record-6074
Open

Guard BadImport's type-annotation move against unpositioned trees#6083
adityaanikam wants to merge 1 commit into
google:masterfrom
adityaanikam:fix-badimport-typeuse-record-6074

Conversation

@adityaanikam

Copy link
Copy Markdown

Fixes #6074

Problem

BadImport.moveTypeAnnotations() unconditionally calls builder.delete(annotation) for every TYPE_USE-targeted annotation on a qualified type, then reinserts it before the qualified identifier. That delete requires a valid start/end source position for the annotation tree.

A record component's type-use annotation can be reached through javac's synthesized members (the desugared field/accessor/canonical constructor), whose copy of the annotation tree has no recorded end position. Building a Replacement from that position throws SourcePositionException (IllegalArgumentException before 2.48.0), aborting the whole compilation. BadImport is on by default, so this needs no special configuration to hit -- any record component whose type is an imported nested class, annotated with something carrying TYPE_USE in its @Target (e.g. Bean Validation's @NotNull), aborts the build.

Fix

Skip the move for annotations without an explicit source position, using ASTHelpers.hasExplicitSource() -- the same helper this codebase already uses elsewhere for exactly this check. The identifier is still qualified by the prefixWith() call in the caller; only the annotation relocation is skipped, so the fix stays useful instead of being dropped entirely.

Testing

Added BadImportTest#recordComponentWithTypeUseAnnotation_doesNotCrash, reproducing the record-component + TYPE_USE-annotation shape from the issue, adjacent to the existing nestedTypeUseAnnotation test this mirrors.

Honest caveat on local verification: I could not get a local Windows build past error_prone_check_api -- ErrorProneSignatureGenerator.java (a file this PR does not touch) fails with an enclosing instance that contains Types.SignatureGenerator is required when maven-compiler-plugin forks a javac subprocess, on both the exact JDK 25 GA build (25+36) and a later 25.0.4 patch, across two vendors (Temurin, Zulu). This reproduces on unmodified master too, so it's unrelated to this change -- and upstream CI is green on the commit I branched from (9fefe418), so it appears to be a Windows-specific forked-javac quirk on my machine rather than a real break. I was not able to run the new test locally as a result. The expected output in the added test is reasoned from the existing nestedTypeUseAnnotation test's behavior, not independently confirmed by a passing local run -- CI will be the first real signal on it, and I'll fix it promptly if it's wrong.

BadImport.moveTypeAnnotations() unconditionally does
builder.delete(annotation) for every TYPE_USE-targeted annotation on
the qualified type, then reinserts it before the qualified type. That
delete requires a valid start/end source position for the annotation
tree.

A record component's type-use annotation can be reached through
javac's synthesized members (the desugared field/accessor/canonical
constructor), whose copy of the annotation tree has no recorded end
position. Building a Replacement from that position throws
SourcePositionException (IllegalArgumentException before 2.48.0),
aborting the whole compilation -- BadImport is on by default, so no
special configuration is needed to hit it.

Skip the move for annotations without an explicit source position,
using ASTHelpers.hasExplicitSource() (the same helper already used
elsewhere in the codebase for this exact check). The identifier is
still qualified by the prefixWith() call in the caller; only the
annotation relocation is skipped, so the fix stays useful instead of
being dropped entirely.

Fixes google#6074
@google-cla

google-cla Bot commented Aug 31, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@adityaanikam

Copy link
Copy Markdown
Author

@googlebot I signed it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BadImport: SourcePositionException building the suggested fix for a record component with a TYPE_USE annotation

1 participant