8167383: Javadoc does not handle packages correctly when used with module option

Reviewed-by: bpatel, jjg

langtools/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/Configuration.java

@@ -308,9 +308,7 @@ public abstract class Configuration {
     public CommentUtils cmtUtils;
 
     /**
-     * A sorted set of packages specified on the command-line merged with a
+     * A sorted set of included packages.
-     * collection of packages that contain the classes specified on the
-     * command-line.
      */
     public SortedSet<PackageElement> packages = null;
 
@@ -399,10 +397,8 @@ public abstract class Configuration {
 
     private void initPackages() {
         packages = new TreeSet<>(utils.makePackageComparator());
-        packages.addAll(getSpecifiedPackages());
+        // add all the included packages
-        for (TypeElement aClass : getSpecifiedClasses()) {
+        packages.addAll(docEnv.getIncludedPackageElements());
-            packages.add(utils.containingPackage(aClass));
-        }
     }
 
     public Set<Doclet.Option> getSupportedOptions() {
@@ -647,7 +643,7 @@ public abstract class Configuration {
         if (docencoding == null) {
             docencoding = encoding;
         }
-        typeElementCatalog = new TypeElementCatalog(getSpecifiedClasses(), this);
+        typeElementCatalog = new TypeElementCatalog(docEnv.getIncludedTypeElements(), this);
         initTagletManager(customTagStrs);
         groups.stream().forEach((grp) -> {
             group.checkPackageGroups(grp.value1, grp.value2);

langtools/src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/ElementsTable.java

@@ -587,18 +587,28 @@ public class ElementsTable {
     }
 
     private Set<PackageElement> computeModulePackages() throws ToolException {
-        final AccessKind accessValue = accessFilter.getAccessValue(ElementKind.PACKAGE);
+        AccessKind accessValue = accessFilter.getAccessValue(ElementKind.PACKAGE);
         final boolean documentAllModulePackages = (accessValue == AccessKind.PACKAGE ||
                 accessValue == AccessKind.PRIVATE);
 
+        accessValue = accessFilter.getAccessValue(ElementKind.MODULE);
+        final boolean moduleDetailedMode = (accessValue == AccessKind.PACKAGE ||
+                accessValue == AccessKind.PRIVATE);
         Set<PackageElement> expandedModulePackages = new LinkedHashSet<>();
 
         for (ModuleElement mdle : specifiedModuleElements) {
-            // add all exported packages belonging to a specified module
+            if (documentAllModulePackages) { // include all packages
-            if (specifiedModuleElements.contains(mdle)) {
+                List<PackageElement> packages = ElementFilter.packagesIn(mdle.getEnclosedElements());
+                expandedModulePackages.addAll(packages);
+                expandedModulePackages.addAll(getAllModulePackages(mdle));
+            } else { // selectively include required packages
                 List<ExportsDirective> exports = ElementFilter.exportsIn(mdle.getDirectives());
                 for (ExportsDirective export : exports) {
-                    expandedModulePackages.add(export.getPackage());
+                    // add if fully exported or add qualified exports only if desired
+                    if (export.getTargetModules() == null
+                            || documentAllModulePackages || moduleDetailedMode) {
+                        expandedModulePackages.add(export.getPackage());
+                    }
                 }
             }
 
@@ -613,27 +623,6 @@ public class ElementsTable {
                     }
                 }
             }
-
-            if (!documentAllModulePackages) {
-                List<ExportsDirective> exports = ElementFilter.exportsIn(mdle.getDirectives());
-                // check exported packages
-                for (ExportsDirective export : exports) {
-                    List<? extends ModuleElement> targetModules = export.getTargetModules();
-                    if (targetModules == null) { // no qualified exports, add 'em all
-                        expandedModulePackages.add(export.getPackage());
-                    } else { // qualified export, add only if target module is being considered
-                        for (ModuleElement target : targetModules) {
-                            if (specifiedModuleElements.contains(target)) {
-                                expandedModulePackages.add(export.getPackage());
-                            }
-                        }
-                    }
-                }
-            } else { // add all exported and module private packages
-                List<PackageElement> packages = ElementFilter.packagesIn(mdle.getEnclosedElements());
-                expandedModulePackages.addAll(packages);
-                expandedModulePackages.addAll(getAllModulePackages(mdle));
-            }
         }
         return expandedModulePackages;
     }
@@ -668,8 +657,7 @@ public class ElementsTable {
             if (!mdle.isUnnamed())
                 imodules.add(mdle);
             PackageElement pkg = toolEnv.elements.getPackageOf(klass);
-            if (!pkg.isUnnamed())
+            ipackages.add(pkg);
-                ipackages.add(pkg);
             addAllClasses(iclasses, klass, true);
         });
 

langtools/test/jdk/javadoc/tool/modules/FilterOptions.java

@@ -73,6 +73,10 @@ public class FilterOptions extends ModuleTestBase {
                 "--module", "m1", "--show-module-contents", "api");
 
         checkModuleMode("API");
+        checkModulesSpecified("m1");
+        checkModulesIncluded("m1");
+        checkPackagesIncluded("pub");
+        checkPackagesNotIncluded("pro", "pqe");
     }
 
     @Test
@@ -81,6 +85,10 @@ public class FilterOptions extends ModuleTestBase {
                 "--module", "m1", "--show-module-contents", "all");
 
         checkModuleMode("ALL");
+        checkModulesSpecified("m1");
+        checkModulesIncluded("m1");
+        checkPackagesIncluded("pub", "pqe");
+        checkPackagesNotIncluded("pro");
     }
 
     @Test
@@ -92,6 +100,7 @@ public class FilterOptions extends ModuleTestBase {
         checkModulesSpecified("m1");
         checkModulesIncluded("m1");
         checkPackagesIncluded("pub");
+        checkPackagesNotIncluded("pqe", "pro");
         checkTypesIncluded("pub.A", "pub.A.ProtectedNested", "pub.A.PublicNested");
     }
 
@@ -102,9 +111,10 @@ public class FilterOptions extends ModuleTestBase {
                 "--show-packages", "all");
         checkModulesSpecified("m1");
         checkModulesIncluded("m1");
-        checkPackagesIncluded("pub", "pro");
+        checkPackagesIncluded("pub", "pqe", "pro");
 
         checkTypesIncluded("pub.A", "pub.A.ProtectedNested", "pub.A.PublicNested",
+                           "pqe.A", "pqe.A.ProtectedNested", "pqe.A.PublicNested",
                            "pro.A", "pro.A.ProtectedNested", "pro.A.PublicNested");
     }
 
@@ -221,6 +231,7 @@ public class FilterOptions extends ModuleTestBase {
         checkModulesSpecified("m1");
         checkModulesIncluded("m1");
         checkPackagesIncluded("pub");
+        checkPackagesNotIncluded("pqe", "pro");
         checkTypesIncluded("pub.A", "pub.A.PublicNested");
 
         checkMembers(Visibility.PUBLIC);
@@ -235,6 +246,7 @@ public class FilterOptions extends ModuleTestBase {
         checkModulesSpecified("m1");
         checkModulesIncluded("m1");
         checkPackagesIncluded("pub");
+        checkPackagesNotIncluded("pqe", "pro");
         checkTypesIncluded("pub.A", "pub.A.ProtectedNested", "pub.A.PublicNested");
 
         checkMembers(Visibility.PROTECTED);
@@ -250,6 +262,7 @@ public class FilterOptions extends ModuleTestBase {
         checkModulesSpecified("m1");
         checkModulesIncluded("m1");
         checkPackagesIncluded("pub");
+        checkPackagesNotIncluded("pqe", "pro");
         checkTypesIncluded("pub.A", "pub.A.ProtectedNested", "pub.A.PublicNested");
 
         checkMembers(Visibility.PROTECTED);
@@ -264,10 +277,10 @@ public class FilterOptions extends ModuleTestBase {
         checkModuleMode("ALL");
         checkModulesSpecified("m1");
         checkModulesIncluded("m1");
-        checkPackagesIncluded("pub");
+        checkPackagesIncluded("pub", "pqe", "pro");
-        checkPackagesIncluded("pro");
         checkTypesIncluded("pub.B", "pub.B.Nested", "pub.B.ProtectedNested", "pub.B.PublicNested",
                            "pub.A", "pub.A.Nested", "pub.A.ProtectedNested", "pub.A.PublicNested",
+                           "pqe.A", "pqe.A.Nested", "pqe.A.ProtectedNested", "pqe.A.PublicNested",
                            "pro.B", "pro.B.Nested", "pro.B.ProtectedNested", "pro.B.PublicNested",
                            "pro.A", "pro.A.Nested", "pro.A.ProtectedNested", "pro.A.PublicNested");
 
@@ -283,12 +296,13 @@ public class FilterOptions extends ModuleTestBase {
         checkModuleMode("ALL");
         checkModulesSpecified("m1");
         checkModulesIncluded("m1");
-        checkPackagesIncluded("pub");
+        checkPackagesIncluded("pub", "pqe", "pro");
-        checkPackagesIncluded("pro");
         checkTypesIncluded("pub.B", "pub.B.PrivateNested", "pub.B.Nested", "pub.B.ProtectedNested",
                            "pub.B.PublicNested",
                            "pub.A", "pub.A.PrivateNested", "pub.A.Nested", "pub.A.ProtectedNested",
                            "pub.A.PublicNested",
+                           "pqe.A", "pqe.A.PrivateNested", "pqe.A.Nested", "pqe.A.ProtectedNested",
+                           "pqe.A.PublicNested",
                            "pro.B", "pro.B.PrivateNested", "pro.B.Nested", "pro.B.ProtectedNested",
                            "pro.B.PublicNested",
                            "pro.A", "pro.A.PrivateNested", "pro.A.Nested", "pro.A.ProtectedNested",
@@ -365,8 +379,17 @@ public class FilterOptions extends ModuleTestBase {
                 .classes(createClass("pub", "B", false))
                 .classes(createClass("pro", "A", true))
                 .classes(createClass("pro", "B", false))
+                .classes(createClass("pqe", "A", true))
                 .exports("pub")
+                .exportsTo("pqe", "m2")
                 .write(src);
+
+        ModuleBuilder mb2 = new ModuleBuilder(tb, "m2");
+        mb2.comment("The second module")
+                .classes(createClass("m2pub", "A", true))
+                .requires("m1")
+                .write(src);
+
         return src.toString();
     }
 

langtools/test/jdk/javadoc/tool/modules/Modules.java

@@ -222,7 +222,10 @@ public class Modules extends ModuleTestBase {
                 .classes("package pkg2; /** @see pkg1.A */ public class B { }")
                 .write(src);
 
+        Path out = base.resolve("out-1");
+        Files.createDirectories(out);
         String log = new JavadocTask(tb)
+                .outdir(out)
                 .options("--module-source-path", src.toString(),
                         "--module-path", modulePath.toString(),
                         "--module", "m2")
@@ -233,7 +236,10 @@ public class Modules extends ModuleTestBase {
             throw new Exception("Error not found");
         }
 
+        out = base.resolve("out-2");
+        Files.createDirectories(out);
         new JavadocTask(tb)
+                .outdir(out)
                 .options("--module-source-path", src.toString(),
                         "--module-path", modulePath.toString(),
                         "--add-modules", "m1",

langtools/test/tools/lib/toolbox/JavadocTask.java

@@ -38,7 +38,9 @@ import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
 import javax.tools.DocumentationTool.DocumentationTask;
+import javax.tools.DocumentationTool;
 import javax.tools.JavaFileManager;
+import javax.tools.JavaFileManager.Location;
 import javax.tools.JavaFileObject;
 import javax.tools.StandardJavaFileManager;
 import javax.tools.StandardLocation;
@@ -303,7 +305,8 @@ public class JavadocTask extends AbstractTask<JavadocTask> {
             if (fileManager == null)
                 fileManager = internalFileManager = jdtool.getStandardFileManager(null, null, null);
             if (outdir != null)
-                setLocationFromPaths(StandardLocation.CLASS_OUTPUT, Collections.singletonList(outdir));
+                setLocationFromPaths(DocumentationTool.Location.DOCUMENTATION_OUTPUT,
+                        Collections.singletonList(outdir));
             if (classpath != null)
                 setLocationFromPaths(StandardLocation.CLASS_PATH, classpath);
             if (sourcepath != null)
@@ -326,7 +329,7 @@ public class JavadocTask extends AbstractTask<JavadocTask> {
         }
     }
 
-    private void setLocationFromPaths(StandardLocation location, List<Path> files) throws IOException {
+    private void setLocationFromPaths(Location location, List<Path> files) throws IOException {
         if (!(fileManager instanceof StandardJavaFileManager))
             throw new IllegalStateException("not a StandardJavaFileManager");
         ((StandardJavaFileManager) fileManager).setLocationFromPaths(location, files);