RFR: 8344079: Minor fixes and cleanups to compiler lint-related code [v2]

[Maurizio Cimadamore]

On Mon, 2 Dec 2024 17:53:53 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> Changing the key is possible. I even prototyped that - but ran out of steam when I realized that all compiler negative tests required some fixing. So I'm a bit wary of going down that path.
>> 
>> IMHO, the issue with stuff like:
>> 
>> 
>> if (lint.isEnabled(LintCategory.DEP_ANN))
>>     log.warn(LintCategory.DEP_ANN, pos, Warnings.MissingDeprecatedAnnotation));
>> 
>> 
>> Is that compiler code should even bother to do this! Ideally, you might want to have a single routine:
>> 
>> 
>> log.warnIfEnabled(DEP_ANN, pos, Warnings.MissingDeprecatedAnnotation)
>> 
>> 
>> This could test whether `DEP_ANN` is enabled _and_ also check that the provided warning key has the correct category set
>
>> `log.warnIfEnabled(DEP_ANN, pos, Warnings.MissingDeprecatedAnnotation)`
> 
> Or, even better:
> 
> `log.warnIfEnabled(pos, Warnings.MissingDeprecatedAnnotation)`
> 
> (e.g. just use the warning key option for the `isEnabled` check)

Another observation: it would be better if we had two distinct set of diagnostics; warnings and lint warnings. If one couldn't be passed in place of the other that will force clients to use the correct `log` method. Which might make it easier to check that all the places issuing a lint warning also check as to whether the warning is enabled.

(that said, we can't check for enable-ment in all cases - e.g. for lint warnings in the tokenizer, so there will always be some ad-hocness somewhere, I think. I think we win if we can find something that works in most cases, but we can't make everything statically type-checkable, not without significant cost and effort).

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22056#discussion_r1866336502