On Mon, 27 Jun 2022 16:11:10 GMT, Pavel Rappo <[email protected]> wrote:
>> JDK 19: >> >> Please review a medium-simple fix for some broken links on the >> constant-values page, from the "contents" index at the top of the page to >> the summary tables lower down on the page. >> >> There are two root causes: >> >> The packages are grouped into "groups" with headings based on abbreviated >> package names, meaning "at most two leading components of each package >> name". At one point there is a probable typo, when a full package name is >> used instead of the abbreviated name, thus causing the wrong "id" to be >> generated, thus causing broken links. The fix for this is obvious: use the >> correct name. >> >> More seriously, the abbreviated package name is modeled using a >> PackageElement. While seemingly a good idea, the construction of these >> elements inherits the module element of the originating package. Then, the >> package elements are used to build a `Set` of headings, based on these >> package elements. The problem is that when the subpackages are found in >> different modules, they give rise to distinct "abbreviated package elements" >> in different modules and thus cause repeated instances of like-named headers >> in the output. This is easily visible as repeated headers for com.sun.* in >> (for example) the [JDK 18 >> API](https://docs.oracle.com/en/java/javase/18/docs/api/constant-values.html). >> The fix is to use a Set<String> to model the set of headers and associated >> links, instead of Set<PackageElement>. >> >> Along with the two fixes described above, there is minor code cleanup, >> including the removal of some ill-considered code in both Workarounds and >> Utils. Apart from those classes, the changes are confined to the "constant >> summary" builder and writers. The use of a member called "first" and related >> shenanigans is a holdover from the days when the page was generated >> literally top-to-bottom with print statements. It is easily removed. >> >> The only existing test for the constant values page was laughably >> insufficient, and was effectively just a regression test for a minor issue >> in JDK 1.4.2. The test is enhanced with a bunch of new test cases, that >> exercise different scenarios for the list of contents at the top of the >> page, and the links to the corresponding sections lower down. The new test >> cases leverage the implicit support for checking links in the generated >> output, although they do forcibly enable that support, just to make sure it >> is active. >> >> When comparing the new JDK API docs against recent JDK API docs, the only >> changes are the expected changes in the links and headings on the >> constant-values.html page. >> >> There is more cleanup that could be done to this page ... such as >> suppressing the "contents:" list when it is just a single entry ... but that >> is an enhancement that is out of scope for this bug fix. > > src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/ConstantsSummaryWriterImpl.java > line 106: > >> 104: contents.defaultPackageLabel, ""); >> 105: } else { >> 106: Content packageNameContent = Text.of(abbrevPackageName + >> ".*"); > > Separately. This is somewhat surprising to see that javadoc has been using > `.*` to denote "subpackages" (or "related packages" as we call them > elsewhere), whereas Java import statements use `.*` to denote immediately > enclosed classes and interfaces, or static members thereof. Arguably so, but that's a different issue. > src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/ConstantsSummaryWriter.java > line 61: > >> 59: * @param content the content to which the link will be added >> 60: */ >> 61: void addLinkToPackageContent(String abbrevPackageName, Content >> content); > > It feels strange to downgrade a package representation from PackageElement to > String. Especially considering that this interface expresses other program > elements through javax.lang.model. Agreed it may seem strange. Two points: 1. There is no representation for a package that ignores the enclosing module 2. These are not packages so much as headings, for the purpose of provides a "Contents" list at the head of the page, for ease of navigation. ------------- PR: https://git.openjdk.org/jdk19/pull/62
