8155026: javac grants implied readability to explicit modules
authorjlahoda
Tue, 28 Jun 2016 13:33:04 +0200
changeset 39366 8bf5fe72ca88
parent 39365 79f4425ad27c
child 39367 3a9d6e8c6fde
8155026: javac grants implied readability to explicit modules Summary: Automatic modules should not 'requires public' ordinary named modules Reviewed-by: jjg
langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Modules.java
langtools/test/tools/javac/modules/AutomaticModules.java
--- a/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Modules.java	Tue Jun 28 17:39:44 2016 -0700
+++ b/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Modules.java	Tue Jun 28 13:33:04 2016 +0200
@@ -509,7 +509,9 @@
             for (ModuleSymbol ms : allModules()) {
                 if (ms == syms.unnamedModule || ms == msym)
                     continue;
-                RequiresDirective d = new RequiresDirective(ms, EnumSet.of(RequiresFlag.PUBLIC));
+                Set<RequiresFlag> flags = (ms.flags_field & Flags.AUTOMATIC_MODULE) != 0 ?
+                        EnumSet.of(RequiresFlag.PUBLIC) : EnumSet.noneOf(RequiresFlag.class);
+                RequiresDirective d = new RequiresDirective(ms, flags);
                 directives.add(d);
                 requires.add(d);
             }
@@ -1006,29 +1008,19 @@
 
         Set<ModuleSymbol> readable = new LinkedHashSet<>();
         Set<ModuleSymbol> requiresPublic = new HashSet<>();
