google / google/error-prone

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

Open
#6,074 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
7.2k
Forks
820
Avg merge
5h 9m
Merged PRs (30d)
50

Description

### What version of Error Prone are you using?

2.50.0

### Does this issue reproduce with the latest release?

Yes. 2.50.0 is the latest release at time of writing, and the crash is present in every release
back to 2.36.0 — see the version table below. It is not a regression.

### What did you do?

`BadImport` crashes with `SourcePositionException` (aborting the whole compilation) when the
bad-named nested class it wants to qualify appears as the type of a **record component that also
carries an annotation whose `@Target` includes `ElementType.TYPE_USE`**.

This reproduces with **default Error Prone settings** — `BadImport` is on by default, so no
configuration is needed to hit it.

Three files, no dependencies beyond Error Prone itself:

`repro/model/Outer.java`

```java
package repro.model;

public class Outer {
public enum Key {
A
}
}
```

`repro/TypeUse.java`

```java
package repro;

import java.lang.annotation.ElementType;
import java.lang.annotation.Target;

@Target(ElementType.TYPE_USE)
public @interface TypeUse {}
```

`repro/Repro.java`

```java
package repro;

import repro.model.Outer.Key;

public record Repro(@TypeUse Key key) {}
```

Compile (JDK 25; `$EP_CP` is the `error_prone_core:2.50.0` runtime classpath):

```sh
javac \
-J--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED \
-J--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED \
-J--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED \
-J--add-exports=jdk.compiler/com.sun.tools.javac.model=ALL-UNNAMED \
-J--add-exports=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED \
-J--add-exports=jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED \
-J--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED \
-J--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED \
-J--add-opens=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED \
-J--add-opens=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED \
-XDcompilePolicy=simple \
--should-stop=ifError=FLOW \
-processorpath "$EP_CP" \
'-Xplugin:ErrorProne' \
-d out \
repro/model/Outer.java repro/TypeUse.java repro/Repro.java
```

### What did you expect to see?

The ordinary `BadImport` finding with a working suggested fix, exactly as produced when the
annotation is removed:

```
Repro.java:5: warning: [BadImport] Importing nested classes/static methods/static fields with
commonly-used names can make code harder to read [...] Here we recommend using qualified class:
Outer.
public record Repro(Key key) {}
^
(see https://errorprone.info/bugpattern/BadImport)
Did you mean 'public record Repro(Outer.Key key) {}'?
```

### What did you see instead?

