RFR: 8280713: Related to comment inheritance jdk.javadoc cleanup and refactoring [v4]

[Jonathan Gibbons]

On Mon, 28 Feb 2022 14:03:15 GMT, Pavel Rappo <prappo at openjdk.org> wrote:

>> Explorative refactoring performed while looking into multiple `@inheritDoc` issues. The easiest way to review it is to, probably, go commit by commit; they are quite focused and commented. Not only the branch as a whole, but all the constituent commits should pass tests and leave JDK API Documentation unchanged.
>
> Pavel Rappo has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 28 commits:
> 
>  - Merge branch 'master' into 8280713
>  - Fix outdated code
>    
>    Uses a set instead of list for quick method search. In a future commit we'll try to figure out why `found` are not unique.
>  - Fix outdated code
>    
>    Fix outdated inline comments and names. Both the assertion and the `contains` method will be removed in a future commit.
>  - Fix ImplementedMethods.overridingMethodFound
>    
>    Removes code from VisibleMemberTable.ImplementedMethods.overridingMethodFound; the removed code is dead for the reasons similar to those in the previous commit. This commit makes overridingMethodFound degrade to linear search, which could be substituted with hashing in a later commit.
>  - Remove ImplementedMethods::removeOverriddenMethod
>    
>    First of all, this commit has been tested and the before and after JDK API Documentation compared.
>    
>    `removeOverriddenMethod` does not seem to do any visible work and only confuses the reader. Here are some problems, all of which has been confirmed while debugging. Hopefully, the bellow bullet points in conjunction with the source code will help you see these problems.
>    
>    Outside:
>    
>    1. `removeOverriddenMethod` is private and is only used in one place: VisibleMemberTable.java:1015
>    2. `VisibleMemberTable` passes `removeOverriddenMethod` with a direct interface method, an interface method that is either defined or overridden by an interface, but not passively inherited.
>    
>    Inside:
>    
>    1. `overriddenClass` is either a class that first defined a method that the passed `method` overrides, or `null` if there's no such class.
>    2. `overriddenClass` is searched for recursively, using only supertypes.
>    3. Regardless of any interfaces it might extend, an interface's supertype is `java.lang.Object`; see, for example, `javax.lang.model.element.TypeElement#getSuperclass`.
>    4. `method` belongs to an interface; see "Outside" (2).
>    5. Given 1-4, the only class `overriddenClass` can be is `java.lang.Object`. This can happen, for example, if `method` is an interface-override of one of the methods defined in `java.lang.Object`. Typically those are `equals`, `hashCode` or `toString`.
>    7. `isSubclassOf(a, b)` checks if `a` is a subtype of `b`.
>    8. Since `a` is `java.lang.Object`, condition `b == a || isSubclassOf(a, b)` can be `true` only if `b` is also `java.lang.Object`. `java.lang.Object` is not a subclass of anything but itself.
>    9. `b` can never be a class, let alone `java.lang.Object`. This is because `b` is an immediately enclosing type of an interface method (by construction of `methlist`), i.e. `b` is an interface.
>    10. So the above check always fails, which means that `methlist.remove(i)` is never executed.
>  - Simplify VisibleMemberTable.ImplementedMethods
>    
>    Also adds a TODO to remind us to investigate an important issue later; either in this or a follow-up PR.
>  - Fix confusing doc
>  - Merge branch 'master' into 8280713
>  - Refine overriddenType(ExecutableElement)
>    
>    Makes it clear that the returned TypeMirror is DeclaredType which represents a class and never an interface.
>  - Refactor how superinterfaces are collected
>    
>    Improves readability of Utils.getAllInterfaces and friends.
>  - ... and 18 more: https://git.openjdk.java.net/jdk/compare/efd3967b...41a00ce9

Good cleanup, as a step along the way to a better documented/specified form, even if parts of it were a little obscure.

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

Marked as reviewed by jjg (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/7233