-        if ((msym.flags() & Flags.AUTOMATIC_MODULE) == 0) {
-            for (RequiresDirective d : msym.requires) {
-                d.module.complete();
-                readable.add(d.module);
-                Set<ModuleSymbol> s = retrieveRequiresPublic(d.module);
-                Assert.checkNonNull(s, () -> "no entry in cache for " + d.module);
-                readable.addAll(s);
-                if (d.flags.contains(RequiresFlag.PUBLIC)) {
-                    requiresPublic.add(d.module);
-                    requiresPublic.addAll(s);
-                }
+
+        for (RequiresDirective d : msym.requires) {
+            d.module.complete();
+            readable.add(d.module);
+            Set<ModuleSymbol> s = retrieveRequiresPublic(d.module);
+            Assert.checkNonNull(s, () -> "no entry in cache for " + d.module);
+            readable.addAll(s);
+            if (d.flags.contains(RequiresFlag.PUBLIC)) {
+                requiresPublic.add(d.module);
+                requiresPublic.addAll(s);
             }
-        } else {
-            //the module graph may contain cycles involving automatic modules
-            //handle automatic modules separatelly:
-            Set<ModuleSymbol> s = retrieveRequiresPublic(msym);
+        }
 
-            readable.addAll(s);
-            requiresPublic.addAll(s);
-
-            //ensure the unnamed module is added (it is not requires public):
-            readable.add(syms.unnamedModule);
-        }
         requiresPublicCache.put(msym, requiresPublic);
         initVisiblePackages(msym, readable);
         for (ExportsDirective d: msym.exports) {
--- a/langtools/test/tools/javac/modules/AutomaticModules.java	Tue Jun 28 17:39:44 2016 -0700
+++ b/langtools/test/tools/javac/modules/AutomaticModules.java	Tue Jun 28 13:33:04 2016 +0200
@@ -23,9 +23,11 @@
 
 /**
  * @test
+ * @bug 8155026
  * @summary Test automatic modules
  * @library /tools/lib
  * @modules
+ *      java.desktop
  *      jdk.compiler/com.sun.tools.javac.api
  *      jdk.compiler/com.sun.tools.javac.main
  * @build toolbox.ToolBox toolbox.JavacTask toolbox.JarTask ModuleTestBase
@@ -34,11 +36,12 @@
 
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.util.Arrays;
+import java.util.List;
 
 import toolbox.JarTask;
 import toolbox.JavacTask;
 import toolbox.Task;
-import toolbox.ToolBox;
 
 public class AutomaticModules extends ModuleTestBase {
 
@@ -86,11 +89,11 @@
         Files.createDirectories(classes);
 
         tb.writeJavaFiles(m1,
-                          "module m1 { requires test.api; }",
+                          "module m1 { requires test.api; requires java.desktop; }",
                           "package impl; public class Impl { public void e(api.Api api) { api.actionPerformed(null); } }");
 
         new JavacTask(tb)
-                .options("-modulesourcepath", moduleSrc.toString(), "-modulepath", modulePath.toString(), "-addmods", "java.desktop")
+                .options("-modulesourcepath", moduleSrc.toString(), "-modulepath", modulePath.toString())
                 .outdir(classes)
                 .files(findJavaFiles(moduleSrc))
                 .run()
@@ -224,4 +227,85 @@
                 .run()
                 .writeAll();
     }
+
+    @Test
+    public void testAutomaticAndNamedModules(Path base) throws Exception {
+        Path modulePath = base.resolve("module-path");
+
+        Files.createDirectories(modulePath);
+
+        for (char c : new char[] {'A', 'B'}) {
+            Path automaticSrc = base.resolve("automaticSrc" + c);
+            tb.writeJavaFiles(automaticSrc, "package api" + c + "; public class Api {}");
+            Path automaticClasses = base.resolve("automaticClasses" + c);
+            tb.createDirectories(automaticClasses);
+
+            String automaticLog = new JavacTask(tb)
+                                    .outdir(automaticClasses)
+                                    .files(findJavaFiles(automaticSrc))
+                                    .run()
+                                    .writeAll()
+                                    .getOutput(Task.OutputKind.DIRECT);
+
+            if (!automaticLog.isEmpty())
+                throw new Exception("expected output not found: " + automaticLog);
+
+            Path automaticJar = modulePath.resolve("automatic" + c + "-1.0.jar");
+
+            new JarTask(tb, automaticJar)
+              .baseDir(automaticClasses)
+              .files("api" + c + "/Api.class")
+              .run();
+        }
+
+        Path moduleSrc = base.resolve("module-src");
+
+        tb.writeJavaFiles(moduleSrc.resolve("m1"),
+                          "module m1 { requires automaticA; }",
+                          "package impl; public class Impl { apiA.Api a; apiB.Api b; m2.M2 m;}");
+
+        tb.writeJavaFiles(moduleSrc.resolve("m2"),
+                          "module m2 { exports m2; }",
+                          "package m2; public class M2 { }");
+
+        Path classes = base.resolve("classes");
+
+        Files.createDirectories(classes);
+
+        List<String> log = new JavacTask(tb)
+                .options("-modulesourcepath", moduleSrc.toString(),
+                         "-modulepath", modulePath.toString(),
+                         "-addmods", "automaticB",
+                         "-XDrawDiagnostics")
+                .outdir(classes)
+                .files(findJavaFiles(moduleSrc))
+                .run(Task.Expect.FAIL)
+                .writeAll()
+                .getOutputLines(Task.OutputKind.DIRECT);
+
+        List<String> expected = Arrays.asList("Impl.java:1:61: compiler.err.not.def.access.package.cant.access: m2.M2, m2",
+                                              "1 error");
+
+        if (!expected.equals(log)) {
+            throw new Exception("expected output not found: " + log);
+        }
+
+        log = new JavacTask(tb)
+                .options("-modulesourcepath", moduleSrc.toString(),
+                         "-modulepath", modulePath.toString(),
+                         "-XDrawDiagnostics")
+                .outdir(classes)
+                .files(findJavaFiles(moduleSrc))
+                .run(Task.Expect.FAIL)
+                .writeAll()
+                .getOutputLines(Task.OutputKind.DIRECT);
+
+        expected = Arrays.asList("Impl.java:1:51: compiler.err.doesnt.exist: apiB",
+                                 "Impl.java:1:61: compiler.err.not.def.access.package.cant.access: m2.M2, m2",
+                                 "2 errors");
+
+        if (!expected.equals(log)) {
+            throw new Exception("expected output not found: " + log);
+        }
+    }
 }