8155026: javac grants implied readability to explicit modules
Summary: Automatic modules should not 'requires public' ordinary named modules
Reviewed-by: jjg
--- 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);
+ }
+ }
}