8240169: javadoc fails to link to non-modular api docs

Reviewed-by: jjg

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Extern.java

@@ -89,7 +89,7 @@
     /**
      * Stores the info for one external doc set
      */
-    private class Item {
+    private static class Item {
 
         /**
          * Element name, found in the "element-list" file in the {@link #path}.
@@ -252,7 +252,7 @@ private boolean link(String url, String elemlisturl, Reporter reporter, boolean
         }
     }
 
-    private class Fault extends Exception {
+    private static class Fault extends Exception {
         private static final long serialVersionUID = 0;
 
         Fault(String msg, Exception cause) {
@@ -275,12 +275,6 @@ else if (element instanceof PackageElement) {
             ModuleElement moduleElement = utils.containingModule(packageElement);
             Map<String, Item> pkgMap = packageItems.get(utils.getModuleName(moduleElement));
             item = (pkgMap != null) ? pkgMap.get(utils.getPackageName(packageElement)) : null;
-            if (item == null && isAutomaticModule(moduleElement)) {
-                pkgMap = packageItems.get(utils.getModuleName(null));
-                if (pkgMap != null) {
-                    item = pkgMap.get(utils.getPackageName(packageElement));
-                }
-            }
         }
         return item;
     }
@@ -384,7 +378,7 @@ private void readElementList(DocFile file, String path) throws Fault, DocFileIOE
      * @throws IOException if there is a problem reading or closing the stream
      */
     private void readElementList(InputStream input, String path, boolean relative)
-                         throws Fault, IOException {
+                         throws IOException {
         try (BufferedReader in = new BufferedReader(new InputStreamReader(input))) {
             String elemname;
             DocPath elempath;
@@ -404,35 +398,48 @@ private void readElementList(InputStream input, String path, boolean relative)
                         } else {
                             elempath = elempath.resolve(pkgPath);
                         }
-                        checkLinkCompatibility(elemname, moduleName, path);
+                        String actualModuleName = checkLinkCompatibility(elemname, moduleName, path);
                         Item item = new Item(elemname, elempath, relative);
-                        packageItems.computeIfAbsent(
-                                moduleName == null ? DocletConstants.DEFAULT_ELEMENT_NAME : moduleName,
-                                k -> new TreeMap<>())
+                        packageItems.computeIfAbsent(actualModuleName, k -> new TreeMap<>())
                             .putIfAbsent(elemname, item); // first-one-wins semantics
                     }
                 }
             }
         }
     }
 
