On Tue, 17 May 2022 00:18:07 GMT, Jonathan Gibbons <j...@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 three commits: > > - merge with upstream master > - fix copyright; update test description > - JDK-8285939: javadoc java.lang.Record should not have "Direct Known > Subclasses:" section This is a nice cleanup! My only objection is that some refactored/moved methods have lost their doc comment. src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/AbstractTreeWriter.java line 94: > 92: } > 93: > 94: protected void addTree(Hierarchy hierarchy, String heading, Content > content) { You should keep and adapt the doc comment of the addTree method you removed above. src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/ClassTree.java line 100: > 98: } > 99: > 100: public SortedSet<TypeElement> allSubtypes(TypeElement > typeElement) { Again I think it would be nice to preserve (and update) the doc comment of the method. I also think the naming of `subtypes` and `allSubtypes` is not super clear, so maybe the comments should explicitly mention returning direct or "recursive" subclasses. ------------- Marked as reviewed by hannesw (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8523