RFR: 8345263: Make sure that lint categories are used correctly when logging lint warnings [v2]

[Vicente Romero]

On Thu, 5 Dec 2024 08:55:19 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> This PR tightens up the logic by which javac reports lint warnings. Currently, lint warnings are typically emitted using the following idiom:
>> 
>> 
>> if (lint.isEnabled(LintCategory.DIVZERO) {
>>     log.warning(LintCategory.DIVZERO, pos, Warnings.DIVZERO);
>> }
>> 
>> There are some issues with this approach:
>> 
>> * logging a lint warning has to be preceded by the correct `isEnabled` check
>> * the check and the `log::warning` call must share the same `LintCategory`
>> * the selected warning key in the `Warnings` class must also make sense for the selected `LintCategory`
>> 
>> This PR addresses these issues, so that the above code is now written as follows:
>> 
>> 
>> lint.logIfEnabled(pos, LintWarnings.DIVZERO);
>> 
>> 
>> The new idiom builds on a number of small improvements:
>> 
>> * the lint category is now tracked directly in the `compiler.properties` file;
>> * a new `LintWarning` class is added to `JCDiagnostic` to model a warning key that is also associated with a speicfic `LintCategory` enum constant;
>> * the `CompilerProperties` class has a new group of compiler keys, nested in the new `LintWarnings` class. This class defines the `LintWarning` objects for all the warning keys in `compiler.properties` that have a lint category set
>> * A new method `Lint::logIfEnabled(Position, LintWarning)` is added - which simplifies the logging of lint warnings in many common cases, by merging the `isEnabled` check together with the logging.
>> 
>> As bonus points, the signatures of some methods in `Check` and `MandatoryWarningHandler` have been tightened to accept not just a `Warning`, but a `LintWarning`. This makes it impossible, for instance, to use `Check::warnUnchecked` with a warning that is not a true lint warning.
>> 
>> Many thanks @archiecobbs for the useful discussions!
>
> Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Add missing lint categories in compiler.properties
>   Simplify some lambda expressions
>   Simplify Lint::logIfEnabled by accepting a log parameter

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java line 2972:

> 2970:                     if (belongsToRestrictedPackage(sym)) {
> 2971:                         log.warning(tree.pos(),
> 2972:                                     Warnings.AccessToMemberFromSerializableLambda(sym));

so this one won't be a lint warning anymore I guess

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22553#discussion_r1872223639