Merge branch 'big-andy-coates-issue_211'

Author: siordache

src/main/java/org/javamodularity/moduleplugin/tasks/CompileTestTask.java

@@ -34,6 +34,7 @@ public void configureCompileTestJava() {
         });
     }
 
+    @SuppressWarnings("Convert2Lambda")
     private void configureCompileTestJava(JavaCompile compileTestJava) {
         if(GradleVersion.current().compareTo(GradleVersion.version("6.4")) >= 0) {
             compileTestJava.getModularity().getInferModulePath().set(false);
@@ -44,7 +45,7 @@ private void configureCompileTestJava(JavaCompile compileTestJava) {
             LOGGER.info(compileTestJava.getName() +  ".compileOnClasspath: {}", moduleOptions.isCompileOnClasspath());
             if(!moduleOptions.isCompileOnClasspath()) {
                 // don't convert to lambda: https://github.com/java9-modularity/gradle-modules-plugin/issues/54
-                compileTestJava.doFirst(new Action<Task>() {
+                compileTestJava.doFirst(new Action<>() {
                     @Override
                     public void execute(Task task) {
                         var compilerArgs = buildCompilerArgs(compileTestJava, moduleOptions);
@@ -86,7 +87,7 @@ private List<String> buildCompilerArgs(
 
         patchModuleContainer.mutator(classpath).mutateArgs(compilerArgs);
 
-        ModuleInfoTestHelper.mutateArgs(project, compilerArgs::add);
+        ModuleInfoTestHelper.mutateArgs(project, true, compilerArgs::add);
 
         return compilerArgs;
     }

src/main/java/org/javamodularity/moduleplugin/tasks/ModuleInfoTestHelper.java

@@ -8,13 +8,17 @@
 import java.io.IOException;
 import java.io.UncheckedIOException;
 import java.nio.file.Files;
+import java.util.Iterator;
+import java.util.List;
 import java.util.function.Consumer;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 class ModuleInfoTestHelper {
 
     private static final Logger LOGGER = Logging.getLogger(ModuleInfoTestHelper.class);
 
-    static void mutateArgs(Project project, Consumer<String> consumer) {
+    static void mutateArgs(Project project, boolean excludeOpens, Consumer<String> consumer) {
         JavaProjectHelper helper = new JavaProjectHelper(project);
 
         String moduleName = helper.moduleName();
@@ -29,13 +33,42 @@ static void mutateArgs(Project project, Consumer<String> consumer) {
         var moduleInfoTestPath = files.getSingleFile().toPath();
         LOGGER.info("Using lines of '{}' to patch module {}...", moduleInfoTestPath, moduleName);
         try (var lines = Files.lines(moduleInfoTestPath)) {
-            lines.map(String::trim)
-                    .filter(line -> !line.isEmpty())
-                    .filter(line -> !line.startsWith("//"))
-                    .peek(line -> LOGGER.debug("  {}", line))
-                    .forEach(consumer);
+            consumeLines(lines, excludeOpens, consumer);
         } catch (IOException e) {
             throw new UncheckedIOException("Reading " + moduleInfoTestPath + " failed", e);
         }
     }
+
+    // Visible for testing.
+    static void consumeLines(Stream<String> s, boolean excludeOpens, Consumer<String> consumer) {
+        final List<String> lines = s.map(String::trim)
+                .filter(line -> !line.isBlank())
+                .filter(line -> !line.startsWith("//"))
+                .collect(Collectors.toList());
+
+        if (excludeOpens) {
+            excludeOpens(lines);
+        }
+
+        lines.stream()
+                .peek(line -> LOGGER.debug("  {}", line))
+                .forEach(consumer);
+    }
+
+    private static void excludeOpens(final List<String> lines) {
+        final Iterator<String> it = lines.iterator();
+        while (it.hasNext()) {
+            final String next = it.next();
+            if (!next.equals("--add-opens")) {
+                continue;
+            }
+
+            it.remove();
+            if (it.hasNext()) {
+                final String opens = it.next();
+                LOGGER.debug("  Excluding --add-opens {}", opens);
+                it.remove();
+            }
+        }
+    }
 }

src/main/java/org/javamodularity/moduleplugin/tasks/TestTask.java

@@ -40,14 +40,15 @@ public void configureTestJava() {
                 .ifPresent(this::configureTestJava);
     }
 
+    @SuppressWarnings("Convert2Lambda")
     private void configureTestJava(Test testJava) {
         var testModuleOptions = testJava.getExtensions().create("moduleOptions", TestModuleOptions.class, project);
 
         if(GradleVersion.current().compareTo(GradleVersion.version("6.4")) >= 0) {
             testJava.getModularity().getInferModulePath().set(false);
         }
         // don't convert to lambda: https://github.com/java9-modularity/gradle-modules-plugin/issues/54
-        testJava.doFirst(new Action<Task>() {
+        testJava.doFirst(new Action<>() {
             @Override
             public void execute(Task task) {
                 if (testModuleOptions.getRunOnClasspath()) {
@@ -86,7 +87,7 @@ private List<String> buildJvmArgs(Test testJava, TestModuleOptions testModuleOpt
             testEngine.additionalTaskOptions.forEach(option -> option.mutateArgs(jvmArgs));
         });
 
-        ModuleInfoTestHelper.mutateArgs(project, jvmArgs::add);
+        ModuleInfoTestHelper.mutateArgs(project, false, jvmArgs::add);
 
         return jvmArgs;
     }

src/test/java/org/javamodularity/moduleplugin/ModulePluginSmokeTest.java

@@ -81,6 +81,7 @@ void smokeTest(String projectName, GradleVersion gradleVersion) {
             assertTasksSuccessful(result, "greeter.provider.testfixture", "build");
         }
         assertTasksSuccessful(result, "greeter.runner", "build", "run");
+        assertOutputDoesNotContain(result, "warning: [options] --add-opens has no effect at compile time");
     }
 
     @CartesianProductTest(name = "smokeTestRun({arguments})")
@@ -275,6 +276,11 @@ private static void assertTasksSuccessful(BuildResult result, String subprojectN
         }
     }
 
+    private static void assertOutputDoesNotContain(BuildResult result, String text) {
+        final String output = result.getOutput();
+        assertFalse(output.contains(text), "Output should not contain '" + text + "', but was: " + output);
+    }
+
     private static boolean checkCombination(String projectName, GradleVersion gradleVersion) {
         final boolean kotlin_NotSupported = projectName.startsWith("test-project-kotlin") && gradleVersion.toString().compareTo("6.4") < 0;
         final boolean kotlin1_7_NotSupported = projectName.equals("test-project-kotlin") && gradleVersion.toString().compareTo("6.6") < 0;

src/test/java/org/javamodularity/moduleplugin/tasks/ModuleInfoTestHelperTest.java

@@ -0,0 +1,68 @@
+package org.javamodularity.moduleplugin.tasks;
+
+import org.junit.jupiter.api.Test;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.stream.Stream;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+class ModuleInfoTestHelperTest {
+
+    private final List<String> output = new ArrayList<>();
+
+    @Test
+    void shouldFilterOutBlankLines() {
+        final Stream<String> lines = Stream.of("", " ", "\t");
+
+        ModuleInfoTestHelper.consumeLines(lines, false, output::add);
+
+        assertEquals(List.of(), output);
+    }
+
+    @Test
+    void shouldFilterOutCommentLines() {
+        final Stream<String> lines = Stream.of("// comment", " // indented comment");
+
+        ModuleInfoTestHelper.consumeLines(lines, false, output::add);
+
+        assertEquals(List.of(), output);
+    }
+
+    @Test
+    void shouldTrimLines() {
+        final Stream<String> lines = Stream.of(" a ", "\tb\t");
+
+        ModuleInfoTestHelper.consumeLines(lines, false, output::add);
+
+        assertEquals(List.of("a", "b"), output);
+    }
+
+    @Test
+    void shouldNotExcludeOpens() {
+        final Stream<String> lines = Stream.of("a", "--add-opens", "c");
+
+        ModuleInfoTestHelper.consumeLines(lines, false, output::add);
+
+        assertEquals(List.of("a", "--add-opens", "c"), output);
+    }
+
+    @Test
+    void shouldExcludeOpens() {
+        final Stream<String> lines = Stream.of("a", " --add-opens", "x=y", "c");
+
+        ModuleInfoTestHelper.consumeLines(lines, true, output::add);
+
+        assertEquals(List.of("a", "c"), output);
+    }
+
+    @Test
+    void shouldNotBlowUpOnExcludedOpenAtEnd() {
+        final Stream<String> lines = Stream.of("a", "--add-opens");
+
+        ModuleInfoTestHelper.consumeLines(lines, true, output::add);
+
+        assertEquals(List.of("a"), output);
+    }
+}

test-project/greeter.provider.test/src/test/java/module-info.test

@@ -0,0 +1,2 @@
+--add-opens
+  java.base/java.lang=greeter.api