Mercurial > hg > openjdk > jdk9 > langtools

changeset 4068:bef1cba2d0d9

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
8175277: javadoc AssertionError when specified with release 8 Reviewed-by: jjg, jlahoda
author ksrini
date Mon, 27 Mar 2017 17:53:00 -0700
parents 24582dd2649a
children cc3c67b12ef1
files src/jdk.compiler/share/classes/com/sun/tools/javac/main/Arguments.java src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/Start.java src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/resources/javadoc.properties test/jdk/javadoc/tool/modules/ReleaseOptions.java
diffstat 4 files changed, 64 insertions(+), 84 deletions(-) [+]
line wrap: on
line diff
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/main/Arguments.java	Fri Mar 24 06:40:28 2017 -0700
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/main/Arguments.java	Mon Mar 27 17:53:00 2017 -0700
@@ -39,6 +39,7 @@
 import java.util.LinkedHashSet;
 import java.util.Map;
 import java.util.Set;
+import java.util.function.Predicate;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import java.util.stream.Stream;
@@ -283,23 +284,14 @@
     }
 
     /**
-     * Processes strings containing options and operands.
-     * @param args the strings to be processed
-     * @param allowableOpts the set of option declarations that are applicable
-     * @param helper a help for use by Option.process
-     * @param allowOperands whether or not to check for files and classes
-     * @param checkFileManager whether or not to check if the file manager can handle
-     *      options which are not recognized by any of allowableOpts
-     * @return true if all the strings were successfully processed; false otherwise
-     * @throws IllegalArgumentException if a problem occurs and errorMode is set to
-     *      ILLEGAL_ARGUMENT
+     * Handles the {@code --release} option.
+     *
+     * @param additionalOptions a predicate to handle additional options implied by the
+     * {@code --release} option. The predicate should return true if all the additional
+     * options were processed successfully.
+     * @return true if successful, false otherwise
      */
-    private boolean processArgs(Iterable<String> args,
-            Set<Option> allowableOpts, OptionHelper helper,
-            boolean allowOperands, boolean checkFileManager) {
-        if (!doProcessArgs(args, allowableOpts, helper, allowOperands, checkFileManager))
-            return false;
-
+    public boolean handleReleaseOptions(Predicate<Iterable<String>> additionalOptions) {
         String platformString = options.get(Option.RELEASE);
 
         checkOptionAllowed(platformString == null,
@@ -323,7 +315,7 @@
 
             context.put(PlatformDescription.class, platformDescription);
 
-            if (!doProcessArgs(platformDescription.getAdditionalOptions(), allowableOpts, helper, allowOperands, checkFileManager))
+            if (!additionalOptions.test(platformDescription.getAdditionalOptions()))
                 return false;
 
             Collection<Path> platformCP = platformDescription.getPlatformPath();
@@ -348,6 +340,30 @@
             }
         }
 
+        return true;
+    }
+
+    /**
+     * Processes strings containing options and operands.
+     * @param args the strings to be processed
+     * @param allowableOpts the set of option declarations that are applicable
+     * @param helper a help for use by Option.process
+     * @param allowOperands whether or not to check for files and classes
+     * @param checkFileManager whether or not to check if the file manager can handle
+     *      options which are not recognized by any of allowableOpts
+     * @return true if all the strings were successfully processed; false otherwise
+     * @throws IllegalArgumentException if a problem occurs and errorMode is set to
+     *      ILLEGAL_ARGUMENT
+     */
+    private boolean processArgs(Iterable<String> args,
+            Set<Option> allowableOpts, OptionHelper helper,
+            boolean allowOperands, boolean checkFileManager) {
+        if (!doProcessArgs(args, allowableOpts, helper, allowOperands, checkFileManager))
+            return false;
+
+        if (!handleReleaseOptions(extra -> doProcessArgs(extra, allowableOpts, helper, allowOperands, checkFileManager)))
+            return false;
+
         options.notifyListeners();
 
         return true;
--- a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/Start.java	Fri Mar 24 06:40:28 2017 -0700
+++ b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/Start.java	Mon Mar 27 17:53:00 2017 -0700
@@ -513,12 +513,22 @@
         } catch (com.sun.tools.javac.main.Option.InvalidValueException ignore) {
         }
 
+        Arguments arguments = Arguments.instance(context);
+        arguments.init(ProgramName);
+        arguments.allowEmpty();
+
         doclet.init(locale, messager);
         parseArgs(argList, javaNames);
 
-        Arguments arguments = Arguments.instance(context);
-        arguments.init(ProgramName);
-        arguments.allowEmpty();
+        if (!arguments.handleReleaseOptions(extra -> true)) {
+            // Arguments does not always increase the error count in the
+            // case of errors, so increment the error count only if it has
+            // not been updated previously, preventing complaints by callers
+            if (!messager.hasErrors() && !messager.hasWarnings())
+                messager.nerrors++;
+            return CMDERR;
+        }
+
         if (!arguments.validate()) {
             // Arguments does not always increase the error count in the
             // case of errors, so increment the error count only if it has
@@ -532,49 +542,6 @@
             ((BaseFileManager) fileManager).handleOptions(fileManagerOpts);
         }
 
