RFR: 8331051: Add an `@since` checker test for `java.base` module [v5]

Nizar Benalla nbenalla at openjdk.org
Sat May 18 21:26:27 UTC 2024


On Fri, 17 May 2024 14:49:46 GMT, Jan Lahoda <jlahoda at openjdk.org> wrote:

>> Nizar Benalla has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   - Not checking elements enclosed within a record, I doubt a record class will change after being created
>>   - Added a null check as `toString` can return an exception
>
> test/jdk/tools/sincechecker/SinceChecker.java line 224:
> 
>> 222:         }
>> 223: 
>> 224:         return LEGACY_PREVIEW_METHODS.containsKey(currentVersion)
> 
> Nit: could probably be `LEGACY_PREVIEW_METHODS.getOrDefault(currentVersion, Map.of()).contains(uniqueId)`

Fixed, thanks.

> test/jdk/tools/sincechecker/SinceChecker.java line 309:
> 
>> 307:                 error("module-info.java not found or couldn't be opened AND this module has no unqualified exports");
>> 308:             } catch (NullPointerException e) {
>> 309:                 error("module-info.java does not contain an `@since`");
> 
> Catching a NPE like this feels like a code smell to me. I assume it may happen when `extractSinceVersionFromText(moduleInfoContent)` returns `null` - so just store that in a variable, and check for `null` there.

Fixed in [e82dfbf](https://github.com/openjdk/jdk/pull/18934/commits/e82dfbf0f3df1dfcb1e482aac09fb34e39af9be0), I tried not catching NPE before but found the extra checks a bit ugly.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18934#discussion_r1605898860
PR Review Comment: https://git.openjdk.org/jdk/pull/18934#discussion_r1605898785


More information about the core-libs-dev mailing list