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

[Hannes Wallnöfer]

On Thu, 7 Nov 2024 14:44:29 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 with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains four additional commits since the last revision:
> 
>  - check if refMem is selected
>  - Merge remote-tracking branch 'upstream/master' into 8202617-javadoc-generates-broken-links-to-undocumented
>  - whitespace
>  - javadoc no longer generates broken links to undocumented fields
>    expand test
>    (C)

Unfortunately, there is something fishy about the warnings. There shouldn't be a "reference not accessible" warning for the public and protected method links, because these are in fact accessible in the current type. It turns out these warnings are generated when condition `htmlWriter instanceof ClassWriter cw` is `false`, i.e. when we are generating an index or summary page. 

One way of fixing this would be to rewrite the 3-branch if statement in lines 257-268 in the following way:

                if (utils.configuration.docEnv.isSelected(refMem)) {
                    if (htmlWriter instanceof ClassWriter cw) {
                        containing = cw.getTypeElement();
                    }
                } else {
                    reportWarning.accept("doclet.link.see.reference_not_accessible",
                            new Object[] { refMem });
                }

This retains the quirk in the current code that the member is only linked if we are in the class writer. We could get the containing class by getting the type element for `holder`, but IMO the current behaviour is acceptable for such a corner case.

Note that I changed the second argument for the warning to the actual member reference, since that is what is unaccessible. Also note that the original `else` branch is not needed anymore as the warning is always about the member not being accessible/selected.

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

PR Comment: https://git.openjdk.org/jdk/pull/21802#issuecomment-2462777127