RFR: 8202617: javadoc generates broken links to undocumented (e.g. private) members [v2]

[Hannes Wallnöfer]

On Thu, 31 Oct 2024 13:18:44 GMT, Nizar Benalla <nbenalla at openjdk.org> wrote:

>> Please review this patch to prevent links to private and package-private members to be generated.
>> The bug happens when you link to private/package-private members, and javadoc used to generated links to them (assuming they were inherited because the holder is unreachable).
>> 
>> Taking the code path I changed is very rare, as it only used by 4 anchors in 4 classes in all the JDK.
>> 
>> if (refSignature.trim().startsWith("#") &&
>>                     ! (utils.isPublic(containing) || utils.isLinkable(containing))
>> 
>> 
>> The classes that used it are `StringBuilder`/`StringBuffer` with `#append(java.lang.String)` and `ZipEntry`/`ZipOutputStream` with `#CENHDR`
>> 
>> 
>> I've expanded the test to check whether the links are created when they should be.
>> 
>> The generated documentation before and after the change are identical.
>
> Nizar Benalla has updated the pull request incrementally with one additional commit since the last revision:
> 
>   whitespace

Sorry, my initial feedback was wrong. `utils.isIncluded` is indeed not what we want to use, because `refMem` and `containing` are in fact not included (which is why we want to link to the inherited documentation in the current class). 

So I had to refresh my knowledge of [doclet terminology], because what we want to know is whether the referenced member is _selected_ rather than _included_. In other words, we want to check if selection control allows the member to be documented in the current class.

I'm adding an inline comment with the change I suggest to take this into account.

[doclet terminology]: https://docs.oracle.com/en/java/javase/23/docs/api/jdk.javadoc/jdk/javadoc/doclet/package-summary.html#terminology-heading

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/taglets/LinkTaglet.java line 253:

> 251:             if (refSignature.trim().startsWith("#") &&
> 252:                     ! (utils.isPublic(containing) || utils.isLinkable(containing)) &&
> 253:                     ! (utils.isPrivate(refMem) || utils.isPackagePrivate(refMem))) {

While this fixes the problem in the common case, it hard-codes the accessibility level for linked members to protected and public, and also prevents us from printing a warning for private/package private members a few lines below.

What I suggest to do instead of this is to add a check if `refMem` is **selected** in line 257/258: 

    if (utils.configuration.docEnv.isSelected(refMem) 
            && htmlWriter instanceof ClassWriter cw) {

This takes care of the selected access level (`-private`, `-package`, `-protecdted` etc) and also should cause a warning to be generated for unselected private members.

test/langtools/jdk/javadoc/doclet/5093723/T5093723.java line 78:

> 76:                 }
> 77:                 """);
> 78: 

Kudos for "modernizing" the test!

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

Changes requested by hannesw (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/21802#pullrequestreview-2415876139
PR Review Comment: https://git.openjdk.org/jdk/pull/21802#discussion_r1829502655
PR Review Comment: https://git.openjdk.org/jdk/pull/21802#discussion_r1829524202