Hi Jon, I don’t quite all aspects of this fix, which is why I didn’t send a review in the first place. But since nobody else did either, I’ll just ask a few questions to help me better understand.
My main area of confusion is related to what is considered linkable and what not. This should be well-defined, but from your description below (saying this is just a partial fix) it doesn’t seem to be. Specific to your webrev, why are members of undocumented super types considered linkable? I also looked at the serialised form page of the current API docs and didn’t find the broken links mentioned in the JIRA issue, so I wasn’t able to observe the impact of the fix. Apart from that, the webrev looks good to me. Hannes > Am 13.07.2018 um 02:16 schrieb Jonathan Gibbons <[email protected]>: > > Please review a small but slightly tricky fix related to broken links on the > serialized-form page. > > The underlying problem was that the doclet was generating bad links when an > @see tag > was referencing a member that was not itself being documented. For example, > the following > will normally cause a broken link because privateMethod will not normally be > included in the > documentation. > > public class C { > /** Text. > * @see #privateMethod() > */ > public void publicMethod() { } > > private void privateMethod() { } > } > > > There is a spectrum of possible solutions, some more complicated than others. > The approach here is to fix the perceived original intent of the code which > is to detect > when the link will not be generated, and in that case, just write the label, > without > any enclosing link. > > This is done by creating and using a new method in Utils that carefully tries > to determine > whether the link should be generated. It's not ideal, and may need to be > refined in due course, > as we identify more test cases, but it is arguably "good enough" for now, in > that all existing > tests continue to pass, as well as some new test cases that mimic the problem > as originally > discovered. It is possible that we might be able to simplify this code, in > time, by using > information in the recently-new VisibleMemberTable. > > With regard to the original problem of broken links on the serialized-form > page in the > JDK API, the number of broken links has gone down by about 15%. > > With respect to the webrev, there is some minor cleanup, but primarily, the > expression > on HtmlDocletWriter:916 has been replaced by a call to a new method in Utils, > that > provides a more nuanced determination of whether the link should be generated > or not. > > The new test creates various test scenarios and runs them through javadoc. It > is worth > noting that by default *all* javadoc tests using JavadocTester now run a link > checker > (checkLinks) unless it is explicitly disabled, and that ensures no > regressions in the > output generated in any other of the javadoc tests. > > JBS: https://bugs.openjdk.java.net/browse/JDK-8207214 > Webrev: http://cr.openjdk.java.net/~jjg/8207214/webrev.00/index.html > > -- Jon >