```
Repro.java:5: error: An unhandled exception was thrown by the Error Prone static analysis plugin.
public record Repro(@TypeUse Key key) {}
^
error-prone version: 2.50.0
BugPattern: BadImport
Stack Trace:
com.google.errorprone.SourcePositionException: invalid source position: [67, -1)
at com.google.errorprone.fixes.Replacement.create(Replacement.java:39)
at com.google.errorprone.fixes.SuggestedFix$ReplacementFix.getReplacement(SuggestedFix.java:477)
at com.google.errorprone.fixes.SuggestedFix.getReplacements(SuggestedFix.java:95)
at com.google.errorprone.fixes.AppliedFix.apply(AppliedFix.java:45)
at com.google.errorprone.JavacErrorDescriptionListener.lambda$new$0(JavacErrorDescriptionListener.java:94)
at java.base/java.util.HashMap.computeIfAbsent(HashMap.java:1229)
at com.google.errorprone.JavacErrorDescriptionListener.lambda$new$1(JavacErrorDescriptionListener.java:90)
at com.google.errorprone.JavacErrorDescriptionListener.lambda$onDescribed$3(JavacErrorDescriptionListener.java:109)
at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:214)
at java.base/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:197)
at java.base/java.util.Collections$2.tryAdvance(Collections.java:5182)
at java.base/java.util.Collections$2.forEachRemaining(Collections.java:5190)
at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:570)
at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:560)
at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:921)
at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:265)
at java.base/java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:723)
at com.google.errorprone.JavacErrorDescriptionListener.onDescribed(JavacErrorDescriptionListener.java:111)
at com.google.errorprone.ErrorProneAnalyzer.lambda$finished$3(ErrorProneAnalyzer.java:216)
at com.google.errorprone.VisitorState.reportMatch(VisitorState.java:248)
at com.google.errorprone.scanner.Scanner.reportMatch(Scanner.java:126)
at com.google.errorprone.scanner.ErrorProneScanner.processMatchers(ErrorProneScanner.java:540)
at com.google.errorprone.scanner.ErrorProneScanner.visitImport(ErrorProneScanner.java:804)
at com.google.errorprone.scanner.ErrorProneScanner.visitImport(ErrorProneScanner.java:180)
at jdk.compiler/com.sun.tools.javac.tree.JCTree$JCImportBase.accept(JCTree.java:678)
at jdk.compiler/com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:92)
at com.google.errorprone.scanner.Scanner.scan(Scanner.java:74)
at com.google.errorprone.scanner.Scanner.scan(Scanner.java:48)
at jdk.compiler/com.sun.source.util.TreeScanner.scan(TreeScanner.java:110)
at jdk.compiler/com.sun.source.util.TreeScanner.scanAndReduce(TreeScanner.java:118)
at jdk.compiler/com.sun.source.util.TreeScanner.visitCompilationUnit(TreeScanner.java:150)
at com.google.errorprone.scanner.ErrorProneScanner.visitCompilationUnit(ErrorProneScanner.java:663)
at com.google.errorprone.scanner.ErrorProneScanner.visitCompilationUnit(ErrorProneScanner.java:180)
at jdk.compiler/com.sun.tools.javac.tree.JCTree$JCCompilationUnit.accept(JCTree.java:627)
at jdk.compiler/com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:66)
at com.google.errorprone.scanner.Scanner.scan(Scanner.java:58)
at com.google.errorprone.scanner.ErrorProneScannerTransformer.apply(ErrorProneScannerTransformer.java:43)
at com.google.errorprone.ErrorProneAnalyzer.finished(ErrorProneAnalyzer.java:231)
at jdk.compiler/com.sun.tools.javac.api.MultiTaskListener.finished(MultiTaskListener.java:133)
at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.flow(JavaCompiler.java:1423)
at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.flow(JavaCompiler.java:1370)
at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.compile(JavaCompiler.java:955)
at jdk.compiler/com.sun.tools.javac.main.Main.compile(Main.java:319)
at jdk.compiler/com.sun.tools.javac.main.Main.compile(Main.java:178)
at jdk.compiler/com.sun.tools.javac.Main.compile(Main.java:66)
at jdk.compiler/com.sun.tools.javac.Main.main(Main.java:52)
1 error
```

### Narrowing

Only the combination *record component* + *TYPE_USE-targeted annotation* crashes. Every other
shape of the same declaration produces a correct suggested fix:

| declaration | result |
| -------------------------------------------------------------------------- | -------------------------- |
| record component + TYPE_USE-targeted annotation | **CRASH** |
| record component, no annotation | finding only, fix is clean |
| record component + FIELD / PARAMETER / METHOD / RECORD_COMPONENT-only annotation | finding only, fix is clean |
| class field / method parameter / return type / local + TYPE_USE annotation | finding only, fix is clean |

Further observations:

- Severity is irrelevant: it crashes at default severity, at `:WARN`, and at `:ERROR`.
- The generic wrapper is irrelevant: `@TypeUse Key key` and `@TypeUse Map key`
crash identically.
- The reported start offset (`67` above) is the offset of the `@` of `@TypeUse`, **not** of the
`Key` identifier at offset `76`. So the replacement region is being computed over the
`AnnotatedType` node wrapping the component's type rather than over the identifier itself, and
that node has no end position recorded (`-1`) — presumably because the record component's type
tree is reached through a synthesized member rather than the original source tree.

### Is this a regression?

**No — it is not a regression.** It reproduces on every Error Prone release I could run on JDK 25:
**2.36.0 through 2.50.0, 15 consecutive releases** spanning 2024-11-19 to 2026-06-10.

