8072595: nashorn should not use obj.getClass() for null checks

Reviewed-by: hannesw, attila

nashorn/samples/javashell.js

@@ -40,6 +40,7 @@
 var Arrays = Java.type("java.util.Arrays");
 var BufferedReader = Java.type("java.io.BufferedReader");
 var FileWriter = Java.type("java.io.FileWriter");
+var List = Java.type("java.util.List");
 var LocalDateTime = Java.type("java.time.LocalDateTime");
 var InputStreamReader = Java.type("java.io.InputStreamReader");
 var PrintWriter = Java.type("java.io.PrintWriter");
@@ -121,7 +122,7 @@ EOF
 // execute code command
 function exec(args) {
     // build child process and start it!
-    new ProcessBuilder(Arrays.asList(args.split(' ')))
+    new ProcessBuilder(Java.to(args.split(' '), List))
          .inheritIO()
          .start()
          .waitFor();

nashorn/samples/shell.js

@@ -42,6 +42,7 @@
     var Arrays = Java.type("java.util.Arrays");
     var BufferedReader = Java.type("java.io.BufferedReader");
     var InputStreamReader = Java.type("java.io.InputStreamReader");
+    var List = Java.type("java.util.List");
     var ProcessBuilder = Java.type("java.lang.ProcessBuilder");
     var System = Java.type("java.lang.System");
 
@@ -66,7 +67,7 @@
                     }
                 } else {
                     // build child process and start it!
-                    new ProcessBuilder(Arrays.asList(args))
+                    new ProcessBuilder(Java.to(args, List))
                         .inheritIO()
                         .start()
                         .waitFor();

nashorn/src/jdk.scripting.nashorn/share/classes/jdk/internal/dynalink/DynamicLinker.java

@@ -88,6 +88,7 @@ import java.lang.invoke.MethodHandles;
 import java.lang.invoke.MethodType;
 import java.lang.invoke.MutableCallSite;
 import java.util.List;
+import java.util.Objects;
 import jdk.internal.dynalink.linker.GuardedInvocation;
 import jdk.internal.dynalink.linker.GuardingDynamicLinker;
 import jdk.internal.dynalink.linker.LinkRequest;
@@ -252,7 +253,7 @@ public class DynamicLinker {
         // Make sure we filter the invocation before linking it into the call site. This is typically used to match the
         // return type of the invocation to the call site.
         guardedInvocation = prelinkFilter.filter(guardedInvocation, linkRequest, linkerServices);
-        guardedInvocation.getClass(); // null pointer check
+        Objects.requireNonNull(guardedInvocation);
 
         int newRelinkCount = relinkCount;
         // Note that the short-circuited "&&" evaluation below ensures we'll increment the relinkCount until

nashorn/src/jdk.scripting.nashorn/share/classes/jdk/internal/dynalink/beans/StaticClass.java

@@ -84,6 +84,7 @@
 package jdk.internal.dynalink.beans;
 
 import java.io.Serializable;
+import java.util.Objects;
 
 /**
  * Object that represents the static facet of a class (its static methods, properties, and fields, as well as
@@ -106,8 +107,7 @@ public class StaticClass implements Serializable {
     private final Class<?> clazz;
 
     /*private*/ StaticClass(final Class<?> clazz) {
-        clazz.getClass(); // NPE check
-        this.clazz = clazz;
+        this.clazz = Objects.requireNonNull(clazz);
     }
 
     /**

nashorn/src/jdk.scripting.nashorn/share/classes/jdk/internal/dynalink/linker/GuardedInvocation.java

@@ -91,6 +91,7 @@ import java.lang.invoke.MethodType;
 import java.lang.invoke.SwitchPoint;
 import java.lang.invoke.WrongMethodTypeException;
 import java.util.List;
+import java.util.Objects;
 import jdk.internal.dynalink.CallSiteDescriptor;
 import jdk.internal.dynalink.support.Guards;
 
@@ -170,8 +171,7 @@ public class GuardedInvocation {
      * @throws NullPointerException if invocation is null.
      */
     public GuardedInvocation(final MethodHandle invocation, final MethodHandle guard, final SwitchPoint switchPoint, final Class<? extends Throwable> exception) {
-        invocation.getClass(); // NPE check
-        this.invocation = invocation;
+        this.invocation = Objects.requireNonNull(invocation);
         this.guard = guard;
         this.switchPoints = switchPoint == null ? null : new SwitchPoint[] { switchPoint };
         this.exception = exception;
@@ -190,8 +190,7 @@ public class GuardedInvocation {
      * @throws NullPointerException if invocation is null.
      */
     public GuardedInvocation(final MethodHandle invocation, final MethodHandle guard, final SwitchPoint[] switchPoints, final Class<? extends Throwable> exception) {
-        invocation.getClass(); // NPE check
-        this.invocation = invocation;
+        this.invocation = Objects.requireNonNull(invocation);
         this.guard = guard;
         this.switchPoints = switchPoints == null ? null : switchPoints.clone();
         this.exception = exception;

nashorn/src/jdk.scripting.nashorn/share/classes/jdk/internal/dynalink/support/CallSiteDescriptorFactory.java

@@ -91,6 +91,7 @@ import java.lang.ref.WeakReference;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
+import java.util.Objects;
 import java.util.StringTokenizer;
 import java.util.WeakHashMap;
 import jdk.internal.dynalink.CallSiteDescriptor;
@@ -123,9 +124,9 @@ public class CallSiteDescriptorFactory {
      * in fact return a weakly-referenced canonical instance.
      */
     public static CallSiteDescriptor create(final Lookup lookup, final String name, final MethodType methodType) {
-        name.getClass(); // NPE check
-        methodType.getClass(); // NPE check
-        lookup.getClass(); // NPE check
+        Objects.requireNonNull(name);
+        Objects.requireNonNull(methodType);
+        Objects.requireNonNull(lookup);
         final String[] tokenizedName = tokenizeName(name);
         if(isPublicLookup(lookup)) {
             return getCanonicalPublicDescriptor(createPublicCallSiteDescriptor(tokenizedName, methodType));

nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/api/scripting/NashornScriptEngine.java

@@ -39,6 +39,7 @@ import java.security.PrivilegedAction;
 import java.security.ProtectionDomain;
 import java.text.MessageFormat;
 import java.util.Locale;
+import java.util.Objects;
 import java.util.ResourceBundle;
 import javax.script.AbstractScriptEngine;
 import javax.script.Bindings;
@@ -360,7 +361,7 @@ public final class NashornScriptEngine extends AbstractScriptEngine implements C
     }
 
     private Object invokeImpl(final Object selfObject, final String name, final Object... args) throws ScriptException, NoSuchMethodException {
-        name.getClass(); // null check
+        Objects.requireNonNull(name);
         assert !(selfObject instanceof ScriptObject) : "raw ScriptObject not expected here";
 
         Global invokeGlobal = null;

nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/api/scripting/NashornScriptEngineFactory.java

@@ -28,6 +28,7 @@ package jdk.nashorn.api.scripting;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
+import java.util.Objects;
 import javax.script.ScriptEngine;
 import javax.script.ScriptEngineFactory;
 import jdk.nashorn.internal.runtime.Context;
@@ -177,7 +178,7 @@ public final class NashornScriptEngineFactory implements ScriptEngineFactory {
      *         denies {@code RuntimePermission("nashorn.setConfig")}
      */
     public ScriptEngine getScriptEngine(final ClassFilter classFilter) {
-        classFilter.getClass(); // null check
+        Objects.requireNonNull(classFilter);
         return newEngine(DEFAULT_OPTIONS, getAppClassLoader(), classFilter);
     }
 
@@ -192,7 +193,7 @@ public final class NashornScriptEngineFactory implements ScriptEngineFactory {
      *         denies {@code RuntimePermission("nashorn.setConfig")}
      */
     public ScriptEngine getScriptEngine(final String... args) {
-        args.getClass(); // null check
+        Objects.requireNonNull(args);
         return newEngine(args, getAppClassLoader(), null);
     }
 
@@ -208,7 +209,7 @@ public final class NashornScriptEngineFactory implements ScriptEngineFactory {
      *         denies {@code RuntimePermission("nashorn.setConfig")}
      */
     public ScriptEngine getScriptEngine(final String[] args, final ClassLoader appLoader) {
-        args.getClass(); // null check
+        Objects.requireNonNull(args);
         return newEngine(args, appLoader, null);
     }
 
@@ -225,8 +226,8 @@ public final class NashornScriptEngineFactory implements ScriptEngineFactory {
      *         denies {@code RuntimePermission("nashorn.setConfig")}
      */
     public ScriptEngine getScriptEngine(final String[] args, final ClassLoader appLoader, final ClassFilter classFilter) {
-        args.getClass(); // null check
-        classFilter.getClass(); // null check
+        Objects.requireNonNull(args);
+        Objects.requireNonNull(classFilter);
         return newEngine(args, appLoader, classFilter);
     }
 

nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/api/scripting/ScriptObjectMirror.java

@@ -39,6 +39,7 @@ import java.util.Iterator;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 import java.util.concurrent.Callable;
 import javax.script.Bindings;
@@ -180,7 +181,7 @@ public final class ScriptObjectMirror extends AbstractJSObject implements Bindin
      * @return return value of function
      */
     public Object callMember(final String functionName, final Object... args) {
-        functionName.getClass(); // null check
+        Objects.requireNonNull(functionName);
         final Global oldGlobal = Context.getGlobal();
         final boolean globalChanged = (oldGlobal != global);
 
@@ -213,7 +214,7 @@ public final class ScriptObjectMirror extends AbstractJSObject implements Bindin
 
     @Override
     public Object getMember(final String name) {
-        name.getClass();
+        Objects.requireNonNull(name);
         return inGlobal(new Callable<Object>() {
             @Override public Object call() {
                 return wrap(sobj.get(name), global);
@@ -232,7 +233,7 @@ public final class ScriptObjectMirror extends AbstractJSObject implements Bindin
 
     @Override
     public boolean hasMember(final String name) {
-        name.getClass();
+        Objects.requireNonNull(name);
         return inGlobal(new Callable<Boolean>() {
             @Override public Boolean call() {
                 return sobj.has(name);
@@ -251,13 +252,13 @@ public final class ScriptObjectMirror extends AbstractJSObject implements Bindin
 
     @Override
     public void removeMember(final String name) {
-        name.getClass();
+        Objects.requireNonNull(name);
         remove(name);
     }
 
     @Override
     public void setMember(final String name, final Object value) {
-        name.getClass();
+        Objects.requireNonNull(name);
         put(name, value);
     }
 
@@ -425,9 +426,7 @@ public final class ScriptObjectMirror extends AbstractJSObject implements Bindin
 
     @Override
     public void putAll(final Map<? extends String, ? extends Object> map) {
-        if (map == null) {
-            throw new NullPointerException("map is null");
-        }
+        Objects.requireNonNull(map, "map is null");
         final ScriptObject oldGlobal = Context.getGlobal();
         final boolean globalChanged = (oldGlobal != global);
         inGlobal(new Callable<Object>() {
@@ -804,9 +803,9 @@ public final class ScriptObjectMirror extends AbstractJSObject implements Bindin
      * @throws IllegalArgumentException if key is empty string
      */
     private static void checkKey(final Object key) {
-        if (key == null) {
-            throw new NullPointerException("key can not be null");
-        } else if (!(key instanceof String)) {
+        Objects.requireNonNull(key, "key can not be null");
+
+        if (!(key instanceof String)) {
             throw new ClassCastException("key should be a String. It is " + key.getClass().getName() + " instead.");
         } else if (((String)key).length() == 0) {
             throw new IllegalArgumentException("key can not be empty");

nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/api/scripting/URLReader.java

@@ -30,6 +30,7 @@ import java.io.IOException;
 import java.io.Reader;
 import java.net.URL;
 import java.nio.charset.Charset;
+import java.util.Objects;
 import jdk.nashorn.internal.runtime.Source;
 
 /**
@@ -77,8 +78,7 @@ public final class URLReader extends Reader {
      * @throws NullPointerException if url is null
      */
     public URLReader(final URL url, final Charset cs) {
-        // null check
-        url.getClass();
+        Objects.requireNonNull(url);
         this.url = url;
         this.cs  = cs;
     }

nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/CompileUnit.java

@@ -26,6 +26,7 @@
 package jdk.nashorn.internal.codegen;
 
 import java.io.Serializable;
+import java.util.Objects;
 import java.util.Set;
 import java.util.TreeSet;
 import jdk.nashorn.internal.ir.CompileUnitHolder;
@@ -113,7 +114,7 @@ public final class CompileUnit implements Comparable<CompileUnit>, Serializable
      * @param clazz class with code for this compile unit
      */
     void setCode(final Class<?> clazz) {
-        clazz.getClass(); // null check
+        Objects.requireNonNull(clazz);
         this.clazz = clazz;
         // Revisit this - refactor to avoid null-ed out non-final fields
         // null out emitter

nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/objects/Global.java

@@ -41,6 +41,7 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.concurrent.Callable;
 import java.util.concurrent.ConcurrentHashMap;
 import javax.script.ScriptContext;
@@ -463,8 +464,7 @@ public final class Global extends ScriptObject implements Scope {
             sm.checkPermission(new RuntimePermission(Context.NASHORN_CREATE_GLOBAL));
         }
 
-        // null check on context
-        context.getClass();
+        Objects.requireNonNull(context);
 
         return $nasgenmap$;
     }
@@ -488,7 +488,7 @@ public final class Global extends ScriptObject implements Scope {
      */
     public static Global instance() {
         final Global global = Context.getGlobal();
-        global.getClass(); // null check
+        Objects.requireNonNull(global);
         return global;
     }
 

nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/Context.java

@@ -60,6 +60,7 @@ import java.util.Collection;
 import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.Map;
+import java.util.Objects;
 import java.util.concurrent.atomic.AtomicLong;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.Consumer;
@@ -904,7 +905,7 @@ public final class Context {
      * @throw SecurityException if not accessible
      */
     private static void checkPackageAccess(final SecurityManager sm, final String fullName) {
-        sm.getClass(); // null check
+        Objects.requireNonNull(sm);
         final int index = fullName.lastIndexOf('.');
         if (index != -1) {
             final String pkgName = fullName.substring(0, index);

nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/ScriptLoader.java

@@ -26,6 +26,7 @@
 package jdk.nashorn.internal.runtime;
 
 import java.security.CodeSource;
+import java.util.Objects;
 
 /**
  * Responsible for loading script generated classes.
@@ -69,8 +70,7 @@ final class ScriptLoader extends NashornLoader {
      * @return Installed class.
      */
     synchronized Class<?> installClass(final String name, final byte[] data, final CodeSource cs) {
-        // null check
-        cs.getClass();
+        Objects.requireNonNull(cs);
         return defineClass(name, data, 0, data.length, cs);
     }
 }

nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/linker/JavaSuperAdapter.java

@@ -25,6 +25,8 @@
 
 package jdk.nashorn.internal.runtime.linker;
 
+import java.util.Objects;
+
 /**
  * Represents a an adapter for invoking superclass methods on an adapter instance generated by
  * {@code JavaAdapterBytecodeGenerator}. Note that objects of this class are just wrappers around the adapter instances,
@@ -34,7 +36,7 @@ class JavaSuperAdapter {
     private final Object adapter;
 
     JavaSuperAdapter(final Object adapter) {
-        adapter.getClass(); // NPE check
+        Objects.requireNonNull(adapter);
         this.adapter = adapter;
     }
 

nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/options/Options.java

@@ -42,6 +42,7 @@ import java.util.List;
 import java.util.Locale;
 import java.util.Map;
 import java.util.MissingResourceException;
+import java.util.Objects;
 import java.util.PropertyPermission;
 import java.util.ResourceBundle;
 import java.util.StringTokenizer;
@@ -143,7 +144,7 @@ public final class Options {
      * @return true if set to true, default value if unset or set to false
      */
     public static boolean getBooleanProperty(final String name, final Boolean defValue) {
-        name.getClass(); // null check
+        Objects.requireNonNull(name);
         if (!name.startsWith("nashorn.")) {
             throw new IllegalArgumentException(name);
         }
@@ -184,7 +185,7 @@ public final class Options {
      * @return string property if set or default value
      */
     public static String getStringProperty(final String name, final String defValue) {
-        name.getClass(); // null check
+        Objects.requireNonNull(name);
         if (! name.startsWith("nashorn.")) {
             throw new IllegalArgumentException(name);
         }
@@ -211,7 +212,7 @@ public final class Options {
      * @return integer property if set or default value
      */
     public static int getIntProperty(final String name, final int defValue) {
-        name.getClass(); // null check
+        Objects.requireNonNull(name);
         if (! name.startsWith("nashorn.")) {
             throw new IllegalArgumentException(name);
         }

nashorn/test/src/jdk/nashorn/api/scripting/ScriptEngineTest.java

@@ -721,6 +721,15 @@ public class ScriptEngineTest {
         assertTrue(invoked.get());
     }
 
+    @Test
+    public void testLengthOnArrayLikeObjects() throws Exception {
+        final ScriptEngine e = new ScriptEngineManager().getEngineByName("nashorn");
+        final Object val = e.eval("var arr = { length: 1, 0: 1}; arr.length");
+
+        assertTrue(Number.class.isAssignableFrom(val.getClass()));
+        assertTrue(((Number)val).intValue() == 1);
+    }
+
     // @bug JDK-8068603: NashornScriptEngine.put/get() impls don't conform to NPE, IAE spec assertions
     @Test
     public void illegalBindingsValuesTest() throws Exception {