RFR: JDK-8075778: Add javadoc tag to avoid duplication of return information in simple situations. [v6]

[Jonathan Gibbons]

On Tue, 8 Dec 2020 13:14:19 GMT, Pavel Rappo <prappo at openjdk.org> wrote:

>> Jonathan Gibbons 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. The pull request contains 12 additional commits since the last revision:
>> 
>>  - Merge remote-tracking branch 'upstream/master' into new-return
>>  - Update JShell to handle inline `{@return}`
>>  - Merge remote-tracking branch 'upstream/master' into new-return
>>  - fix test
>>  - Update for new `@return` tag
>>  - Merge remote-tracking branch 'upstream/master' into new-return
>>  - Update DocCommentParser to permit nested inline tags in specified cases: @return
>>  - Add default impl for new method
>>  - Fix test failure
>>  - Fix trailing whitespace in test
>>  - ... and 2 more: https://git.openjdk.java.net/jdk/compare/c3337822...a25dadca
>
> src/jdk.compiler/share/classes/com/sun/source/util/DocTreeFactory.java line 277:
> 
>> 275:      * @param description the description of the return value of a method
>> 276:      * @return a {@code ReturnTree} object
>> 277:      * @throws UnsupportedOperationException if inline {@code {@return} } tags are
> 
> To be consistent with the rest of the file, I suggest using `{@code {@return }}` instead of `{@code {@return} }` (note the position of the whitespace).

fixed

> src/jdk.compiler/share/classes/com/sun/tools/javac/parser/DocCommentParser.java line 275:
> 
>> 273:                         return tp.parse(p, TagParser.Kind.BLOCK);
>> 274:                     } else {
>> 275:                             return erroneous("dc.bad.inline.tag", p);
> 
> This indentation fits the current `switch` but not the proposed `if` statement.

fixed

> src/jdk.compiler/share/classes/com/sun/source/doctree/ReturnTree.java line 44:
> 
>> 42:      * Returns whether this instance is an inline tag.
>> 43:      *
>> 44:      * @return {@code true} if this instance is an inline tag
> 
> Although I'm not sure how `boolean` queries are generally specified in the `DocTree` API, consider explicitly specifying that this method returns `false` if called on a non-inline instance.

hmm, OK, seems a bit redundant but will add `and {@code false} otherwise`

> src/jdk.compiler/share/classes/com/sun/tools/javac/parser/DocCommentParser.java line 333:
> 
>> 331:                     DCTree text = inlineText(WhitespaceRetentionPolicy.REMOVE_ALL); // skip content
>> 332:                     nextChar();
>> 333:                     return m.at(p).newUnknownInlineTagTree(name, List.of(text)).setEndPos(bp);
> 
> There's a discrepancy between the inline comment and code. While the comment says that the `else` clause handles a block tag, the returned tree corresponds to an (unknown) inline tag.

The code is correct, and even the comment: which refers to "block tags in inline content".
The case being handled is one like this:

`
    /**
     * This is a bad comment. {@see Object} or {@since 7}.
     */
`

> src/jdk.compiler/share/classes/com/sun/tools/javac/parser/DocCommentParser.java line 1132:
> 
>> 1130: 
>> 1131:     /**
>> 1132:      * @see <a href="https://docs.oracle.com/en/java/javase/14/docs/specs/javadoc/doc-comment-spec.html">JavaDoc Tags</a>
> 
> Consider updating the version to 15. Later, we'd hopefully be able to use the [`@spec` tag](https://bugs.openjdk.java.net/browse/JDK-8226279) here.

done

Yes, this could eventually be a use of the proposed `@spec` tag with a relative URL

> src/jdk.compiler/share/classes/com/sun/tools/javac/tree/DCTree.java line 690:
> 
>> 688: 
>> 689:         @Override
>> 690:         public String getTagName() {
> 
> Isn't this method missing `@DefinedBy(Api.COMPILER_TREE)`?

Oops, yes; fixed; I guess I'm mildly surprised the checker didn't catch it, but that's a different issue

> src/jdk.compiler/share/classes/jdk/internal/shellsupport/doc/JavadocFormatter.java line 268:
> 
>> 266:         /**
>> 267:          * {@inheritDoc}
>> 268:          * {@code @return} is a bimodfal tage and can be used as either a block tag or an inline
> 
> Fix the typos in "bimodfal tage".

Oops, also fixed an instance of `null` to `{@code null}`

> test/langtools/jdk/internal/shellsupport/doc/JavadocFormatterTest.java line 140:
> 
>> 138:                    "1234 1234 1234 1234 1234 1234 1234 1234 1234 1234 1234 1234 1234\n" +
>> 139:                    "1234 1234 1234 1234 1234 1234 1234 1234 1234 1234 1234 1234 1234\n" +
>> 140:                    "1234 1234 1234 1234 1234 1234 1234 1234 1234 \n";
> 
> What's that about? I mean is it related to this work on `{@return}`?

It was needed to fix a test failure

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

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