From a86244f83cc4e234bdf8dc2c8d87640b6aab8463 Mon Sep 17 00:00:00 2001 From: Archie Cobbs Date: Mon, 29 Jul 2024 19:16:39 +0000 Subject: [PATCH] 6381729: Javadoc for generic constructor doesn't document type parameter Reviewed-by: jjg, liach --- .../html/AbstractExecutableMemberWriter.java | 21 ++++++- .../formats/html/AbstractMemberWriter.java | 12 +--- .../doclets/formats/html/ClassUseWriter.java | 2 +- .../formats/html/ConstructorWriter.java | 59 ++++++++++++++----- .../doclets/formats/html/Signatures.java | 6 +- .../doclet/testErasure/TestErasure.java | 15 +++-- .../testTypeParams/TestTypeParameters.java | 24 +++++++- .../testTypeParams/pkg/CtorTypeParam.java | 29 +++++++++ 8 files changed, 133 insertions(+), 35 deletions(-) create mode 100644 test/langtools/jdk/javadoc/doclet/testTypeParams/pkg/CtorTypeParam.java diff --git a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/AbstractExecutableMemberWriter.java b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/AbstractExecutableMemberWriter.java index 4e929130b66..f6ad4cf05df 100644 --- a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/AbstractExecutableMemberWriter.java +++ b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/AbstractExecutableMemberWriter.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1997, 2024, 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 @@ -31,6 +31,7 @@ import javax.lang.model.element.Element; import javax.lang.model.element.ElementKind; import javax.lang.model.element.ExecutableElement; import javax.lang.model.element.TypeElement; +import javax.lang.model.element.TypeParameterElement; import javax.lang.model.element.VariableElement; import javax.lang.model.type.DeclaredType; import javax.lang.model.type.ExecutableType; @@ -130,6 +131,24 @@ public abstract class AbstractExecutableMemberWriter extends AbstractMemberWrite target.add(writer.getDocLink(PLAIN, te, member, name(member))); } + /** + * Adds the generic type parameters. + * + * @param member the member to add the generic type parameters for + * @param target the content to which the generic type parameters will be added + */ + protected void addTypeParameters(ExecutableElement member, Content target) { + Content typeParameters = getTypeParameters(member); + target.add(typeParameters); + // Add explicit line break between method type parameters and + // return type in member summary table to avoid random wrapping. + if (typeParameters.charCount() > 10) { + target.add(new HtmlTree(TagName.BR)); + } else { + target.add(Entity.NO_BREAK_SPACE); + } + } + /** * Add the parameter for the executable member. * diff --git a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/AbstractMemberWriter.java b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/AbstractMemberWriter.java index caeb6c35e6d..af1f7ef1fe6 100644 --- a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/AbstractMemberWriter.java +++ b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/AbstractMemberWriter.java @@ -466,16 +466,8 @@ public abstract class AbstractMemberWriter { ? ((ExecutableElement)member).getTypeParameters() : null; if (list != null && !list.isEmpty()) { - Content typeParameters = ((AbstractExecutableMemberWriter) this) - .getTypeParameters((ExecutableElement)member); - code.add(typeParameters); - // Add explicit line break between method type parameters and - // return type in member summary table to avoid random wrapping. - if (typeParameters.charCount() > 10) { - code.add(new HtmlTree(TagName.BR)); - } else { - code.add(Entity.NO_BREAK_SPACE); - } + ((AbstractExecutableMemberWriter) this) + .addTypeParameters((ExecutableElement)member, code); } code.add( writer.getLink(new HtmlLinkInfo(configuration, diff --git a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/ClassUseWriter.java b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/ClassUseWriter.java index e949b4ddc68..94e66f1eed6 100644 --- a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/ClassUseWriter.java +++ b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/ClassUseWriter.java @@ -142,7 +142,7 @@ public class ClassUseWriter extends SubWriterHolderWriter { methodSubWriter = new MethodWriter(this); constrSubWriter = new ConstructorWriter(this); - constrSubWriter.setFoundNonPubConstructor(true); + constrSubWriter.setShowConstructorModifiers(true); fieldSubWriter = new FieldWriter(this); classSubWriter = new NestedClassWriter(this); } diff --git a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/ConstructorWriter.java b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/ConstructorWriter.java index 1c4af2faf97..c5fb332a008 100644 --- a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/ConstructorWriter.java +++ b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/ConstructorWriter.java @@ -31,6 +31,7 @@ import java.util.List; import javax.lang.model.element.Element; import javax.lang.model.element.ExecutableElement; import javax.lang.model.element.TypeElement; +import javax.lang.model.element.TypeParameterElement; import jdk.javadoc.internal.doclets.formats.html.markup.ContentBuilder; import jdk.javadoc.internal.doclets.formats.html.markup.Entity; @@ -53,7 +54,17 @@ public class ConstructorWriter extends AbstractExecutableMemberWriter { */ private ExecutableElement currentConstructor; - private boolean foundNonPubConstructor = false; + /** + * If any constructors are non-public, then we want the modifiers shown in the summary. + * This implies we need a three-column summary. + */ + private boolean showConstructorModifiers = false; + + /** + * Whether any constructors have type parameters. + * This implies we need a three column summary. + */ + private boolean hasTypeParamsConstructor = false; /** * Construct a new member writer for constructors. @@ -65,11 +76,7 @@ public class ConstructorWriter extends AbstractExecutableMemberWriter { // the following must be done before the summary table is generated var constructors = getVisibleMembers(VisibleMemberTable.Kind.CONSTRUCTORS); - for (Element constructor : constructors) { - if (utils.isProtected(constructor) || utils.isPrivate(constructor)) { - setFoundNonPubConstructor(true); - } - } + analyzeConstructors(constructors); } /** @@ -94,11 +101,7 @@ public class ConstructorWriter extends AbstractExecutableMemberWriter { protected void buildConstructorDoc(Content target) { var constructors = getVisibleMembers(VisibleMemberTable.Kind.CONSTRUCTORS); if (!constructors.isEmpty()) { - for (Element constructor : constructors) { - if (utils.isProtected(constructor) || utils.isPrivate(constructor)) { - setFoundNonPubConstructor(true); - } - } + analyzeConstructors(constructors); Content constructorDetailsHeader = getConstructorDetailsHeader(target); Content memberList = getMemberList(); @@ -126,6 +129,24 @@ public class ConstructorWriter extends AbstractExecutableMemberWriter { } } + // Calculate "showConstructorModifiers" and "hasTypeParamsConstructor" + private void analyzeConstructors(List constructors) { + for (Element constructor : constructors) { + if (utils.isProtected(constructor) || utils.isPrivate(constructor)) { + setShowConstructorModifiers(true); + } + List list = ((ExecutableElement)constructor).getTypeParameters(); + if (list != null && !list.isEmpty()) { + hasTypeParamsConstructor = true; + } + } + } + + // Does the constructor summary need three columnns or just two? + protected boolean threeColumnSummary() { + return showConstructorModifiers || hasTypeParamsConstructor; + } + @Override protected void buildSignature(Content target) { target.add(getSignature(currentConstructor)); @@ -201,6 +222,7 @@ public class ConstructorWriter extends AbstractExecutableMemberWriter { protected Content getSignature(ExecutableElement constructor) { return new Signatures.MemberSignature(constructor, this) + .setTypeParameters(getTypeParameters(constructor)) .setParameters(getParameters(constructor, true)) .setExceptions(getExceptions(constructor)) .setAnnotations(writer.getAnnotationInfo(constructor, true)) @@ -231,8 +253,8 @@ public class ConstructorWriter extends AbstractExecutableMemberWriter { .add(memberDetails)); } - protected void setFoundNonPubConstructor(boolean foundNonPubConstructor) { - this.foundNonPubConstructor = foundNonPubConstructor; + protected void setShowConstructorModifiers(boolean showConstructorModifiers) { + this.showConstructorModifiers = showConstructorModifiers; } @Override @@ -244,7 +266,7 @@ public class ConstructorWriter extends AbstractExecutableMemberWriter { @Override public TableHeader getSummaryTableHeader(Element member) { - if (foundNonPubConstructor) { + if (threeColumnSummary()) { return new TableHeader(contents.modifierLabel, contents.constructorLabel, contents.descriptionLabel); } else { @@ -256,7 +278,7 @@ public class ConstructorWriter extends AbstractExecutableMemberWriter { protected Table createSummaryTable() { List bodyRowStyles; - if (foundNonPubConstructor) { + if (threeColumnSummary()) { bodyRowStyles = Arrays.asList(HtmlStyle.colFirst, HtmlStyle.colConstructorName, HtmlStyle.colLast); } else { @@ -276,7 +298,7 @@ public class ConstructorWriter extends AbstractExecutableMemberWriter { @Override protected void addSummaryType(Element member, Content content) { - if (foundNonPubConstructor) { + if (threeColumnSummary()) { var code = new HtmlTree(TagName.CODE); if (utils.isProtected(member)) { code.add("protected "); @@ -288,6 +310,11 @@ public class ConstructorWriter extends AbstractExecutableMemberWriter { code.add( resources.getText("doclet.Package_private")); } + ExecutableElement constructor = (ExecutableElement)member; + List list = constructor.getTypeParameters(); + if (list != null && !list.isEmpty()) { + addTypeParameters(constructor, code); + } content.add(code); } } diff --git a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/Signatures.java b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/Signatures.java index f4f472de9f9..afc0648fa79 100644 --- a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/Signatures.java +++ b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/Signatures.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2020, 2024, 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 @@ -529,6 +529,7 @@ public class Signatures { private int appendTypeParameters(Content target, int lastLineSeparator) { // Apply different wrapping strategies for type parameters // depending on the combined length of type parameters and return type. + // Note return type will be null if this is a constructor. int typeParamLength = typeParameters.charCount(); if (typeParamLength >= TYPE_PARAMS_MAX_INLINE_LENGTH) { @@ -539,9 +540,10 @@ public class Signatures { int lineLength = target.charCount() - lastLineSeparator; int newLastLineSeparator = lastLineSeparator; + int returnTypeLength = returnType != null ? returnType.charCount() : 0; // sum below includes length of modifiers plus type params added above - if (lineLength + returnType.charCount() > RETURN_TYPE_MAX_LINE_LENGTH) { + if (lineLength + returnTypeLength > RETURN_TYPE_MAX_LINE_LENGTH) { target.add(Text.NL); newLastLineSeparator = target.charCount(); } else { diff --git a/test/langtools/jdk/javadoc/doclet/testErasure/TestErasure.java b/test/langtools/jdk/javadoc/doclet/testErasure/TestErasure.java index 3c9f9b6fd9d..f80074c50e2 100644 --- a/test/langtools/jdk/javadoc/doclet/testErasure/TestErasure.java +++ b/test/langtools/jdk/javadoc/doclet/testErasure/TestErasure.java @@ -82,15 +82,19 @@ public class TestErasure extends JavadocTester {