-        String platformString = compOpts.get("--release");
-
-        if (platformString != null) {
-            if (compOpts.isSet("-source")) {
-                String text = messager.getText("main.release.bootclasspath.conflict", "-source");
-                throw new ToolException(CMDERR, text);
-            }
-            if (fileManagerOpts.containsKey(BOOT_CLASS_PATH)) {
-                String text = messager.getText("main.release.bootclasspath.conflict",
-                        BOOT_CLASS_PATH.getPrimaryName());
-                throw new ToolException(CMDERR, text);
-            }
-
-            PlatformDescription platformDescription =
-                    PlatformUtils.lookupPlatformDescription(platformString);
-
-            if (platformDescription == null) {
-                String text = messager.getText("main.unsupported.release.version", platformString);
-                throw new IllegalArgumentException(text);
-            }
-
-            compOpts.put(SOURCE, platformDescription.getSourceVersion());
-
-            context.put(PlatformDescription.class, platformDescription);
-
-            Collection<Path> platformCP = platformDescription.getPlatformPath();
-
-            if (platformCP != null) {
-                if (fileManager instanceof StandardJavaFileManager) {
-                    StandardJavaFileManager sfm = (StandardJavaFileManager) fileManager;
-                    try {
-                        sfm.setLocationFromPaths(StandardLocation.PLATFORM_CLASS_PATH, platformCP);
-                    } catch (IOException ioe) {
-                        throw new ToolException(SYSERR, ioe.getMessage(), ioe);
-                    }
-                } else {
-                    String text = messager.getText("main.release.not.standard.file.manager",
-                                                    platformString);
-                    throw new ToolException(ABNORMAL, text);
-                }
-            }
-        }
-
         compOpts.notifyListeners();
         List<String> modules = (List<String>) jdtoolOpts.computeIfAbsent(ToolOption.MODULE,
                 s -> Collections.EMPTY_LIST);
--- a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/resources/javadoc.properties	Fri Mar 24 06:40:28 2017 -0700
+++ b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/resources/javadoc.properties	Mon Mar 27 17:53:00 2017 -0700
@@ -284,9 +284,6 @@
 main.illegal_class_name=Illegal class name: "{0}"
 main.illegal_package_name=Illegal package name: "{0}"
 main.illegal_option_value=Illegal option value: "{0}"
-main.release.bootclasspath.conflict=option {0} cannot be used together with -release
-main.unsupported.release.version=release version {0} not supported
-main.release.not.standard.file.manager=-release option specified, but the provided JavaFileManager is not a StandardJavaFileManager.
 main.file.manager.list=FileManager error listing files: "{0}"
 main.assertion.error=assertion failed: "{0}}"
 main.unknown.error=an unknown error has occurred
--- a/test/jdk/javadoc/tool/modules/ReleaseOptions.java	Fri Mar 24 06:40:28 2017 -0700
+++ b/test/jdk/javadoc/tool/modules/ReleaseOptions.java	Mon Mar 27 17:53:00 2017 -0700
@@ -23,7 +23,7 @@
 
 /**
  * @test
- * @bug 8175346
+ * @bug 8175346 8175277
  * @summary Test release option interactions
  * @modules
  *      jdk.javadoc/jdk.javadoc.internal.api
@@ -58,7 +58,7 @@
         Task.Result result = execNegativeTask("--release", "8",
                 "--patch-module", "m=" + mpath.toString(),
                 "p");
-        assertMessagePresent(".*No source files for package p.*");
+        assertMessagePresent(".*not allowed with target 1.8.*");
         assertMessageNotPresent(".*Exception*");
         assertMessageNotPresent(".java.lang.AssertionError.*");
     }
@@ -80,20 +80,20 @@
         assertMessageNotPresent(".java.lang.AssertionError.*");
     }
 
-//    @Test TBD, JDK-8175277, argument validation should fail on this
-//    public void testReleaseWithModuleSourcepath(Path base) throws Exception {
-//        Path src = Paths.get(base.toString(), "src");
-//        Path mpath = Paths.get(src.toString(), "m");
-//
-//        tb.writeJavaFiles(mpath,
-//                "module m { exports p; }",
-//                "package p; public class C { }");
-//
-//        Task.Result result = execNegativeTask("--release", "8",
-//                "--module-source-path", src.toString(),
-//                "--module", "m");
-//        assertMessagePresent(".*(use -source 9 or higher to enable modules).*");
-//        assertMessageNotPresent(".*Exception*");
-//        assertMessageNotPresent(".java.lang.AssertionError.*");
-//    }
+    @Test
+    public void testReleaseWithModuleSourcepath(Path base) throws Exception {
+        Path src = Paths.get(base.toString(), "src");
+        Path mpath = Paths.get(src.toString(), "m");
+
+        tb.writeJavaFiles(mpath,
+                "module m { exports p; }",
+                "package p; public class C { }");
+
+        Task.Result result = execNegativeTask("--release", "8",
+                "--module-source-path", src.toString(),
+                "--module", "m");
+        assertMessagePresent(".*not allowed with target 1.8.*");
+        assertMessageNotPresent(".*Exception*");
+        assertMessageNotPresent(".java.lang.AssertionError.*");
+    }
 }