RFR: JDK-8285939: javadoc java.lang.Record should not have "Direct Known Subclasses:" section [v3]

[Pavel Rappo]

On Wed, 25 May 2022 20:55:21 GMT, Jonathan Gibbons <jjg at openjdk.org> wrote:

>> As described in the JBS issue, the observed problem is a side-effect of a related but different issue, which is that record classes are not treated the same was as enum classes when it comes to generating the class hierarchy in `ClassTree`. Because record classes are not treated specially/differently, they are treated as ordinary/plain classes, with the side-effect that the page for `java.lang.Record` shows `Direct Known Subclasses`.
>> 
>> The underlying fix is therefore to clone the enum support in `ClassTree` and related classes, to provide support for record classes. It is possible to do an extreme minimal clone, but that just clones some of the messy evolution already there. Instead, the code in `ClassTree` and its clients is refactored and modernized.
>> 
>> Previously there were four explicit pairs of member fields, containing data for different groups (hierarchies) of classes, namely: plain classes, enum classes, interfaces and annotation interfaces. These fields are most of the code to support them are moved into some new abstractions to encapsulate related functionality.
>> 
>> 1. The main new abstraction in `ClassTree` is `Hierarchy` which provides the support for the different hierarchies displayed in the generated pages.
>> 2. A new enum `HierarchyKind` identifies the four existing hierarchies (listed above) and now a new one, for record classes. The hierarchies correspond to the different kinds of declared type.
>> 3. A small new class `SubtypeMap` which is a multi-map for mapping a type element to its subtypes.  This is used in `Hierarchy` and to record the classes that implement an interfaces.
>> 
>> Generally, the naming should be clearer and more obvious. The most confusing name in the old code was `enumSubtypes` which was weird because enum classes don't have subtypes.  It meant "subtypes of supertypes of enum classes". This was a prime motivator to switch to the `hierarchy` terminology.   The variant spellings of `intfacs` have all been replaced  by `interfaces`, and `classtree` becomes `classTree`.
>> 
>> *Testing*: a new test case has been added to the `TestRecordTypes.java` test, which verifies the new record hierarchy is displayed on a a package tree page.  It is not simple to directly test the observed/reported behavior, because it is specific to the JDK API documentation, and because it is about the content of the `java.lang.Record` API page. However, manual inspection and diff comparison between JDK API documentation generated before and after this change reveals only expected differences. These differences are on the `java.lang.R4cord` page (where the offending section is no longer displayed) and on the pages related to the two existing records in JDK ... which are now listed in `Record Class Hierarchy` sections in the appropriate `package-tree.html` files.
>
> Jonathan Gibbons has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains five commits:
> 
>  - Merge remote-tracking branch 'upstream/master' into 8285939.record-subtypes
>  - address review comments: add doc comments to new methods
>  - merge with upstream master
>  - fix copyright; update test description
>  - JDK-8285939: javadoc java.lang.Record should not have "Direct Known Subclasses:" section

Looks good.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/ClassTree.java line 78:

> 76:         /**
> 77:          * {@return the roots of the hierarchy}
> 78:          * The roots are the classes or interfaces with no superclass or superinterfaces.

Use singular "root" for clarity:
Suggestion:

         * A root is a class or an interface with no superclass or superinterfaces.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/ClassTree.java line 88:

> 86:          * {@return a map containing the type elements in this hierarchy and their subtypes}
> 87:          */
> 88:         public Map<TypeElement, SortedSet<TypeElement>> subtypes() {

This method is unused; consider deleting it.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/ClassTree.java line 103:

> 101:          * {@return the set of all subtypes of the given type element, or an empty set if there are none}
> 102:          *
> 103:          * The set of all subtypes is the transitive closure of the {@linkplain #subtypes() immediate subtypes}

Did you mean to link to #subtypes(TypeElement)?

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/ClassTree.java line 128:

> 126:     /**
> 127:      * A map giving the subtypes for each of a collection of type elements.
> 128:      * The subtypes may be subclasses or subinterfaces, depending on the context.

For a separate PR: we should consider dropping hyphens from "super-" and "sub-" in words that relate to classes, interfaces, types, and packages.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/ClassTree.java line 228:

> 226: 
> 227:     /**
> 228:      * Generate the hierarchies for the given set of type elements.

Suggestion:

     * Generate the hierarchies for the given type elements.

test/langtools/jdk/javadoc/doclet/testRecordTypes/TestRecordTypes.java line 610:

> 608: 
> 609:     @Test
> 610:     public void testPackageTree(Path base) throws IOException {

Am I right saying that we cannot easily test the original bug symptom, which is the presence of "Direct Known Subclasses:" on the api/java.base/java/lang/Record.html page?

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

Marked as reviewed by prappo (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/8523