Skip to content

Commit

Permalink
8240169: javadoc fails to link to non-modular api docs
Browse files Browse the repository at this point in the history 
Reviewed-by: jjg
  • Loading branch information
@hns
hns committed Apr 8, 2020
1 parent cdb379e commit ac8d420
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 40 deletions.
45 changes: 26 additions & 19 deletions src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Extern.java
View file
Expand Up @@ -89,7 +89,7 @@ public class Extern {
/**
* 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}.
Expand Down Expand Up @@ -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) {
Expand All @@ -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;
}
Expand Down Expand Up @@ -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;
Expand All @@ -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
Expand Down
25 changes: 21 additions & 4 deletions test/langtools/jdk/javadoc/doclet/testClassCrossReferences/TestClassCrossReferences.java
View file
Expand Up @@ -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
Expand Down Expand Up @@ -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>");
}
}
41 changes: 24 additions & 17 deletions test/langtools/jdk/javadoc/doclet/testLinkOption/TestLinkOptionWithModule.java
View file
Expand Up @@ -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
Expand All @@ -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();
Expand All @@ -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();
}

Expand All @@ -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());

Expand All @@ -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());

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

}

0 comments on commit ac8d420

Please sign in to comment.