RFR: JDK-8290243: move seeTagToContent from HtmlDocletWriter to TagletWriterImpl

[Pavel Rappo]

On Thu, 14 Jul 2022 19:44:22 GMT, Jonathan Gibbons <jjg at openjdk.org> wrote:

> Please review a pure-cleanup change to move refactor code from `HtmlDocletWriter` to `TagletWriterImpl`.
> 
> The methods in question are `seeTagToContent`, and `linkToContent`, which is a partial clone of `seeTagToContent` introduced to handle the `link` markup tag in snippets.
> 
> Despite the name, `seeTagToContent` handles both `@see` and `{@link ...}` tags, both of which have a more natural home in `TagletWriterImpl`.
> 
> The change is performed in a series of steps/commits:
> 
> 1. The methods `seeTagToContent` and `linkToContent` are moved from `HtmlDocletWriter` to `TagletWriterImpl`, with minimal fixup, such as imports, and either qualifying access to members of `HtmlDocletWriter`, or removing qualifiers to members of `TagletWriterImpl`. And, spoiler alert: `linkToContent` eventually goes away altogether.
> 
> 2. An unrelated annoying typo in a test was found and fixed. The copyright date is left unchanged and the test is not marked with the bug number for the issue.
> 
> 3. `seeTagToContent`, now in `TagletWriterImpl` was split in two, into a "front end", to handle the differences between `@see` and `{@link...}` tags, and a "back end", to handle the common behavior. The API for the back end was informed by the needs of `linkToContent`. Note: there are later refinements to the API for the back end.
> 
> 4. The "front end" method, `seeTagToContent` is further split into two separate methods for `{@link ...}` tags and `@see` tags, eliminating the existing ugly `switch` statement. At this time, an "impossible" case was removed from `HtmlDocletWriter.commentTagsToContent`.  The method is for handling text and inline tags, so the `visitSee` method to handle an isolated `@see` tag can never be called in practice. The method is deleted. A future improvement would be to verify that there are no block tags in  the argument list passed to `commentTagsToContent`. Somewhat surprising, this is not a trivial check, and so not done here at this time. It would maybe help to have a new method `isBlockTag` on `DocTree` but that would be a public API changed nd out of scope for this work.
> 
> 5. Finally, the signature of the "back end" method, now called `seeLinkReferenceOutput`, is adjusted, and used in `snippetTagOutput` instead of using the original `linkToContent`, which was a partial clone of `seeTagToOutput`, and which can now be deleted.
> 
> At this point, it is worth noting that all tests pass without change (the typo fix is unrelated) and the generated API docs  before and after these changes compare the same (apart from the obvious minor differences in timestamps, etc).
> That being said, there was notable difference in one line of code in `linkToContent` and the corresponding line in `seeTagToContent`.  It corresponds to the `TODO` in the original `HtmlDocletWriter` line 1201.
> 
> 
> // TODO:
> //                  TypeMirror refType = ch.getReferencedType(target);
>                     TypeMirror refType = target.asType();
> 
> 
> The change is related to not having a `DocTree target` available in the snippet world. and so the code uses the `TypeMirror` of the referenced element.  If that change is made in the new code, references to raw types, such as `List` get generated with their type parameters, such as `List<T>` ... that's good ... but if the reference in the `DocTree` provided type arguments, such as `List<String>`, then the arguments were lost and the type parameters were given instead ... that list, `List<T>` ... and that's bad. The solution may be to use a hybrid solution ... if the reference in the source contains type arguments, show them, otherwise if the reference is to a raw type, show the type parameters. However, any such solution will require new tests and will affect a number of existing tests, and so is better left to a separate update. The fix for now is just to make the use of the `DocTree` reference optional, so that it need not be supplied when `linkSeeReferenceOutput` is called from 
 `snippetTagOutput`.

I like the idea, but I have to spend more time with this PR. And we have time, since we're waiting for the conflicting change (JDK-8281969) to be forward-ported from JDK 19.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java line 1682:

> 1680:                     return false;
> 1681:                 }
> 1682: 

Did you remove this method because the enclosing method, `commentTagsToContent`, is called only for a series of inline tags, which `@see` is not?

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/TagletWriterImpl.java line 449:

> 447:         TypeElement refClass = ch.getReferencedClass(ref);
> 448:         Element refMem =       ch.getReferencedMember(ref);
> 449:         String refMemName =    ch.getReferencedMemberName(refSignature);

Why is it that `refMemName` is now derived from the signature rather than from the node?

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/taglets/TagletWriter.java line 119:

> 117:      * @return the output
> 118:      */
> 119:     protected abstract Content linkTagOutput(Element element, LinkTree tag);

Finally, a separate method for `{@link}` and `{@linkplain}` tags!

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

PR: https://git.openjdk.org/jdk/pull/9499