8300517: Refactor VisibleMemberTable (method members)

Reviewed-by: jjg

Author: pavelrappo

src/jdk.javadoc/share/classes/jdk/javadoc/doclet/package-info.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2023, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -79,10 +79,13 @@
  *   </dd>
  *
  *   <dt><a id="included"></a>Included</dt>
- *   <dd>An element is considered to be <em>included</em>, if it is
- *       <em>specified</em> if it contains a <em>specified</em> element,
- *       or it is enclosed in a <em>specified</em> element, and is <em>selected</em>.
- *       Included elements will be documented.
+ *   <dd>An element is considered to be <em>included</em>, if it is <em>selected</em> and any of the following is true:
+ *     <ul>
+ *       <li>the element is <em>specified</em>, or
+ *       <li>the element contains a <em>specified</em> element, or
+ *       <li>the element is enclosed in a <em>specified</em> element.
+ *     </ul>
+ *     Included elements will be documented.
  *   </dd>
  *
  * </dl>

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java

@@ -289,23 +289,30 @@ public String replaceDocRootDir(String htmlstr) {
      * @param dl the content to which the method information will be added
      */
     private void addMethodInfo(ExecutableElement method, Content dl) {
-        TypeElement enclosing = utils.getEnclosingTypeElement(method);
-        List<? extends TypeMirror> intfacs = enclosing.getInterfaces();
-        ExecutableElement overriddenMethod = utils.overriddenMethod(method);
-        VisibleMemberTable vmt = configuration.getVisibleMemberTable(enclosing);
-        // Check whether there is any implementation or overridden info to be
-        // printed. If no overridden or implementation info needs to be
-        // printed, do not print this section.
-        if ((!intfacs.isEmpty()
-                && !vmt.getImplementedMethods(method).isEmpty())
-                || overriddenMethod != null) {
-            MethodWriterImpl.addImplementsInfo(this, method, dl);
-            if (overriddenMethod != null) {
-                MethodWriterImpl.addOverridden(this,
-                        utils.overriddenType(method),
-                        overriddenMethod,
-                        dl);
-            }
+        var enclosing = (TypeElement) method.getEnclosingElement();
+        var overrideInfo = utils.overriddenMethod(method);
+        var enclosingVmt = configuration.getVisibleMemberTable(enclosing);
+        var implementedMethods = enclosingVmt.getImplementedMethods(method);
+        if ((!enclosing.getInterfaces().isEmpty()
+                && !implementedMethods.isEmpty())
+                || overrideInfo != null) {
+            // TODO note that if there are any overridden interface methods throughout the
+            //   hierarchy, !enclosingVmt.getImplementedMethods(method).isEmpty(), their information
+            //   will be printed if *any* of the below is true:
+            //     * the enclosing has _directly_ implemented interfaces
+            //     * the overridden method is not null
+            //   If both are false, the information will not be printed: there will be no
+            //   "Specified by" documentation. The examples of that can be seen in documentation
+            //   for these methods:
+            //     * ForkJoinPool.execute(java.lang.Runnable)
+            //  This is a long-standing bug, which must be fixed separately: JDK-8302316
+            MethodWriterImpl.addImplementsInfo(this, method, implementedMethods, dl);
+        }
+        if (overrideInfo != null) {
+            MethodWriterImpl.addOverridden(this,
+                    overrideInfo.overriddenMethodOwner(),
+                    overrideInfo.overriddenMethod(),
+                    dl);
         }
     }
 

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/MethodWriterImpl.java

@@ -25,6 +25,7 @@
 
 package jdk.javadoc.internal.doclets.formats.html;
 
+import java.util.Collection;
 import java.util.SortedSet;
 import java.util.TreeSet;
 
@@ -293,26 +294,28 @@ protected static void addOverridden(HtmlDocletWriter writer,
      * Adds "implements" information for a method (if appropriate)
      * into a definition list.
      *
-     * @param writer the writer for the method
-     * @param method the method
-     * @param dl     the definition list
+     * @param writer  the writer for the method
+     * @param method  the method
+     * @param methods implemented methods
+     * @param dl      the definition list
      */
     protected static void addImplementsInfo(HtmlDocletWriter writer,
                                             ExecutableElement method,
+                                            Collection<ExecutableElement> methods,
                                             Content dl) {
         Utils utils = writer.utils;
-        if (utils.isStatic(method) || writer.options.noComment()) {
+        if (writer.options.noComment()) {
             return;
         }
         Contents contents = writer.contents;
-        VisibleMemberTable vmt = writer.configuration
-                .getVisibleMemberTable(utils.getEnclosingTypeElement(method));
+        var enclosing = (TypeElement) method.getEnclosingElement();
+        VisibleMemberTable vmt = writer.configuration.getVisibleMemberTable(enclosing);
         SortedSet<ExecutableElement> implementedMethods =
                 new TreeSet<>(utils.comparators.makeOverrideUseComparator());
-        implementedMethods.addAll(vmt.getImplementedMethods(method));
+        implementedMethods.addAll(methods);
         for (ExecutableElement implementedMeth : implementedMethods) {
             TypeMirror intfac = vmt.getImplementedMethodHolder(method, implementedMeth);
-            intfac = utils.getDeclaredType(utils.getEnclosingTypeElement(method), intfac);
+            intfac = utils.getDeclaredType(enclosing, intfac);
             Content intfaclink = writer.getLink(new HtmlLinkInfo(
                     writer.configuration, HtmlLinkInfo.Kind.LINK_TYPE_PARAMS_AND_BOUNDS, intfac));
             var codeIntfacLink = HtmlTree.CODE(intfaclink);

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/TagletWriterImpl.java

@@ -573,8 +573,9 @@ private Content linkSeeReferenceOutput(Element holder,
                 VisibleMemberTable vmt = configuration.getVisibleMemberTable(containing);
                 overriddenMethod = vmt.getOverriddenMethod((ExecutableElement)refMem);
 
-                if (overriddenMethod != null)
+                if (overriddenMethod != null) {
                     containing = utils.getEnclosingTypeElement(overriddenMethod);
+                }
             }
             if (refSignature.trim().startsWith("#") &&
                     ! (utils.isPublic(containing) || utils.isLinkable(containing))) {

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/WorkArounds.java

@@ -194,74 +194,6 @@ public TypeElement searchClass(TypeElement klass, String className) {
         return elementUtils.getTypeElement(className);
     }
 
-    // TODO:  need to re-implement this using j.l.m. correctly!, this has
-    //        implications on testInterface, the note here is that javac's supertype
-    //        does the right thing returning Parameters in scope.
-    /*
-     * Returns the closest superclass (not the superinterface) that contains
-     * a method that is both:
-     *
-     *   - overridden by the specified method, and
-     *   - is not itself a *simple* override
-     *
-     * If no such class can be found, returns null.
-     *
-     * If the specified method belongs to an interface, the only considered
-     * superclass is java.lang.Object no matter how many other interfaces
-     * that interface extends.
-     */
-    public DeclaredType overriddenType(ExecutableElement method) {
-        if (utils.isStatic(method)) {
-            return null;
-        }
-        MethodSymbol sym = (MethodSymbol) method;
-        ClassSymbol origin = (ClassSymbol) sym.owner;
-        for (Type t = javacTypes.supertype(origin.type);
-             t.hasTag(TypeTag.CLASS);
-             t = javacTypes.supertype(t)) {
-            ClassSymbol c = (ClassSymbol) t.tsym;
-            for (Symbol sym2 : c.members().getSymbolsByName(sym.name)) {
-                if (sym.overrides(sym2, origin, javacTypes, true)) {
-                    // Ignore those methods that may be a simple override
-                    // and allow the real API method to be found.
-                    if (utils.isSimpleOverride((MethodSymbol)sym2)) {
-                        continue;
-                    }
-                    assert t.hasTag(TypeTag.CLASS) && !t.isInterface();
-                    return (Type.ClassType) t;
-                }
-            }
-        }
-        return null;
-    }
-
-    // TODO: the method jx.l.m.Elements::overrides does not check
-    // the return type, see JDK-8174840 until that is resolved,
-    // use a  copy of the same method, with a return type check.
-
-    // Note: the rider.overrides call in this method *must* be consistent
-    // with the call in overrideType(....), the method above.
-    public boolean overrides(ExecutableElement e1, ExecutableElement e2, TypeElement cls) {
-        MethodSymbol rider = (MethodSymbol)e1;
-        MethodSymbol ridee = (MethodSymbol)e2;
-        ClassSymbol origin = (ClassSymbol)cls;
-
-        return rider.name == ridee.name &&
-
-               // not reflexive as per JLS
-               rider != ridee &&
-
-               // we don't care if ridee is static, though that wouldn't
-               // compile
-               !rider.isStatic() &&
-
-               // Symbol.overrides assumes the following
-               ridee.isMemberOf(origin, javacTypes) &&
-
-               // check access, signatures and check return types
-               rider.overrides(ridee, origin, javacTypes, true);
-    }
-
     // TODO: jx.l.m ?
     public Location getLocationForModule(ModuleElement mdle) {
         ModuleSymbol msym = (ModuleSymbol)mdle;