RFR: 8216497: javadoc should auto-link to platform classes [v2]

[Jonathan Gibbons]

On Mon, 21 Sep 2020 10:47:40 GMT, Hannes Wallnöfer <hannesw at openjdk.org> wrote:

>> This pull request is identical with the RFR previously sent for the Mercurial repository:
>> 
>> https://mail.openjdk.java.net/pipermail/javadoc-dev/2020-August/001796.html
>> 
>> I'm copy-pasting the comments from the original RFR below.
>> 
>> Most of the new code is added to the Extern class where it fits in quite nicely and can use the existing supporting
>> code for setting up external links.
>> The default behaviour is to generate links to docs.oracle.com for released versions and download.java.net for
>> prereleases. Platform documentation URLs can be configured using the new --link-platform-properties <url> option to
>> provide a properties file with URLs pointing to to alternative locations. See the CSR linked above for more details on
>> the new options.  The feature can be disabled using the --no-platform-link option, generating the same output as
>> previously.   One problem I had to solve was how to handle the transition from prerelease versions to final releases,
>> since the location of the documentation changes in this transition. For obvious reasons we don’t want to make that
>> switch via code change at a time shortly before release.  The way it is done is that we determine if the current
>> javadoc instance is a prerelease version as indicated by the Version returned by BaseConfiguration#getDocletVersion(),
>> and then check whether the release/source version of the current javadoc execution uses the same (latest) version. This
>> means that that only the latest version will ever generate prerelease links (e.g. running current javadoc 16 with
>> source version 15 will generate links to the final release documentation) but I think this is acceptable.  Another
>> issue I spent some time getting right was tests. New releases require a new element-list resource*), so tests have to
>> pick up new releases. On the other hand, we don’t want hundreds of tests to fail when a new release is added, ideally
>> there should be one test  with a clear message. Because of this, when a release is encountered for which no
>> element-list is available a warning is generated instead of an error, and the documentation is generated without
>> platform links as if running with --no-platform-link option. This allows most existing tests to pass and just the new
>> test to fail with a relatively clear message of what is wrong.
>> *) I also thought about generating the element-list for the current release at build time. It’s quite involved, and we
>>  still need to maintain element-lists for older versions, so I’m not sure it’s worth it.
>> 
>> For existing tests that check output affected by the new option I added  the --no-platform-link option to disable the
>> feature. Otherwise we’d have to update those tests with each new release (or else freeze the tests to use some static
>> release or source version, which we don’t want either).  I updated the CSR to the new code. It also needs to be
>> reviewed: https://bugs.openjdk.java.net/browse/JDK-8251181
>> 
>> Thanks,
>> Hannes
>
> Hannes Wallnöfer has updated the pull request incrementally with three additional commits since the last revision:
> 
>  - Merge pull request #1 from lahodaj/JDK-8216497
>    
>    Automatically generate the elements-list data from the ct.sym for releases 11+, including the current release under
>    development
>  - Generating current release list for javadoc; using hardcoded lists for versions < 11
>  - Attempting to (mostly) generate the javadoc release manifests from ct.sym historical data.

Generally excellent. Some feedback inline.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/resources/standard.properties line 350:

> 348:
> 349: doclet.usage.excludedocfilessubdir.parameters=\
> 350:     <name>:..

3 dots for ellipsis?   2 dots is "parent directory"

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/resources/standard.properties line 384:

> 382:
> 383: doclet.usage.no-platform-link.description=\
> 384:     Do not generate links to platform documentation

Suggest:
Do not generate links to the platform documentation

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/BaseOptions.java line 194:

> 192:
> 193:     /**
> 194:      * Argument for command-line option {@code --no-platform-link}.

minor: would "--no-platform-links" (plural) be a better name for the option?

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/BaseOptions.java line 435:

> 433:                 },
> 434:
> 435:                 new Option(resources, "--no-platform-link") {

Repeating preceding comment: would `--no-platform-links` (plural) be a better name?

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Extern.java line 236:

> 234:      * @param linkPlatformProperties path or URL to properties file containing
> 235:      *                               platform documentation URLs, or null
> 236:      * @param reporter The {@code DocErrorReporter} used to report errors.

(pedantic) param descriptions are typically phrases (no initial capital, no trailing period)
In this case, your two `@param` descriptions are inconsistent

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Extern.java line 323:

> 321:                 props.load(inputStream);
> 322:             }
> 323:             url = props.getProperty("doclet.platform.docs." + version);

Similar to other file-or-url arguments: good!

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Extern.java line 340:

> 338:     private int getSourceVersionNumber() {
> 339:         SourceVersion sourceVersion = configuration.docEnv.getSourceVersion();
> 340:         // TODO it would be nice if this was provided by SourceVersion

File an RFE for `SourceVersion`

test/langtools/jdk/javadoc/doclet/testAnnotationTypes/TestAnnotationTypes.java line 49:

> 47:         javadoc("-d", "out-1",
> 48:                 "-sourcepath", testSrc,
> 49:                 "--no-platform-link",

I see lots of instances of `no-platform-link` in this and subsequent tests. `JavadocTester` does have the concept of
default options, although that may be more for the behavior after executing javadoc and not for the options given to
javadoc itself. Is it worth supporting default javadoc options, since that the default can be disabled for specific
tests?

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

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