RFR: 8348427: DeferredLintHandler API should use JCTree instead of DiagnosticPosition [v2]

[Maurizio Cimadamore]

On Fri, 24 Jan 2025 23:34:28 GMT, Archie Cobbs <acobbs at openjdk.org> wrote:

>> The purpose of `DeferredLintHandler` is to allow `@SuppressWarnings` to be applied to warnings that are generated before `@SuppressWarnings` annotations themselves have been processed. The way this currently works is that warning callbacks are kept in a `HashMap` keyed by the innermost containing module, package, class, method, or variable declarations (in the form of a `JCModuleDecl`, `JCPackageDecl`, `JCClassDecl`, `JCMethodDecl`, or `JCVariableDecl`). Later, when the compiler executes the attribution phase and the lint categories suppressed at each declaration are known, the corresponding warning callbacks are provided with an appropriately configured `Lint` instance and "flushed".
>> 
>> However, the `DeferredLintHandler` API uses `DiagnosticPosition` instead of `JCTree` for registering and flushing deferred warnings. This opens the door for bugs where warnings are registered to an object which is not a declaration, and therefore ignored.
>> 
>> In fact, this occurs once in the code ([here](https://github.com/openjdk/jdk/blob/48ece0721489c1b357aaa81e89fe59f486079d15/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Annotate.java#L679)) where a `JCExpression` is being passed, although in this case the bug appears to be harmless (because annotation values can't contain any type of declaration).
>> 
>> The API should be tighted up, and furthermore an assertion should be added to verify that the JCTree being passed is actually a declaration supporting `@SuppressWarnings`.
>> 
>> In addition, there is a design flaw in the API: it's not possible to obtain the current immediate mode `Lint` object, so if an immediate mode `Lint` object is pushed/popped more than once, the second `Lint` object will overwrite the first.
>> 
>> To fix this, the API should be adjusted so the stack of current declarations and/or immediate mode `Lint` objects is managed by the `DeferredLintHandler` itself, by providing a `push()` and `pop()` methods in the API.
>
> Archie Cobbs has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains four commits:
> 
>  - Merge branch 'master' into JDK-8348427 to fix conflict.
>  - Refactor the DeferredLintHandler API to maintain the "stack" itself.
>  - Add Javadoc to document a subtley in the API.
>  - Update DeferredLintHandler API to use JCTree instead of DiagnosticPosition.

Question: what happens if we get rid of the "immediate" and we always defer? Does this only affect the order in which diagnostics are reported, or is there something deeper?

-------------

PR Review: https://git.openjdk.org/jdk/pull/23281#pullrequestreview-2595400317