8067757: Incorrect HTML generation for copied javadoc with multiple @throws tags

Reviewed-by: jjg
This commit is contained in:
Pavel Rappo 2022-07-04 16:00:53 +00:00
parent 0dff3276e8
commit f640fc5a1e
5 changed files with 405 additions and 57 deletions
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets
test/langtools/jdk/javadoc/doclet
testThrowsInheritance
testThrowsInheritanceMultiple

@ -504,7 +504,7 @@ public class TagletWriterImpl extends TagletWriter {
excName = htmlWriter.getLink(new HtmlLinkInfo(configuration, HtmlLinkInfo.Kind.MEMBER,
substituteType));
} else if (exception == null) {
excName = new RawHtml(ch.getExceptionName(throwsTag).toString());
excName = new RawHtml(throwsTag.getExceptionName().toString());
} else if (exception.asType() == null) {
excName = new RawHtml(utils.getFullyQualifiedName(exception));
} else {

@ -48,9 +48,7 @@ import com.sun.source.doctree.ThrowsTree;
import jdk.javadoc.doclet.Taglet.Location;
import jdk.javadoc.internal.doclets.toolkit.Content;
import jdk.javadoc.internal.doclets.toolkit.util.CommentHelper;
import jdk.javadoc.internal.doclets.toolkit.util.DocFinder;
import jdk.javadoc.internal.doclets.toolkit.util.Utils;
/**
* A taglet that processes {@link ThrowsTree}, which represents
@ -64,14 +62,14 @@ public class ThrowsTaglet extends BaseTaglet implements InheritableTaglet {
@Override
public void inherit(DocFinder.Input input, DocFinder.Output output) {
Utils utils = input.utils;
var utils = input.utils;
Element target;
CommentHelper ch = utils.getCommentHelper(input.element);
var ch = utils.getCommentHelper(input.element);
if (input.tagId == null) {
var tag = (ThrowsTree) input.docTreeInfo.docTree();
target = ch.getException(tag);
input.tagId = target == null
? ch.getExceptionName(tag).getSignature()
? tag.getExceptionName().getSignature()
: utils.getFullyQualifiedName(target);
} else {
target = input.utils.findClass(input.element, input.tagId);
@ -97,20 +95,20 @@ public class ThrowsTaglet extends BaseTaglet implements InheritableTaglet {
@Override
public Content getAllBlockTagOutput(Element holder, TagletWriter writer) {
Utils utils = writer.configuration().utils;
var utils = writer.configuration().utils;
var executable = (ExecutableElement) holder;
ExecutableType instantiatedType = utils.asInstantiatedMethodType(
writer.getCurrentPageElement(), executable);
List<? extends TypeMirror> thrownTypes = instantiatedType.getThrownTypes();
Map<String, TypeMirror> typeSubstitutions = getSubstitutedThrownTypes(
writer.configuration().utils.typeUtils,
utils.typeUtils,
executable.getThrownTypes(),
thrownTypes);
Map<List<ThrowsTree>, ExecutableElement> tagsMap = new LinkedHashMap<>();
tagsMap.put(utils.getThrowsTrees(executable), executable);
Map<ThrowsTree, ExecutableElement> tagsMap = new LinkedHashMap<>();
utils.getThrowsTrees(executable).forEach(t -> tagsMap.put(t, executable));
Content result = writer.getOutputInstance();
Set<String> alreadyDocumented = new HashSet<>();
result.add(throwsTagsOutput(tagsMap, writer, alreadyDocumented, typeSubstitutions, true));
result.add(throwsTagsOutput(tagsMap, alreadyDocumented, typeSubstitutions, writer));
result.add(inheritThrowsDocumentation(executable, thrownTypes, alreadyDocumented, typeSubstitutions, writer));
result.add(linkToUndocumentedDeclaredExceptions(thrownTypes, alreadyDocumented, writer));
return result;
@ -145,45 +143,48 @@ public class ThrowsTaglet extends BaseTaglet implements InheritableTaglet {
/**
* Returns the generated content for a collection of {@code @throws} tags.
*
* @param throwsTags the collection of tags to be converted
* @param throwsTags the tags to be converted; each tag is mapped to
* a method it appears on
* @param alreadyDocumented the set of exceptions that have already been
* documented and thus must not be documented by
* this method. All exceptions documented by this
* method will be added to this set upon the
* method's return.
* @param writer the taglet-writer used by the doclet
* @param alreadyDocumented the set of exceptions that have already been documented
* @param allowDuplicates {@code true} if we allow duplicate tags to be documented
* @return the generated content for the tags
*/
protected Content throwsTagsOutput(Map<List<ThrowsTree>, ExecutableElement> throwsTags,
TagletWriter writer,
Set<String> alreadyDocumented,
Map<String, TypeMirror> typeSubstitutions,
boolean allowDuplicates) {
Utils utils = writer.configuration().utils;
private Content throwsTagsOutput(Map<ThrowsTree, ExecutableElement> throwsTags,
Set<String> alreadyDocumented,
Map<String, TypeMirror> typeSubstitutions,
TagletWriter writer) {
var utils = writer.configuration().utils;
Content result = writer.getOutputInstance();
for (Entry<List<ThrowsTree>, ExecutableElement> entry : throwsTags.entrySet()) {
var documentedInThisCall = new HashSet<String>();
for (Entry<ThrowsTree, ExecutableElement> entry : throwsTags.entrySet()) {
Element e = entry.getValue();
CommentHelper ch = utils.getCommentHelper(e);
for (ThrowsTree tag : entry.getKey()) {
Element te = ch.getException(tag);
String excName = ch.getExceptionName(tag).toString();
TypeMirror substituteType = typeSubstitutions.get(excName);
if ((!allowDuplicates) &&
(alreadyDocumented.contains(excName) ||
(te != null && alreadyDocumented.contains(utils.getFullyQualifiedName(te, false)))) ||
(substituteType != null && alreadyDocumented.contains(substituteType.toString()))) {
continue;
}
if (alreadyDocumented.isEmpty()) {
result.add(writer.getThrowsHeader());
}
result.add(writer.throwsTagOutput(e, tag, substituteType));
if (substituteType != null) {
alreadyDocumented.add(substituteType.toString());
} else {
alreadyDocumented.add(te != null
? utils.getFullyQualifiedName(te, false)
: excName);
}
var ch = utils.getCommentHelper(e);
ThrowsTree tag = entry.getKey();
Element te = ch.getException(tag);
String excName = tag.getExceptionName().toString();
TypeMirror substituteType = typeSubstitutions.get(excName);
if (alreadyDocumented.contains(excName)
|| (te != null && alreadyDocumented.contains(utils.getFullyQualifiedName(te, false)))
|| (substituteType != null && alreadyDocumented.contains(substituteType.toString()))) {
continue;
}
if (alreadyDocumented.isEmpty() && documentedInThisCall.isEmpty()) {
result.add(writer.getThrowsHeader());
}
result.add(writer.throwsTagOutput(e, tag, substituteType));
if (substituteType != null) {
documentedInThisCall.add(substituteType.toString());
} else {
documentedInThisCall.add(te != null
? utils.getFullyQualifiedName(te, false)
: excName);
}
}
alreadyDocumented.addAll(documentedInThisCall);
return result;
}
@ -205,8 +206,8 @@ public class ThrowsTaglet extends BaseTaglet implements InheritableTaglet {
assert holder.getKind() == ElementKind.CONSTRUCTOR : holder.getKind();
return result;
}
Utils utils = writer.configuration().utils;
Map<List<ThrowsTree>, ExecutableElement> declaredExceptionTags = new LinkedHashMap<>();
var utils = writer.configuration().utils;
Map<ThrowsTree, ExecutableElement> declaredExceptionTags = new LinkedHashMap<>();
for (TypeMirror declaredExceptionType : declaredExceptionTypes) {
var input = new DocFinder.Input(utils, holder, this,
utils.getTypeName(declaredExceptionType, false));
@ -220,14 +221,12 @@ public class ThrowsTaglet extends BaseTaglet implements InheritableTaglet {
if (inheritedDoc.holder == null) {
inheritedDoc.holder = holder;
}
List<ThrowsTree> inheritedTags = inheritedDoc.tagList.stream()
.map(t -> (ThrowsTree) t)
.toList();
declaredExceptionTags.put(inheritedTags, (ExecutableElement) inheritedDoc.holder);
var h = (ExecutableElement) inheritedDoc.holder;
inheritedDoc.tagList.forEach(t -> declaredExceptionTags.put((ThrowsTree) t, h));
}
}
result.add(throwsTagsOutput(declaredExceptionTags, writer, alreadyDocumented,
typeSubstitutions, false));
result.add(throwsTagsOutput(declaredExceptionTags, alreadyDocumented, typeSubstitutions,
writer));
return result;
}
@ -235,7 +234,7 @@ public class ThrowsTaglet extends BaseTaglet implements InheritableTaglet {
Set<String> alreadyDocumented,
TagletWriter writer) {
// TODO: assert declaredExceptionTypes are instantiated
Utils utils = writer.configuration().utils;
var utils = writer.configuration().utils;
Content result = writer.getOutputInstance();
for (TypeMirror declaredExceptionType : declaredExceptionTypes) {
TypeElement te = utils.asTypeElement(declaredExceptionType);

@ -547,10 +547,6 @@ public class CommentHelper {
return dtree.getKind() == SEE ? ((SeeTree)dtree).getReference() : null;
}
public ReferenceTree getExceptionName(ThrowsTree tt) {
return tt.getExceptionName();
}
public IdentifierTree getName(DocTree dtree) {
switch (dtree.getKind()) {
case PARAM:

@ -69,7 +69,7 @@ public class TestThrowsTagInheritance extends JavadocTester {
checkExit(Exit.OK);
// The method should not inherit the IOOBE throws tag from the abstract class,
// for now keep keep this bug compatible, should fix this correctly in
// for now keep this bug compatible, should fix this correctly in
// the future.
checkOutput("pkg/Extender.html", false, "java.lang.IndexOutOfBoundsException");
}

@ -0,0 +1,353 @@
/*
* Copyright (c) 2022, 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 8067757
* @library /tools/lib ../../lib
* @modules jdk.compiler/com.sun.tools.javac.api
* jdk.compiler/com.sun.tools.javac.main
* jdk.javadoc/jdk.javadoc.internal.tool
* @build toolbox.ToolBox javadoc.tester.*
* @run main TestOneToMany
*/
import javadoc.tester.JavadocTester;
import toolbox.ToolBox;
import java.nio.file.Path;
import java.nio.file.Paths;
public class TestOneToMany extends JavadocTester {
public static void main(String... args) throws Exception {
var tester = new TestOneToMany();
tester.runTests(m -> new Object[]{Paths.get(m.getName())});
}
private final ToolBox tb = new ToolBox();
// These tests:
//
// - Use own exceptions to not depend on platform links or a setup with
// the no-platform-links option
// - Enclose files in a package to exercise a typical source layout and
// avoid enumerating separate files in the javadoc command
@Test
public void testUncheckedException(Path base) throws Exception {
var src = base.resolve("src");
tb.writeJavaFiles(src, """
package x;
public class MyRuntimeException extends RuntimeException { }
""", """
package x;
public interface I {
/**
* @throws MyRuntimeException if this
* @throws MyRuntimeException if that
*/
void m();
}
""", """
package x;
public interface I1 extends I {
@Override
void m() throws MyRuntimeException;
}
""", """
package x;
public class IImpl implements I {
@Override
public void m() throws MyRuntimeException { }
}
""", """
package x;
public class C {
/**
* @throws MyRuntimeException if this
* @throws MyRuntimeException if that
*/
public void m();
}
""", """
package x;
public class C1 extends C {
@Override
public void m() throws MyRuntimeException { }
}
""");
javadoc("-d", base.resolve("out").toString(),
"-sourcepath", src.toString(),
"x");
checkExit(Exit.OK);
checkOutput("x/IImpl.html", true, """
<dl class="notes">
<dt>Specified by:</dt>
<dd><code><a href="I.html#m()">m</a></code>&nbsp;in interface&nbsp;<code><a href="I.html" title="interface in x">I</a></code></dd>
<dt>Throws:</dt>
<dd><code><a href="MyRuntimeException.html" title="class in x">MyRuntimeException</a></code> - if this</dd>
<dd><code><a href="MyRuntimeException.html" title="class in x">MyRuntimeException</a></code> - if that</dd>
</dl>""");
checkOutput("x/I1.html", true, """
<dl class="notes">
<dt>Specified by:</dt>
<dd><code><a href="I.html#m()">m</a></code>&nbsp;in interface&nbsp;<code><a href="I.html" title="interface in x">I</a></code></dd>
<dt>Throws:</dt>
<dd><code><a href="MyRuntimeException.html" title="class in x">MyRuntimeException</a></code> - if this</dd>
<dd><code><a href="MyRuntimeException.html" title="class in x">MyRuntimeException</a></code> - if that</dd>
</dl>""");
checkOutput("x/C1.html", true, """
<dl class="notes">
<dt>Overrides:</dt>
<dd><code><a href="C.html#m()">m</a></code>&nbsp;in class&nbsp;<code><a href="C.html" title="class in x">C</a></code></dd>
<dt>Throws:</dt>
<dd><code><a href="MyRuntimeException.html" title="class in x">MyRuntimeException</a></code> - if this</dd>
<dd><code><a href="MyRuntimeException.html" title="class in x">MyRuntimeException</a></code> - if that</dd>
</dl>""");
}
@Test
public void testUncheckedExceptionWithRedundantThrows(Path base) throws Exception {
var src = base.resolve("src");
tb.writeJavaFiles(src, """
package x;
public class MyRuntimeException extends RuntimeException { }
""", """
package x;
public interface I {
/**
* @throws MyRuntimeException if this
* @throws MyRuntimeException if that
*/
void m() throws MyRuntimeException;
}
""", """
package x;
public interface I1 extends I {
@Override
void m() throws MyRuntimeException;
}
""", """
package x;
public class IImpl implements I {
@Override
public void m() throws MyRuntimeException { }
}
""", """
package x;
public class C {
/**
* @throws MyRuntimeException if this
* @throws MyRuntimeException if that
*/
public void m() throws MyRuntimeException;
}
""", """
package x;
public class C1 extends C {
@Override
public void m() throws MyRuntimeException { }
}
""");
javadoc("-d", base.resolve("out").toString(),
"-sourcepath", src.toString(),
"x");
checkExit(Exit.OK);
checkOutput("x/IImpl.html", true, """
<dl class="notes">
<dt>Specified by:</dt>
<dd><code><a href="I.html#m()">m</a></code>&nbsp;in interface&nbsp;<code><a href="I.html" title="interface in x">I</a></code></dd>
<dt>Throws:</dt>
<dd><code><a href="MyRuntimeException.html" title="class in x">MyRuntimeException</a></code> - if this</dd>
<dd><code><a href="MyRuntimeException.html" title="class in x">MyRuntimeException</a></code> - if that</dd>
</dl>""");
checkOutput("x/I1.html", true, """
<dl class="notes">
<dt>Specified by:</dt>
<dd><code><a href="I.html#m()">m</a></code>&nbsp;in interface&nbsp;<code><a href="I.html" title="interface in x">I</a></code></dd>
<dt>Throws:</dt>
<dd><code><a href="MyRuntimeException.html" title="class in x">MyRuntimeException</a></code> - if this</dd>
<dd><code><a href="MyRuntimeException.html" title="class in x">MyRuntimeException</a></code> - if that</dd>
</dl>""");
checkOutput("x/C1.html", true, """
<dl class="notes">
<dt>Overrides:</dt>
<dd><code><a href="C.html#m()">m</a></code>&nbsp;in class&nbsp;<code><a href="C.html" title="class in x">C</a></code></dd>
<dt>Throws:</dt>
<dd><code><a href="MyRuntimeException.html" title="class in x">MyRuntimeException</a></code> - if this</dd>
<dd><code><a href="MyRuntimeException.html" title="class in x">MyRuntimeException</a></code> - if that</dd>
</dl>""");
}
@Test
public void testCheckedException(Path base) throws Exception {
var src = base.resolve("src");
tb.writeJavaFiles(src, """
package x;
public class MyCheckedException extends Exception { }
""", """
package x;
public interface I {
/**
* @throws MyCheckedException if this
* @throws MyCheckedException if that
*/
void m() throws MyCheckedException;
}
""", """
package x;
public interface I1 extends I {
@Override
void m() throws MyCheckedException;
}
""", """
package x;
public class IImpl implements I {
@Override
public void m() throws MyCheckedException { }
}
""", """
package x;
public class C {
/**
* @throws MyCheckedException if this
* @throws MyCheckedException if that
*/
public void m() throws MyCheckedException;
}
""", """
package x;
public class C1 extends C {
@Override
public void m() throws MyCheckedException { }
}
""");
javadoc("-d", base.resolve("out").toString(),
"-sourcepath", src.toString(),
"x");
checkExit(Exit.OK);
checkOutput("x/IImpl.html", true, """
<dl class="notes">
<dt>Specified by:</dt>
<dd><code><a href="I.html#m()">m</a></code>&nbsp;in interface&nbsp;<code><a href="I.html" title="interface in x">I</a></code></dd>
<dt>Throws:</dt>
<dd><code><a href="MyCheckedException.html" title="class in x">MyCheckedException</a></code> - if this</dd>
<dd><code><a href="MyCheckedException.html" title="class in x">MyCheckedException</a></code> - if that</dd>
</dl>""");
checkOutput("x/I1.html", true, """
<dl class="notes">
<dt>Specified by:</dt>
<dd><code><a href="I.html#m()">m</a></code>&nbsp;in interface&nbsp;<code><a href="I.html" title="interface in x">I</a></code></dd>
<dt>Throws:</dt>
<dd><code><a href="MyCheckedException.html" title="class in x">MyCheckedException</a></code> - if this</dd>
<dd><code><a href="MyCheckedException.html" title="class in x">MyCheckedException</a></code> - if that</dd>
</dl>""");
checkOutput("x/C1.html", true, """
<dl class="notes">
<dt>Overrides:</dt>
<dd><code><a href="C.html#m()">m</a></code>&nbsp;in class&nbsp;<code><a href="C.html" title="class in x">C</a></code></dd>
<dt>Throws:</dt>
<dd><code><a href="MyCheckedException.html" title="class in x">MyCheckedException</a></code> - if this</dd>
<dd><code><a href="MyCheckedException.html" title="class in x">MyCheckedException</a></code> - if that</dd>
</dl>""");
}
@Test
public void testSubExceptionDoubleInheritance(Path base) throws Exception {
var src = base.resolve("src");
tb.writeJavaFiles(src, """
package x;
public class MyException extends Exception { }
""", """
package x;
public class MySubException extends MyException { }
""", """
package x;
public interface I {
/**
* @throws MyException if this
* @throws MySubException if that
*/
void m() throws MyException, MySubException;
}
""", """
package x;
public interface I1 extends I {
@Override
void m() throws MyException, MySubException;
}
""");
javadoc("-d", base.resolve("out").toString(),
"-sourcepath", src.toString(),
"x");
checkExit(Exit.OK);
checkOutput("x/I1.html", true, """
<dl class="notes">
<dt>Specified by:</dt>
<dd><code><a href="I.html#m()">m</a></code>&nbsp;in interface&nbsp;<code><a href="I.html" title="interface in x">I</a></code></dd>
<dt>Throws:</dt>
<dd><code><a href="MyException.html" title="class in x">MyException</a></code> - if this</dd>
<dd><code><a href="MySubException.html" title="class in x">MySubException</a></code> - if that</dd>
</dl>""");
}
}