diff --git a/src/jdk.javadoc/share/classes/jdk/javadoc/doclet/package-info.java b/src/jdk.javadoc/share/classes/jdk/javadoc/doclet/package-info.java index 51b83540e51..d6042431dbe 100644 --- a/src/jdk.javadoc/share/classes/jdk/javadoc/doclet/package-info.java +++ b/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 @@ * * *
Included
- *
An element is considered to be included, if it is - * specified if it contains a specified element, - * or it is enclosed in a specified element, and is selected. - * Included elements will be documented. + *
An element is considered to be included, if it is selected and any of the following is true: + * + * Included elements will be documented. *
* * diff --git a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java index 29dd88e31ec..0e61798f7a7 100644 --- a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java +++ b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java @@ -289,23 +289,30 @@ public class HtmlDocletWriter { * @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 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); } } diff --git a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/MethodWriterImpl.java b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/MethodWriterImpl.java index 6a0e647d413..e3b603be76f 100644 --- a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/MethodWriterImpl.java +++ b/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 @@ public class MethodWriterImpl extends AbstractExecutableMemberWriter * 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 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 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); diff --git a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/TagletWriterImpl.java b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/TagletWriterImpl.java index 365e15eb9a5..455f7f2c3a3 100644 --- a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/TagletWriterImpl.java +++ b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/TagletWriterImpl.java @@ -573,8 +573,9 @@ public class TagletWriterImpl extends TagletWriter { 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))) { diff --git a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/WorkArounds.java b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/WorkArounds.java index 1e51120b974..c1c9d0bb6b7 100644 --- a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/WorkArounds.java +++ b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/WorkArounds.java @@ -194,74 +194,6 @@ public class WorkArounds { 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; diff --git a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Utils.java b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Utils.java index 7cd9eaab4c9..42e83aa48bf 100644 --- a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Utils.java +++ b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Utils.java @@ -184,63 +184,6 @@ public class Utils { return getSymbol("java.lang.FunctionalInterface"); } - /** - * Search for the given method in the given class. - * - * @param te Class to search into. - * @param method Method to be searched. - * - * @return Method found, null otherwise. - */ - public ExecutableElement findMethod(TypeElement te, ExecutableElement method) { - for (ExecutableElement m : getMethods(te)) { - if (executableMembersEqual(method, m)) { - return m; - } - } - return null; - } - - /** - * Test whether a class is a subclass of another class. - * - * @param t1 the candidate subclass - * @param t2 the candidate superclass - * @return true if t1 is a superclass of t2 - */ - public boolean isSubclassOf(TypeElement t1, TypeElement t2) { - return typeUtils.isSubtype(typeUtils.erasure(t1.asType()), typeUtils.erasure(t2.asType())); - } - - /** - * @param e1 the first method to compare. - * @param e2 the second method to compare. - * @return true if member1 overrides/hides or is overridden/hidden by member2. - */ - public boolean executableMembersEqual(ExecutableElement e1, ExecutableElement e2) { - // TODO: investigate if Elements.hides(..) will work here. - if (isStatic(e1) && isStatic(e2)) { - List parameters1 = e1.getParameters(); - List parameters2 = e2.getParameters(); - if (e1.getSimpleName().equals(e2.getSimpleName()) && - parameters1.size() == parameters2.size()) { - for (int j = 0; j < parameters1.size(); j++) { - VariableElement v1 = parameters1.get(j); - VariableElement v2 = parameters2.get(j); - if (!typeUtils.isSameType(v1.asType(), v2.asType())) { - return false; - } - } - return true; - } - return false; - } else { - return elementUtils.overrides(e1, e2, getEnclosingTypeElement(e1)) || - elementUtils.overrides(e2, e1, getEnclosingTypeElement(e2)) || - e1.equals(e2); - } - } - /** * According to The Java Language Specification, * all the outer classes and static inner classes are core classes. @@ -332,8 +275,19 @@ public class Utils { return e.getModifiers().contains(Modifier.FINAL); } + /* + * A contemporary JLS term for "package private" or "default access" is + * "package access". For example: "a member is declared with package + * access" or "a member has package access". + * + * This is to avoid confusion with unrelated _default_ methods which + * appeared in JDK 8. + */ public boolean isPackagePrivate(Element e) { - return !(isPublic(e) || isPrivate(e) || isProtected(e)); + var m = e.getModifiers(); + return !m.contains(Modifier.PUBLIC) + && !m.contains(Modifier.PROTECTED) + && !m.contains(Modifier.PRIVATE); } public boolean isPrivate(Element e) { @@ -422,10 +376,6 @@ public class Utils { .compareTo(SourceVersion.RELEASE_8) >= 0; } - public boolean isNoType(TypeMirror t) { - return t.getKind() == NONE; - } - public boolean isUndocumentedEnclosure(TypeElement enclosingTypeElement) { return (isPackagePrivate(enclosingTypeElement) || isPrivate(enclosingTypeElement) || hasHiddenTag(enclosingTypeElement)) @@ -659,6 +609,14 @@ public class Utils { !((DeclaredType)e.getEnclosingElement().asType()).getTypeArguments().isEmpty(); } + /* + * The record is used to pass the method along with the type where that method is visible. + * Passing the type explicitly allows to preserve a complete type information, including + * parameterization. + */ + public record OverrideInfo(ExecutableElement overriddenMethod, + DeclaredType overriddenMethodOwner) { } + /* * Returns the closest superclass (not the superinterface) that contains * a method that is both: @@ -672,43 +630,29 @@ public class Utils { * superclass is java.lang.Object no matter how many other interfaces * that interface extends. */ - public DeclaredType overriddenType(ExecutableElement method) { - return configuration.workArounds.overriddenType(method); - } - - private TypeMirror getType(TypeMirror t) { - return (isNoType(t)) ? getObjectType() : t; - } - - public TypeMirror getSuperType(TypeElement te) { - TypeMirror t = te.getSuperclass(); - return getType(t); - } - - public ExecutableElement overriddenMethod(ExecutableElement method) { - if (isStatic(method)) { - return null; - } - final TypeElement origin = getEnclosingTypeElement(method); - for (TypeMirror t = getSuperType(origin); - t.getKind() == DECLARED; - t = getSuperType(asTypeElement(t))) { - TypeElement te = asTypeElement(t); - if (te == null) { + public OverrideInfo overriddenMethod(ExecutableElement method) { + var t = method.getEnclosingElement().asType(); + // in this context, consider java.lang.Object to be the superclass of an interface + while (true) { + var supertypes = typeUtils.directSupertypes(t); + if (supertypes.isEmpty()) { + // reached the top of the hierarchy + assert typeUtils.isSameType(getObjectType(), t); return null; } + t = supertypes.get(0); + // if non-empty, the first element is always the superclass + var te = (TypeElement) ((DeclaredType) t).asElement(); + assert te.getKind().isClass(); VisibleMemberTable vmt = configuration.getVisibleMemberTable(te); for (Element e : vmt.getMembers(VisibleMemberTable.Kind.METHODS)) { - ExecutableElement ee = (ExecutableElement)e; - if (configuration.workArounds.overrides(method, ee, origin) && + var ee = (ExecutableElement) e; + if (elementUtils.overrides(method, ee, (TypeElement) method.getEnclosingElement()) && !isSimpleOverride(ee)) { - return ee; + return new OverrideInfo(ee, (DeclaredType) t); } } - if (typeUtils.isSameType(t, getObjectType())) - return null; } - return null; } public SortedSet getTypeElementsAsSortedSet(Iterable typeElements) { @@ -1062,17 +1006,6 @@ public class Utils { }.visit(t); } - public TypeElement getSuperClass(TypeElement te) { - if (checkType(te)) { - return null; - } - TypeMirror superclass = te.getSuperclass(); - if (isNoType(superclass) && isClass(te)) { - superclass = getObjectType(); - } - return asTypeElement(superclass); - } - private boolean checkType(TypeElement te) { return isInterface(te) || typeUtils.isSameType(te.asType(), getObjectType()); } @@ -1092,28 +1025,25 @@ public class Utils { * be found. */ public TypeMirror getFirstVisibleSuperClass(TypeMirror type) { - List superTypes = typeUtils.directSupertypes(type); - TypeMirror superType = superTypes.isEmpty() ? getObjectType() : superTypes.get(0); - TypeElement superClass = asTypeElement(superType); - // skip "hidden" classes - while ((superClass != null && hasHiddenTag(superClass)) - || (superClass != null && !isPublic(superClass) && !isLinkable(superClass))) { - TypeMirror supersuperType = superClass.getSuperclass(); - TypeElement supersuperClass = asTypeElement(supersuperType); - if (supersuperClass == null - || supersuperClass.getQualifiedName().equals(superClass.getQualifiedName())) { - break; + // TODO: this computation should be eventually delegated to VisibleMemberTable + Set alreadySeen = null; + // create a set iff assertions are enabled, to assert that no class + // appears more than once in a superclass hierarchy + assert (alreadySeen = new HashSet<>()) != null; + for (var t = type; ;) { + var supertypes = typeUtils.directSupertypes(t); + if (supertypes.isEmpty()) { // end of hierarchy + return null; + } + t = supertypes.get(0); // if non-empty, the first element is always the superclass + var te = asTypeElement(t); + assert alreadySeen.add(te); // it should be the first time we see `te` + if (!hasHiddenTag(te) && (isPublic(te) || isLinkable(te))) { + return t; } - superType = supersuperType; - superClass = supersuperClass; } - if (typeUtils.isSameType(type, superType)) { - return null; - } - return superType; } - /** * Given a class, return the closest visible superclass. * @@ -2454,10 +2384,21 @@ public class Utils { } public ModuleElement containingModule(Element e) { + // TODO: remove this short-circuit after JDK-8302545 has been fixed + // or --ignore-source-errors has been removed + if (e.getKind() == ElementKind.PACKAGE + && e.getEnclosingElement() == null) { + return null; + } return elementUtils.getModuleOf(e); } public PackageElement containingPackage(Element e) { + // TODO: remove this short-circuit after JDK-8302545 has been fixed + // or --ignore-source-errors has been removed + if (e.getKind() == ElementKind.PACKAGE) { + return (PackageElement) e; + } return elementUtils.getPackageOf(e); } @@ -2801,7 +2742,10 @@ public class Utils { } private DocFinder newDocFinder() { - return new DocFinder(this::overriddenMethod, this::implementedMethods); + return new DocFinder(e -> { + var i = overriddenMethod(e); + return i == null ? null : i.overriddenMethod(); + }, this::implementedMethods); } private Iterable implementedMethods(ExecutableElement originalMethod, ExecutableElement m) { diff --git a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/VisibleMemberTable.java b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/VisibleMemberTable.java index 82b31cefcbd..13b0f844ddd 100644 --- a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/VisibleMemberTable.java +++ b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/VisibleMemberTable.java @@ -141,7 +141,12 @@ public class VisibleMemberTable { } } + /** The class or interface described by this table. */ private final TypeElement te; + /** + * The superclass of {@link #te} or null if {@code te} is an + * interface or {@code java.lang.Object}. + */ private final TypeElement parent; private final BaseConfiguration config; @@ -149,15 +154,36 @@ public class VisibleMemberTable { private final Utils utils; private final VisibleMemberCache mcache; - private final List allSuperclasses; + /** + * Tables for direct and indirect superclasses. + * + * Tables for superclasses must be unique: no class can appear multiple + * times in the inheritance hierarchy for some other class. + */ + private final Set allSuperclasses; + /** + * Tables for direct and indirect superinterfaces. + * + * Tables for superinterfaces might not be unique (i.e. an interface + * may be added from different lineages). + */ private final List allSuperinterfaces; - private final List parents; + /** + * Tables for direct superclass and direct superinterfaces. + * + * The position of a table for the superclass in the list is unspecified. + */ + private final Set parents; private Map> visibleMembers; private final Map propertyMap = new HashMap<>(); - // Keeps track of method overrides - private final Map overriddenMethodTable + // FIXME: Figure out why it is one-one and not one-to-many. + /** + * Maps a method m declared in {@code te} to a visible method m' in a + * {@code te}'s supertype such that m overrides m'. + */ + private final Map overriddenMethodTable = new LinkedHashMap<>(); protected VisibleMemberTable(TypeElement typeElement, BaseConfiguration configuration, @@ -166,11 +192,11 @@ public class VisibleMemberTable { utils = configuration.utils; options = configuration.getOptions(); te = typeElement; - parent = utils.getSuperClass(te); + parent = (TypeElement) utils.typeUtils.asElement(te.getSuperclass()); this.mcache = mcache; - allSuperclasses = new ArrayList<>(); + allSuperclasses = new LinkedHashSet<>(); allSuperinterfaces = new ArrayList<>(); - parents = new ArrayList<>(); + parents = new LinkedHashSet<>(); } private void ensureInitialized() { @@ -185,12 +211,12 @@ public class VisibleMemberTable { computeVisibleMembers(); } - List getAllSuperclasses() { + private Set getAllSuperclasses() { ensureInitialized(); return allSuperclasses; } - List getAllSuperinterfaces() { + private List getAllSuperinterfaces() { ensureInitialized(); return allSuperinterfaces; } @@ -227,7 +253,6 @@ public class VisibleMemberTable { */ public List getVisibleMembers(Kind kind, Predicate p) { ensureInitialized(); - return visibleMembers.getOrDefault(kind, List.of()).stream() .filter(p) .toList(); @@ -261,17 +286,26 @@ public class VisibleMemberTable { } /** - * Returns the overridden method, if it is simply overriding or the - * method is a member of a package private type, this method is - * primarily used to determine the location of a possible comment. + * Returns the method overridden by the provided method, or {@code null}. + * + * Sometimes it's not possible to link to a method that a link, linkplain, + * or see tag mentions. This is because the method is a "simple override" + * and, thus, has no useful documentation, or because the method is + * declared in a type that has package access and, thus, has no visible + * documentation. + * + * Call this method to determine if any of the above is the case. If the + * call returns a method element, link to that method element instead of + * the provided method. * * @param e the method to check - * @return the method found or null + * @return the method found or {@code null} */ public ExecutableElement getOverriddenMethod(ExecutableElement e) { + // TODO: consider possible ambiguities: multiple overridden methods ensureInitialized(); - - OverriddenMethodInfo found = overriddenMethodTable.get(e); + assert !overriddenMethodTable.containsKey(null); + OverrideInfo found = overriddenMethodTable.get(e); if (found != null && (found.simpleOverride || utils.isUndocumentedEnclosure(utils.getEnclosingTypeElement(e)))) { return found.overriddenMethod; @@ -285,7 +319,7 @@ public class VisibleMemberTable { * * @param e the method to check */ - public boolean isNotSimpleOverride(ExecutableElement e) { + private boolean isNotSimpleOverride(ExecutableElement e) { ensureInitialized(); var info = overriddenMethodTable.get(e); @@ -411,7 +445,8 @@ public class VisibleMemberTable { if (intfc != null) { VisibleMemberTable vmt = mcache.getVisibleMemberTable(intfc); allSuperinterfaces.add(vmt); - parents.add(vmt); + boolean added = parents.add(vmt); + assert added; // no duplicates allSuperinterfaces.addAll(vmt.getAllSuperinterfaces()); } } @@ -419,10 +454,12 @@ public class VisibleMemberTable { if (parent != null) { VisibleMemberTable vmt = mcache.getVisibleMemberTable(parent); allSuperclasses.add(vmt); + assert Collections.disjoint(allSuperclasses, vmt.getAllSuperclasses()); // no duplicates allSuperclasses.addAll(vmt.getAllSuperclasses()); - // Add direct superinterfaces of a superclass, if any. + // Add direct and indirect superinterfaces of a superclass. allSuperinterfaces.addAll(vmt.getAllSuperinterfaces()); - parents.add(vmt); + boolean added = parents.add(vmt); + assert added; // no duplicates } } @@ -469,10 +506,10 @@ public class VisibleMemberTable { } private boolean allowInheritedMembers(Element e, Kind kind, LocalMemberTable lmt) { - return isInherited(e) && !isMemberHidden(e, kind, lmt); + return isAccessible(e) && !isMemberHidden(e, kind, lmt); } - private boolean isInherited(Element e) { + private boolean isAccessible(Element e) { if (utils.isPrivate(e)) return false; @@ -516,75 +553,127 @@ public class VisibleMemberTable { visibleMembers.put(kind, list); } + // This method computes data structures related to method members + // of a class or an interface. + // + // TODO The computation is performed manually, by applying JLS rules. + // While jdk.javadoc does need custom and specialized data structures, + // this method does not feel DRY. It should be possible to improve + // it by delegating some, if not most, of the JLS wrestling to + // javax.lang.model. For example, while it cannot help us get the + // structures, such as overriddenMethodTable, javax.lang.model can + // help us get all method members of a class or an interface t by calling + // ElementFilter.methodsIn(Elements.getAllMembers(t)). private void computeVisibleMethods(LocalMemberTable lmt) { - Set inheritedMethods = new LinkedHashSet<>(); + // parentMethods is a union of visible methods from all parents. + // It is used to figure out which methods this type should inherit. + // Inherited methods are those parent methods that remain after all + // methods that cannot be inherited are eliminated. + Set parentMethods = new LinkedHashSet<>(); + for (var p : parents) { + // Lists of visible methods from different parents may share some + // methods. These are the methods that the parents inherited from + // their common ancestor. + // + // Such methods won't result in duplicates in parentMethods as we + // purposefully don't track duplicates. + // FIXME: add a test to assert the order (LinkedHashSet) + parentMethods.addAll(p.getAllVisibleMembers(Kind.METHODS)); + } + + // overriddenByTable maps an ancestor (grandparent and above) method + // to parent methods that override it: + // + // key + // : a method overridden by one or more parent methods + // value + // : a list of parent methods that override the key Map> overriddenByTable = new HashMap<>(); - for (VisibleMemberTable pvmt : parents) { + for (var p : parents) { // Merge the lineage overrides into local table - pvmt.overriddenMethodTable.forEach((method, methodInfo) -> { + p.overriddenMethodTable.forEach((method, methodInfo) -> { if (!methodInfo.simpleOverride) { // consider only real overrides - List list = overriddenByTable.computeIfAbsent(methodInfo.overriddenMethod, + var list = overriddenByTable.computeIfAbsent(methodInfo.overriddenMethod, k -> new ArrayList<>()); list.add(method); } }); - inheritedMethods.addAll(pvmt.getAllVisibleMembers(Kind.METHODS)); } - // Filter out inherited methods that: - // a. cannot be overridden (private instance members) - // b. are overridden and should not be visible in this type - // c. are hidden in the type being considered - // see allowInheritedMethod, which performs the above actions + // filter out methods that aren't inherited + // // nb. This statement has side effects that can initialize // members of the overriddenMethodTable field, so it must be // evaluated eagerly with toList(). - List inheritedMethodsList = inheritedMethods.stream() + List inheritedMethods = parentMethods.stream() .filter(e -> allowInheritedMethod((ExecutableElement) e, overriddenByTable, lmt)) .toList(); - // Filter out the local methods, that do not override or simply - // overrides a super method, or those methods that should not - // be visible. - Predicate isVisible = m -> { - OverriddenMethodInfo p = overriddenMethodTable.getOrDefault(m, null); - return p == null || !p.simpleOverride; + // filter out "simple overrides" from local methods + Predicate nonSimpleOverride = m -> { + OverrideInfo i = overriddenMethodTable.get(m); + return i == null || !i.simpleOverride; }; Stream localStream = lmt.getOrderedMembers(Kind.METHODS) .stream() .map(m -> (ExecutableElement)m) - .filter(isVisible); + .filter(nonSimpleOverride); // Merge the above list and stream, making sure the local methods precede the others // Final filtration of elements - List list = Stream.concat(localStream, inheritedMethodsList.stream()) + // FIXME add a test to assert the order or remove that part of the comment above ^ + List list = Stream.concat(localStream, inheritedMethods.stream()) .filter(this::mustDocument) .toList(); visibleMembers.put(Kind.METHODS, list); - // Copy over overridden tables from the lineage, and finish up. + // copy over overridden tables from the lineage for (VisibleMemberTable pvmt : parents) { + // a key in overriddenMethodTable is a method _declared_ in the respective parent; + // no two _different_ parents can share a declared method, by definition; + // if parents in the list are different (i.e. the list of parents doesn't contain duplicates), + // then no keys are equal and thus no replace happens + // if the list of parents contains duplicates, values for the equal keys are equal, + // so no harm if they are replaced in the map + assert putAllIsNonReplacing(overriddenMethodTable, pvmt.overriddenMethodTable); overriddenMethodTable.putAll(pvmt.overriddenMethodTable); } } - boolean isEnclosureInterface(Element e) { - TypeElement enclosing = utils.getEnclosingTypeElement(e); - return utils.isPlainInterface(enclosing); + private static boolean putAllIsNonReplacing(Map dst, Map src) { + for (var e : src.entrySet()) { + if (dst.containsKey(e.getKey()) + && !Objects.equals(dst.get(e.getKey()), e.getValue())) { + return false; + } + } + return true; } - boolean allowInheritedMethod(ExecutableElement inheritedMethod, - Map> overriddenByTable, - LocalMemberTable lmt) { - if (!isInherited(inheritedMethod)) + private boolean allowInheritedMethod(ExecutableElement inheritedMethod, + Map> overriddenByTable, + LocalMemberTable lmt) { + // JLS 8.4.8: A class does not inherit private or static methods from + // its superinterface types. + // + // JLS 9.4.1: An interface does not inherit private or static methods + // from its superinterfaces. + // + // JLS 8.4.8: m is public, protected, or declared with package access + // in the same package as C + // + // JLS 9.4: A method in the body of an interface declaration may be + // declared public or private. If no access modifier is given, the + // method is implicitly public. + if (!isAccessible(inheritedMethod)) return false; final boolean haveStatic = utils.isStatic(inheritedMethod); - final boolean inInterface = isEnclosureInterface(inheritedMethod); + final boolean inInterface = isDeclaredInInterface(inheritedMethod); - // Static methods in interfaces are never documented. + // Static interface methods are never inherited (JLS 8.4.8 and 9.1.3) if (haveStatic && inInterface) { return false; } @@ -601,7 +690,7 @@ public class VisibleMemberTable { List list = overriddenByTable.get(inheritedMethod); if (list != null) { boolean found = list.stream() - .anyMatch(this::isEnclosureInterface); + .anyMatch(this::isDeclaredInInterface); if (found) return false; } @@ -610,11 +699,12 @@ public class VisibleMemberTable { Elements elementUtils = config.docEnv.getElementUtils(); // Check the local methods in this type. + // List contains overloads and probably something else, but one match is enough, hence short-circuiting List lMethods = lmt.getMembers(inheritedMethod, Kind.METHODS); for (Element le : lMethods) { ExecutableElement lMethod = (ExecutableElement) le; // Ignore private methods or those methods marked with - // a "hidden" tag. + // a "hidden" tag. // FIXME I cannot see where @hidden is ignored if (utils.isPrivate(lMethod)) continue; @@ -628,11 +718,16 @@ public class VisibleMemberTable { if (elementUtils.overrides(lMethod, inheritedMethod, utils.getEnclosingTypeElement(lMethod))) { + assert utils.getEnclosingTypeElement(lMethod).equals(te); + // Disallow package-private super methods to leak in TypeElement encl = utils.getEnclosingTypeElement(inheritedMethod); if (utils.isUndocumentedEnclosure(encl)) { + // FIXME + // is simpleOverride=false here to force to be used because + // it cannot be linked to, because package-private? overriddenMethodTable.computeIfAbsent(lMethod, - l -> new OverriddenMethodInfo(inheritedMethod, false)); + l -> new OverrideInfo(inheritedMethod, false)); return false; } @@ -644,13 +739,17 @@ public class VisibleMemberTable { && !overridingSignatureChanged(lMethod, inheritedMethod) && !overriddenByTable.containsKey(inheritedMethod); overriddenMethodTable.computeIfAbsent(lMethod, - l -> new OverriddenMethodInfo(inheritedMethod, simpleOverride)); + l -> new OverrideInfo(inheritedMethod, simpleOverride)); return simpleOverride; } } return true; } + private boolean isDeclaredInInterface(ExecutableElement e) { + return e.getEnclosingElement().getKind() == ElementKind.INTERFACE; + } + // Check whether the signature of an overriding method has any changes worth // being documented compared to the overridden method. private boolean overridingSignatureChanged(ExecutableElement method, ExecutableElement overriddenMethod) { @@ -736,9 +835,9 @@ public class VisibleMemberTable { } /* - * This class encapsulates the details of local members, orderedMembers + * This class encapsulates the details of local members. orderedMembers * contains the members in the declaration order, additionally a - * HashMap is maintained for performance optimization to lookup + * HashMap is maintained for performance optimization to look up * members. As a future enhancement is perhaps to consolidate the ordering * into a Map, capturing the insertion order, thereby eliminating an * ordered list. @@ -754,7 +853,7 @@ public class VisibleMemberTable { LocalMemberTable() { orderedMembers = new EnumMap<>(Kind.class); memberMap = new EnumMap<>(Kind.class); - + // elements directly declared by te List elements = te.getEnclosedElements(); for (Element e : elements) { if (options.noDeprecated() && utils.isDeprecated(e)) { @@ -783,7 +882,7 @@ public class VisibleMemberTable { } break; case CONSTRUCTOR: - addMember(e, Kind.CONSTRUCTORS); + addMember(e, Kind.CONSTRUCTORS); break; case ENUM_CONSTANT: addMember(e, Kind.ENUM_CONSTANTS); @@ -854,8 +953,8 @@ public class VisibleMemberTable { } } - record PropertyMembers(ExecutableElement propertyMethod, VariableElement field, - ExecutableElement getter, ExecutableElement setter) { } + private record PropertyMembers(ExecutableElement propertyMethod, VariableElement field, + ExecutableElement getter, ExecutableElement setter) { } /* * JavaFX convention notes. @@ -988,23 +1087,34 @@ public class VisibleMemberTable { private class ImplementedMethods { - private final Map interfaces = new HashMap<>(); - private final LinkedHashSet methods = new LinkedHashSet<>(); + private final Map interfaces = new LinkedHashMap<>(); - public ImplementedMethods(ExecutableElement method) { - // ExecutableElement.getEnclosingElement() returns "the class or - // interface defining the executable", which has to be TypeElement - TypeElement typeElement = (TypeElement) method.getEnclosingElement(); - Set intfacs = utils.getAllInterfaces(typeElement); - for (TypeMirror interfaceType : intfacs) { - // TODO: this method also finds static methods which are pseudo-inherited; - // this needs to be fixed - ExecutableElement found = utils.findMethod(utils.asTypeElement(interfaceType), method); - if (found != null && !methods.contains(found)) { - methods.add(found); - interfaces.put(found, interfaceType); + public ImplementedMethods(ExecutableElement implementer) { + var typeElement = (TypeElement) implementer.getEnclosingElement(); + for (TypeMirror i : utils.getAllInterfaces(typeElement)) { + TypeElement dst = utils.asTypeElement(i); // a type element to look an implemented method in + ExecutableElement implemented = findImplementedMethod(dst, implementer); + if (implemented == null) { + continue; + } + var prev = interfaces.put(implemented, i); + // no two type elements declare the same method + assert prev == null; + // dst can be generic, while i might be parameterized; but they + // must the same type element. For example, if dst is Set, + // then i is Set + assert Objects.equals(((DeclaredType) i).asElement(), dst); + } + } + + private ExecutableElement findImplementedMethod(TypeElement te, ExecutableElement implementer) { + var typeElement = (TypeElement) implementer.getEnclosingElement(); + for (var m : utils.getMethods(te)) { + if (utils.elementUtils.overrides(implementer, m, typeElement)) { + return m; } } + return null; } /** @@ -1020,7 +1130,7 @@ public class VisibleMemberTable { * @return a collection of implemented methods */ Collection getImplementedMethods() { - return methods; + return interfaces.keySet(); } TypeMirror getMethodHolder(ExecutableElement ee) { @@ -1028,7 +1138,35 @@ public class VisibleMemberTable { } } - private record OverriddenMethodInfo(ExecutableElement overriddenMethod, - boolean simpleOverride) { + /* + * (Here "override" used as a noun, not a verb, for a short and descriptive + * name. Sadly, we cannot use "Override" as a complete name because a clash + * with @java.lang.Override would make it inconvenient.) + * + * Used to provide additional attributes to the otherwise boolean + * "overrides(a, b)" relationship. + * + * Overriding method could be a key in a map and an instance of this + * record could be the value. + */ + private record OverrideInfo(ExecutableElement overriddenMethod, + boolean simpleOverride) { + @Override // for debugging + public String toString() { + return overriddenMethod.getEnclosingElement() + + "::" + overriddenMethod + ", simple=" + simpleOverride; + } + } + + @Override + public int hashCode() { + return te.hashCode(); + } + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof VisibleMemberTable other)) + return false; + return te.equals(other.te); } } diff --git a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/ToolOptions.java b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/ToolOptions.java index f3d3cc6749b..07f53f70924 100644 --- a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/ToolOptions.java +++ b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/ToolOptions.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2012, 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 @@ -139,7 +139,7 @@ public class ToolOptions { private boolean verbose; /** - * Argument for command-line option {@code -xclasses}. + * Argument for command-line option {@code -Xclasses}. * If true, names on the command line that would normally be * treated as package names are treated as class names instead. */ @@ -793,7 +793,7 @@ public class ToolOptions { } /** - * Argument for command-line option {@code -xclasses}. + * Argument for command-line option {@code -Xclasses}. * If true, names on the command line that would normally be * treated as package names are treated as class names instead. */ diff --git a/test/langtools/jdk/javadoc/doclet/testInterface/TestInterface.java b/test/langtools/jdk/javadoc/doclet/testInterface/TestInterface.java index 0f651b07945..4e6bf921df8 100644 --- a/test/langtools/jdk/javadoc/doclet/testInterface/TestInterface.java +++ b/test/langtools/jdk/javadoc/doclet/testInterface/TestInterface.java @@ -144,6 +144,16 @@ public class TestInterface extends JavadocTester { """, + """ +
+

staticMethod

+
public static \ + void staticMethod()
+ """ + ); + + checkOutput("pkg/ClassWithStaticMembers.html", false, """

staticMethod

@@ -286,4 +296,59 @@ public class TestInterface extends JavadocTester { - Interface in pkg2
 
"""); } + + @Test + public void test3() { + javadoc("-d", "out-3", + "--no-platform-links", // disable links to simplify output matching + "-sourcepath", testSrc, + "pkg3"); + + checkExit(Exit.OK); + + checkOutput("pkg3/I.html", true, + """ +
  • +
    +

    hashCode

    +
    \ + int hashCode()
    +
    +
    Overrides:
    +
    hashCode in class java.lang.Object
    +
    +
    +
    • +
  • +
    +

    equals

    +
    \ + boolean equals\ + (java.lang.Object obj)
    +
    +
    Overrides:
    +
    equals in class java.lang.Object
    +
    +
    +
    • +
  • +
    +

    toString

    +
    \ + java.lang.String toString()
    +
    +
    Overrides:
    +
    toString in class java.lang.Object
    +
    +
    +
    • +
  • +
    +

    clone

    +
    \ + java.lang.Object clone()
    +
    +
    • + """); + } } diff --git a/test/langtools/jdk/javadoc/doclet/testInterface/pkg3/I.java b/test/langtools/jdk/javadoc/doclet/testInterface/pkg3/I.java new file mode 100644 index 00000000000..c7d16e0550a --- /dev/null +++ b/test/langtools/jdk/javadoc/doclet/testInterface/pkg3/I.java @@ -0,0 +1,39 @@ +/* + * Copyright (c) 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 + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +package pkg3; + +public interface I { + + int hashCode(); + + boolean equals(Object obj); + + String toString(); + + // No matter what your IDE might show you, from JLS 9.6.4.4 it follows that + // the "clone" (as well as currently deprecated "finalize") method cannot + // be overridden by an interface method in the same way "hashCode", "equals" + // and "toString" can. This is because "clone" is not public. + Object clone(); +} diff --git a/test/langtools/jdk/javadoc/doclet/testOverriddenMethods/TestBadOverride.java b/test/langtools/jdk/javadoc/doclet/testOverriddenMethods/TestBadOverride.java index 6d8e61bd32f..2d81fdae4db 100644 --- a/test/langtools/jdk/javadoc/doclet/testOverriddenMethods/TestBadOverride.java +++ b/test/langtools/jdk/javadoc/doclet/testOverriddenMethods/TestBadOverride.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2017, 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 @@ -49,15 +49,7 @@ public class TestBadOverride extends JavadocTester { javadoc("-d", "out", "-sourcepath", testSrc, "pkg4"); - checkExit(Exit.OK); - - checkOutput("pkg4/Foo.html", true, - """ -
    -

    toString

    -
    public void toString()
    -
    Why can't I do this ?
    -
    """); + // explicitly configure "no crash" check, which is the main interest of this test + setAutomaticCheckNoStacktrace(true); } } diff --git a/test/langtools/jdk/javadoc/doclet/testOverriddenMethods/TestSpecifiedBy.java b/test/langtools/jdk/javadoc/doclet/testOverriddenMethods/TestSpecifiedBy.java new file mode 100644 index 00000000000..58f2c60c78f --- /dev/null +++ b/test/langtools/jdk/javadoc/doclet/testOverriddenMethods/TestSpecifiedBy.java @@ -0,0 +1,95 @@ +/* + * Copyright (c) 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 + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* + * @test + * @bug 4318787 + * @library /tools/lib ../../lib + * @modules jdk.javadoc/jdk.javadoc.internal.tool + * @build toolbox.ToolBox javadoc.tester.* + * @run main TestSpecifiedBy + */ + +import java.nio.file.Path; +import java.util.List; + +import javadoc.tester.JavadocTester; +import toolbox.ToolBox; + +public class TestSpecifiedBy extends JavadocTester { + + public static void main(String... args) throws Exception { + new TestSpecifiedBy().runTests(); + } + + private final ToolBox tb = new ToolBox(); + + @Test + public void test(Path base) throws Exception { + Path src = base.resolve("src"); + tb.writeJavaFiles(src, """ + package pkg; + + public abstract class A { + public abstract void m(); + } + """, """ + package pkg; + + public class B extends A { + public void m() { } + } + """, """ + package pkg; + + public abstract class C extends A { + public void m() { } + } + """, """ + package pkg; + + public abstract class D extends A { + public abstract void m(); + } + """); + + javadoc("-d", base.resolve("out").toString(), + "-sourcepath", src.toString(), + "pkg"); + + checkExit(Exit.OK); + // check that the terminology for an overridden abstract method of an + // abstract class is the same as that of an overridden interface method; + // no matter who the overrider is, the overridden method should be + // listed under "Specified by", not "Overrides" + for (var f : List.of("pkg/B.html", "pkg/C.html", "pkg/D.html")) + checkOutput(f, true, + """ +
    +
    Specified by:
    +
    m in class \ + A
    +
    + """); + } +} diff --git a/test/langtools/jdk/javadoc/tool/IgnoreSourceErrors.java b/test/langtools/jdk/javadoc/tool/IgnoreSourceErrors.java index 44e698d7a88..97b4af12b3b 100644 --- a/test/langtools/jdk/javadoc/tool/IgnoreSourceErrors.java +++ b/test/langtools/jdk/javadoc/tool/IgnoreSourceErrors.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017, 2021, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2017, 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 @@ -24,7 +24,7 @@ /* * @test * @bug 8175219 8268582 - * @summary test --ignore-errors works correctly + * @summary test --ignore-source-errors works correctly * @modules * jdk.javadoc/jdk.javadoc.internal.api * jdk.javadoc/jdk.javadoc.internal.tool