8191468: jdk.scripting.nashorn.shell (jjs) module should use optional dependency for java.compiler module

Reviewed-by: hannesw, jlaskey

src/jdk.scripting.nashorn.shell/share/classes/jdk/nashorn/tools/jjs/JavacPackagesHelper.java

@@ -0,0 +1,160 @@
+/*
+ * Copyright (c) 2017, 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+package jdk.nashorn.tools.jjs;
+
+import java.io.IOException;
+import java.io.File;
+import java.util.Collections;
+import java.util.EnumSet;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import javax.tools.JavaCompiler;
+import javax.tools.JavaFileManager.Location;
+import javax.tools.JavaFileObject;
+import javax.tools.StandardJavaFileManager;
+import javax.tools.StandardLocation;
+import javax.tools.ToolProvider;
+import jdk.nashorn.internal.runtime.Context;
+
+/**
+ * A javac package helper that uses javac to complete package names.
+ */
+final class JavacPackagesHelper extends PackagesHelper {
+    // JavaCompiler may be null on certain platforms (eg. JRE)
+    private static final JavaCompiler compiler;
+    static {
+        // Use javac only if security manager is not around!
+        compiler = System.getSecurityManager() == null? ToolProvider.getSystemJavaCompiler() : null;
+    }
+
+    /**
+     * Is this class available?
+     *
+     * @return true if javac is available
+     */
+    static boolean isAvailable() {
+        return compiler != null;
+    }
+
+    private final boolean modulePathSet;
+    private final StandardJavaFileManager fm;
+    private final Set<JavaFileObject.Kind> fileKinds;
+
+    /**
+     * Construct a new JavacPackagesHelper.
+     *
+     * @param context the current Nashorn Context
+     */
+    JavacPackagesHelper(final Context context) throws IOException {
+        super(context);
+        final String modulePath = context.getEnv()._module_path;
+        this.modulePathSet = modulePath != null && !modulePath.isEmpty();
+        if (isAvailable()) {
+            final String classPath = context.getEnv()._classpath;
+            fm = compiler.getStandardFileManager(null, null, null);
+            fileKinds = EnumSet.of(JavaFileObject.Kind.CLASS);
+
+            if (this.modulePathSet) {
+                fm.setLocation(StandardLocation.MODULE_PATH, getFiles(modulePath));
+            }
+
+            if (classPath != null && !classPath.isEmpty()) {
+                fm.setLocation(StandardLocation.CLASS_PATH, getFiles(classPath));
+            } else {
+                // no classpath set. Make sure that it is empty and not any default like "."
+                fm.setLocation(StandardLocation.CLASS_PATH, Collections.<File>emptyList());
+            }
+        } else {
+            // javac is not available - caller should have checked!
+            throw new IllegalStateException("JavacPackagesHelper is not available!");
+        }
+    }
+
+
+    @Override
+    void close() throws IOException {
+        if (fm != null) {
+            fm.close();
+        }
+    }
+
+    @Override
+    Set<String> listPackage(final String pkg) throws IOException {
+        final Set<String> props = new HashSet<>();
+        listPackage(StandardLocation.PLATFORM_CLASS_PATH, pkg, props);
+        if (this.modulePathSet) {
+            for (Set<Location> locs : fm.listLocationsForModules(StandardLocation.MODULE_PATH)) {
+                for (Location loc : locs) {
+                    listPackage(loc, pkg, props);
+                }
+            }
+        }
+        listPackage(StandardLocation.CLASS_PATH, pkg, props);
+        return props;
+    }
+
+    private void listPackage(final Location loc, final String pkg, final Set<String> props)
+            throws IOException {
+        for (JavaFileObject file : fm.list(loc, pkg, fileKinds, true)) {
+            final String binaryName = fm.inferBinaryName(loc, file);
+            // does not start with the given package prefix
+            if (!binaryName.startsWith(pkg + ".")) {
+                continue;
+            }
+
+            final int nextDot = binaryName.indexOf('.', pkg.length() + 1);
+            final int start = pkg.length() + 1;
+
+            if (nextDot != -1) {
+                // subpackage - eg. "regex" for "java.util"
+                final String pkgName = binaryName.substring(start, nextDot);
+                if (isPackageAccessible(binaryName.substring(0, nextDot))) {
+                    props.add(binaryName.substring(start, nextDot));
+                }
+            } else {
+                // class - filter out nested, inner, anonymous, local classes.
+                // Dynalink supported public nested classes as properties of
+                // StaticClass object anyway. We don't want to expose those
+                // "$" internal names as properties of package object.
+
+                final String clsName = binaryName.substring(start);
+                if (clsName.indexOf('$') == -1 && isClassAccessible(binaryName)) {
+                    props.add(clsName);
+                }
+            }
+        }
+    }
+
+    // return list of File objects for the given class path
+    private static List<File> getFiles(final String classPath) {
+        return Stream.of(classPath.split(File.pathSeparator))
+                    .map(File::new)
+                    .collect(Collectors.toList());
+    }
+}

