RFR: 8304135: Introduce unified overridden methods query [v2]

Fri Jun 2 12:41:07 UTC 2023

On Fri, 2 Jun 2023 12:10:45 GMT, Pavel Rappo <prappo at openjdk.org> wrote:

>> Please review this change.
>> 
>> It turns out that JavaDoc's model for overriding and inheritance for methods is simplistic. As a result, it fails to accommodate upcoming changes in the "method comments algorithm" and the new feature called "directed documentation inheritance".
>> 
>> This change aligns JavaDoc's model for overriding and inheritance for methods with that of the Java Language Specification. The most important part of the change pertains to overridden methods. The model now recognizes that "overrides" is not a transitive relation: from the fact that A overrides B and B overrides C, it does not necessarily follow that A overrides C. The model also recognizes that "overrides" depends on three parameters, not two: the method that overrides, the method that is being overridden, and the class where the override takes place. For details, see the test that this change adds: `TestMethodMembers.java`.
>> 
>> Additionally, the change provides a unified query for method overrides: be it defined in a class or interface, be it abstract or concrete, an overridden method can be queried through a single new API method in `VisibleMemberTable`: 
>> 
>>     public OverrideSequence overrideAt(ExecutableElement method)
>> 
>> That query accepts a method and returns a sequence of methods that the accepted method overrides from the class or interface described by the instance of `VisibleMemberTable` that the query has been called on. That sequence is totally ordered and can be traversed in either direction.
>> 
>> It's quite remarkable that there are only 2 existing tests that required some adjustment after the change. However, there are approximately 250 changed files in the generated JDK API Documentation (i.e. the result of `make docs`). Most of them are benign or beneficial (e.g. they fix a bug). However, there is at least one that will need to be fixed after the upcoming but separate "directed documentation inheritance" update: `java.util.concurrent.LinkedBlockingDeque`.
>> 
>> With this change, it takes the same time (as measured with `TIME(1)`) to run the tests and produce JDK API documentation. Proper benchmarks will be done later on.
>> 
>> ---
>> 
>> _Meanwhile, I'll try to figure out a convenient way to attach diffs for JDK API Documentation to this PR._
>
> Pavel Rappo 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 five additional commits since the last revision:
> 
>  - Merge branch 'master' into 8304135
>  - Extract getActualMethod
>  - Impose (almost) legacy order on implemented methods
>    
>    The legacy order is generated by an application of
>    Utils.overriddenMethod followed by application of
>    Utils.addSuperInterfaces.
>  - Fix errors reported by jcheck
>  - Initial commit

This comment describes changes to approximately 250 files in the generated JDK API Documentation which are mentioned in this PR description. Since the publication of this PR, I've been able to shave off a few changes that I previously categorized as non-benign.

As a sidenote, while analysing the changes to the JDK API Documentation, I once again saw how brittle and sometimes non-intuitive the automatic documentation inheritance algorithm is. No mater the search order that the algorithm imposes, some results are surprising. The published documentation for the current JDK likely contains many unnoticed bugs of the type "Gee, why does it inherit this text instead of that?". I've seen a few of those, but won't bring them into this discussion not to muddy the waters further. Hopefully, soon we'll have directed documentation inheritance, which will enable fixing some of those immediately and the rest of those in principle.

Back to the PR. Here are the changed files:

 * https://cr.openjdk.org/~prappo/8304135/before/api/
 * https://cr.openjdk.org/~prappo/8304135/after/api/

The majority of those changes can be described as an improvement to the following consistency aspect: a method should appear either in the "Method Summary" and "Method Details" sections, or in one or more "Methods declared in ..." lists. This aspect corresponds to the fact that a method can either be declared or inherited, but never both. Declared methods are summarised and detailed, while inherited methods are merely mentioned.

If a method is mentioned in N > 1 "Methods declared in ..." lists, these are actually N different methods. The Java Language Specification allows a class or interface to inherit multiple abstract methods whose signatures are override-equivalent. In JDK API it rarely happens: I can see fewer than 10 cases like that. One case is AbstractList which inherits two abstract `size` methods: [one] from AbstractCollection (which in turn inherits it from Collection) and the [other] from List.

Here are representative examples for some kinds of changes in those files:

1. DataOutputStream.write(byte[] b): a previously unrepresentable simple override via inheritance. It now only appears in "Methods declared in class java.io.FilterOutputStream". In particular, it does not appear in "Methods declared in interface java.io.DataOutput" or "Method Details".
2. PushbackReader.close overrides FilterReader.close, which is a non-simple override of Reader.close because Reader is abstract and FilterReader isn't. That was a bug.
3. "Specified by" and "Overrides" related to generic methods in java.lang.Class, java.time.chrono.ChronoLocalDateTime and java.time.chrono.ChronoZonedDateTime now correctly refer to wildcard `?` instead of the generic parameter.
4.  StringBuffer's chars() and codePoints() are no longer mentioned in "Methods declared in interface java.lang.CharSequence" as they are really taken from AbstractStringBuilder, an "undocumented enclosure", where they override corresponding methods declared in CharSequence.
   That mechanics also applies to java.time.chrono.HijrahDate's methods `toString`, `until` and their counterparts in java.time.chrono.ChronoLocalDateImpl.
5. Inline links to `#length()` in StringBuffer now correctly refer to the local method, not the `CharSequence.html#length()` supermethod.
6. StringBuffer.toString now correctly mentions that it overrides Object.toString. Previously it didn't mention any override because its immediate superclass, AbstractStringBuilder, is an "undocumented enclosure".
7. Finally, there are changes to class-use and index files, mostly related to the above changes in documentation.

[one]: https://cr.openjdk.org/~prappo/8304135/after/api/java.base/java/util/AbstractList.html#methods-inherited-from-class-java.util.Collection
[other]: https://cr.openjdk.org/~prappo/8304135/after/api/java.base/java/util/AbstractList.html#methods-inherited-from-class-java.util.List

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

PR Comment: https://git.openjdk.org/jdk/pull/14221#issuecomment-1573670298