[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-2d-dev
Subject:    Re: RFR: 8294377: Prepare to stop auto-inheriting documentation for subclasses of exceptions whose d
From:       Phil Race <prr () openjdk ! org>
Date:       2022-09-30 19:17:22
Message-ID: PlQjfMvDRNRyYFL9xVnKoirPgtOk_QhEMl_QbNRnzrU=.0b472944-6d3a-48f2-bc79-ec592df83c79 () github ! com
[Download RAW message or body]

On Tue, 27 Sep 2022 12:14:23 GMT, Pavel Rappo <prappo@openjdk.org> wrote:

> > This adds exception documentation to JDK methods that would otherwise lose that \
> > documentation once JDK-8287796 is integrated. While adding this exception \
> > documentation now does not change [^1] the JDK API Documentation, it will \
> > automatically counterbalance the effect that JDK-8287796 will have. 
> > JDK-8287796 seeks to remove an old, undocumented, and esoteric javadoc feature \
> > that cannot be suppressed [^2]. The feature is so little known about that IDEs \
> > either do not implement it correctly or do not implement it at all, thus \
> > rendering documentation differently from how the javadoc tool renders it. 
> > That feature was introduced in JDK-4947455 and manifests as follows. If a method \
> > inherits documentation for an exception, along with that documentation the method \
> > automatically inherits documentation for all exceptions that are subclasses of \
> > that former exception and are documented in an overridden method. 
> > To illustrate that behavior, consider the following example. A method `Subtype.m` \
> > inherits documentation for `SuperException`, while the overridden method \
> > `Supertype.m` documents `SuperException`, `SubExceptionA` and `SubExceptionB`, \
> > where the latter two exceptions are subclasses of the former exception: 
> > public class Subtype extends Supertype {
> > 
> > @Override
> > public void m() throws SuperException {
> > ...
> > 
> > public class Supertype {
> > 
> > /**
> > * @throws SuperException general problem
> > * @throws SubExceptionA  specific problem A
> > * @throws SubExceptionB  specific problem B
> > */
> > public void m() throws SuperException {
> > ...
> > 
> > public class SuperException extends Exception {
> > ...
> > 
> > public class SubExceptionA extends SuperException {
> > ...
> > 
> > public class SubExceptionB extends SuperException {
> > ...
> > 
> > For the above example, the API Documentation for `Subtype.m` will contain \
> > documentation for all three exceptions; the page fragment for `Subtype.m` in \
> > "Method Details" will look like this: 
> > public void m()
> > throws SuperException
> > 
> > Overrides:
> > m in class Supertype
> > Throws:
> > SuperException - general problem
> > SubExceptionA - specific problem A
> > SubExceptionB - specific problem B 
> > 
> > It works for checked and unchecked exceptions, for methods in classes and \
> > interfaces. 
> > 
> > If it's the first time you hear about that behavior and this change affects your \
> > area, it's a good opportunity to reflect on whether the exception documentation \
> > this change adds is really needed or you were simply unaware of the fact that it \
> > was automatically added before. If exception documentation is not needed, please \
> > comment on this PR. Otherwise, consider approving it. 
> > Keep in mind that if some exception documentation is not needed, **leaving it \
> > out** from this PR might require a CSR. 
> > 
> > [^1]: Aside from insignificant changes on class-use and index pages. There's one \
> > relatively significant change. This change is in jdk.jshell, where some methods \
> > disappeared from "Methods declared in ..." section and other irregularities. We \
> > are aware of this and have filed JDK-8291803 to fix the root cause. [^2]: In \
> > reality, the feature can be suppressed, but it looks like a bug rather than \
> > intentional design. If we legitimize the feature and its suppression behavior, \
> > the model for exception documentation inheritance might become much more \
> > complicated.
> 
> Pavel Rappo has updated the pull request incrementally with one additional commit \
> since the last revision: 
> revert an extraneous change to jdk.javadoc

src/java.desktop/share/classes/javax/imageio/stream/ImageInputStreamImpl.java line \
206:

> 204:      * @throws EOFException {@inheritDoc}
> 205:      */
> 206:     public boolean readBoolean() throws IOException {

If the docs end up the same as you say that is fine by me since for the two ImageIO \
classes that seems to be what we'd want.

But inheritDoc behaviour is still "surprising".
I've never been sure I understood it and now I'm just sure I don't.

1) The two ImageIO methods don't have @override or *anything* and yet javadoc infers
they'd want to inherit the docs from the interface .. clever javadoc .. I guess I can \
see that some doc is better than none so this is OK

2) When it does inherit I'd expected that it copies the EXACT and ENTIRE javadoc
from the super-interface, which for your change to work can't be all its doing.
You've added JUST
/**
 * @throws SomeExceptionSubType blah-blah
 */

and yet javadoc somehow figures out this is to be ADDED to the super-type doc
for the method and not replace it .. ???

3) What the old code was doing seems to me to be "natural" to the extent any of
this does and the fix you are preparing would add to the surprising behaviours ..

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

PR: https://git.openjdk.org/jdk/pull/10449


[prev in list] [next in list] [prev in thread] [next in thread]