-    private void checkLinkCompatibility(String packageName, String moduleName, String path) throws Fault {
+    /**
+     * Check if the external documentation format matches our internal model of the code.
+     * Returns the module name to use for external reference lookup according to the actual
+     * modularity of the external package (and regardless of modularity of documentation).
+     *
+     * @param packageName the package name
+     * @param moduleName the module name or null
+     * @param path the documentation path
+     * @return the module name to use according to actual modularity of the package
+     */
+    private String checkLinkCompatibility(String packageName, String moduleName, String path)  {
         PackageElement pe = utils.elementUtils.getPackageElement(packageName);
         if (pe != null) {
             ModuleElement me = (ModuleElement)pe.getEnclosingElement();
             if (me == null || me.isUnnamed()) {
                 if (moduleName != null) {
-                    throw new Fault(resources.getText("doclet.linkMismatch_PackagedLinkedtoModule",
-                            path), null);
+                    configuration.getReporter().print(Kind.WARNING,
+                            resources.getText("doclet.linkMismatch_PackagedLinkedtoModule", path));
                 }
+                // library is not modular, ignore module name even if documentation is modular
+                return DocletConstants.DEFAULT_ELEMENT_NAME;
             } else if (moduleName == null) {
-                // suppress the error message in the case of automatic modules
+                // suppress the warning message in the case of automatic modules
                 if (!isAutomaticModule(me)) {
-                    throw new Fault(resources.getText("doclet.linkMismatch_ModuleLinkedtoPackage",
-                            path), null);
+                    configuration.getReporter().print(Kind.WARNING,
+                            resources.getText("doclet.linkMismatch_ModuleLinkedtoPackage", path));
                 }
+                // library is modular, use module name for lookup even though documentation is not
+                return utils.getModuleName(me);
             }
         }
+        return moduleName == null ? DocletConstants.DEFAULT_ELEMENT_NAME : moduleName;
     }
 
     // The following should be replaced by a new method such as Elements.isAutomaticModule

test/langtools/jdk/javadoc/doclet/testClassCrossReferences/TestClassCrossReferences.java

@@ -23,7 +23,7 @@
 
 /*
  * @test
- * @bug 4652655 4857717 8025633 8026567 8071982 8164407 8182765 8205593
+ * @bug 4652655 4857717 8025633 8026567 8071982 8164407 8182765 8205593 8240169
  * @summary This test verifies that class cross references work properly.
  * @library ../../lib
  * @modules jdk.javadoc/jdk.javadoc.internal.tool
@@ -71,15 +71,32 @@ public void test() {
     }
 
     @Test
-    public void test_error() {
-        javadoc("-d", "out-error",
+    public void test_warning() {
+        javadoc("-d", "out-warning",
                 "-Xdoclint:none",
                 "-sourcepath", testSrc,
                 "-linkoffline", uri, testSrc,
                 testSrc("C.java"));
-        checkExit(Exit.ERROR);
+        checkExit(Exit.OK);
+
         checkOutput(Output.OUT, true,
                 "The code being documented uses modules but the packages defined"
                 + " in http://docs.oracle.com/javase/8/docs/api/ are in the unnamed module");
+
+        checkOutput("C.html", true,
+                "<a href=\"" + uri + "java/math/package-summary.html\" class=\"external-link\">"
+                + "<code>Link to math package</code></a>",
+                "<a href=\"" + uri + "javax/swing/text/AbstractDocument.AttributeContext.html\" "
+                + "title=\"class or interface in javax.swing.text\" class=\"external-link\"><code>Link to AttributeContext innerclass</code></a>",
+                "<a href=\"" + uri + "java/math/BigDecimal.html\" "
+                + "title=\"class or interface in java.math\" class=\"external-link\"><code>Link to external class BigDecimal</code></a>",
+                "<a href=\"" + uri + "java/math/BigInteger.html#gcd(java.math.BigInteger)\" "
+                + "title=\"class or interface in java.math\" class=\"external-link\"><code>Link to external member gcd</code></a>",
+                "<a href=\"" + uri + "javax/tools/SimpleJavaFileObject.html#uri\" "
+                + "title=\"class or interface in javax.tools\" class=\"external-link\"><code>Link to external member URI</code></a>",
+                "<dl class=\"notes\">\n"
+                + "<dt>Overrides:</dt>\n"
+                + "<dd><code>toString</code>&nbsp;in class&nbsp;<code>java.lang.Object</code></dd>\n"
+                + "</dl>");
     }
 }

test/langtools/jdk/javadoc/doclet/testLinkOption/TestLinkOptionWithModule.java

@@ -23,7 +23,7 @@
 
 /*
  * @test
- * @bug 8205593
+ * @bug 8205593 8240169
  * @summary Javadoc -link makes broken links if module name matches package name
  * @library /tools/lib ../../lib
  * @modules
@@ -48,7 +48,7 @@
 public class TestLinkOptionWithModule extends JavadocTester {
 
     final ToolBox tb;
-    private final Path src;
+    private final Path moduleSrc, packageSrc;
 
     public static void main(String... args) throws Exception {
         TestLinkOptionWithModule tester = new TestLinkOptionWithModule();
@@ -57,7 +57,8 @@ public static void main(String... args) throws Exception {
 
     TestLinkOptionWithModule() throws Exception {
         tb = new ToolBox();
-        src = Paths.get("src");
+        moduleSrc = Paths.get("src", "modules");
+        packageSrc = Paths.get("src", "packages");
         initModulesAndPackages();
     }
 
@@ -66,11 +67,11 @@ public void testModuleLinkedToModule(Path base) throws Exception {
         Path out1 = base.resolve("out1a"), out2 = base.resolve("out1b");
 
         javadoc("-d", out1.toString(),
-                "--module-source-path", src.toString(),
+                "--module-source-path", moduleSrc.toString(),
                 "--module", "com.ex1");
 
         javadoc("-d", out2.toString(),
-                "--module-source-path", src.toString(),
+                "--module-source-path", moduleSrc.toString(),
                 "--module", "com.ex2",
                 "-link", "../" + out1.getFileName());
 
@@ -85,11 +86,11 @@ public void testPackageLinkedToPackage(Path base) throws Exception {
         Path out1 = base.resolve("out2a"), out2 = base.resolve("out2b");
 
         javadoc("-d", out1.toString(),
-                "-sourcepath", src.toString(),
+                "-sourcepath", packageSrc.toString(),
                 "-subpackages", "com.ex1");
 
         javadoc("-d", out2.toString(),
-                "-sourcepath", src.toString(),
+                "-sourcepath", packageSrc.toString(),
                 "-subpackages", "com.ex2",
                 "-link", "../" + out1.getFileName());
 
@@ -104,45 +105,51 @@ public void testModuleLinkedToPackage(Path base) throws Exception {
         Path out1 = base.resolve("out3a"), out2 = base.resolve("out3b");
 
         javadoc("-d", out1.toString(),
-                "-sourcepath", src.toString(),
+                "-sourcepath", packageSrc.toString(),
                 "-subpackages", "com.ex1");
 
         javadoc("-d", out2.toString(),
-                "--module-source-path", src.toString(),
+                "--module-source-path", moduleSrc.toString(),
                 "--module", "com.ex2",
                 "-link", "../" + out1.getFileName());
 
-        checkExit(Exit.ERROR);
+        checkExit(Exit.OK);
         checkOutput(Output.OUT, true,
                 "The code being documented uses modules but the packages defined "
                 + "in ../out3a/ are in the unnamed module");
+        checkOutput("com.ex2/com/ex2/B.html", true,
+                "<a href=\"../../../../out3a/com/ex1/A.html\" "
+                + "title=\"class or interface in com.ex1\" class=\"external-link\">A</a>");
     }
 
     @Test
     public void testPackageLinkedToModule(Path base) throws Exception {
         Path out1 = base.resolve("out4a"), out2 = base.resolve("out4b");
 
         javadoc("-d", out1.toString(),
-                "--module-source-path", src.toString(),
+                "--module-source-path", moduleSrc.toString(),
                 "--module", "com.ex1");
 
         javadoc("-d", out2.toString(),
-                "-sourcepath", src.toString(),
+                "-sourcepath", packageSrc.toString(),
                 "-subpackages", "com.ex2",
                 "-link", "../" + out1.getFileName());
 
-        checkExit(Exit.ERROR);
+        checkExit(Exit.OK);
         checkOutput(Output.OUT, true,
                 "The code being documented uses packages in the unnamed module, but the packages defined "
                 + "in ../out4a/ are in named modules");
+        checkOutput("com/ex2/B.html", true,
+                "<a href=\"../../../out4a/com.ex1/com/ex1/A.html\" "
+                + "title=\"class or interface in com.ex1\" class=\"external-link\">A</a>");
     }
 
 
     void initModulesAndPackages() throws Exception{
         new ModuleBuilder(tb, "com.ex1")
                 .exports("com.ex1")
                 .classes("package com.ex1; public class A{}")
-                .write(src);
+                .write(moduleSrc);
 
         new ModuleBuilder(tb, "com.ex2")
                 .requires("com.ex1")
@@ -152,17 +159,17 @@ void initModulesAndPackages() throws Exception{
                         + "public class B{\n"
                         + "public B(A obj){}\n"
                         + "}\n")
-                .write(src);
+                .write(moduleSrc);
 
         new ClassBuilder(tb, "com.ex1.A")
                 .setModifiers("public","class")
-                .write(src);
+                .write(packageSrc);
 
         new ClassBuilder(tb, "com.ex2.B")
                 .addImports("com.ex1.A")
                 .setModifiers("public","class")
                 .addMembers(MethodBuilder.parse("public void foo(A a)"))
-                .write(src);
+                .write(packageSrc);
     }
 
 }