Constructor Summary

Constructors
-
-
Constructor
+
+
Modifier
+
Constructor
Description
+
 
\ Foo(T arg)
 
+
 <T extends X>
\ Foo(T arg)
 
+
 <T extends Y>
\ Foo(T arg)
 
@@ -188,13 +192,16 @@ public class TestErasure extends JavadocTester {

Constructor Summary

Constructors
-
-
Constructor
+
+
Modifier
+
Constructor
Description
+
 
\ Foo\ (T arg)
 
+
 <T extends X>
\ Foo(T arg)
 
diff --git a/test/langtools/jdk/javadoc/doclet/testTypeParams/TestTypeParameters.java b/test/langtools/jdk/javadoc/doclet/testTypeParams/TestTypeParameters.java index 7f586b3539d..0d95ee38cdc 100644 --- a/test/langtools/jdk/javadoc/doclet/testTypeParams/TestTypeParameters.java +++ b/test/langtools/jdk/javadoc/doclet/testTypeParams/TestTypeParameters.java @@ -23,12 +23,13 @@ /* * @test - * @bug 4927167 4974929 7010344 8025633 8081854 8182765 8187288 8261976 + * @bug 4927167 4974929 6381729 7010344 8025633 8081854 8182765 8187288 8261976 * @summary When the type parameters are more than 10 characters in length, * make sure there is a line break between type params and return type * in member summary. Also, test for type parameter links in package-summary and * class-use pages. The class/annotation pages should check for type * parameter links in the class/annotation signature section when -linksource is set. + * Verify that generic type parameters on constructors are documented. * @library ../../lib * @modules jdk.javadoc/jdk.javadoc.internal.tool * @build javadoc.tester.* @@ -94,4 +95,25 @@ public class TestTypeParameters extends JavadocTester { le="class in pkg">ParamTest2<java.util.List<? extends Foo4>>>"""); } + + @Test + public void test3() { + javadoc("-d", "out-3", + "-Xdoclint:none", + "--no-platform-links", + "-sourcepath", testSrc, + "pkg"); + checkExit(Exit.OK); + + checkOutput("pkg/CtorTypeParam.html", true, + """ +
 <T extends java.lang.Runnable>
+
\ + CtorTypeParam()
+
 
""", + """ +
public\ +  <T extends java.lang.Runnable>\ +  CtorTypeParam()
"""); + } } diff --git a/test/langtools/jdk/javadoc/doclet/testTypeParams/pkg/CtorTypeParam.java b/test/langtools/jdk/javadoc/doclet/testTypeParams/pkg/CtorTypeParam.java new file mode 100644 index 00000000000..a7f2309475d --- /dev/null +++ b/test/langtools/jdk/javadoc/doclet/testTypeParams/pkg/CtorTypeParam.java @@ -0,0 +1,29 @@ +/* + * Copyright (c) 2024, 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 pkg; + +public class CtorTypeParam { + public CtorTypeParam() { + } +}