JDK-8323093 : type annotations are not visible to javac plugins across compilation boundaries
  • Type: CSR
  • Component: tools
  • Sub-Component: javac
  • Priority: P3
  • Status: Closed
  • Resolution: Approved
  • Fix Versions: 22
  • Submitted: 2024-01-05
  • Updated: 2025-01-07
  • Resolved: 2024-01-19
Related Reports
CSR :  
Description
Summary
-------

Update javac's implementation of `TypeMirror` to allow accessing type use annotations on types loaded from class files.

Problem
-------

The implementation currently does not associate type use annotations loaded from class files with the corresponding types. This means that `TypeMirror` APIs including `getAnnotationMirrors` and `toString` do not accurately report type use annotations for types loaded from class files.

Solution
--------

If a type annotation is present on a type read from a class file, the implementation should associate the annotation with its type.

Specification
-------------

No specification change; behavioral change only.
Comments
Moving to Approved for JDK 22. Given behavioral compatibility impact seen in the field, not currently approved for JDK 21 update or earlier release trains.
19-01-2024

Another note on the compatibility impact: JDK-8225356 was an analogous behavioural change to the `toString` of `javax.lang.model` classes. For context on compatibility analysis from Google's code base, JDK-8225356 required updates to roughly 22 annotation processors, vs. the change in this CSR which required updates to 6 processors in Google's code base. So based on that comparision, the impact of this change is smaller than the analogous change in JDK-8225356.
11-01-2024

More notes on the compatibility impact: After this was merged it was discovered that it impacted micronaut, which has an annotation process that was using `javax.lang.model.type.TypeVariable#toString` as a map key, which meant that a use of the type variable with annotations couldn't be used to look up one without, and vice versa. The issue has been fixed: https://github.com/micronaut-projects/micronaut-core/pull/10293 As I noted in the backport requests, Google has applied this fix internally, and our codebase is building with the fix in place. I saw some compatibility impact from rolling this change out. We have a relatively large codebase that contains many annotation processors and makes use of type use annotations, so I think it's a meaningful test case, but of course the impact here will vary across codebases. Overall the internal impact of this change was comparable to other routine fixes: whenever a string representation changes, we usually find someone who was depending on it (see also https://www.hyrumslaw.com/). I'd also note that there would be some impact from backing the change out and leaving the bug unfixed, since it is preventing annotation processors from being able to handle type annotations. For example: a dependency injection framework might use an annotation processor to generate code, and that code would want to know if a formal parameter type accepted `null` values, or if a method returned `null` values. This is currently not possible to do reliably today, because the type annotation information is not available for dependencies. The full list of compatibility impact I saw inside Google was: * An annotation processor was using `TypeMirror#toString` as a key for dependency injection graph analysis, the fix was to create a stable string representation of the key instead of relying on `TypeMirror#toString` * An annotation processor was using `TypeMirror#toString` to compare different `TypeMirror` instances for equality, the fix was to use `javax.lang.model.util.Types#isSameType` * An annotation processor was using the output of `TypeMirror#toString` in a generated identifier, and using a regular expression to replace characters that can appear in the output that are not valid identifier characters (e.g. `<`). The logic did not handle output that contained type annotations. This was worked around by removing `@` and whitespace from the output, a more principled fix would be to convert the `TypeMirror` to a stable string representation for use in an identifier, e.g. with a custom `TypeVisitor`. * A test that compared actual to expected compilation diagnostics included diagnostics containing the string representation of types, and the string representation of some of the types changed to include type annotations. * An annotation processor that expected APIs to be consistently annotated for nullness (e.g. for getters to match setters) was failing to report diagnostics for elements loaded from bytecode that were missing annotations, and this change fixed that bug. The resolution was to update a single source file to add a missing `@Nullable` annotation. * An annotation processor was testing types for equality by calling `TypeMirror#toString` and comparing to the expected string representation, which stopped working when the string representation included type annotations. The fix was to use `javax.lang.model.util.Types#isSameType`.
08-01-2024