RFR: JDK-8288058: Broken links on constant-values page

Jonathan Gibbons jjg at openjdk.java.net
Wed Jun 15 03:29:10 UTC 2022


On Wed, 15 Jun 2022 03:19:57 GMT, Jonathan Gibbons <jjg at openjdk.org> 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:
> 
> 1. 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.
> 
> 2. 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/toolkit/builders/ConstantsSummaryBuilder.java line 66:

> 64:      * The set of package-group headings.
> 65:      */
> 66:     protected final Set<String> packageGroupHeadings;

This line is the basis for the fix for item 2 that I referred to run the main description, about the inappropriate use of `PackageElement` to model the headings.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/builders/ConstantsSummaryBuilder.java line 172:

> 170:         String abbrevPkgName = getAbbrevPackageName(currentPackage);
> 171:         if (!packageGroupHeadings.contains(abbrevPkgName)) {
> 172:             writer.addPackageGroup(abbrevPkgName, target);

This line is the fix for the "typo" that I referred to in item 1 of the main description.

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

PR: https://git.openjdk.org/jdk/pull/9162


More information about the javadoc-dev mailing list