8207032: Compilation succeeds without checking readability when --add-exports used

Summary: Ensuring --add-exports are only propagated when the target module reads the exporting module.
Reviewed-by: vromero, jjg

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Modules.java

@@ -1552,7 +1552,9 @@ public class Modules extends JCTree.Visitor {
         }
 
         addExports.forEach((exportsFrom, exports) -> {
+            if (msym.readModules.contains(exportsFrom)) {
                 addVisiblePackages(msym, seen, exportsFrom, exports);
+            }
         });
     }
 

test/jdk/tools/launcher/modules/addexports/AddExportsTestWarningError.java

@@ -80,7 +80,7 @@ public class AddExportsTestWarningError {
 
         builder.compile("m1", MODS_DIR);
         builder.compile("m2", MODS_DIR, "--add-exports", "m1/p1=m2");
-        builder.compile("m3", MODS_DIR, "--add-exports", "m1/p1=m3");
+        builder.compile("m3", MODS_DIR, "--add-exports", "m1/p1=m3", "--add-reads", "m3=m1");
     }
 
 

test/langtools/tools/javac/modules/AddExportsTest.java

@@ -23,6 +23,7 @@
 
 /*
  * @test
+ * @bug 8207032
  * @summary Test the --add-exports option
  * @library /tools/lib
  * @modules jdk.compiler/com.sun.tools.javac.api
@@ -35,6 +36,8 @@ import java.nio.file.Path;
 
 import toolbox.JavacTask;
 import toolbox.Task;
+import toolbox.Task.Expect;
+import toolbox.Task.OutputKind;
 
 public class AddExportsTest extends ModuleTestBase {
 
@@ -74,11 +77,11 @@ public class AddExportsTest extends ModuleTestBase {
                           "package p1; public class C1 { }");
         Path src_m2 = src.resolve("m2x");
         tb.writeJavaFiles(src_m2,
-                          "module m2x { }",
+                          "module m2x { requires m1x; }",
                           "package p2; class C2 { p1.C1 c1; }");
         Path src_m3 = src.resolve("m3x");
         tb.writeJavaFiles(src_m3,
-                          "module m3x { }",
+                          "module m3x { requires m1x; }",
                           "package p3; class C3 { p1.C1 c1; }");
         Path classes = base.resolve("classes");
         tb.createDirectories(classes);
@@ -275,7 +278,7 @@ public class AddExportsTest extends ModuleTestBase {
                           "package p1; public class C1 { }");
         Path src_m2 = src.resolve("m2x");
         tb.writeJavaFiles(src_m2,
-                          "module m2x { }",
+                          "module m2x { requires m1x; }",
                           "package p2; class C2 { p1.C1 c1; }");
         Path classes = base.resolve("classes");
         tb.createDirectories(classes);
@@ -298,7 +301,7 @@ public class AddExportsTest extends ModuleTestBase {
                           "package p1; public class C1 { }");
         Path src_m2 = src.resolve("m2x");
         tb.writeJavaFiles(src_m2,
-                          "module m2x { }",
+                          "module m2x { requires m1x; }",
                           "package p2; class C2 { p1.C1 c1; }");
         Path classes = base.resolve("classes");
         tb.createDirectories(classes);
@@ -322,11 +325,11 @@ public class AddExportsTest extends ModuleTestBase {
                           "package p1; public class C1 { }");
         Path src_m2 = src.resolve("m2x");
         tb.writeJavaFiles(src_m2,
-                          "module m2x { }",
+                          "module m2x { requires m1x; }",
                           "package p2; class C2 { p1.C1 c1; }");
         Path src_m3 = src.resolve("m3x");
         tb.writeJavaFiles(src_m3,
-                          "module m3x { }",
+                          "module m3x { requires m1x; }",
                           "package p3; class C3 { p1.C1 c1; }");
         Path classes = base.resolve("classes");
         tb.createDirectories(classes);
@@ -340,4 +343,89 @@ public class AddExportsTest extends ModuleTestBase {
                 .run()
                 .writeAll();
     }
+
+    @Test
+    public void testNoReads(Path base) throws Exception {
+        Path src = base.resolve("src");
+        Path src_m1 = src.resolve("m1x");
+        tb.writeJavaFiles(src_m1,
+                          "module m1x { }",
+                          "package p1; public class C1 { }");
+        Path src_m2 = src.resolve("m2x");
+        tb.writeJavaFiles(src_m2,
+                          "module m2x { }",
+                          "package p2; class C2 { p1.C1 c1; }");
+        Path classes = base.resolve("classes");
+        tb.createDirectories(classes);
+
+        String log;
+
+        log = new JavacTask(tb)
+                .options("--module-source-path", src.toString(),
+                         "-XDrawDiagnostics")
+                .outdir(classes)
+                .files(findJavaFiles(src))
+                .run(Expect.FAIL)
+                .writeAll()
+                .getOutput(OutputKind.DIRECT);
+
+        checkOutputContains(log,
+            "C2.java:1:24: compiler.err.package.not.visible: p1, (compiler.misc.not.def.access.does.not.read: m2x, p1, m1x)");
+
+        log = new JavacTask(tb)
+                .options("--module-source-path", src.toString(),
+                         "-XDrawDiagnostics",
+                         "--add-exports", "m1x/p1=m2x")
+                .outdir(classes)
+                .files(findJavaFiles(src))
+                .run(Expect.FAIL)
+                .writeAll()
+                .getOutput(OutputKind.DIRECT);
+
+        checkOutputContains(log,
+            "C2.java:1:24: compiler.err.package.not.visible: p1, (compiler.misc.not.def.access.does.not.read: m2x, p1, m1x)");
+
+        Path mp = base.resolve("mp");
+        tb.createDirectories(mp);
+
+        new JavacTask(tb)
+                .options("--module-source-path", src.toString(),
+                         "-XDrawDiagnostics",
+                         "--add-exports", "m1x/p1=m2x",
+                         "--add-reads", "m2x=m1x")
+                .outdir(mp)
+                .files(findJavaFiles(src))
+                .run(Expect.SUCCESS)
+                .writeAll();
+
+        log = new JavacTask(tb)
+                .options("-XDrawDiagnostics",
+                         "--add-exports", "m1x/p1=m2x",
+                         "--add-reads", "m2x=m1x",
+                         "--module-path", mp.toString())
+                .outdir(classes)
+                .files(findJavaFiles(src_m2))
+                .run(Expect.FAIL)
+                .writeAll()
+                .getOutput(OutputKind.DIRECT);
+
+        checkOutputContains(log,
+            "C2.java:1:24: compiler.err.package.not.visible: p1, (compiler.misc.not.def.access.does.not.read: m2x, p1, m1x)");
+        checkOutputContains(log,
+            "- compiler.warn.module.for.option.not.found: --add-reads, m1x");
+        checkOutputContains(log,
+            "- compiler.warn.module.for.option.not.found: --add-exports, m1x");
+
+        new JavacTask(tb)
+                .options("-XDrawDiagnostics",
+                         "--add-exports", "m1x/p1=m2x",
+                         "--add-reads", "m2x=m1x",
+                         "--module-path", mp.toString(),
+                         "--add-modules", "m1x")
+                .outdir(classes)
+                .files(findJavaFiles(src_m2))
+                .run(Expect.SUCCESS)
+                .writeAll()
+                .getOutput(OutputKind.DIRECT);
+    }
 }