Re: RFR: JDK-8285939: javadoc java.lang.Record should not have "Direct Known Subclasses:" section [v2]
On Tue, 17 May 2022 00:18:07 GMT, Jonathan Gibbons 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 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
Re: RFR: JDK-6251738: Want a top-level summary page that itemizes all spec documents referenced from javadocs (OEM spec) [v5]
On Tue, 24 May 2022 21:30:59 GMT, Pavel Rappo wrote:
>> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/BaseOptions.java
>> line 302:
>>
>>> 300:
>>> 301: /**
>>> 302: * Argument for command-line option {@code --spec-base-URI}.
>>
>> Have my earlier comment on the upper case in `--spec-base-URI` disappeared?
>> I cannot seem to find it.
>
> Ah, found it: there are two of those cases.
I believe these have all been fixed
-
PR: https://git.openjdk.java.net/jdk/pull/8439
Re: RFR: JDK-6251738: Want a top-level summary page that itemizes all spec documents referenced from javadocs (OEM spec) [v4]
On Tue, 24 May 2022 16:53:51 GMT, Pavel Rappo wrote: >> Jonathan Gibbons has updated the pull request incrementally with one >> additional commit since the last revision: >> >> address review feedback > > src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/taglets/SpecTaglet.java > line 2: > >> 1: /* >> 2: * Copyright (c) 2001, 2022, Oracle and/or its affiliates. All rights >> reserved. > > 2001? removed - PR: https://git.openjdk.java.net/jdk/pull/8439
Re: RFR: JDK-6251738: Want a top-level summary page that itemizes all spec documents referenced from javadocs (OEM spec) [v7]
On Tue, 3 May 2022 22:16:15 GMT, Jonathan Gibbons wrote: >> test/langtools/jdk/javadoc/doclet/testConditionalPages/TestConditionalPages.java >> line 2: >> >>> 1: /* >>> 2: * Copyright (c) 2020, 2022, Oracle and/or its affiliates. All rights >>> reserved. >> >> Consider adding 6251738 to `@bug`. > > Mmmm, OK done - PR: https://git.openjdk.java.net/jdk/pull/8439
Re: RFR: JDK-6251738: Want a top-level summary page that itemizes all spec documents referenced from javadocs (OEM spec) [v8]
> Please review the code and tests to add support for a new `@spec url title` > tag to javadoc. Note, this does not include any changes to API doc comments > to use the new tag in JDK API documentation; that will come later. Jonathan Gibbons has updated the pull request incrementally with one additional commit since the last revision: fix copyright dates remove archaic obsolete build flags - Changes: - all: https://git.openjdk.java.net/jdk/pull/8439/files - new: https://git.openjdk.java.net/jdk/pull/8439/files/b03bbb62..c24c02ac Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=8439&range=07 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=8439&range=06-07 Stats: 7 lines in 5 files changed: 1 ins; 2 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/8439.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8439/head:pull/8439 PR: https://git.openjdk.java.net/jdk/pull/8439
RFR: 8287333: Clean up ParamTaglet and ThrowsTaglet
A cleanup to facilitate fixing bugs like JDK-6509045: `{@inheritDoc}` only
copies one instance of the specified exception.
-
Commit messages:
- Update top-level doc comment
- Trivially re-order assignments
- Reformat for clarity
- Remove dead condition
- Remove emptiness check
- Remove comments
- Use consistent order
- Rename and clarify
- Improve `inherit`
- Rearrange methods
- ... and 16 more: https://git.openjdk.java.net/jdk/compare/e990fec1...03ec6e6f
Changes: https://git.openjdk.java.net/jdk/pull/8886/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=8886&range=00
Issue: https://bugs.openjdk.java.net/browse/JDK-8287333
Stats: 382 lines in 3 files changed: 111 ins; 158 del; 113 mod
Patch: https://git.openjdk.java.net/jdk/pull/8886.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/8886/head:pull/8886
PR: https://git.openjdk.java.net/jdk/pull/8886
Re: RFR: JDK-8284037: Snippet-files subdirectory not automatically detected when in unnamed package [v2]
On Tue, 24 May 2022 19:46:51 GMT, Pavel Rappo wrote: >> Jonathan Gibbons has updated the pull request incrementally with one >> additional commit since the last revision: >> >> address review feedback > > src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/taglets/SnippetTaglet.java > line 223: > >> 221: JavaFileManager.Location l = >> utils.getLocationForPackage(pkg); >> 222: String relativeName = "snippet-files/" + v; >> 223: String packageName = packageName(pkg, utils); > > Drop the now-unused `packageName` method. Good catch! Done. - PR: https://git.openjdk.java.net/jdk/pull/8708
Re: RFR: JDK-8284037: Snippet-files subdirectory not automatically detected when in unnamed package [v3]
> Please review a small fix to address use of snippets in source code in the > unnamed package. > > The core of the fix is to replace `packageName(pkg, utils)` (which returns > `""` for the unnamed package) with plain old > `pkg.getQualifiedName().toString()` (which returns an empty string for the > unnamed package.) > > There's some minor localized cleanup and rearrangement as well. > > The test verifies behavior without and then with a source path, since that is > the likely path of discovery by a user playing with the unnamed package. Jonathan Gibbons has updated the pull request incrementally with one additional commit since the last revision: address review feedback: remove dead code - Changes: - all: https://git.openjdk.java.net/jdk/pull/8708/files - new: https://git.openjdk.java.net/jdk/pull/8708/files/ccfb80b7..c265fffb Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=8708&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=8708&range=01-02 Stats: 4 lines in 1 file changed: 0 ins; 4 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/8708.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8708/head:pull/8708 PR: https://git.openjdk.java.net/jdk/pull/8708
Re: RFR: JDK-8279513: jdk/javadoc/doclet/testDocletExample/TestDocletExample.java fails after 8278795
On Tue, 24 May 2022 18:39:06 GMT, Pavel Rappo wrote:
>> Please review a test-only fix to make a recent new test more robust.
>>
>> The test was not fundamentally broken, so the essential functionality
>> remains the same. However, it does assume access to the `src/jdk.javadoc`
>> directory, and crashed when that was not available.
>>
>> The mitigation is two-fold:
>>
>> 1. introduce and use a new `SnippetUtils.ConfigurationException` that is
>> thrown when the test cannot find the necessary directories.
>> 2. introduce and use 2 new test-suite keywords, `needs-src`
>> `needs-src-jdk_javadoc` that can be used on the jtreg command-line to filter
>> out this test, and any similar tests in the future.
>>
>> Tested locally, manually, by temporarily renaming key directories, to test
>> the different code paths. In all cases, if the test is run and any
>> necessary directories are missing, the test _will still fail_, but now with
>> a more useful and specific exception and detail message.
>
> test/langtools/jdk/javadoc/doclet/testDocletExample/TestDocletExample.java
> line 71:
>
>> 69: var entryPointSnippet = snippets.getSnippetById(dc,
>> "entry-point");
>> 70: if (entryPointSnippet == null) {
>> 71: throw new Error("Cannot find snippet \"entry-point\"");
>
> I understand it as follows. Although this code now throws generic `Error`
> instead of `NullPointerException`, which the bug reporter observed, we
> shouldn't see that `Error` in circumstances similar to those of bug reporter.
> This is because in those circumstances the code will throw
> `ConfigurationException` earlier, at construction time, so we won't reach
> this check. Do I understand it correctly?
Yes. Although we could throw NPE even here, I was wanting to throw something
that indicates the test is dysfunctional, as compared to failing or crashing.
> test/langtools/jdk/javadoc/doclet/testDocletExample/TestDocletExample.java
> line 97:
>
>> 95: var dc = snippets.getDocTrees().getDocCommentTree(docletPkg);
>> 96: var exampleSnippet = snippets.getSnippetById(dc, "Example.java");
>> 97: if (exampleSnippet == null) {
>
> Same as above.
Yes, same reasoning. `SnippetUtils` is broken if we get a null result.
-
PR: https://git.openjdk.java.net/jdk/pull/8796
Re: RFR: JDK-8284037: Snippet-files subdirectory not automatically detected when in unnamed package [v3]
On Wed, 25 May 2022 17:00:16 GMT, Jonathan Gibbons wrote: >> Please review a small fix to address use of snippets in source code in the >> unnamed package. >> >> The core of the fix is to replace `packageName(pkg, utils)` (which returns >> `""` for the unnamed package) with plain old >> `pkg.getQualifiedName().toString()` (which returns an empty string for the >> unnamed package.) >> >> There's some minor localized cleanup and rearrangement as well. >> >> The test verifies behavior without and then with a source path, since that >> is the likely path of discovery by a user playing with the unnamed package. > > Jonathan Gibbons has updated the pull request incrementally with one > additional commit since the last revision: > > address review feedback: remove dead code Marked as reviewed by prappo (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8708
Re: RFR: JDK-8279513: jdk/javadoc/doclet/testDocletExample/TestDocletExample.java fails after 8278795
On Wed, 25 May 2022 17:02:15 GMT, Jonathan Gibbons wrote: > `SnippetUtils` is broken if we get a null result. If it's unconditionally so, which it seems to be, wouldn't it be better to throw something from `SnippetUtils.getSnippetById` instead? - PR: https://git.openjdk.java.net/jdk/pull/8796
Re: RFR: JDK-8279513: jdk/javadoc/doclet/testDocletExample/TestDocletExample.java fails after 8278795
On Wed, 25 May 2022 17:07:36 GMT, Pavel Rappo wrote: >> Yes, same reasoning. `SnippetUtils` is broken if we get a null result. > >> `SnippetUtils` is broken if we get a null result. > > If it's unconditionally so, which it seems to be, wouldn't it be better to > throw something from `SnippetUtils.getSnippetById` instead? Hmmm, The current spec of `SnippetUtils` is to return `null` if an item cannot be found. So, it is only broken if the user really expects a non-null result. I guess it depends if there is enough use case where you might want a null result instead of an exception. - PR: https://git.openjdk.java.net/jdk/pull/8796
Re: RFR: JDK-8279513: jdk/javadoc/doclet/testDocletExample/TestDocletExample.java fails after 8278795
On Wed, 25 May 2022 16:59:25 GMT, Jonathan Gibbons wrote:
>> test/langtools/jdk/javadoc/doclet/testDocletExample/TestDocletExample.java
>> line 71:
>>
>>> 69: var entryPointSnippet = snippets.getSnippetById(dc,
>>> "entry-point");
>>> 70: if (entryPointSnippet == null) {
>>> 71: throw new Error("Cannot find snippet \"entry-point\"");
>>
>> I understand it as follows. Although this code now throws generic `Error`
>> instead of `NullPointerException`, which the bug reporter observed, we
>> shouldn't see that `Error` in circumstances similar to those of bug
>> reporter. This is because in those circumstances the code will throw
>> `ConfigurationException` earlier, at construction time, so we won't reach
>> this check. Do I understand it correctly?
>
> Yes. Although we could throw NPE even here, I was wanting to throw something
> that indicates the test is dysfunctional, as compared to failing or crashing.
The rationale that you provided reminds of `new FileInputStream(java.io.File)`
that throws `FileNotFoundException`. Unlike not finding a file, not finding a
particular snippet is an unexpected, unrecoverable programming error. I cannot
imagine a reasonable test that checks that some snippet is not there.
-
PR: https://git.openjdk.java.net/jdk/pull/8796
Re: RFR: JDK-8279513: jdk/javadoc/doclet/testDocletExample/TestDocletExample.java fails after 8278795
On Wed, 25 May 2022 17:19:22 GMT, Pavel Rappo wrote: >> Yes. Although we could throw NPE even here, I was wanting to throw something >> that indicates the test is dysfunctional, as compared to failing or crashing. > > The rationale that you provided reminds of `new > FileInputStream(java.io.File)` that throws `FileNotFoundException`. Unlike > not finding a file, not finding a particular snippet is an unexpected, > unrecoverable programming error. I cannot imagine a reasonable test that > checks that some snippet is not there. It's a reasonable analogy. - PR: https://git.openjdk.java.net/jdk/pull/8796
Re: RFR: JDK-8279513: jdk/javadoc/doclet/testDocletExample/TestDocletExample.java fails after 8278795
On Wed, 25 May 2022 17:11:59 GMT, Jonathan Gibbons wrote: >>> `SnippetUtils` is broken if we get a null result. >> >> If it's unconditionally so, which it seems to be, wouldn't it be better to >> throw something from `SnippetUtils.getSnippetById` instead? > > Hmmm, The current spec of `SnippetUtils` is to return `null` if an item > cannot be found. So, it is only broken if the user really expects a non-null > result. > > I guess it depends if there is enough use case where you might want a null > result instead of an exception. I guess the `SnippetUtils` spec does not currently define behavior if not found. I would be more inclined to define a "returns null" behavior than to use `Optional` or throw and exception. But more generally, I'd prefer to leave updates to `SnippetIUtils` to its own separate Enhancement. - PR: https://git.openjdk.java.net/jdk/pull/8796
Re: RFR: JDK-8279513: jdk/javadoc/doclet/testDocletExample/TestDocletExample.java fails after 8278795
On Thu, 19 May 2022 22:05:50 GMT, Jonathan Gibbons wrote: > Please review a test-only fix to make a recent new test more robust. > > The test was not fundamentally broken, so the essential functionality remains > the same. However, it does assume access to the `src/jdk.javadoc` directory, > and crashed when that was not available. > > The mitigation is two-fold: > > 1. introduce and use a new `SnippetUtils.ConfigurationException` that is > thrown when the test cannot find the necessary directories. > 2. introduce and use 2 new test-suite keywords, `needs-src` > `needs-src-jdk_javadoc` that can be used on the jtreg command-line to filter > out this test, and any similar tests in the future. > > Tested locally, manually, by temporarily renaming key directories, to test > the different code paths. In all cases, if the test is run and any necessary > directories are missing, the test _will still fail_, but now with a more > useful and specific exception and detail message. > I'd prefer to leave updates to SnippetIUtils to its own separate Enhancement. Sounds good. - Marked as reviewed by prappo (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8796
Re: RFR: JDK-8279513: jdk/javadoc/doclet/testDocletExample/TestDocletExample.java fails after 8278795
On Wed, 25 May 2022 17:24:22 GMT, Pavel Rappo wrote: > > I'd prefer to leave updates to SnippetIUtils to its own separate > > Enhancement. > > Sounds good. [JDK-8287337](https://bugs.openjdk.java.net/browse/JDK-8287337) - PR: https://git.openjdk.java.net/jdk/pull/8796
Integrated: JDK-8284037: Snippet-files subdirectory not automatically detected when in unnamed package
On Fri, 13 May 2022 21:51:32 GMT, Jonathan Gibbons wrote: > Please review a small fix to address use of snippets in source code in the > unnamed package. > > The core of the fix is to replace `packageName(pkg, utils)` (which returns > `""` for the unnamed package) with plain old > `pkg.getQualifiedName().toString()` (which returns an empty string for the > unnamed package.) > > There's some minor localized cleanup and rearrangement as well. > > The test verifies behavior without and then with a source path, since that is > the likely path of discovery by a user playing with the unnamed package. This pull request has now been integrated. Changeset: f786e2a2 Author:Jonathan Gibbons URL: https://git.openjdk.java.net/jdk/commit/f786e2a22fca7c9785b2daa5667271f418c414f4 Stats: 114 lines in 2 files changed: 102 ins; 11 del; 1 mod 8284037: Snippet-files subdirectory not automatically detected when in unnamed package Reviewed-by: prappo - PR: https://git.openjdk.java.net/jdk/pull/8708
Integrated: JDK-8279513: jdk/javadoc/doclet/testDocletExample/TestDocletExample.java fails after 8278795
On Thu, 19 May 2022 22:05:50 GMT, Jonathan Gibbons wrote: > Please review a test-only fix to make a recent new test more robust. > > The test was not fundamentally broken, so the essential functionality remains > the same. However, it does assume access to the `src/jdk.javadoc` directory, > and crashed when that was not available. > > The mitigation is two-fold: > > 1. introduce and use a new `SnippetUtils.ConfigurationException` that is > thrown when the test cannot find the necessary directories. > 2. introduce and use 2 new test-suite keywords, `needs-src` > `needs-src-jdk_javadoc` that can be used on the jtreg command-line to filter > out this test, and any similar tests in the future. > > Tested locally, manually, by temporarily renaming key directories, to test > the different code paths. In all cases, if the test is run and any necessary > directories are missing, the test _will still fail_, but now with a more > useful and specific exception and detail message. This pull request has now been integrated. Changeset: 7156f98e Author:Jonathan Gibbons URL: https://git.openjdk.java.net/jdk/commit/7156f98e324ffd0ab17105b2cb8cb6ce0d718e5b Stats: 44 lines in 3 files changed: 37 ins; 0 del; 7 mod 8279513: jdk/javadoc/doclet/testDocletExample/TestDocletExample.java fails after 8278795 Reviewed-by: prappo - PR: https://git.openjdk.java.net/jdk/pull/8796
RFR: JDK-8287337: SnippetUtils should throw exceptions if snippets not found
Please review a simple change for `SnippetUtils` to throw a checked exception if a snippet cannot be found. - Commit messages: - JDK-8287337: SnippetUtils should throw exceptions if snippets not found Changes: https://git.openjdk.java.net/jdk/pull/8892/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=8892&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8287337 Stats: 41 lines in 2 files changed: 31 ins; 6 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/8892.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8892/head:pull/8892 PR: https://git.openjdk.java.net/jdk/pull/8892
Re: RFR: JDK-8285939: javadoc java.lang.Record should not have "Direct Known Subclasses:" section [v2]
On Wed, 25 May 2022 10:22:15 GMT, Hannes Wallnöfer wrote:
>> 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
>
> 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.
Noted; will do.
-
PR: https://git.openjdk.java.net/jdk/pull/8523
Re: RFR: JDK-8285939: javadoc java.lang.Record should not have "Direct Known Subclasses:" section [v2]
On Wed, 25 May 2022 19:43:55 GMT, Jonathan Gibbons wrote:
>> 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.
>
> Noted; will do.
done
-
PR: https://git.openjdk.java.net/jdk/pull/8523
Re: RFR: JDK-8285939: javadoc java.lang.Record should not have "Direct Known Subclasses:" section [v3]
> 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 - Changes: https://git.openjdk.java.net/jdk/pull/8523/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=8523&range=02 Stats: 446 lines in 11 files changed: 150 ins; 123 del; 173 mod Patch: https://git.openjdk.java.net/jdk/pull/8523.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8523/head:pull/8523 PR: https://git.openjdk.java.net/jdk/pull/8523
Re: RFR: JDK-8285939: javadoc java.lang.Record should not have "Direct Known Subclasses:" section [v2]
On Wed, 25 May 2022 10:48:43 GMT, Hannes Wallnöfer wrote:
>> 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
>
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/ClassTree.java
> line 100:
>
>> 98: }
>> 99:
>> 100: public SortedSet 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.
done
-
PR: https://git.openjdk.java.net/jdk/pull/8523
