RFR: 8346294: Invalid lint category specified in compiler.properties [v2]

[Archie Cobbs]

On Mon, 16 Dec 2024 18:35:44 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> Archie Cobbs has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Cleanups & refactoring based on review suggestions.
>
> src/jdk.compiler/share/classes/com/sun/tools/javac/code/Lint.java line 383:
> 
>> 381:          * @throws IllegalArgumentException if no such lint category exists
>> 382:          */
>> 383:         public static LintCategory forOption(String option) {
> 
> Maybe we should call this "getOrThrow" ?

Yeah, or perhaps the most modern & proper thing to do would be to have `LintCategory.get()` return `Optional<LintCategory>`.

This method is only used in a couple of places so that refactoring seems safe.

Updated in 3f792a8b650.

> test/langtools/tools/javac/lint/LintWarningCategoryTest.java line 36:
> 
>> 34: import com.sun.tools.javac.resources.CompilerProperties.LintWarnings;
>> 35: 
>> 36: public class LintWarningCategoryTest {
> 
> Do we need this specific test? Or will either the build, or other tests fail in case there's a lint category/name mismatch?

We don't strictly need this test, because build will fail now if any `lint:` label is invalid, so the test is redundant.

But I also wasn't sure if it was OK to not have any test at all.

Another option would be to split this change two separate bugs/PRs, but that seems like overkill.

(flips through OpenJDK developer guide...) Maybe the best answer would to remove the test and add the `noreg-build` label, as this can be considered a build fix.

Let's try that route... updated in 3f792a8b650.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22769#discussion_r1887370472
PR Review Comment: https://git.openjdk.org/jdk/pull/22769#discussion_r1887370371