RFR: JDK-8290243: move seeTagToContent from HtmlDocletWriter to TagletWriterImpl [v2]

[Jonathan Gibbons]

On Tue, 19 Jul 2022 12:48:39 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 pull request now contains six commits:
>> 
>>  - Merge with upstream/master; resolve conflict
>>  - Replace `linkToContent` by using the new `linkSeeReferenceOutput` method
>>  - Further split seeTagToContent into linkTagOutput and seeTagOutput
>>  - Split seeTagToContent in two: a front end and a back end
>>  - Fix typo in test
>>  - Move seeTagToContent, linkToContent from HtmlDocletWriter to TagletWriterImpl
>
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java line 1353:
> 
>> 1351:                         content.add(label);
>> 1352:                     } else {
>> 1353:                         TagletWriterImpl t = getTagletWriterInstance(context.within(node));
> 
> Separate to this PR: when I see a cast to or a variable declaration of the `TagletWriterImpl` type, I cannot help but wonder why most of the methods in `TagletWriter` are protected. Do you have any idea how we ended up like that?

My reverse-engineered understanding is that using `protected` allows the methods to be overridden/implemented, but discourages direct calling, in favor of using `get[Block|Inline]TagOutput`, which is the way to get to those tag-specific methods. For better or worse, that does allow the built-in behavior to be modified.

The whole `TagletWriter` system is somewhat questionable, but revising it would be something of a Big Deal, and does not seem like the most important issue to deal with at this time.

> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/TagletWriterImpl.java line 471:
> 
>> 469:                                    String tagName, boolean isLinkPlain,
>> 470:                                    Content label,
>> 471:                                    BiConsumer<String, Object[]> reportWarning) {
> 
> This method is huge: it has 8 parameters and spans 120 lines. We should refactor it in a later PR. For now, consider the below suggestion that changes access level, addresses a few doc typos, and formats parameters one per line.
> 
> Suggestion:
> 
>     /**
>      * Worker method to generate a link from the information in different kinds of tags,
>      * such as {@code {@link ...}} tags, {@code @see ...} tags and the {@code link} markup tag
>      * in a {@code {@snippet ...}} tag.
>      *
>      * @param holder        the element that has the documentation comment containing the information
>      * @param refTree       the tree node containing the information, or {@code null} if not available
>      * @param refSignature  the normalized signature of the target of the reference
>      * @param ref           the target of the reference
>      * @param tagName       the name of the tag in the source, to be used in diagnostics
>      * @param isLinkPlain   {@code true} if the link should be presented in "plain" font,
>      *                      or {@code false} for "code" font
>      * @param label         the label for the link,
>      *                      or an empty content to use a default label derived from the signature
>      * @param reportWarning a function to report warnings about issues found in the reference
>      *
>      * @return the output containing the generated link, or content indicating an error
>      */
>     private Content linkSeeReferenceOutput(Element holder,
>                                            DocTree refTree,
>                                            String refSignature,
>                                            Element ref,
>                                            String tagName,
>                                            boolean isLinkPlain,
>                                            Content label,
>                                            BiConsumer<String, Object[]> reportWarning) {

Yes, it's a big method; my defense is that it is smaller than before!

I'm open to further refactoring if anything obvious becomes apparent.

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

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