8262886: javadoc generates broken links with {@inheritDoc}

Reviewed-by: jjg

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

@@ -183,11 +183,6 @@ public class HtmlDocletWriter {
      */
     protected boolean printedAnnotationHeading = false;
 
-    /**
-     * To check whether annotation field heading is printed or not.
-     */
-    protected boolean printedAnnotationFieldHeading = false;
-
     /**
      * To check whether the repeated annotations is documented or not.
      */
@@ -1647,13 +1642,41 @@ public class HtmlDocletWriter {
     }
 
     /**
-     * Return true if relative links should not be redirected.
+     * Returns true if relative links should be redirected.
      *
-     * @return Return true if a relative link should not be redirected.
+     * @return true if a relative link should be redirected.
      */
-    private boolean shouldNotRedirectRelativeLinks() {
+    private boolean shouldRedirectRelativeLinks(Element element) {
-        return  this instanceof ClassWriter ||
+        if (element == null || utils.isOverviewElement(element)) {
-                this instanceof PackageSummaryWriter;
+            // Can't redirect unless there is a valid source element.
+            return false;
+        }
+        // Retrieve the element of this writer if it is a "primary" writer for an element.
+        // Note: It would be nice to have getCurrentPageElement() return package and module elements
+        // in their respective writers, but other uses of the method are only interested in TypeElements.
+        Element currentPageElement = getCurrentPageElement();
+        if (currentPageElement == null) {
+            if (this instanceof PackageWriterImpl packageWriter) {
+                currentPageElement = packageWriter.packageElement;
+            } else if (this instanceof ModuleWriterImpl moduleWriter) {
+                currentPageElement = moduleWriter.mdle;
+            }
+        }
+        // Redirect link if the current writer is not the primary writer for the source element.
+        return currentPageElement == null
+                || (currentPageElement != element
+                    &&  currentPageElement != utils.getEnclosingTypeElement(element));
+    }
+
+    /**
+     * Returns true if element lives in the same package as the type or package
+     * element of this writer.
+     */
+    private boolean inSamePackage(Element element) {
+        Element currentPageElement = (this instanceof PackageWriterImpl packageWriter)
+                ? packageWriter.packageElement : getCurrentPageElement();
+        return currentPageElement != null && !utils.isModule(element)
+                && utils.containingPackage(currentPageElement) == utils.containingPackage(element);
     }
 
     /**
@@ -1681,47 +1704,66 @@ public class HtmlDocletWriter {
      */
     private String redirectRelativeLinks(Element element, TextTree tt) {
         String text = tt.getBody();
-        if (element == null || utils.isOverviewElement(element) || shouldNotRedirectRelativeLinks()) {
+        if (!shouldRedirectRelativeLinks(element)) {
-            return text;
-        }
-
-        DocPath redirectPathFromRoot = new SimpleElementVisitor14<DocPath, Void>() {
-            @Override
-            public DocPath visitType(TypeElement e, Void p) {
-                return docPaths.forPackage(utils.containingPackage(e));
-            }
-
-            @Override
-            public DocPath visitPackage(PackageElement e, Void p) {
-                return docPaths.forPackage(e);
-            }
-
-            @Override
-            public DocPath visitVariable(VariableElement e, Void p) {
-                return docPaths.forPackage(utils.containingPackage(e));
-            }
-
-            @Override
-            public DocPath visitExecutable(ExecutableElement e, Void p) {
-                return docPaths.forPackage(utils.containingPackage(e));
-            }
-
-            @Override
-            protected DocPath defaultAction(Element e, Void p) {
-                return null;
-            }
-        }.visit(element);
-        if (redirectPathFromRoot == null) {
             return text;
         }
         String lower = Utils.toLowerCase(text);
-        if (!(lower.startsWith("mailto:")
+        if (lower.startsWith("mailto:")
                 || lower.startsWith("http:")
                 || lower.startsWith("https:")
-                || lower.startsWith("file:"))) {
+                || lower.startsWith("file:")) {
-            text = "{@" + (new DocRootTaglet()).getName() + "}/"
+            return text;
-                    + redirectPathFromRoot.resolve(text).getPath();
+        }
-            text = replaceDocRootDir(text);
+        if (text.startsWith("#")) {
+            // Redirected fragment link: prepend HTML file name to make it work
+            if (utils.isModule(element)) {
+                text = "module-summary.html" + text;
+            } else if (utils.isPackage(element)) {
+                text = DocPaths.PACKAGE_SUMMARY.getPath() + text;
+            } else {
+                TypeElement typeElement = element instanceof TypeElement
+                        ? (TypeElement) element : utils.getEnclosingTypeElement(element);
+                text = docPaths.forName(typeElement).getPath() + text;
+            }
+        }
+
+        if (!inSamePackage(element)) {
+            DocPath redirectPathFromRoot = new SimpleElementVisitor14<DocPath, Void>() {
+                @Override
+                public DocPath visitType(TypeElement e, Void p) {
+                    return docPaths.forPackage(utils.containingPackage(e));
+                }
+
+                @Override
+                public DocPath visitPackage(PackageElement e, Void p) {
+                    return docPaths.forPackage(e);
+                }
+
+                @Override
+                public DocPath visitVariable(VariableElement e, Void p) {
+                    return docPaths.forPackage(utils.containingPackage(e));
+                }
+
+                @Override
+                public DocPath visitExecutable(ExecutableElement e, Void p) {
+                    return docPaths.forPackage(utils.containingPackage(e));
+                }
+
+                @Override
+                public DocPath visitModule(ModuleElement e, Void p) {
+                    return DocPaths.forModule(e);
+                }
+
+                @Override
+                protected DocPath defaultAction(Element e, Void p) {
+                    return null;
+                }
+            }.visit(element);
+            if (redirectPathFromRoot != null) {
+                text = "{@" + (new DocRootTaglet()).getName() + "}/"
+                        + redirectPathFromRoot.resolve(text).getPath();
+                return replaceDocRootDir(text);
+            }
         }
         return text;
     }

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2021, 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
@@ -23,7 +23,7 @@
 
 /*
  * @test
- * @bug      4460354 8014636 8043186 8195805 8182765 8196202
+ * @bug      4460354 8014636 8043186 8195805 8182765 8196202 8262886
  * @summary  Test to make sure that relative paths are redirected in the
  *           output so that they are not broken.
  * @library  ../../lib
@@ -46,77 +46,157 @@ public class TestRelativeLinks extends JavadocTester {
     }
 
     @Test
-    public void test() {
+    public void testRelativeLinks() {
         javadoc("-d", "out",
                 "-use",
                 "-sourcepath", testSrc,
-                "pkg", "pkg2");
+                "pkg", "pkg2", "pkg.sub");
-        checkExit(Exit.ERROR);
+        checkExit(Exit.OK);
-
-        checkOutput(Output.OUT, true,
-                "attribute not supported in HTML5: name");
 
         // These relative paths should stay relative because they appear
         // in the right places.
         checkOutput("pkg/C.html", true,
             """
                 <a href="relative-class-link.html">relative class link</a>""",
+            """
+                <a href="#class-fragment">fragment class link</a>""",
+            """
+                <a id="class-fragment">Class fragment</a>""",
             """
                 <a href="relative-field-link.html">relative field link</a>""",
             """
                 <a href="relative-method-link.html">relative method link</a>""",
             """
-                \s<a href="relative-multi-line-link.html">relative-multi-line-link</a>.""");
+                <a href="#method-fragment">fragment method link</a>""",
+            """
+                <a href="relative-multi-line-link.html">relative-multi-line-link</a>""");
+
         checkOutput("pkg/package-summary.html", true,
             """
-                <a href="relative-package-link.html">relative package link</a>""");
+                <a href="relative-package-link.html">relative package link</a>""",
+            """
+                <a href="#package-fragment">package fragment link</a>""",
+            """
+                <a id="package-fragment">Package fragment</a>""",
+            """
+                <a href="relative-class-link.html">relative class link</a>""",
+            """
+                <a href="C.html#class-fragment">fragment class link</a>""");
+
+        // subclass in same pacakge
+        checkOutput("pkg/D.html", true,
+                """
+                    <a href="relative-class-link.html">relative class link</a>""",
+                """
+                    <a href="C.html#class-fragment">fragment class link</a>""",
+                """
+                    <a href="relative-method-link.html">relative method link</a>""",
+                """
+                    <a href="C.html#method-fragment">fragment method link</a>""");
 
         // These relative paths should be redirected because they are in different
         // places.
 
+        // subclass in subpackage
+        checkOutput("pkg/sub/F.html", true,
+                """
+                    <a href="../../pkg/relative-class-link.html">relative class link</a>""",
+                """
+                    <a href="../../pkg/C.html#class-fragment">fragment class link</a>""",
+                """
+                    <a href="../../pkg/relative-method-link.html">relative method link</a>""",
+                """
+                    <a href="../../pkg/C.html#method-fragment">fragment method link</a>""");
+
         // INDEX PAGE
         checkOutput("index-all.html", true,
             """
                 <a href="./pkg/relative-class-link.html">relative class link</a>""",
+            """
+                <a href="./pkg/C.html#class-fragment">fragment class link</a>""",
             """
                 <a href="./pkg/relative-field-link.html">relative field link</a>""",
             """
                 <a href="./pkg/relative-method-link.html">relative method link</a>""",
+            """
+                <a href="./pkg/C.html#method-fragment">fragment method link</a>""",
             """
                 <a href="./pkg/relative-package-link.html">relative package link</a>""",
             """
-                \s<a href="./pkg/relative-multi-line-link.html">relative-multi-line-link</a>.""");
+                <a href="./pkg/relative-multi-line-link.html">relative-multi-line-link</a>""");
 
         // This is not a relative path and should not be redirected.
         checkOutput("index-all.html", true,
             """
-                <div class="block"><a name="masters"></a>""");
+                <div class="block"><a id="masters"></a>""");
         checkOutput("index-all.html", false,
             """
-                <div class="block"><a name="./pkg/masters"></a>""");
+                <div class="block"><a id="./pkg/masters"></a>""");
 
         // PACKAGE USE
         checkOutput("pkg/package-use.html", true,
             """
-                <a href="../pkg/relative-package-link.html">relative package link</a>.""",
+                <a href="../pkg/relative-package-link.html">relative package link</a>""",
             """
-                <a href="../pkg/relative-class-link.html">relative class link</a>""");
+                <a href="../pkg/package-summary.html#package-fragment">package fragment link</a>""",
+            """
+                <a href="../pkg/relative-class-link.html">relative class link</a>""",
+            """
+                <a href="../pkg/C.html#class-fragment">fragment class link</a>""");
 
         // CLASS_USE
         checkOutput("pkg/class-use/C.html", true,
+            """
+                <a href="../../pkg/relative-class-link.html">relative class link</a>""",
+            """
+                <a href="../../pkg/C.html#class-fragment">fragment class link</a>""",
             """
                 <a href="../../pkg/relative-field-link.html">relative field link</a>""",
             """
                 <a href="../../pkg/relative-method-link.html">relative method link</a>""",
+            """
+                <a href="../../pkg/C.html#method-fragment">fragment method link</a>""",
             """
                 <a href="../../pkg/relative-package-link.html">relative package link</a>""",
             """
-                \s<a href="../../pkg/relative-multi-line-link.html">relative-multi-line-link</a>.""");
+                <a href="../../pkg/relative-multi-line-link.html">relative-multi-line-link</a>""");
 
         // PACKAGE OVERVIEW
         checkOutput("index.html", true,
             """
                 <a href="./pkg/relative-package-link.html">relative package link</a>""");
+
+        // subpackage summary
+        checkOutput("pkg/sub/package-summary.html", true,
+            // related packages
+            """
+                <a href="../../pkg/relative-package-link.html">relative package link</a>""",
+            """
+                <a href="../../pkg/package-summary.html#package-fragment">package fragment link</a>""",
+            // subclass inheriting relative link doc
+            """
+                <a href="../../pkg/relative-class-link.html">relative class link</a>""",
+            """
+                <a href="../../pkg/C.html#class-fragment">fragment class link</a>""");
+
+        // sibling package summary
+        checkOutput("pkg2/package-summary.html", true,
+            """
+                <a href="../pkg/relative-class-link.html">relative class link</a>""",
+            """
+                 <a href="../pkg/C.html#class-fragment">fragment class link</a>""");
+    }
+
+    @Override
+    public void checkLinks() {
+        // since the test uses explicit links to non-existent files,
+        // we create those files to avoid false positive errors from checkLinks
+        touch("pkg/relative-class-link.html");
+        touch("pkg/relative-field-link.html");
+        touch("pkg/relative-method-link.html");
+        touch("pkg/relative-package-link.html");
+        touch("pkg/relative-multi-line-link.html");
+        super.checkLinks();
     }
 
     private void touch(String file) {

test/langtools/jdk/javadoc/doclet/testRelativeLinks/TestRelativeModuleLinks.java

@@ -0,0 +1,115 @@
+/*
+ * Copyright (c) 2021, 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 8262886
+ * @summary javadoc generates broken links with {@inheritDoc}
+ * @modules jdk.javadoc/jdk.javadoc.internal.api
+ *          jdk.javadoc/jdk.javadoc.internal.tool
+ *          jdk.compiler/com.sun.tools.javac.api
+ *          jdk.compiler/com.sun.tools.javac.main
+ * @library ../../lib /tools/lib
+ * @build toolbox.ToolBox toolbox.ModuleBuilder javadoc.tester.*
+ * @run main TestRelativeModuleLinks
+ */
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+
+import javadoc.tester.JavadocTester;
+import toolbox.ModuleBuilder;
+import toolbox.ToolBox;
+
+public class TestRelativeModuleLinks extends JavadocTester {
+
+    public final ToolBox tb;
+    public static void main(String... args) throws Exception {
+        TestRelativeModuleLinks tester = new TestRelativeModuleLinks();
+        tester.runTests(m -> new Object[] { Paths.get(m.getName()) });
+    }
+
+    public TestRelativeModuleLinks() {
+        tb = new ToolBox();
+    }
+
+    @Test
+    public void testRelativeModuleLinks(Path base) throws IOException {
+        Path src = base.resolve("src");
+        new ModuleBuilder(tb, "ma")
+                .classes("package pa; public class A {}")
+                .exports("pa")
+                .comment("""
+                         <a href="doc-files/file.html">relative module link</a>,
+                         <a href="#module-fragment">fragment module link</a>.
+                         <a id="module-fragment">Module fragment</a>.""")
+                .write(src);
+        new ModuleBuilder(tb, "mb")
+                .classes("package pb; public class B {}")
+                .exports("pb")
+                .requiresTransitive("ma")
+                .write(src);
+        Path docFiles = src.resolve("ma").resolve("doc-files");
+        tb.writeFile(docFiles.resolve("file.html"),
+                """
+                    <html>
+                    <head><title>Module HTML File</title></head>
+                    <body><h1>Module HTML File</h1>
+                    File content</body>
+                    </html>
+                    """);
+        javadoc("-d", base.resolve("api").toString(),
+                "-quiet",
+                "--module-source-path", src.toString(),
+                "--module", "ma,mb");
+
+        checkExit(Exit.OK);
+
+        // Main page
+        checkOutput("index.html", true,
+                """
+                <div class="block"><a href="./ma/doc-files/file.html">relative module link</a>,
+                 <a href="./ma/module-summary.html#module-fragment">fragment module link</a>.</div>""");
+
+        // Index page
+        checkOutput("index-all.html", true,
+                """
+                <div class="block"><a href="./ma/doc-files/file.html">relative module link</a>,
+                 <a href="./ma/module-summary.html#module-fragment">fragment module link</a>.</div>""");
+
+        // Own module page
+        checkOutput("ma/module-summary.html", true,
+                """
+                <div class="block"><a href="doc-files/file.html">relative module link</a>,
+                 <a href="#module-fragment">fragment module link</a>.
+                 <a id="module-fragment">Module fragment</a>.</div>""");
+
+        // Other module page
+        checkOutput("mb/module-summary.html", true,
+                """
+                <div class="block"><a href="../ma/doc-files/file.html">relative module link</a>,
+                 <a href="../ma/module-summary.html#module-fragment">fragment module link</a>.</div>""");
+    }
+}
+

test/langtools/jdk/javadoc/doclet/testRelativeLinks/pkg/C.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2021, 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,8 +24,11 @@
 package pkg;
 
 /**
- * Here is a relative link in a class:
+ * Here are two relative links in a class:
- * <a href="relative-class-link.html">relative class link</a>.
+ * <a href="relative-class-link.html">relative class link</a>,
+ * <a href="#class-fragment">fragment class link</a>.
+ *
+ * <a id="class-fragment">Class fragment</a>.
  */
 public class C {
 
@@ -36,8 +39,9 @@ public class C {
     public C field = null;
 
     /**
-     * Here is a relative link in a method:
+     * Here are two relative links in a method:
-     * <a href="relative-method-link.html">relative method link</a>.
+     * <a href="relative-method-link.html">relative method link</a>,
+     * <a href="#method-fragment">fragment method link</a>.
      */
     public C method() { return null;}
 
@@ -45,11 +49,13 @@ public class C {
      * Here is a relative link in a method:
      * <a
      * href="relative-multi-line-link.html">relative-multi-line-link</a>.
+     *
+     * <a id="method-fragment">Method fragment</a>.
      */
     public C multipleLineTest() { return null;}
 
     /**
-     * <a name="masters"></a>
+     * <a id="masters"></a>
      * Something that goes holy cow. Second line.
      */
     public static class WithAnAnchor{}

test/langtools/jdk/javadoc/doclet/testRelativeLinks/pkg/D.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2021, 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
@@ -21,9 +21,21 @@
  * questions.
  */
 
-package pkg2;
+package pkg;
 
 /**
- * Just a dummy class to force the overview page to generate.
+ * {@inheritDoc}
+ *
+ * A class that extends C and inherits some of its comments.
  */
-public class Foo {}
+public class D extends C {
+
+    /**
+     * {@inheritDoc}
+     */
+    @Override
+    public D method() {
+        return null;
+    }
+
+}

test/langtools/jdk/javadoc/doclet/testRelativeLinks/pkg/package.html

@@ -1,7 +1,10 @@
 <html>
 	<body>
-		Here is a relative link in a package: 
+		Here are two relative links in a package:
-       <a href="relative-package-link.html">relative package link</a>.
+       <a href="relative-package-link.html">relative package link</a>,
+	   <a href="#package-fragment">package fragment link</a>.
+
+	   <a id="package-fragment">Package fragment</a>.
 	</body>
 </html>
 

test/langtools/jdk/javadoc/doclet/testRelativeLinks/pkg/sub/F.java

@@ -0,0 +1,43 @@
+/*
+ * Copyright (c) 2021, 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.sub;
+
+import pkg.C;
+
+/**
+ * {@inheritDoc}
+ *
+ * A class that extends C and inherits some of its comments.
+ */
+public class F extends C {
+
+    /**
+     * {@inheritDoc}
+     */
+    @Override
+    public F method() {
+        return null;
+    }
+
+}

test/langtools/jdk/javadoc/doclet/testRelativeLinks/pkg2/E.java

@@ -0,0 +1,43 @@
+/*
+ * Copyright (c) 2003, 2021, 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 pkg2;
+
+import pkg.C;
+
+/**
+ * {@inheritDoc}
+ *
+ * A class that extends pkg.C from onother package and inherits some of its comments.
+ */
+public class E extends C {
+
+    /**
+     * {@inheritDoc}
+     */
+    @Override
+    public E method() {
+        return null;
+    }
+
+}