On Wed, 24 Apr 2024 14:10:29 GMT, Nizar Benalla <d...@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