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

[Maurizio Cimadamore]

On Thu, 6 Feb 2025 02:41:54 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 seven commits:
> 
>  - Eliminate code branch that never occurs.
>  - Clean up imports.
>  - Merge branch 'master' into JDK-8348427 to resolve conflict.
>  - 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.

Regardless of the separate discussion re. "pushImmediate", this PR contains a lot of good changes, and I'm ok with it being integrated as is. We can see if we can get rid of the "immediate" state in a separate PR.

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

Marked as reviewed by mcimadamore (Reviewer).

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