6381729: Javadoc for generic constructor doesn't document type parameter

Reviewed-by: jjg, liach
This commit is contained in:
Archie Cobbs 2024-07-29 19:16:39 +00:00
parent c4e6255ac3
commit a86244f83c
8 changed files with 133 additions and 35 deletions

View File

@ -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.
*

View File

@ -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,

View File

@ -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);
}

View File

@ -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<? extends Element> constructors) {
for (Element constructor : constructors) {
if (utils.isProtected(constructor) || utils.isPrivate(constructor)) {
setShowConstructorModifiers(true);
}
List<? extends TypeParameterElement> 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<Element> createSummaryTable() {
List<HtmlStyle> 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<? extends TypeParameterElement> list = constructor.getTypeParameters();
if (list != null && !list.isEmpty()) {
addTypeParameters(constructor, code);
}
content.add(code);
}
}

View File

@ -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 {

View File

@ -82,15 +82,19 @@ public class TestErasure extends JavadocTester {
<section class="constructor-summary" id="constructor-summary">
<h2>Constructor Summary</h2>
<div class="caption"><span>Constructors</span></div>
<div class="summary-table two-column-summary">
<div class="table-header col-first">Constructor</div>
<div class="summary-table three-column-summary">
<div class="table-header col-first">Modifier</div>
<div class="table-header col-second">Constructor</div>
<div class="table-header col-last">Description</div>
<div class="col-first even-row-color"><code>&nbsp;</code></div>
<div class="col-constructor-name even-row-color"><code>\
<a href="#%3Cinit%3E(T)" class="member-name-link">Foo</a><wbr>(T&nbsp;arg)</code></div>
<div class="col-last even-row-color">&nbsp;</div>
<div class="col-first odd-row-color"><code>&nbsp;&lt;T extends X&gt;<br></code></div>
<div class="col-constructor-name odd-row-color"><code>\
<a href="#%3Cinit%3E(X)" class="member-name-link">Foo</a><wbr>(T&nbsp;arg)</code></div>
<div class="col-last odd-row-color">&nbsp;</div>
<div class="col-first even-row-color"><code>&nbsp;&lt;T extends Y&gt;<br></code></div>
<div class="col-constructor-name even-row-color"><code>\
<a href="#%3Cinit%3E(Y)" class="member-name-link">Foo</a><wbr>(T&nbsp;arg)</code></div>
<div class="col-last even-row-color">&nbsp;</div>
@ -188,13 +192,16 @@ public class TestErasure extends JavadocTester {
<section class="constructor-summary" id="constructor-summary">
<h2>Constructor Summary</h2>
<div class="caption"><span>Constructors</span></div>
<div class="summary-table two-column-summary">
<div class="table-header col-first">Constructor</div>
<div class="summary-table three-column-summary">
<div class="table-header col-first">Modifier</div>
<div class="table-header col-second">Constructor</div>
<div class="table-header col-last">Description</div>
<div class="col-first even-row-color"><code>&nbsp;</code></div>
<div class="col-constructor-name even-row-color"><code>\
<a href="#%3Cinit%3E(T)" class="member-name-link">Foo</a>\
<wbr>(<a href="Foo.html" title="type parameter in Foo">T</a>&nbsp;arg)</code></div>
<div class="col-last even-row-color">&nbsp;</div>
<div class="col-first odd-row-color"><code>&nbsp;&lt;T extends X&gt;<br></code></div>
<div class="col-constructor-name odd-row-color"><code>\
<a href="#%3Cinit%3E(X)" class="member-name-link">Foo</a><wbr>(T&nbsp;arg)</code></div>
<div class="col-last odd-row-color">&nbsp;</div>

View File

@ -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</a>&lt;java.util.List&lt;? extends <a href="Foo4.ht\
ml" title="class in pkg">Foo4</a>&gt;&gt;&gt;""");
}
@Test
public void test3() {
javadoc("-d", "out-3",
"-Xdoclint:none",
"--no-platform-links",
"-sourcepath", testSrc,
"pkg");
checkExit(Exit.OK);
checkOutput("pkg/CtorTypeParam.html", true,
"""
<div class="col-first even-row-color"><code>&nbsp;&lt;T extends java.lang.Runnable&gt;<br></code></div>
<div class="col-constructor-name even-row-color"><code>\
<a href="#%3Cinit%3E()" class="member-name-link">CtorTypeParam</a>()</code></div>
<div class="col-last even-row-color">&nbsp;</div>""",
"""
<div class="member-signature"><span class="modifiers">public</span>\
&nbsp;<span class="type-parameters">&lt;T extends java.lang.Runnable&gt;</span>\
&nbsp;<span class="element-name">CtorTypeParam</span>()</div>""");
}
}

View File

@ -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 <T extends Runnable> CtorTypeParam() {
}
}