Hannes,
Thanks for looking at this, and for your questions. Answers inline.
-- Jon
On 07/20/2018 08:24 AM, Hannes Wallnöfer wrote:
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.
Yes, it's not well-defined, at least not in any moderately formal
documentation. This is another area in which javadoc uses and reuses
low-level constructs without wrapping them in higher level abstractions.
The point about this just being a partial fix was because I see the new
method isLinkable to be the place to encapsulate what it means to be
linkable, where "linkable" means "the ability the provide a non-broken,
clickable link to a useful and relevant target." As we investigate more
broken links in the docs, we may see the new method being enhanced to
accommodate that cases.
Specific to your webrev, why are members of undocumented super types considered
linkable?
This is another undocumented "quirk" of javadoc, that is reasonable in
hindsight when you think about it. If a package-private superclass has
public methods, those public methods can be invoked by a client using a
public subtype. The question then becomes, where should those methods be
documented? If the immediately enclosing type is undocumented, then the
first public subtype becomes a "reasonable" place to document the
methods. Maybe there are other "reasonable" ways to present the
information, but this is the current (long-term) behavior of the doclet.
Kumar found this behavior (the hard way) when writing the new
VisibleMemberTable, which replaces the old VisibleMemberMap. This sort
of behavior is the primary reason for comparing "before" and "after"
versions of generate docs when replacing or updating javadoc internals.
"If you don't think you're making any changes to the output, verify
you're not making any changes to the output!"
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.
Most links on the serialized form page are OK. Serialization is "weird"
because it includes the doc for private fields, and in a number of
places, the doc comment on the private fields contains @see references
to other private members which are not being documented. It is those
links which were broken and which now should not be broken, at least
from a clickable-link perspective. It is still "not great" that the
doclet posts an (unlinked) reference to an undocumented member. It
could be an RFE to improve that, but while I can imagine better behavior
of @see links, I have no good suggestion for the equivalent inline
{@link} tags.
Here is an example of the broken link:
In the JDK 10 API docs, look at the Serialized form for java.awt.MenuItem:
https://docs.oracle.com/javase/10/docs/api/serialized-form.html#java.awt.MenuItem
Look at the last entry in the See Also list:
|MenuItem.writeObject(ObjectOutputStream)|
<https://docs.oracle.com/javase/10/docs/api/java/awt/MenuItem.html#writeObject%28java.io.ObjectOutputStream%29>
If you click on it you go to the page for MenuItem, but because there is
no id for the writeObject method, you just go to the top of the page.
Now look at the corresponding page in the docs with this fix:
http://cr.openjdk.java.net/~jjg/8207214/api.00/serialized-form.html#java.awt.MenuItem
Look at that same last entry again. This time, it is just plain text,
not linked.
Arguably, a better solution for a followup RFE would be to link the
class name (because the class is documented) but not the member name.
But that is a more tricky fix.
-- Jon
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
