RFR: 8331051: Add an `@since` checker test for `java.base` module
Jonathan Gibbons
jjg at openjdk.org
Wed May 1 21:19:54 UTC 2024
On Wed, 24 Apr 2024 14:10:29 GMT, Nizar Benalla <duke at openjdk.org> wrote:
> This checker checks the values of the `@since` tag found in the documentation comment for an element against the release in which the element first appeared.
>
> Real since value of an API element is computed as the oldest release in which the given API element was introduced. That is:
> - for modules, classes and interfaces, the release in which the element with the given qualified name was introduced
> - for constructors, the release in which the constructor with the given VM descriptor was introduced
> - for methods and fields, the release in which the given method or field with the given VM descriptor became a member of its enclosing class or interface, whether direct or inherited
>
> Effective since value of an API element is computed as follows:
> - if the given element has a `@since` tag in its javadoc, it is used
> - in all other cases, return the effective since value of the enclosing element
>
> The since checker verifies that for every API element, the real since value and the effective since value are the same, and reports an error if they are not.
>
> Preview method are handled as per JEP 12, if `@PreviewFeature` is used consistently going forward then the checker doesn't need to be updated with every release. The checker has explicit knowledge of preview elements that came before `JDK 14` because they weren't marked in a machine understandable way and preview elements that came before `JDK 17` that didn't have `@PreviewFeature`.
>
> Important note : We only check code written since `JDK 9` as the releases used to determine the expected value of `@since` tags are taken from the historical data built into `javac` which only goes back that far
>
> The intial comment at the beginning of `SinceChecker.java` holds more information into the program.
>
> I already have filed issues and fixed some wrong tags like in #18640, #18032, #18030, #18055, #18373, #18954, #18972.
test/jdk/TEST.groups line 669:
> 667: # Set of tests for `@since` checks in source code documentation
> 668: jdk_since_check = \
> 669: tools/sincechecker/testjavabase/CheckSince_javaBase.java
Generally, the symbol of a red line in a red circle is indicative of a missing final newline at the end of the file.
It is recommend that files end with exactly one newline character.
test/jdk/tools/sincechecker/SinceChecker.java line 53:
> 51: - This checker checks the values of the `@since` tag found in the documentation comment for an element against the release in which the element first appeared.
> 52: - The source code containing the documentation comments is read from `src.zip` in the release of JDK used to run the test.
> 53: - The releases used to determine the expected value of `@since` tags are taken from the historical data built into `javac`.
(Minor, suggest wrapping lines somewhere between 80-100 characters long)
test/jdk/tools/sincechecker/SinceChecker.java line 72:
> 70:
> 71: Real since value of an API element is computed as the oldest release in which the given API element was introduced. That is:
> 72: - for modules, classes and interfaces, the release in which the element with the given qualified name was introduced
What about packages? packages should be in this list as well.
test/jdk/tools/sincechecker/SinceChecker.java line 223:
> 221: try (FileSystem zipFO = FileSystems.newFileSystem(uri, Collections.emptyMap())) {
> 222: Path root = zipFO.getRootDirectories().iterator().next();
> 223: Path packagePath = root.resolve(moduleName);
Here, and in uses of this name, including in method signatures, it is strange to see the variable named `packagePath` since it appears to be the path of the unnamed package in the module.
`modulePath` would likely be a better name, unless you are worried about possible confusion with the `--module-path` option, in which case maybe `moduleDir` or `moduleDirectory` would work.
test/jdk/tools/sincechecker/SinceChecker.java line 239:
> 237: } catch (Exception e) {
> 238: e.printStackTrace();
> 239: error("Initiating javadocHelperFailed " + e.getMessage());
You probably want just `e`, not `e.getMessage()` because `e` (actually `e.toString()`) will include the name of the exception as well, whereas `e.getMessage()` does not.)
test/jdk/tools/sincechecker/SinceChecker.java line 249:
> 247: }
> 248:
> 249: private void processModuleCheck(ModuleElement moduleElement, JavacTask ct, Path packagePath, EffectiveSourceSinceHelper javadocHelper) {
see comment line 223, about use of `packagePath`
test/jdk/tools/sincechecker/SinceChecker.java line 280:
> 278: }
> 279:
> 280: private String getModuleVersionFromFile(Path packagePath) {
see comment line 223, about `packagePath`
test/jdk/tools/sincechecker/SinceChecker.java line 296:
> 294: }
> 295:
> 296: private String getPackageVersionFromFile(Path packagePath, ModuleElement.ExportsDirective ed) {
see comment line 223, about `packagePath`
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18934#discussion_r1586830124
PR Review Comment: https://git.openjdk.org/jdk/pull/18934#discussion_r1586813217
PR Review Comment: https://git.openjdk.org/jdk/pull/18934#discussion_r1586834143
PR Review Comment: https://git.openjdk.org/jdk/pull/18934#discussion_r1586823529
PR Review Comment: https://git.openjdk.org/jdk/pull/18934#discussion_r1586817831
PR Review Comment: https://git.openjdk.org/jdk/pull/18934#discussion_r1586824342
PR Review Comment: https://git.openjdk.org/jdk/pull/18934#discussion_r1586824992
PR Review Comment: https://git.openjdk.org/jdk/pull/18934#discussion_r1586825818
More information about the core-libs-dev
mailing list