8176481: javadoc does not consider mandated modules
authorksrini
Thu, 23 Mar 2017 14:18:25 -0700
changeset 44389 a745e6ff79a5
parent 44388 4d0903f1f311
child 44390 c8d72fa23c7f
8176481: javadoc does not consider mandated modules Reviewed-by: jjg
langtools/src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/ElementsTable.java
langtools/src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/resources/javadoc.properties
langtools/test/jdk/javadoc/tool/modules/MissingSourceModules.java
langtools/test/jdk/javadoc/tool/modules/Modules.java
langtools/test/jdk/javadoc/tool/modules/PatchModules.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<JavaFileObject> list = null;
+    /* Call fm.list and wrap any IOException that occurs in a ToolException */
+    private Iterable<JavaFileObject> fmList(Location location,
+                                            String packagename,
+                                            Set<JavaFileObject.Kind> 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<ModuleElement> getModuleRequires(ModuleElement mdle, boolean isPublic) {
+    private Set<ModuleElement> getModuleRequires(ModuleElement mdle, boolean onlyTransitive) throws ToolException {
         Set<ModuleElement> 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<ModuleSymbol, Boolean> 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<Location> 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<PackageElement> 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();
--- 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
--- /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.");
+    }
+}
--- 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; }")
--- 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