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