src/jdk.scripting.nashorn.shell/share/classes/jdk/nashorn/tools/jjs/JrtPackagesHelper.java

@@ -0,0 +1,90 @@
+/*
+ * Copyright (c) 2017, 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+package jdk.nashorn.tools.jjs;
+
+import java.io.IOException;
+import java.net.URI;
+import java.nio.file.DirectoryStream;
+import java.nio.file.Files;
+import java.nio.file.FileSystem;
+import java.nio.file.FileSystems;
+import java.nio.file.Path;
+import java.util.HashSet;
+import java.util.Set;
+import jdk.nashorn.internal.runtime.Context;
+
+/**
+ * A java packages helper that uses jrt file system.
+ */
+final class JrtPackagesHelper extends PackagesHelper {
+    private final FileSystem jrtfs;
+
+    /**
+     * Construct a new JrtPackagesHelper.
+     *
+     * @param context the current Nashorn Context
+     */
+    JrtPackagesHelper(final Context context) throws IOException {
+        super(context);
+        jrtfs = FileSystems.getFileSystem(URI.create("jrt:/"));
+    }
+
+    @Override
+    void close() throws IOException {
+    }
+
+    @Override
+    Set<String> listPackage(final String pkg) throws IOException {
+        final Set<String> props = new HashSet<>();
+        // look for the /packages/<package_name> directory
+        Path pkgDir = jrtfs.getPath("/packages/" + pkg);
+        if (Files.exists(pkgDir)) {
+            String pkgSlashName = pkg.replace('.', '/');
+            try (DirectoryStream<Path> ds = Files.newDirectoryStream(pkgDir)) {
+                // it has module links under which this package occurs
+                for (Path mod : ds) {
+                    // get the package directory under /modules
+                    Path pkgUnderMod = jrtfs.getPath(mod.toString() + "/" + pkgSlashName);
+                    try (DirectoryStream<Path> ds2 = Files.newDirectoryStream(pkgUnderMod)) {
+                        for (Path p : ds2) {
+                            String str = p.getFileName().toString();
+                            // get rid of ".class", if any
+                            if (str.endsWith(".class")) {
+                                final String clsName = str.substring(0, str.length() - ".class".length());
+                                if (clsName.indexOf('$') == -1 && isClassAccessible(pkg + "." + clsName)) {
+                                    props.add(str);
+                                }
+                            } else if (isPackageAccessible(pkg + "." + str)) {
+                                props.add(str);
+                            }
+                        }
+                    }
+                }
+            }
+        }
+        return props;
+    }
+}

src/jdk.scripting.nashorn.shell/share/classes/jdk/nashorn/tools/jjs/PackagesHelper.java

@@ -27,58 +27,21 @@ package jdk.nashorn.tools.jjs;
 
 import java.lang.reflect.Modifier;
 import java.io.IOException;
-import java.io.File;
-import java.net.URI;
-import java.nio.file.DirectoryStream;
-import java.nio.file.Files;
-import java.nio.file.FileSystem;
-import java.nio.file.FileSystems;
-import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.Collections;
-import java.util.EnumSet;
-import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-import java.util.stream.Collectors;
-import java.util.stream.Stream;
-import javax.tools.JavaCompiler;
-import javax.tools.JavaFileManager.Location;
-import javax.tools.JavaFileObject;
-import javax.tools.StandardJavaFileManager;
-import javax.tools.StandardLocation;
-import javax.tools.ToolProvider;
 import jdk.nashorn.internal.runtime.Context;
 
 /**
- * A helper class to compute properties of a Java package object. Properties of
+ * Abstract helper class to compute properties of a Java package object. Properties of
  * package object are (simple) top level class names in that java package and
  * immediate subpackages of that package.
  */