| Error Prone | result |
| ----------------- | --------------------------------------------------------------------- |
| 2.50.0 (latest) | **CRASH** — `SourcePositionException: invalid source position: [67, -1)` |
| 2.49.0 | **CRASH** — `SourcePositionException` |
| 2.48.0 | **CRASH** — `SourcePositionException` |
| 2.47.0 | **CRASH** — `IllegalArgumentException: invalid replacement: [67, -1)` |
| 2.46.0 | **CRASH** — `IllegalArgumentException` |
| 2.45.0 | **CRASH** — `IllegalArgumentException` |
| 2.44.0 | **CRASH** — `IllegalArgumentException` |
| 2.43.0 | **CRASH** — `IllegalArgumentException` |
| 2.42.0 | **CRASH** — `IllegalArgumentException` |
| 2.41.0 | **CRASH** — `IllegalArgumentException` |
| 2.40.0 | **CRASH** — `IllegalArgumentException` |
| 2.39.0 | **CRASH** — `IllegalArgumentException` |
| 2.38.0 | **CRASH** — `IllegalArgumentException` |
| 2.37.0 | **CRASH** — `IllegalArgumentException` |
| 2.36.0 | **CRASH** — `IllegalArgumentException` |
| 2.35.1 and older | not testable — Error Prone does not run on JDK 25 (`NoSuchFieldError: Class com.sun.tools.javac.code.TypeTag does not have member field 'UNKNOWN'` from `ASTHelpers.`) |

Two notes on reading that table:

- The change of exception type at 2.48.0 is cosmetic, not behavioural: both are thrown from the
same `Replacement.create` call site with the identical offsets `[67, -1)`.
`SourcePositionException` is a wrapper introduced around the pre-existing
`Preconditions.checkArgument` failure.
- The floor at 2.35.1 is a **JDK 25 incompatibility in Error Prone itself**, unrelated to this bug.
I had only JDK 25 available, so I cannot say whether the crash predates 2.36.0 — only that it has
been present in every release since.

Consequently there is no "pin to an older version" workaround; disabling `BadImport` is the only
option.

### Impact

Because an annotation with a `TYPE_USE` target is enough, this is easy to hit in ordinary code.
`jakarta.validation.constraints.@NotNull` (and the rest of Bean Validation) lists `TYPE_USE` among
its targets, so any validated record DTO whose component type is an imported nested class named
`Builder` / `Key` / `Type` / `Id` / `Factory` / … aborts the build. In our case a single such DTO
made `BadImport` unusable repository-wide, and there is no per-site suppression that helps because
the crash happens while rendering the fix for the *import* statement.

### Version history

Not a regression — it crashes in every release I tested, back to 2.30.0. Only the failure mode
changed: 2.36.0 introduced `SourcePositionException`, before that the same bad region surfaced as
`IllegalArgumentException`, and 2.35.1 and older report it as a generic unhandled plugin exception.

| Error Prone | Result | Reported as |
| ----------- | ------ | ----------- |
| 2.50.0 - 2.48.0 | CRASH | `SourcePositionException: invalid source position: [67, -1)` |
| 2.47.0 - 2.36.0 | CRASH | `IllegalArgumentException: invalid replacement: [67, -1)` |
| 2.35.1 - 2.30.0 | CRASH | unhandled exception thrown by the Error Prone plugin |

### Suggested direction

Either skip the fix (emit the finding without a `SuggestedFix`) when the end position of the
usage's enclosing tree is unavailable, or resolve the replacement region from the identifier node
rather than from its enclosing `AnnotatedType`.

Contributor guide

Open the contributing guide

Research direction

Reproduce the crash with repro/model/Outer.java, repro/TypeUse.java, and repro/Repro.java using the provided javac command, then trace BadImport through the SuggestedFix and Replacement.create entries named in the stack trace. Done means the record component with a TYPE_USE annotation produces the ordinary BadImport finding and a working qualified-name fix without aborting compilation.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
tooling
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
70/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.