RFR: 8308715: Create a mechanism for Implicitly Declared Class javadoc [v2]

[Jonathan Gibbons]

On Tue, 28 Nov 2023 23:15:38 GMT, Pavel Rappo <prappo at openjdk.org> wrote:

>> Please review this PR to support _JEP 463 Implicitly Declared Classes and Instance Main Method (Second Preview)_ in JavaDoc.
>> 
>> [JEP 463](https://openjdk.org/jeps/463) is the next iteration of [JEP 445](https://openjdk.org/jeps/445), which introduced the ability to have a source file as a launchable, "classless" method bag
>> 
>> 
>> % cat HelloWorld.java
>> /** Run me. */
>> void main() {
>>     print("Hello, world!");
>> }
>> 
>> /** Shortcut for printing strings. */
>> void print(String arg) {
>>     System.out.println(arg);
>> }
>> 
>> 
>> which the compiler interprets as a familiar class
>> 
>> 
>> final class HelloWorld {
>> 
>>     HelloWorld() {
>>     }
>> 
>>     /** Run me. */
>>     void main() {
>>         print("Hello, world!");
>>     }
>> 
>>     /** Shortcut for printing strings. */
>>     void print(String arg) {
>>         System.out.println(arg);
>>     }    
>> }
>> 
>> 
>> ### How JEP 445 works with JavaDoc today
>> 
>> In JDK 21, javadoc can document such a file **without any changes to the javadoc tool**. The only thing that the user needs to do is to make sure that the following options are present:
>> 
>> * `--enable-preview` and `--source=21`
>> * `-package`
>> 
>> The first pair of options tells javadoc to use preview features, which JEP 445 is one of. Without these preview-related options, javadoc will raise the following error:
>> 
>> 
>> % javadoc --version
>> javadoc 21
>> 
>> % javadoc HelloWorld.java -d /tmp/throwaway
>> Loading source file HelloWorld.java...
>> HelloWorld.java:2: error: unnamed classes are a preview feature and are disabled by default.
>> void main() {
>> ^
>>   (use --enable-preview to enable unnamed classes)
>> 1 error
>> 
>> 
>> The second option, `-package`, tells javadoc to document classes that are public, protected, or declared with package access (colloquially known as "package private"). Without this option, javadoc will only document public and protected classes, which do not include the interpreted class:
>> 
>> 
>> % javadoc --enable-preview --source=21 HelloWorld.java -d /tmp/throwaway
>> Loading source file HelloWorld.java...
>> Constructing Javadoc information...
>> error: No public or protected classes found to document.
>> 1 error
>> 
>> 
>> When those additional options are present, javadoc does its job:
>> 
>> 
>> % javadoc --enable-preview --source=21 -package HelloWorld.java -d /tmp/throwaway
>> Loading source file HelloWorld.java...
>> Constructing Javadoc information...
>> Creating destination directory: "/tmp/throwaway/"
>> Building index for all the...
>
> Pavel Rappo has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Utils.java line 315:

> 313: 
> 314:     /*
> 315:      * If a similar query is added to javax.lang.model, use that instead.

The `Trees` API would be a better place than `java.lang.model` since this is just a query on an AST node, not on an `Element`.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Utils.java line 324:

> 322:         return (classDecl.mods.flags & com.sun.tools.javac.code.Flags.IMPLICIT_CLASS) != 0;
> 323:     }
> 324: 

Generally, this should all be in the `Workarounds` class, which is the intended enclosure for any code that needs to access `javac` internals that is not directly available via any public API.

Alternatively, you could put this now into `Trees` (not `java.lang.model`, since this is an AST-only method, not an `Element`-based method.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/Checker.java line 105:

> 103: import jdk.javadoc.internal.doclint.HtmlTag.ElemKind;
> 104: import static jdk.javadoc.internal.doclint.Messages.Group.*;
> 105: import jdk.javadoc.internal.doclets.toolkit.util.Utils;

This (ugly) dependency would go away if you put that new method into the `Trees` API.

test/langtools/jdk/javadoc/doclet/testImplicitlyDeclaredClasses/TestImplicitlyDeclaredClasses.java line 61:

> 59:             for (Method otherMethod : otherMethods()) {
> 60:                 var methods = List.of(main, otherMethod);
> 61:                 String index = String.valueOf(i++);

Why `String`, not `var` ?

test/langtools/jdk/javadoc/doclet/testImplicitlyDeclaredClasses/TestImplicitlyDeclaredClasses.java line 117:

> 115:                 // is generated, but the warning for the first method is not.
> 116:                 // Numbers are equal, test passes.
> 117:                 checking("");

Better to give a non-empty string identifying the nature of the check

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16853#discussion_r1409903080
PR Review Comment: https://git.openjdk.org/jdk/pull/16853#discussion_r1409882607
PR Review Comment: https://git.openjdk.org/jdk/pull/16853#discussion_r1409883559
PR Review Comment: https://git.openjdk.org/jdk/pull/16853#discussion_r1409885485
PR Review Comment: https://git.openjdk.org/jdk/pull/16853#discussion_r1409887159