-final class PackagesHelper {
+abstract class PackagesHelper {
-    // JavaCompiler may be null on certain platforms (eg. JRE)
-    private static final JavaCompiler compiler;
-    static {
-        // Use javac only if security manager is not around!
-        compiler = System.getSecurityManager() == null? ToolProvider.getSystemJavaCompiler() : null;
-    }
-
-    /**
-     * Is javac available?
-     *
-     * @return true if javac is available
-     */
-    private static boolean isJavacAvailable() {
-        return compiler != null;
-    }
-
     private final Context context;
-    private final boolean modulePathSet;
-    private final StandardJavaFileManager fm;
-    private final Set<JavaFileObject.Kind> fileKinds;
-    private final FileSystem jrtfs;
 
     /**
      * Construct a new PackagesHelper.
@@ -87,31 +50,10 @@ final class PackagesHelper {
      */
     PackagesHelper(final Context context) throws IOException {
         this.context = context;
-        final String modulePath = context.getEnv()._module_path;
-        this.modulePathSet = modulePath != null && !modulePath.isEmpty();
-        if (isJavacAvailable()) {
-            final String classPath = context.getEnv()._classpath;
-            fm = compiler.getStandardFileManager(null, null, null);
-            fileKinds = EnumSet.of(JavaFileObject.Kind.CLASS);
-
-            if (this.modulePathSet) {
-                fm.setLocation(StandardLocation.MODULE_PATH, getFiles(modulePath));
     }
 
-            if (classPath != null && !classPath.isEmpty()) {
+    static PackagesHelper create(final Context context) throws IOException {
-                fm.setLocation(StandardLocation.CLASS_PATH, getFiles(classPath));
+        return isJavacHelperAvailable()? new JavacPackagesHelper(context) : new JrtPackagesHelper(context);
-            } else {
-                // no classpath set. Make sure that it is empty and not any default like "."
-                fm.setLocation(StandardLocation.CLASS_PATH, Collections.<File>emptyList());
-            }
-            jrtfs = null;
-        } else {
-            // javac is not available - directly use jrt fs
-            // to support at least platform classes.
-            fm = null;
-            fileKinds = null;
-            jrtfs = FileSystems.getFileSystem(URI.create("jrt:/"));
-        }
     }
 
     // LRU cache for java package properties lists
@@ -132,7 +74,7 @@ final class PackagesHelper {
      * @param pkg Java package name or package prefix name
      * @return the list of properties of the given Java package or package prefix
      */
-    List<String> getPackageProperties(final String pkg) {
+    final List<String> getPackageProperties(final String pkg) {
         // check the cache first
         if (propsCache.containsKey(pkg)) {
             return propsCache.get(pkg);
@@ -152,95 +94,20 @@ final class PackagesHelper {
         }
     }
 
-    public void close() throws IOException {
+    /**
-        if (fm != null) {
+     * Close resources (like file system) used, if any.
-            fm.close();
+     */
-        }
+    abstract void close() throws IOException;
-    }
 
-    private Set<String> listPackage(final String pkg) throws IOException {
+    /**
-        final Set<String> props = new HashSet<>();
+     * Return the set of properties of a given package object.
-        if (fm != null) {
+     *
-            listPackage(StandardLocation.PLATFORM_CLASS_PATH, pkg, props);
+     * @param pkg package start string
-            if (this.modulePathSet) {
+     * @return set of properties of the given Java package
-                for (Set<Location> locs : fm.listLocationsForModules(StandardLocation.MODULE_PATH)) {
+     */
-                    for (Location loc : locs) {
+    abstract Set<String> listPackage(final String pkg) throws IOException;
-                        listPackage(loc, pkg, props);
-                    }
-                }
-            }
-            listPackage(StandardLocation.CLASS_PATH, pkg, props);
-        } else if (jrtfs != null) {
-            // look for the /packages/<package_name> directory
-            Path pkgDir = jrtfs.getPath("/packages/" + pkg);
-            if (Files.exists(pkgDir)) {
-                String pkgSlashName = pkg.replace('.', '/');
-                try (DirectoryStream<Path> ds = Files.newDirectoryStream(pkgDir)) {
-                    // it has module links under which this package occurs
-                    for (Path mod : ds) {
-                        // get the package directory under /modules
-                        Path pkgUnderMod = jrtfs.getPath(mod.toString() + "/" + pkgSlashName);
-                        try (DirectoryStream<Path> ds2 = Files.newDirectoryStream(pkgUnderMod)) {
-                            for (Path p : ds2) {
-                                String str = p.getFileName().toString();
-                                // get rid of ".class", if any
-                                if (str.endsWith(".class")) {
-                                    final String clsName = str.substring(0, str.length() - ".class".length());
-                                    if (clsName.indexOf('$') == -1 && isClassAccessible(pkg + "." + clsName)) {
-                                        props.add(str);
-                                    }
-                                } else if (isPackageAccessible(pkg + "." + str)) {
-                                    props.add(str);
-                                }
-                            }
-                        }
-                    }
-                }
-            }
-        }
-        return props;
-    }
 
-    private void listPackage(final Location loc, final String pkg, final Set<String> props)
+    final boolean isClassAccessible(final String className) {
-            throws IOException {
-        for (JavaFileObject file : fm.list(loc, pkg, fileKinds, true)) {
-            final String binaryName = fm.inferBinaryName(loc, file);
-            // does not start with the given package prefix
-            if (!binaryName.startsWith(pkg + ".")) {
-                continue;
-            }
-
-            final int nextDot = binaryName.indexOf('.', pkg.length() + 1);
-            final int start = pkg.length() + 1;
-
-            if (nextDot != -1) {
-                // subpackage - eg. "regex" for "java.util"
-                final String pkgName = binaryName.substring(start, nextDot);
-                if (isPackageAccessible(binaryName.substring(0, nextDot))) {
-                    props.add(binaryName.substring(start, nextDot));
-                }
-            } else {
-                // class - filter out nested, inner, anonymous, local classes.
-                // Dynalink supported public nested classes as properties of
-                // StaticClass object anyway. We don't want to expose those
-                // "$" internal names as properties of package object.
-
-                final String clsName = binaryName.substring(start);
-                if (clsName.indexOf('$') == -1 && isClassAccessible(binaryName)) {
-                    props.add(clsName);
-                }
-            }
-        }
-    }
-
-    // return list of File objects for the given class path
-    private static List<File> getFiles(final String classPath) {
-        return Stream.of(classPath.split(File.pathSeparator))
-                    .map(File::new)
-                    .collect(Collectors.toList());
-    }
-
-    private boolean isClassAccessible(final String className) {
         try {
             final Class<?> clz = context.findClass(className);
             return Modifier.isPublic(clz.getModifiers());
@@ -249,7 +116,7 @@ final class PackagesHelper {
         return false;
     }
 
-    private boolean isPackageAccessible(final String pkgName) {
+    final boolean isPackageAccessible(final String pkgName) {
         try {
             Context.checkPackageAccess(pkgName);
             return true;
@@ -257,4 +124,20 @@ final class PackagesHelper {
             return false;
         }
     }
+
+    private static boolean isJavacHelperAvailable() {
+        try {
+            boolean result = JavacPackagesHelper.isAvailable();
+            if (Main.DEBUG && !result) {
+                System.err.println("javac packages helper is not available");
+            }
+            return result;
+        } catch (final LinkageError err) {
+            if (Main.DEBUG) {
+                System.err.println("javac packages helper is not available");
+                err.printStackTrace();
+            }
+            return false;
+        }
+    }
 }

src/jdk.scripting.nashorn.shell/share/classes/jdk/nashorn/tools/jjs/PropertiesHelper.java

@@ -57,7 +57,7 @@ final class PropertiesHelper {
      */
     PropertiesHelper(final Context context) {
         try {
-            this.pkgsHelper = new PackagesHelper(context);
+            this.pkgsHelper = PackagesHelper.create(context);
         } catch (final IOException exp) {
             if (Main.DEBUG) {
                 exp.printStackTrace();

src/jdk.scripting.nashorn.shell/share/classes/module-info.java

@@ -38,7 +38,7 @@
  * @since 9
  */
 module jdk.scripting.nashorn.shell {
-    requires java.compiler;
+    requires static java.compiler;
     requires jdk.internal.le;
     requires jdk.scripting.nashorn;
     requires jdk.internal.ed;

test/nashorn/script/basic/JDK-8191468.js

@@ -0,0 +1,48 @@
+/*
+ * Copyright (c) 2017, 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/**
+ * JDK-8191468: jdk.scripting.nashorn.shell (jjs) module should use optional dependency for java.compiler module
+ *
+ * @test
+ * @run
+ */
+
+var optJjsMod = java.lang.ModuleLayer.boot().findModule("jdk.scripting.nashorn.shell");
+
+// make sure that the module exists!
+Assert.assertTrue(optJjsMod.isPresent());
+
+// jdk.scripting.nashorn.shell should use optional dependency for java.compiler
+var javaCompilerDependency = optJjsMod.get().
+        descriptor.requires().
+        stream().
+        filter(function(mod) { return mod.name() == "java.compiler" }).
+        findFirst();
+
+// java.compiler dependency should be present
+Assert.assertTrue(javaCompilerDependency.isPresent());
+
+var Modifier = java.lang.module.ModuleDescriptor.Requires.Modifier;
+// java.compiler requires should be "requires static"
+Assert.assertTrue(javaCompilerDependency.get().modifiers().contains(Modifier.STATIC));