On Thu, 1 Jul 2021 13:50:25 GMT, Hannes Wallnöfer <hann...@openjdk.org> wrote:

> This change adds support for generating HTML links to the type arguments of 
> enclosing classes when creating a link to an inner class. Previously, only a 
> link to the inner class was created and the type arguments were even omitted 
> from the link label.
> 
> The new feature to create separate links to the enclosing class and its type 
> arguments is only activated if the enclosing class has type arguments. If the 
> enclosing class is non-generic, the old behavior is preserved to create a 
> single link to the inner class. The reason for this is that a dedicated link 
> to the enclosing class itself provides little benefit, since it can be easily 
> reached via the "Enclosing class" link of the inner class. Also, linking the 
> enclosing type in absence of type arguments makes it hard to see that there 
> are two links and easy to click on the wrong link by mistake.
> 
> On the other hand, for type arguments a separate link should be useful since 
> it is often not a "nearby" type. It is also easier to detect the different 
> links than for non-generic nested classes. I came to like this "mixed" 
> solution best after trying several other approaches.

What is the type arguments are themselves type variables: e.g. `@see Map<P,Q>`

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

> 1080:             // Must be a class reference since refClass is not null and 
> refMemName is null.
> 1081:             if (labelContent.isEmpty()) {
> 1082:                 if (seeText.contains("<")) {

Hmmm, string-based checks are generally something of a red-flag, as compared to 
more abstract methods on appropriate classes. And, this is a string operation 
on the output of  `CommentHelper.getText`. Is there a way that we have may this 
a more robust test ... for example introduce 
`CommentHelper.isGeneric(ReferenceTree)` or something like that?

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/links/LinkInfo.java
 line 132:

> 130:         // from the documentation of an inner class, so rendering it as 
> separate link would
> 131:         // add little benefit but add considerable noise, while type 
> arguments may not
> 132:         // otherwise be reachable from the documentation of the inner 
> class.

Do you render the _enclosing types_ as separate links, or just  the _type 
arguments of enclosing types_?

Even if the enclosing type has type arguments, your argument about the noise 
from linking the name of the enclosing type is still valid.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/links/LinkInfo.java
 line 154:

> 152:         } else if (isLinkable()) {
> 153:             Content tlabel = newContent();
> 154:             tlabel.add(type instanceof DeclaredType dt && 
> linkEnclosingTypes(dt)

There's a philosophical discussion to be had, separately, with Joe, about using 
`instanceof` with Language Model types. since the beginning, i`nstanceof` has 
been discouraged in favor of using `getKind()` but the reasons for that may 
have faded, and "instanced patterns" may be enough of a reason to change the 
guidelines.

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

PR: https://git.openjdk.java.net/jdk17/pull/195

Reply via email to