RFR: 8177280: @see {@link} syntax should allow generic types

[Jonathan Gibbons]

HtmlDocletWriter:1044

Changing RawHtml to StringContent is a significant behavioral change. 
It's not explicitly stated in the doc comment spec whether the text may 
contain HTML, but it is reasonable to infer from the descriptions of 
{@code} and {@literal} that it may be HTML content.  Is this change 
necessary?

CommentHelper ... clever changes, I think ;-)

In the new test, the direct/explicit use of 
https://docs.oracle.com/en/java/javase/12/docs/api/ may cause issues 
later, in that "sanity scripts" may discover the string and wonder if it 
is an out of date reference. At a minimum, the use of the URL should be 
significantly commented in the test.  But, despite the comment on the 
`testValidLinks` method, I don't think it actually checks that the links 
exist (as compared to 404-Not Found).  Since you are using -linkoffline, 
it may be sufficient to just do a global replace of

	https://docs.oracle.com/en/java/javase/12/docs/api

to

	http://example.com/docs/api

or something like that.

-- Jon

On 02/18/2020 03:26 AM, Hannes Wallnöfer wrote:
> I have updated the patch to recent changes in CommentHelper, no changes otherwise.
>
> http://cr.openjdk.java.net/~hannesw/8177280/webrev.01/
>
> Hannes
>
>> Am 07.02.2020 um 19:34 schrieb Hannes Wallnöfer <hannes.wallnoefer at oracle.com>:
>>
>> Please review:
>>
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8177280
>> Webrev: http://cr.openjdk.java.net/~hannesw/8177280/webrev.00/
>>
>> As I said previously there are some things in this patch I’m unsure or not quite happy about.
>>
>> Some notes:
>>
>> - While the implementation of the new DocTrees method is quite simple, I’m not sure if I should install a new Log.DeferredDiagnosticHandler for the runtime of the method, like the attributeDocReference method in the same class does.
>>
>> - In HtmlDocletWriter.seeTagToContent I changed the handling of the tag text to escape HTML characters if no label is specified. This causes „<„ and „>“ to be displayed in the browser instead of being interpreted as HTML tag if a generic link target can’t be resolved. This is potentially problematic since @see and @link can contain plain HTML content, but I think it’s ok since those cases are handled further up in HtmlDocletWriter.seeTagToContent.
>>
>> - That same method in HtmlDocletWriter contained some never-used code (dependent on the BaseConfiguration.backwardCompatibility flag which is always true) to use the fully qualified class name for links in some cases. I removed that code and the field in BaseConfiguration since it adds to that already long-winding method.
>>
>> - Setting the label on a LinkInfoImpl basically disables rendering of type arguments. While I understand the rationale behind this, it might still be useful to set the label for the main link of a generic type. I’ve tried to remove the restriction, but ran into a lot of problems (i.e. failing tests) in other places. Since the current behaviour does what we need I decided to not change this.
>>
>> - There was a potential infinite recursion in LinkFactoryImpl.getTypeParameterLinks caused by following the type parameters of linkInfo.typeElement when linkInfo.type is set. The problem was that linkInfo.typeElement may be set to the owner of linkInfo.type, and the solution is to only follow that path if linkInfo.type is null.
>>
>> - I removed two unused LinkInfo Kind enum values.
>>
>> - I CommentHelper there were previously two and now three Visitors to extract ReferenceTrees, so I added a common base class called ReferenceDocTreeVisitor.
>>
>> - As an completely unrelated cleanup I removed the unused javafx field in IndexBuilder.
>>
>> Thanks,
>> Hannes