8208608: Update --module-source-path to allow explicit source paths for specific modules

Reviewed-by: jlahoda

src/jdk.compiler/share/classes/com/sun/tools/javac/file/Locations.java

@@ -97,11 +97,8 @@ import com.sun.tools.javac.util.StringUtils;
 import static javax.tools.StandardLocation.PLATFORM_CLASS_PATH;
 
 import static com.sun.tools.javac.main.Option.BOOT_CLASS_PATH;
-import static com.sun.tools.javac.main.Option.DJAVA_ENDORSED_DIRS;
-import static com.sun.tools.javac.main.Option.DJAVA_EXT_DIRS;
 import static com.sun.tools.javac.main.Option.ENDORSEDDIRS;
 import static com.sun.tools.javac.main.Option.EXTDIRS;
-import static com.sun.tools.javac.main.Option.XBOOTCLASSPATH;
 import static com.sun.tools.javac.main.Option.XBOOTCLASSPATH_APPEND;
 import static com.sun.tools.javac.main.Option.XBOOTCLASSPATH_PREPEND;
 
@@ -1533,7 +1530,62 @@ public class Locations {
             return true;
         }
 
+        /**
+         * Initializes the module table, based on a string containing the composition
+         * of a series of command-line options.
+         * At most one pattern to initialize a series of modules can be given.
+         * At most one module-specific search path per module can be given.
+         *
+         * @param value a series of values, separated by NUL.
+         */
         void init(String value) {
+            Pattern moduleSpecificForm = Pattern.compile("([\\p{Alnum}$_.]+)=(.*)");
+            List<String> pathsForModules = new ArrayList<>();
+            String modulePattern = null;
+            for (String v : value.split("\0")) {
+                if (moduleSpecificForm.matcher(v).matches()) {
+                    pathsForModules.add(v);
+                } else {
+                    modulePattern = v;
+                }
+            }
+            // set the general module pattern first, if given
+            if (modulePattern != null) {
+                initFromPattern(modulePattern);
+            }
+            pathsForModules.forEach(this::initForModule);
+        }
+
+        /**
+         * Initializes a module-specific override, using {@code setPathsForModule}.
+         *
+         * @param value a string of the form: module-name=search-path
+         */
+        void initForModule(String value) {
+            int eq = value.indexOf('=');
+            String name = value.substring(0, eq);
+            List<Path> paths = new ArrayList<>();
+            for (String v : value.substring(eq + 1).split(File.pathSeparator)) {
+                try {
+                    paths.add(Paths.get(v));
+                } catch (InvalidPathException e) {
+                    throw new IllegalArgumentException("invalid path: " + v, e);
+                }
+            }
+            try {
+                setPathsForModule(name, paths);
+            } catch (IOException e) {
+                e.printStackTrace();
+                throw new IllegalArgumentException("cannot set path for module " + name, e);
+            }
+        }
+
+        /**
+         * Initializes the module table based on a custom option syntax.
+         *
+         * @param value the value such as may be given to a --module-source-path option
+         */
+        void initFromPattern(String value) {
             Collection<String> segments = new ArrayList<>();
             for (String s: value.split(File.pathSeparator)) {
                 expandBraces(s, segments);

src/jdk.compiler/share/classes/com/sun/tools/javac/main/Option.java

@@ -182,7 +182,48 @@ public enum Option {
 
     SOURCE_PATH("--source-path -sourcepath", "opt.arg.path", "opt.sourcepath", STANDARD, FILEMANAGER),
 
-    MODULE_SOURCE_PATH("--module-source-path", "opt.arg.mspath", "opt.modulesourcepath", STANDARD, FILEMANAGER),
+    MODULE_SOURCE_PATH("--module-source-path", "opt.arg.mspath", "opt.modulesourcepath", STANDARD, FILEMANAGER) {
+        // The deferred filemanager diagnostics mechanism assumes a single value per option,
+        // but --module-source-path-module can be used multiple times, once in the old form
+        // and once per module in the new form.  Therefore we compose an overall value for the
+        // option containing the individual values given on the command line, separated by NULL.
+        // The standard file manager code knows to split apart the NULL-separated components.
+        @Override
+        public void process(OptionHelper helper, String option, String arg) throws InvalidValueException {
+            if (arg.isEmpty()) {
+                throw helper.newInvalidValueException(Errors.NoValueForOption(option));
+            }
+            Pattern moduleSpecificForm = getPattern();
+            String prev = helper.get(MODULE_SOURCE_PATH);
+            if (prev == null) {
+                super.process(helper, option, arg);
+            } else  if (moduleSpecificForm.matcher(arg).matches()) {
+                String argModule = arg.substring(0, arg.indexOf('='));
+                boolean isRepeated = Arrays.stream(prev.split("\0"))
+                        .filter(s -> moduleSpecificForm.matcher(s).matches())
+                        .map(s -> s.substring(0, s.indexOf('=')))
+                        .anyMatch(s -> s.equals(argModule));
+                if (isRepeated) {
+                    throw helper.newInvalidValueException(Errors.RepeatedValueForModuleSourcePath(argModule));
+                } else {
+                    super.process(helper, option, prev + '\0' + arg);
+                }
+            } else {
+                boolean isPresent = Arrays.stream(prev.split("\0"))
+                        .anyMatch(s -> !moduleSpecificForm.matcher(s).matches());
+                if (isPresent) {
+                    throw helper.newInvalidValueException(Errors.MultipleValuesForModuleSourcePath);
+                } else {
+                    super.process(helper, option, prev + '\0' + arg);
+                }
+            }
+        }
+
+        @Override
+        public Pattern getPattern() {
+            return Pattern.compile("([\\p{Alnum}$_.]+)=(.*)");
+        }
+    },
 
     MODULE_PATH("--module-path -p", "opt.arg.path", "opt.modulepath", STANDARD, FILEMANAGER),
 
@@ -194,7 +235,7 @@ public enum Option {
         // The deferred filemanager diagnostics mechanism assumes a single value per option,
         // but --patch-module can be used multiple times, once per module. Therefore we compose
         // a value for the option containing the last value specified for each module, and separate
-        // the the module=path pairs by an invalid path character, NULL.
+        // the module=path pairs by an invalid path character, NULL.
         // The standard file manager code knows to split apart the NULL-separated components.
         @Override
         public void process(OptionHelper helper, String option, String arg) throws InvalidValueException {

src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties

@@ -3417,7 +3417,14 @@ compiler.err.no.value.for.option=\
 
 # 0: string
 compiler.err.repeated.value.for.patch.module=\
-    --patch-module specified more than once for {0}
+    --patch-module specified more than once for module {0}
+
+# 0: string
+compiler.err.repeated.value.for.module.source.path=\
+    --module-source-path specified more than once for module {0}
+
+compiler.err.multiple.values.for.module.source.path=\
+    --module-source-path specified more than once with a pattern argument
 
 # 0: string
 compiler.err.unmatched.quote=\

test/langtools/tools/javac/diags/examples.not-yet.txt

@@ -174,12 +174,14 @@ compiler.err.invalid.flag
 compiler.err.invalid.profile
 compiler.err.invalid.source
 compiler.err.invalid.target
+compiler.err.multiple.values.for.module.source.path
 compiler.err.no.source.files.classes
 compiler.err.no.value.for.option
 compiler.err.option.not.allowed.with.target
 compiler.err.option.too.many
 compiler.err.profile.bootclasspath.conflict
 compiler.err.release.bootclasspath.conflict
+compiler.err.repeated.value.for.module.source.path
 compiler.err.repeated.value.for.patch.module
 compiler.err.req.arg
 compiler.err.sourcepath.modulesourcepath.conflict

test/langtools/tools/javac/file/SetLocationForModule.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2017, 2018, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -160,8 +160,7 @@ public class SetLocationForModule extends TestRunner {
             Path src1 = Files.createDirectories(base.resolve("src1"));
             Path src1_m = src1.resolve("m");
             tb.writeJavaFiles(src1_m, "module m { }");
-//            fm.setLocationFromPaths(locn, List.of(src1));
+            fm.setLocationFromPaths(locn, List.of(src1));
-            fm.handleOption("--module-source-path", List.of(src1.toString()).iterator());
 
             Location m = fm.getLocationForModule(locn, "m");
             checkEqual("default setting",
@@ -186,8 +185,7 @@ public class SetLocationForModule extends TestRunner {
             Path src2 = Files.createDirectories(base.resolve("src2"));
             Path src2_m = src2.resolve("m");
             tb.writeJavaFiles(src2_m, "module m { }");
-//            fm.setLocationFromPaths(locn, List.of(src2));
+            fm.setLocationFromPaths(locn, List.of(src2));
-            fm.handleOption("--module-source-path", List.of(src2.toString()).iterator());
 
             m = fm.getLocationForModule(locn, "m");
 

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

@@ -519,11 +519,175 @@ public class ModuleSourcePathTest extends ModuleTestBase {
         }
     }
 
+    @Test
+    public void moduleSpecificFormsOnly(Path base) throws Exception {
+        // The dirs for the modules do not use a subdirectory named for the module,
+        // meaning they can only be used by the module-specific form of the option.
+        String[] srcDirs = {
+                "src0",         // m0x
+                "src1",         // m1x
+                "src2",         // m2x
+                "src3"          // m3x
+        };
+        generateModules(base, false, srcDirs);
+
+        final Path modules = base.resolve("modules");
+        tb.createDirectories(modules);
+
+        new JavacTask(tb, Task.Mode.CMDLINE)
+                .options("-XDrawDiagnostics",
+                        "--module-source-path", "m0x=" + base.resolve("src0"),
+                        "--module-source-path", "m1x=" + base.resolve("src1"),
+                        "--module-source-path", "m2x=" + base.resolve("src2"),
+                        "--module-source-path", "m3x=" + base.resolve("src3"))
+                .files(findJavaFiles(base.resolve(srcDirs[srcDirs.length - 1])))
+                .outdir(modules)
+                .run()
+                .writeAll();
+
+        for (int i = 0; i < srcDirs.length; i++) {
+            checkFiles(modules.resolve("m" + i + "x/module-info.class"));
+        }
+        checkFiles(modules.resolve("m3x/pkg3/A.class"));
+    }
+
+    @Test
+    public void modulePatternWithEquals(Path base) throws Exception {
+        // The dirs for the modules contain an '=' character, but
+        // the option should still be recognized as the module pattern form.
+        String[] srcDirs = {
+                "src=",         // m0x
+                "src=",         // m1x
+                "src=",         // m2x
+                "src="  // m3x
+        };
+        generateModules(base, true, srcDirs);
+
+        final Path modules = base.resolve("modules");
+        tb.createDirectories(modules);
+
+        new JavacTask(tb, Task.Mode.CMDLINE)
+                .options("-XDrawDiagnostics",
+                        "--module-source-path", base.resolve("src=").toString())
+                .files(findJavaFiles(base.resolve(srcDirs[srcDirs.length - 1])))
+                .outdir(modules)
+                .run()
+                .writeAll();
+
+        for (int i = 0; i < srcDirs.length; i++) {
+            checkFiles(modules.resolve("m" + i + "x/module-info.class"));
+        }
+        checkFiles(modules.resolve("m3x/pkg3/A.class"));
+    }
+
+    @Test
+    public void duplicateModuleSpecificForms(Path base) throws Exception {
+        // The dirs for the modules do not use a subdirectory named for the module,
+        // meaning they can only be used by the module-specific form of the option.
+        String[] srcDirs = {
+                "src0",         // m0x
+                "src1",         // m1x
+                "src2",         // m2x
+                "src3"          // m3x
+        };
+        generateModules(base, false, srcDirs);
+
+        final Path modules = base.resolve("modules");
+        tb.createDirectories(modules);
+
+        // in the following, it should not matter that src1 does not contain
+        // a definition of m0x; it is bad/wrong to specify the option for m0x twice.
+        String log = new JavacTask(tb, Task.Mode.CMDLINE)
+                .options("-XDrawDiagnostics",
+                        "--module-source-path", "m0x=" + base.resolve("src0"),
+                        "--module-source-path", "m0x=" + base.resolve("src1"))
+                .files(findJavaFiles(base.resolve(srcDirs[srcDirs.length - 1])))
+                .outdir(modules)
+                .run(Task.Expect.FAIL)
+                .writeAll()
+                .getOutput(Task.OutputKind.DIRECT);
+
+        if (!log.contains("error: --module-source-path specified more than once for module m0x"))
+            throw new Exception("Expected error message not found");
+    }
+
+    @Test
+    public void duplicateModulePatternForms(Path base) throws Exception {
+        // module-specific subdirs are used to allow for use of module-pattern form
+        String[] srcDirs = {
+                "src",  // m0x
+                "src",  // m1x
+                "src",  // m2x
+                "src"   // m3x
+        };
+        generateModules(base, true, srcDirs);
+
+        final Path modules = base.resolve("modules");
+        tb.createDirectories(modules);
+
+        // in the following, it should not matter that the same pattern
+        // is used for both occurrences; it is bad/wrong to give any two patterns
+        String log = new JavacTask(tb, Task.Mode.CMDLINE)
+                .options("-XDrawDiagnostics",
+                        "--module-source-path", base.resolve("src").toString(),
+                        "--module-source-path", base.resolve("src").toString())
+                .files(findJavaFiles(base.resolve(srcDirs[srcDirs.length - 1])))
+                .outdir(modules)
+                .run(Task.Expect.FAIL)
+                .writeAll()
+                .getOutput(Task.OutputKind.DIRECT);
+
+        if (!log.contains("error: --module-source-path specified more than once with a pattern argument"))
+            throw new Exception("Expected error message not found");
+    }
+
+    @Test
+    public void mixedOptionForms(Path base) throws Exception {
+        // The dirs for m0x, m2x use a subdirectory named for the module,
+        // meaning they can be used in the module pattern form of the option;
+        // the dirs for m1x, m3x do not use a subdirectory named for the module,
+        // meaning they can only be used by the module-specific form of the option
+        String[] srcDirs = {
+                "src/m0x",      // m0x
+                "src1",         // m1x
+                "src/m2x",      // m2x
+                "src3"          // m3x
+        };
+        generateModules(base, false, srcDirs);
+
+        final Path modules = base.resolve("modules");
+        tb.createDirectories(modules);
+
+        new JavacTask(tb, Task.Mode.CMDLINE)
+                .options("-XDrawDiagnostics",
+                        "--module-source-path", base.resolve("src").toString(), // for m0x, m2x
+                        "--module-source-path", "m1x=" + base.resolve("src1"),
+                        "--module-source-path", "m3x=" + base.resolve("src3"))
+                .files(findJavaFiles(base.resolve(srcDirs[srcDirs.length - 1])))
+                .outdir(modules)
+                .run()
+                .writeAll();
+
+        for (int i = 0; i < srcDirs.length; i++) {
+            checkFiles(modules.resolve("m" + i + "x/module-info.class"));
+        }
+        checkFiles(modules.resolve("m3x/pkg3/A.class"));
+    }
+
     private void generateModules(Path base, String... paths) throws IOException {
+        generateModules(base, true, paths);
+    }
+
+    private void generateModules(Path base, boolean useModuleSubdirs, String... paths)
+                throws IOException {
         for (int i = 0; i < paths.length; i++) {
             String moduleName = "m" + i + "x";
             String dependency = i > 0 ? "requires m" + (i - 1) + "x;" : "";
-            tb.writeJavaFiles(base.resolve(paths[i]).resolve(moduleName),
+            Path dir = base.resolve(paths[i]);
+            if (useModuleSubdirs) {
+                dir = dir.resolve(moduleName);
+            }
+            tb.writeJavaFiles(dir,
                     "module " + moduleName + " { " + dependency + " }",
                     "package pkg" + i + "; class A { }");
         }

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

@@ -98,7 +98,7 @@ public class PatchModulesTest extends ModuleTestBase {
     @Test
     public void testDuplicates(Path base) throws Exception {
         test(asList("java.base=a", "java.compiler=b", "java.base=c"),
-            false, "error: --patch-module specified more than once for java.base");
+            false, "error: --patch-module specified more than once for module java.base");
     }
 
     @Test