changeset 3757:6afd59d40256

8169447: javac should detect/reject repeated use of --patch-module on command line Reviewed-by: jlahoda
author jjg
date Thu, 10 Nov 2016 13:29:34 -0800
parents d4deb115da83
children 6cc2220006f2
files src/jdk.compiler/share/classes/com/sun/tools/javac/main/Option.java src/jdk.compiler/share/classes/com/sun/tools/javac/resources/javac.properties test/tools/javac/modules/PatchModulesTest.java
diffstat 3 files changed, 62 insertions(+), 25 deletions(-) [+]
line wrap: on
line diff
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/main/Option.java	Thu Nov 10 17:27:01 2016 +0100
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/main/Option.java	Thu Nov 10 13:29:34 2016 -0800
@@ -35,6 +35,7 @@
 import java.util.Collections;
 import java.util.Comparator;
 import java.util.EnumSet;
+import java.util.HashSet;
 import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
@@ -43,6 +44,7 @@
 import java.util.ServiceLoader;
 import java.util.Set;
 import java.util.TreeSet;
+import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 import java.util.stream.StreamSupport;
@@ -200,31 +202,37 @@
         // The standard file manager code knows to split apart the NULL-separated components.
         @Override
         public boolean process(OptionHelper helper, String option, String arg) {
-            if (!arg.contains("=")) { // could be more strict regeex, e.g. "(?i)[a-z0-9_.]+=.*"
-                helper.error(Errors.LocnInvalidArgForXpatch(arg));
+            if (arg.isEmpty()) {
+                helper.error("err.no.value.for.option", option);
+                return true;
+            } else if (getPattern().matcher(arg).matches()) {
+                String prev = helper.get(PATCH_MODULE);
+                if (prev == null) {
+                    super.process(helper, option, arg);
+                    return false;
+                } else {
+                    String argModulePackage = arg.substring(0, arg.indexOf('='));
+                    boolean isRepeated = Arrays.stream(prev.split("\0"))
+                            .map(s -> s.substring(0, s.indexOf('=')))
+                            .collect(Collectors.toSet())
+                            .contains(argModulePackage);
+                    if (isRepeated) {
+                        helper.error("err.repeated.value.for.patch.module", argModulePackage);
+                        return true;
+                    } else {
+                        super.process(helper, option, prev + '\0' + arg);
+                        return false;
+                    }
+                }
+            } else {
+                helper.error("err.bad.value.for.option", option, arg);
+                return true;
             }
+        }
 
-            String previous = helper.get(this);
-            if (previous == null) {
-                return super.process(helper, option, arg);
-            }
-
-            Map<String,String> map = new LinkedHashMap<>();
-            for (String s : previous.split("\0")) {
-                int sep = s.indexOf('=');
-                map.put(s.substring(0, sep), s.substring(sep + 1));
-            }
-
-            int sep = arg.indexOf('=');
-            map.put(arg.substring(0, sep), arg.substring(sep + 1));
-
-            StringBuilder sb = new StringBuilder();
-            map.forEach((m, p) -> {
-                if (sb.length() > 0)
-                    sb.append('\0');
-                sb.append(m).append('=').append(p);
-            });
-            return super.process(helper, option, sb.toString());
+        @Override
+        public Pattern getPattern() {
+            return Pattern.compile("([^/]+)=(,*[^,].*)");
         }
     },
 
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/javac.properties	Thu Nov 10 17:27:01 2016 +0100
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/javac.properties	Thu Nov 10 13:29:34 2016 -0800
@@ -361,6 +361,8 @@
     bad value for {0} option: ''{1}''
 javac.err.no.value.for.option=\
     no value for {0} option
+javac.err.repeated.value.for.patch.module=\
+    --patch-module specified more than once for {0}
 
 ## messages
 
--- a/test/tools/javac/modules/PatchModulesTest.java	Thu Nov 10 17:27:01 2016 +0100
+++ b/test/tools/javac/modules/PatchModulesTest.java	Thu Nov 10 13:29:34 2016 -0800
@@ -91,12 +91,28 @@
     }
 
     @Test
-    public void testLastOneWins(Path base) throws Exception {
+    public void testDuplicates(Path base) throws Exception {
         test(asList("java.base=a", "java.compiler=b", "java.base=c"),
-            "{java.base=[c], java.compiler=[b]}");
+            false, "--patch-module specified more than once for java.base");
+    }
+
+    @Test
+    public void testEmpty(Path base) throws Exception {
+        test(asList(""),
+            false, "no value for --patch-module option");
+    }
+
+    @Test
+    public void testInvalid(Path base) throws Exception {
+        test(asList("java.base/java.lang=."),
+            false, "bad value for --patch-module option: 'java.base/java.lang=.'");
     }
 
     void test(List<String> patches, String expect) throws Exception {
+        test(patches, true, expect);
+    }
+
+    void test(List<String> patches, boolean expectOK, String expect) throws Exception {
         JavacTool tool = (JavacTool) ToolProvider.getSystemJavaCompiler();
         StringWriter sw = new StringWriter();
         try (PrintWriter pw = new PrintWriter(sw)) {
@@ -121,6 +137,17 @@
                 tb.out.println("Found:  " + found);
                 error("output not as expected");
             }
+        } catch (IllegalArgumentException e) {
+            if (expectOK) {
+                error("unexpected exception: " + e);
+                throw e;
+            }
+            String found = e.getMessage();
+            if (!found.equals(expect)) {
+                tb.out.println("Expect: " + expect);
+                tb.out.println("Found:  " + found);
+                error("output not as expected");
+            }
         }
     }
 }