# HG changeset patch # User ksrini # Date 1490303905 25200 # Node ID a745e6ff79a55a5bfe9e71ea48bc58797e456ffe # Parent 4d0903f1f311aa5aad57ebe1196bff14d30cedf5 8176481: javadoc does not consider mandated modules Reviewed-by: jjg diff -r 4d0903f1f311 -r a745e6ff79a5 langtools/src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/ElementsTable.java --- a/langtools/src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/ElementsTable.java Thu Mar 23 10:58:16 2017 -0700 +++ b/langtools/src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/ElementsTable.java Thu Mar 23 14:18:25 2017 -0700 @@ -30,6 +30,7 @@ import java.util.Collections; import java.util.EnumMap; import java.util.EnumSet; +import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashMap; import java.util.LinkedHashSet; @@ -75,6 +76,7 @@ import static com.sun.tools.javac.code.Scope.LookupKind.NON_RECURSIVE; +import static javax.lang.model.util.Elements.Origin.*; import static javax.tools.JavaFileObject.Kind.*; import static jdk.javadoc.internal.tool.Main.Result.*; @@ -520,15 +522,21 @@ } } - private void addPackagesFromLocations(Location packageLocn, ModulePackage modpkg) throws ToolException { - Iterable list = null; + /* Call fm.list and wrap any IOException that occurs in a ToolException */ + private Iterable fmList(Location location, + String packagename, + Set kinds, + boolean recurse) throws ToolException { try { - list = fm.list(packageLocn, modpkg.packageName, sourceKinds, true); + return fm.list(location, packagename, kinds, recurse); } catch (IOException ioe) { - String text = messager.getText("main.file.manager.list", modpkg.packageName); + String text = messager.getText("main.file.manager.list", packagename); throw new ToolException(SYSERR, text, ioe); } - for (JavaFileObject fo : list) { + } + + private void addPackagesFromLocations(Location packageLocn, ModulePackage modpkg) throws ToolException { + for (JavaFileObject fo : fmList(packageLocn, modpkg.packageName, sourceKinds, true)) { String binaryName = fm.inferBinaryName(packageLocn, fo); String pn = getPackageName(binaryName); String simpleName = getSimpleName(binaryName); @@ -555,25 +563,51 @@ /** * Returns the "requires" modules for the target module. * @param mdle the target module element - * @param isPublic true gets all the public requires, otherwise - * gets all the non-public requires + * @param onlyTransitive true gets all the requires transitive, otherwise + * gets all the non-transitive requires * * @return a set of modules */ - private Set getModuleRequires(ModuleElement mdle, boolean isPublic) { + private Set getModuleRequires(ModuleElement mdle, boolean onlyTransitive) throws ToolException { Set result = new HashSet<>(); for (RequiresDirective rd : ElementFilter.requiresIn(mdle.getDirectives())) { - if (isPublic && rd.isTransitive()) { - result.add(rd.getDependency()); - } - if (!isPublic && !rd.isTransitive()) { - result.add(rd.getDependency()); + ModuleElement dep = rd.getDependency(); + if (result.contains(dep)) + continue; + if (!isMandated(mdle, rd) && onlyTransitive == rd.isTransitive()) { + if (!haveModuleSources(dep)) { + messager.printWarning(dep, "main.module_not_found", dep.getSimpleName()); + } + result.add(dep); + } else if (isMandated(mdle, rd) && haveModuleSources(dep)) { + result.add(dep); } } return result; } - private void computeSpecifiedModules() { + private boolean isMandated(ModuleElement mdle, RequiresDirective rd) { + return toolEnv.elements.getOrigin(mdle, rd) == MANDATED; + } + + Map haveModuleSourcesCache = new HashMap<>(); + private boolean haveModuleSources(ModuleElement mdle) throws ToolException { + ModuleSymbol msym = (ModuleSymbol)mdle; + if (msym.sourceLocation != null) { + return true; + } + if (msym.patchLocation != null) { + Boolean value = haveModuleSourcesCache.get(msym); + if (value == null) { + value = fmList(msym.patchLocation, "", sourceKinds, true).iterator().hasNext(); + haveModuleSourcesCache.put(msym, value); + } + return value; + } + return false; + } + + private void computeSpecifiedModules() throws ToolException { if (expandRequires == null) { // no expansion requested specifiedModuleElements = Collections.unmodifiableSet(specifiedModuleElements); return; @@ -617,20 +651,14 @@ ModuleSymbol msym = (ModuleSymbol) mdle; List msymlocs = getModuleLocation(locations, msym.name.toString()); for (Location msymloc : msymlocs) { - try { - for (JavaFileObject fo : fm.list(msymloc, "", sourceKinds, true)) { - if (fo.getName().endsWith("module-info.java")) { - continue; - } - String binaryName = fm.inferBinaryName(msymloc, fo); - String pn = getPackageName(binaryName); - PackageSymbol psym = syms.enterPackage(msym, names.fromString(pn)); - result.add((PackageElement) psym); + for (JavaFileObject fo : fmList(msymloc, "", sourceKinds, true)) { + if (fo.getName().endsWith("module-info.java")) { + continue; } - - } catch (IOException ioe) { - String text = messager.getText("main.file.manager.list", msymloc.getName()); - throw new ToolException(SYSERR, text, ioe); + String binaryName = fm.inferBinaryName(msymloc, fo); + String pn = getPackageName(binaryName); + PackageSymbol psym = syms.enterPackage(msym, names.fromString(pn)); + result.add((PackageElement) psym); } } return result; @@ -727,10 +755,9 @@ Set packlist = new LinkedHashSet<>(); cmdLinePackages.forEach((modpkg) -> { - ModuleElement mdle = null; PackageElement pkg; if (modpkg.hasModule()) { - mdle = toolEnv.elements.getModuleElement(modpkg.moduleName); + ModuleElement mdle = toolEnv.elements.getModuleElement(modpkg.moduleName); pkg = toolEnv.elements.getPackageElement(mdle, modpkg.packageName); } else { pkg = toolEnv.elements.getPackageElement(modpkg.toString()); @@ -821,17 +848,12 @@ } String pname = modpkg.packageName; for (Location packageLocn : locs) { - try { - for (JavaFileObject fo : fm.list(packageLocn, pname, sourceKinds, recurse)) { - String binaryName = fm.inferBinaryName(packageLocn, fo); - String simpleName = getSimpleName(binaryName); - if (isValidClassName(simpleName)) { - lb.append(fo); - } + for (JavaFileObject fo : fmList(packageLocn, pname, sourceKinds, recurse)) { + String binaryName = fm.inferBinaryName(packageLocn, fo); + String simpleName = getSimpleName(binaryName); + if (isValidClassName(simpleName)) { + lb.append(fo); } - } catch (IOException ioe) { - String text = messager.getText("main.file.manager.list", pname); - throw new ToolException(SYSERR, text, ioe); } } return lb.toList(); diff -r 4d0903f1f311 -r a745e6ff79a5 langtools/src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/resources/javadoc.properties --- a/langtools/src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/resources/javadoc.properties Thu Mar 23 10:58:16 2017 -0700 +++ b/langtools/src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/resources/javadoc.properties Thu Mar 23 14:18:25 2017 -0700 @@ -85,10 +85,10 @@ main.opt.expand.requires.desc=\ Instructs the tool to expand the set of modules to be\n\ documented. By default, only the modules given explicitly on\n\ - the command line will be documented. A value of "transitive" will\n\ - additionally include all "requires transitive" dependencies of\n\ - those modules. A value of "all" will include all dependencies\n\ - of those modules. + the command line will be documented. A value of "transitive"\n\ + will additionally include all "requires transitive"\n\ + dependencies of those modules. A value of "all" will include\n\ + all dependencies of those modules. main.opt.help.desc=\ Display command line options and exit diff -r 4d0903f1f311 -r a745e6ff79a5 langtools/test/jdk/javadoc/tool/modules/MissingSourceModules.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/langtools/test/jdk/javadoc/tool/modules/MissingSourceModules.java Thu Mar 23 14:18:25 2017 -0700 @@ -0,0 +1,136 @@ +/* + * Copyright (c) 2017, 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 8176481 + * @summary Tests behavior of the tool, when modules are present as + * binaries. + * @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 /tools/lib + * @build toolbox.ToolBox toolbox.TestRunner + * @run main MissingSourceModules + */ + +import java.nio.file.Path; +import java.nio.file.Paths; + +import toolbox.*; + +public class MissingSourceModules extends ModuleTestBase { + + public static void main(String... args) throws Exception { + new MissingSourceModules().runTests(); + } + + @Test + public void testExplicitBinaryModuleOnModulePath(Path base) throws Exception { + Path modulePath = base.resolve("modules"); + + ModuleBuilder ma = new ModuleBuilder(tb, "ma"); + ma.comment("Module ma.") + .exports("pkg1") + .classes("package pkg1; /** Class A */ public class A { }") + .classes("package pkg1.pkg2; /** Class B */ public class B { }") + .build(modulePath); + + execNegativeTask("--module-path", modulePath.toString(), + "--module", "ma"); + assertMessagePresent("module ma not found."); + } + + @Test + public void testExplicitBinaryModuleOnLegacyPaths(Path base) throws Exception { + Path modulePath = base.resolve("modules"); + + ModuleBuilder ma = new ModuleBuilder(tb, "ma"); + ma.comment("Module ma.") + .exports("pkg1") + .classes("package pkg1; /** Class A */ public class A { }") + .classes("package pkg1.pkg2; /** Class B */ public class B { }") + .build(modulePath); + + Path mPath = Paths.get(modulePath.toString(), "ma"); + execNegativeTask("--source-path", mPath.toString(), + "--module", "ma"); + assertMessagePresent("module ma not found."); + + execNegativeTask("--class-path", mPath.toString(), + "--module", "ma"); + assertMessagePresent("module ma not found."); + } + + @Test + public void testImplicitBinaryRequiresModule(Path base) throws Exception { + Path src = base.resolve("src"); + Path modulePath = base.resolve("modules"); + + ModuleBuilder mb = new ModuleBuilder(tb, "mb"); + mb.comment("Module mb.") + .exports("pkgb") + .classes("package pkgb; /** Class A */ public class A { }") + .build(modulePath); + + ModuleBuilder ma = new ModuleBuilder(tb, "ma"); + ma.comment("Module ma.") + .exports("pkga") + .requires("mb", modulePath) + .classes("package pkga; /** Class A */ public class A { }") + .write(src); + + execTask("--module-path", modulePath.toString(), + "--module-source-path", src.toString(), + "--expand-requires", "all", + "--module", "ma"); + assertMessagePresent("module mb not found."); + } + + @Test + public void testImplicitBinaryTransitiveModule(Path base) throws Exception { + Path src = base.resolve("src"); + Path modulePath = base.resolve("modules"); + + ModuleBuilder mb = new ModuleBuilder(tb, "mb"); + mb.comment("Module mb.") + .exports("pkgb") + .classes("package pkgb; /** Class A */ public class A { }") + .build(modulePath); + + ModuleBuilder ma = new ModuleBuilder(tb, "ma"); + ma.comment("Module ma.") + .exports("pkga") + .requiresTransitive("mb", modulePath) + .classes("package pkga; /** Class A */ public class A { }") + .write(src); + + execTask("--module-path", modulePath.toString(), + "--module-source-path", src.toString(), + "--expand-requires", "transitive", + "--module", "ma"); + assertMessagePresent("module mb not found."); + } +} diff -r 4d0903f1f311 -r a745e6ff79a5 langtools/test/jdk/javadoc/tool/modules/Modules.java --- a/langtools/test/jdk/javadoc/tool/modules/Modules.java Thu Mar 23 10:58:16 2017 -0700 +++ b/langtools/test/jdk/javadoc/tool/modules/Modules.java Thu Mar 23 14:18:25 2017 -0700 @@ -23,7 +23,7 @@ /* * @test - * @bug 8159305 8166127 8175860 + * @bug 8159305 8166127 8175860 8176481 * @summary Tests primarily the module graph computations. * @modules * jdk.javadoc/jdk.javadoc.internal.api @@ -35,7 +35,6 @@ * @run main Modules */ -import java.io.File; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; @@ -421,6 +420,7 @@ checkPackagesIncluded("p"); checkTypesIncluded("p.Main"); checkPackagesNotIncluded(".*open.*"); + assertMessageNotPresent("warning"); } @Test @@ -442,9 +442,46 @@ "--expand-requires", "transitive"); checkModulesSpecified("M", "N", "O"); + checkModulesNotSpecified("java.base"); checkModulesIncluded("M", "N", "O"); + checkModulesNotIncluded("java.base"); checkPackagesIncluded("p", "openN", "openO"); checkTypesIncluded("p.Main", "openN.N", "openO.O"); + assertMessageNotPresent("warning"); + } + + @Test + public void testExpandRequiresTransitiveWithMandated(Path base) throws Exception { + Path src = base.resolve("src"); + + createAuxiliaryModules(src); + + Path patchSrc = Paths.get(src.toString(), "patch"); + + new ModuleBuilder(tb, "M") + .comment("The M module.") + .requiresTransitive("N", src) + .requires("L", src) + .exports("p") + .classes("package p; public class Main { openO.O o; openN.N n; openL.L l; }") + .write(src); + + // build the patching module + tb.writeJavaFiles(patchSrc, "package pkg1;\n" + + "/** Class A */ public class A extends java.util.ArrayList { }"); + tb.writeJavaFiles(patchSrc, "package pkg1;\n" + + "/** Class B */ public class B { }"); + + execTask("--module-source-path", src.toString(), + "--patch-module", "java.base=" + patchSrc.toString(), + "--module", "M", + "--expand-requires", "transitive"); + + checkModulesSpecified("java.base", "M", "N", "O"); + checkModulesIncluded("java.base", "M", "N", "O"); + checkPackagesIncluded("p", "openN", "openO"); + checkTypesIncluded("p.Main", "openN.N", "openO.O"); + assertMessageNotPresent("warning"); } @Test @@ -466,13 +503,14 @@ "--module", "M", "--expand-requires", "all"); - checkModulesSpecified("M", "java.base", "N", "L", "O"); - checkModulesIncluded("M", "java.base", "N", "L", "O"); + checkModulesSpecified("M", "N", "L", "O"); + checkModulesIncluded("M", "N", "L", "O"); checkModulesNotIncluded("P", "J", "Q"); checkPackagesIncluded("p", "openN", "openL", "openO"); checkPackagesNotIncluded(".*openP.*", ".*openJ.*"); checkTypesIncluded("p.Main", "openN.N", "openL.L", "openO.O"); checkTypesNotIncluded(".*openP.*", ".*openJ.*"); + assertMessageNotPresent("warning"); } @Test @@ -577,7 +615,7 @@ new ModuleBuilder(tb, "L") .comment("The L module.") .exports("openL") - . requiresTransitive("P") + .requiresTransitive("P") .classes("package openL; /** Class L open */ public class L { }") .classes("package closedL; /** Class L closed */ public class L { }") .write(src); @@ -599,7 +637,7 @@ .write(src); new ModuleBuilder(tb, "P") - .comment("The O module.") + .comment("The P module.") .exports("openP") .requires("J") .classes("package openP; /** Class O open. */ public class O { openJ.J j; }") diff -r 4d0903f1f311 -r a745e6ff79a5 langtools/test/jdk/javadoc/tool/modules/PatchModules.java --- a/langtools/test/jdk/javadoc/tool/modules/PatchModules.java Thu Mar 23 10:58:16 2017 -0700 +++ b/langtools/test/jdk/javadoc/tool/modules/PatchModules.java Thu Mar 23 14:18:25 2017 -0700 @@ -129,7 +129,6 @@ // Case B.1: (jsr166) augment and override system module @Test public void testPatchModuleModifyingSystemModule(Path base) throws Exception { - Path src = base.resolve("src"); Path patchSrc = base.resolve("patch"); // build the patching sources