RFR: 8224228: No way to locally suppress lint warnings in parser/tokenizer or preview features

Archie Cobbs acobbs at openjdk.org
Wed Jan 22 20:33:31 UTC 2025


On Wed, 22 Jan 2025 16:15:40 GMT, Archie Cobbs <acobbs at openjdk.org> wrote:

> This PR updates the `DeferredLintHandler` so that deferred warnings can be registered during parsing, before any `JCTree` nodes have been created, and it uses this new capability to update the `"preview"` and `"text-blocks"` Lint warnings so that they can be suppressed via `@SuppressWarnings` annotations. More details are provided in additional comments.

**Overview**

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".

During lexical scanning, `JCTree` nodes don't exist yet, so it is impossible to follow this protocol. Similarly, during parsing, `JCTree` nodes are in the process of being created, so  at best, some awkward gymnastics are required, e.g., see the handling for `"dangling-doc-comments"`.

This PR refactors `DeferredLintHandler` so that it can handle warnings generated during scanning and parsing:
* During scanning and parsing, warnings are temporarily stored by lexical position
* After parsing, the lexical warnings are remapped to the innermost containing `JCTree` declaration

These changes allow us to add `@SuppressWarnings` support for `"preview"` and `"text-blocks"`, and also facilitate any future lexically-based lint categories (e.g., [JDK-8271171](https://bugs.openjdk.org/browse/JDK-8271171), [JDK-8210681](https://bugs.openjdk.org/browse/JDK-8210681)).

One downside is that parsing must now always be done with an `EndPositionTable`, because one is required in order to calculate the lexical extent of declarations. I'm not sure if/how this will meaningfully impact compile time. In any case,  after parsing this table is discarded if no longer needed.

**Per-file notes**

`Preview.java`
* Lexical warnings are now deferred via `DeferredLintHandler`
* Normal warnings via `warnPreview()` are now passed the current `Lint` instance
* Normal warnings via `checkSourceLevel()` are now passed the current `Lint` instance

`Annotate.java`
* Updated for new `DeferredLintHandler` methods for `push()` and `pop()`
* Tighten `DiagnosticPosition deferPos` to `JCTree deferDecl` which is what's actually required
* Line 680: Fix a bug where a `JCExpression` (which is not a declaration supporting `@SuppressWarnings`) was being used for `deferPos`. In this case the correct `deferDecl` should already be established, so we can replace with null.

`Attr.java`
* Updated for new `DeferredLintHandler` methods for `push()` and `pop()`
* Add a couple of missed `flush()`s for variable declarations:
  * Pattern matching binding
  * Enhanced `for()` loop variables

`Check.java`
* Updated for new `DeferredLintHandler` methods for `push()` and `pop()`
* Pass current `Lint` to `preview.reportPreviewWarning()`
* No need to defer in `checkPreview()`
* Update `SuperThisChecker` to handle `@SuppressWarnings("preview")` on constructors

`Modules.java`
* Updated for new `DeferredLintHandler` methods for `push()` and `pop()`
* Update to support `@SuppressWarnings("preview")` on module declarations

`JavaCompiler.java`
* Update `DeferredLintHandler` when entering and exiting parsing
* `EndPositionTable` is now required while parsing, but can be discarded afterwards

`JavaTokenizer.java`
* `lexWarning()` can now defer warnings via `DeferredLintHandler` (this fixes `"text-blocks"`)

`JavacParser.java`
* Make `EmptyEndPosTable` public so `JavaCompiler` can create one

`MandatoryWarningHandler.java`
* Allow overriding `verbose` on a per-report basis to reflect `@SuppressWarnings`

Changes to existing tests:

`ImplicitClass/ErrorRecovery.java`
* The compiler error now results in elimination of the "recompile" note, which was inappropriate in light of the error

`preview/PreviewAutoSuppress.java`
* Warnings reordered as side effect of deferral, but note they are now consistent with the previous test

`preview/PreviewErrors.java`
* Updated to reflect the new effectiveness of `@SuppressWarnings("preview")`

**Other notes**

I tried to cleanup the existing logic in `JavacParser.java` for handling `"dangling-doc-comments"`(+13, -46), but it led to this test failure:

    @SuppressWarnings("dangling-doc-comments")
    /// Bad/misplaced/suppressed comment.
    public void m2() { }

The current code applies the suppression to the misplaced comment, thereby suppressing the warning. But lexically speaking, that comment is not actually within the declaration of `m2()` to which the `@SuppressWarnings` annotation applies, so one could argue that the `@SuppressWarnings` should not apply to it and therefore this test is wrong. Refactoring this code results in the latter interpretation, causing the test to fail, so I didn't do that.

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

PR Comment: https://git.openjdk.org/jdk/pull/23237#issuecomment-2607810168


More information about the compiler-dev mailing list