Skip to content
This repository was archived by the owner on Sep 2, 2022. It is now read-only.

Commit 962f1c1

Browse files
hnshns
committed
8262886: javadoc generates broken links with {@inheritdoc}
Reviewed-by: jjg
1 parent f7ffd58 commit 962f1c1

File tree

8 files changed

+415
-71
lines changed

8 files changed

+415
-71
lines changed

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

Lines changed: 85 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -183,11 +183,6 @@ public class HtmlDocletWriter {
183183
*/
184184
protected boolean printedAnnotationHeading = false;
185185

186-
/**
187-
* To check whether annotation field heading is printed or not.
188-
*/
189-
protected boolean printedAnnotationFieldHeading = false;
190-
191186
/**
192187
* To check whether the repeated annotations is documented or not.
193188
*/
@@ -1647,13 +1642,41 @@ protected Boolean defaultAction(DocTree node, Content c) {
16471642
}
16481643

16491644
/**
1650-
* Return true if relative links should not be redirected.
1645+
* Returns true if relative links should be redirected.
16511646
*
1652-
* @return Return true if a relative link should not be redirected.
1647+
* @return true if a relative link should be redirected.
1648+
*/
1649+
private boolean shouldRedirectRelativeLinks(Element element) {
1650+
if (element == null || utils.isOverviewElement(element)) {
1651+
// Can't redirect unless there is a valid source element.
1652+
return false;
1653+
}
1654+
// Retrieve the element of this writer if it is a "primary" writer for an element.
1655+
// Note: It would be nice to have getCurrentPageElement() return package and module elements
1656+
// in their respective writers, but other uses of the method are only interested in TypeElements.
1657+
Element currentPageElement = getCurrentPageElement();
1658+
if (currentPageElement == null) {
1659+
if (this instanceof PackageWriterImpl packageWriter) {
1660+
currentPageElement = packageWriter.packageElement;
1661+
} else if (this instanceof ModuleWriterImpl moduleWriter) {
1662+
currentPageElement = moduleWriter.mdle;
1663+
}
1664+
}
1665+
// Redirect link if the current writer is not the primary writer for the source element.
1666+
return currentPageElement == null
1667+
|| (currentPageElement != element
1668+
&& currentPageElement != utils.getEnclosingTypeElement(element));
1669+
}
1670+
1671+
/**
1672+
* Returns true if element lives in the same package as the type or package
1673+
* element of this writer.
16531674
*/
1654-
private boolean shouldNotRedirectRelativeLinks() {
1655-
return this instanceof ClassWriter ||
1656-
this instanceof PackageSummaryWriter;
1675+
private boolean inSamePackage(Element element) {
1676+
Element currentPageElement = (this instanceof PackageWriterImpl packageWriter)
1677+
? packageWriter.packageElement : getCurrentPageElement();
1678+
return currentPageElement != null && !utils.isModule(element)
1679+
&& utils.containingPackage(currentPageElement) == utils.containingPackage(element);
16571680
}
16581681

16591682
/**
@@ -1681,47 +1704,66 @@ private boolean shouldNotRedirectRelativeLinks() {
16811704
*/
16821705
private String redirectRelativeLinks(Element element, TextTree tt) {
16831706
String text = tt.getBody();
1684-
if (element == null || utils.isOverviewElement(element) || shouldNotRedirectRelativeLinks()) {
1707+
if (!shouldRedirectRelativeLinks(element)) {
16851708
return text;
16861709
}
1687-
1688-
DocPath redirectPathFromRoot = new SimpleElementVisitor14<DocPath, Void>() {
1689-
@Override
1690-
public DocPath visitType(TypeElement e, Void p) {
1691-
return docPaths.forPackage(utils.containingPackage(e));
1710+
String lower = Utils.toLowerCase(text);
1711+
if (lower.startsWith("mailto:")
1712+
|| lower.startsWith("http:")
1713+
|| lower.startsWith("https:")
1714+
|| lower.startsWith("file:")) {
1715+
return text;
1716+
}
1717+
if (text.startsWith("#")) {
1718+
// Redirected fragment link: prepend HTML file name to make it work
1719+
if (utils.isModule(element)) {
1720+
text = "module-summary.html" + text;
1721+
} else if (utils.isPackage(element)) {
1722+
text = DocPaths.PACKAGE_SUMMARY.getPath() + text;
1723+
} else {
1724+
TypeElement typeElement = element instanceof TypeElement
1725+
? (TypeElement) element : utils.getEnclosingTypeElement(element);
1726+
text = docPaths.forName(typeElement).getPath() + text;
16921727
}
1728+
}
16931729

1694-
@Override
1695-
public DocPath visitPackage(PackageElement e, Void p) {
1696-
return docPaths.forPackage(e);
1697-
}
1730+
if (!inSamePackage(element)) {
1731+
DocPath redirectPathFromRoot = new SimpleElementVisitor14<DocPath, Void>() {
1732+
@Override
1733+
public DocPath visitType(TypeElement e, Void p) {
1734+
return docPaths.forPackage(utils.containingPackage(e));
1735+
}
16981736

1699-
@Override
1700-
public DocPath visitVariable(VariableElement e, Void p) {
1701-
return docPaths.forPackage(utils.containingPackage(e));
1702-
}
1737+
@Override
1738+
public DocPath visitPackage(PackageElement e, Void p) {
1739+
return docPaths.forPackage(e);
1740+
}
17031741

1704-
@Override
1705-
public DocPath visitExecutable(ExecutableElement e, Void p) {
1706-
return docPaths.forPackage(utils.containingPackage(e));
1707-
}
1742+
@Override
1743+
public DocPath visitVariable(VariableElement e, Void p) {
1744+
return docPaths.forPackage(utils.containingPackage(e));
1745+
}
17081746

1709-
@Override
1710-
protected DocPath defaultAction(Element e, Void p) {
1711-
return null;
1747+
@Override
1748+
public DocPath visitExecutable(ExecutableElement e, Void p) {
1749+
return docPaths.forPackage(utils.containingPackage(e));
1750+
}
1751+
1752+
@Override
1753+
public DocPath visitModule(ModuleElement e, Void p) {
1754+
return DocPaths.forModule(e);
1755+
}
1756+
1757+
@Override
1758+
protected DocPath defaultAction(Element e, Void p) {
1759+
return null;
1760+
}
1761+
}.visit(element);
1762+
if (redirectPathFromRoot != null) {
1763+
text = "{@" + (new DocRootTaglet()).getName() + "}/"
1764+
+ redirectPathFromRoot.resolve(text).getPath();
1765+
return replaceDocRootDir(text);
17121766
}
1713-
}.visit(element);
1714-
if (redirectPathFromRoot == null) {
1715-
return text;
1716-
}
1717-
String lower = Utils.toLowerCase(text);
1718-
if (!(lower.startsWith("mailto:")
1719-
|| lower.startsWith("http:")
1720-
|| lower.startsWith("https:")
1721-
|| lower.startsWith("file:"))) {
1722-
text = "{@" + (new DocRootTaglet()).getName() + "}/"
1723-
+ redirectPathFromRoot.resolve(text).getPath();
1724-
text = replaceDocRootDir(text);
17251767
}
17261768
return text;
17271769
}

‎test/langtools/jdk/javadoc/doclet/testRelativeLinks/TestRelativeLinks.java‎

Lines changed: 96 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2003, 2019, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2003, 2021, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -23,7 +23,7 @@
2323

2424
/*
2525
* @test
26-
* @bug 4460354 8014636 8043186 8195805 8182765 8196202
26+
* @bug 4460354 8014636 8043186 8195805 8182765 8196202 8262886
2727
* @summary Test to make sure that relative paths are redirected in the
2828
* output so that they are not broken.
2929
* @library ../../lib
@@ -46,77 +46,157 @@ public static void main(String... args) throws Exception {
4646
}
4747

4848
@Test
49-
public void test() {
49+
public void testRelativeLinks() {
5050
javadoc("-d", "out",
5151
"-use",
5252
"-sourcepath", testSrc,
53-
"pkg", "pkg2");
54-
checkExit(Exit.ERROR);
55-
56-
checkOutput(Output.OUT, true,
57-
"attribute not supported in HTML5: name");
53+
"pkg", "pkg2", "pkg.sub");
54+
checkExit(Exit.OK);
5855

5956
// These relative paths should stay relative because they appear
6057
// in the right places.
6158
checkOutput("pkg/C.html", true,
6259
"""
6360
<a href="relative-class-link.html">relative class link</a>""",
61+
"""
62+
<a href="#class-fragment">fragment class link</a>""",
63+
"""
64+
<a id="class-fragment">Class fragment</a>""",
6465
"""
6566
<a href="relative-field-link.html">relative field link</a>""",
6667
"""
6768
<a href="relative-method-link.html">relative method link</a>""",
6869
"""
69-
\s<a href="relative-multi-line-link.html">relative-multi-line-link</a>.""");
70+
<a href="#method-fragment">fragment method link</a>""",
71+
"""
72+
<a href="relative-multi-line-link.html">relative-multi-line-link</a>""");
73+
7074
checkOutput("pkg/package-summary.html", true,
7175
"""
72-
<a href="relative-package-link.html">relative package link</a>""");
76+
<a href="relative-package-link.html">relative package link</a>""",
77+
"""
78+
<a href="#package-fragment">package fragment link</a>""",
79+
"""
80+
<a id="package-fragment">Package fragment</a>""",
81+
"""
82+
<a href="relative-class-link.html">relative class link</a>""",
83+
"""
84+
<a href="C.html#class-fragment">fragment class link</a>""");
85+
86+
// subclass in same pacakge
87+
checkOutput("pkg/D.html", true,
88+
"""
89+
<a href="relative-class-link.html">relative class link</a>""",
90+
"""
91+
<a href="C.html#class-fragment">fragment class link</a>""",
92+
"""
93+
<a href="relative-method-link.html">relative method link</a>""",
94+
"""
95+
<a href="C.html#method-fragment">fragment method link</a>""");
7396

7497
// These relative paths should be redirected because they are in different
7598
// places.
7699

100+
// subclass in subpackage
101+
checkOutput("pkg/sub/F.html", true,
102+
"""
103+
<a href="../../pkg/relative-class-link.html">relative class link</a>""",
104+
"""
105+
<a href="../../pkg/C.html#class-fragment">fragment class link</a>""",
106+
"""
107+
<a href="../../pkg/relative-method-link.html">relative method link</a>""",
108+
"""
109+
<a href="../../pkg/C.html#method-fragment">fragment method link</a>""");
110+
77111
// INDEX PAGE
78112
checkOutput("index-all.html", true,
79113
"""
80114
<a href="./pkg/relative-class-link.html">relative class link</a>""",
115+
"""
116+
<a href="./pkg/C.html#class-fragment">fragment class link</a>""",
81117
"""
82118
<a href="./pkg/relative-field-link.html">relative field link</a>""",
83119
"""
84120
<a href="./pkg/relative-method-link.html">relative method link</a>""",
121+
"""
122+
<a href="./pkg/C.html#method-fragment">fragment method link</a>""",
85123
"""
86124
<a href="./pkg/relative-package-link.html">relative package link</a>""",
87125
"""
88-
\s<a href="./pkg/relative-multi-line-link.html">relative-multi-line-link</a>.""");
126+
<a href="./pkg/relative-multi-line-link.html">relative-multi-line-link</a>""");
89127

90128
// This is not a relative path and should not be redirected.
91129
checkOutput("index-all.html", true,
92130
"""
93-
<div class="block"><a name="masters"></a>""");
131+
<div class="block"><a id="masters"></a>""");
94132
checkOutput("index-all.html", false,
95133
"""
96-
<div class="block"><a name="./pkg/masters"></a>""");
134+
<div class="block"><a id="./pkg/masters"></a>""");
97135

98136
// PACKAGE USE
99137
checkOutput("pkg/package-use.html", true,
100138
"""
101-
<a href="../pkg/relative-package-link.html">relative package link</a>.""",
139+
<a href="../pkg/relative-package-link.html">relative package link</a>""",
140+
"""
141+
<a href="../pkg/package-summary.html#package-fragment">package fragment link</a>""",
102142
"""
103-
<a href="../pkg/relative-class-link.html">relative class link</a>""");
143+
<a href="../pkg/relative-class-link.html">relative class link</a>""",
144+
"""
145+
<a href="../pkg/C.html#class-fragment">fragment class link</a>""");
104146

105147
// CLASS_USE
106148
checkOutput("pkg/class-use/C.html", true,
149+
"""
150+
<a href="../../pkg/relative-class-link.html">relative class link</a>""",
151+
"""
152+
<a href="../../pkg/C.html#class-fragment">fragment class link</a>""",
107153
"""
108154
<a href="../../pkg/relative-field-link.html">relative field link</a>""",
109155
"""
110156
<a href="../../pkg/relative-method-link.html">relative method link</a>""",
157+
"""
158+
<a href="../../pkg/C.html#method-fragment">fragment method link</a>""",
111159
"""
112160
<a href="../../pkg/relative-package-link.html">relative package link</a>""",
113161
"""
114-
\s<a href="../../pkg/relative-multi-line-link.html">relative-multi-line-link</a>.""");
162+
<a href="../../pkg/relative-multi-line-link.html">relative-multi-line-link</a>""");
115163

116164
// PACKAGE OVERVIEW
117165
checkOutput("index.html", true,
118166
"""
119167
<a href="./pkg/relative-package-link.html">relative package link</a>""");
168+
169+
// subpackage summary
170+
checkOutput("pkg/sub/package-summary.html", true,
171+
// related packages
172+
"""
173+
<a href="../../pkg/relative-package-link.html">relative package link</a>""",
174+
"""
175+
<a href="../../pkg/package-summary.html#package-fragment">package fragment link</a>""",
176+
// subclass inheriting relative link doc
177+
"""
178+
<a href="../../pkg/relative-class-link.html">relative class link</a>""",
179+
"""
180+
<a href="../../pkg/C.html#class-fragment">fragment class link</a>""");
181+
182+
// sibling package summary
183+
checkOutput("pkg2/package-summary.html", true,
184+
"""
185+
<a href="../pkg/relative-class-link.html">relative class link</a>""",
186+
"""
187+
<a href="../pkg/C.html#class-fragment">fragment class link</a>""");
188+
}
189+
190+
@Override
191+
public void checkLinks() {
192+
// since the test uses explicit links to non-existent files,
193+
// we create those files to avoid false positive errors from checkLinks
194+
touch("pkg/relative-class-link.html");
195+
touch("pkg/relative-field-link.html");
196+
touch("pkg/relative-method-link.html");
197+
touch("pkg/relative-package-link.html");
198+
touch("pkg/relative-multi-line-link.html");
199+
super.checkLinks();
120200
}
121201

122202
private void touch(String file) {

0 commit comments

Comments
 (0)