8027128: jdk.nashorn.api.scripting.JSObject should be an interface

Reviewed-by: hannesw, attila, jlaskey

src/jdk/nashorn/api/scripting/AbstractJSObject.java

+/*
+ * Copyright (c) 2010, 2013, 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.api.scripting;
+
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Set;
+
+/**
+ * This is the base class for nashorn ScriptObjectMirror class.
+ *
+ * This class can also be subclassed by an arbitrary Java class. Nashorn will
+ * treat objects of such classes just like nashorn script objects. Usual nashorn
+ * operations like obj[i], obj.foo, obj.func(), delete obj.foo will be glued
+ * to appropriate method call of this class.
+ */
+public abstract class AbstractJSObject implements JSObject {
+    /**
+     * Call this object as a JavaScript function. This is equivalent to
+     * 'func.apply(thiz, args)' in JavaScript.
+     *
+     * @param thiz 'this' object to be passed to the function
+     * @param args arguments to method
+     * @return result of call
+     */
+    @Override
+    public Object call(final Object thiz, final Object... args) {
+        throw new UnsupportedOperationException("call");
+    }
+
+    /**
+     * Call this 'constructor' JavaScript function to create a new object.
+     * This is equivalent to 'new func(arg1, arg2...)' in JavaScript.
+     *
+     * @param args arguments to method
+     * @return result of constructor call
+     */
+    @Override
+    public Object newObject(final Object... args) {
+        throw new UnsupportedOperationException("newObject");
+    }
+
+    /**
+     * Evaluate a JavaScript expression.
+     *
+     * @param s JavaScript expression to evaluate
+     * @return evaluation result
+     */
+    @Override
+    public Object eval(final String s) {
+        throw new UnsupportedOperationException("eval");
+    }
+
+    /**
+     * Retrieves a named member of this JavaScript object.
+     *
+     * @param name of member
+     * @return member
+     */
+    @Override
+    public Object getMember(final String name) {
+        return null;
+    }
+
+    /**
+     * Retrieves an indexed member of this JavaScript object.
+     *
+     * @param index index slot to retrieve
+     * @return member
+     */
+    @Override
+    public Object getSlot(final int index) {
+        return null;
+    }
+
+    /**
+     * Does this object have a named member?
+     *
+     * @param name name of member
+     * @return true if this object has a member of the given name
+     */
+    @Override
+    public boolean hasMember(final String name) {
+        return false;
+    }
+
+    /**
+     * Does this object have a indexed property?
+     *
+     * @param slot index to check
+     * @return true if this object has a slot
+     */
+    @Override
+    public boolean hasSlot(final int slot) {
+        return false;
+    }
+
+    /**
+     * Remove a named member from this JavaScript object
+     *
+     * @param name name of the member
+     */
+    @Override
+    public void removeMember(final String name) {
+        //empty
+    }
+
+    /**
+     * Set a named member in this JavaScript object
+     *
+     * @param name  name of the member
+     * @param value value of the member
+     */
+    @Override
+    public void setMember(final String name, final Object value) {
+        //empty
+    }
+
+    /**
+     * Set an indexed member in this JavaScript object
+     *
+     * @param index index of the member slot
+     * @param value value of the member
+     */
+    @Override
+    public void setSlot(final int index, final Object value) {
+        //empty
+    }
+
+    // property and value iteration
+
+    /**
+     * Returns the set of all property names of this object.
+     *
+     * @return set of property names
+     */
+    @Override
+    @SuppressWarnings("unchecked")
+    public Set<String> keySet() {
+        return Collections.EMPTY_SET;
+    }
+
+    /**
+     * Returns the set of all property values of this object.
+     *
+     * @return set of property values.
+     */
+    @Override
+    @SuppressWarnings("unchecked")
+    public Collection<Object> values() {
+        return Collections.EMPTY_SET;
+    }
+
+    // JavaScript instanceof check
+
+    /**
+     * Checking whether the given object is an instance of 'this' object.
+     *
+     * @param instance instace to check
+     * @return true if the given 'instance' is an instance of this 'function' object
+     */
+    @Override
+    public boolean isInstance(final Object instance) {
+        return false;
+    }
+
+    /**
+     * Checking whether this object is an instance of the given 'clazz' object.
+     *
+     * @param clazz clazz to check
+     * @return true if this object is an instance of the given 'clazz'
+     */
+    @Override
+    public boolean isInstanceOf(final Object clazz) {
+        if (clazz instanceof JSObject) {
+            return ((JSObject)clazz).isInstance(this);
+        }
+
+        return false;
+    }
+
+    /**
+     * ECMA [[Class]] property
+     *
+     * @return ECMA [[Class]] property value of this object
+     */
+    @Override
+    public String getClassName() {
+        return getClass().getName();
+    }
+
+    /**
+     * Is this a function object?
+     *
+     * @return if this mirror wraps a ECMAScript function instance
+     */
+    @Override
+    public boolean isFunction() {
+        return false;
+    }
+
+    /**
+     * Is this a 'use strict' function object?
+     *
+     * @return true if this mirror represents a ECMAScript 'use strict' function
+     */
+    @Override
+    public boolean isStrictFunction() {
+        return false;
+    }
+
+    /**
+     * Is this an array object?
+     *
+     * @return if this mirror wraps a ECMAScript array object
+     */
+    @Override
+    public boolean isArray() {
+        return false;
+    }
+
+    /**
+     * Returns this object's numeric value.
+     *
+     * @return this object's numeric value.
+     */
+    @Override
+    public double toNumber() {
+        return Double.NaN;
+    }
+}

src/jdk/nashorn/api/scripting/JSObject.java

@@ -30,14 +30,12 @@ import java.util.Collections;
 import java.util.Set;
 
 /**
- * This is the base class for nashorn ScriptObjectMirror class.
- *
- * This class can also be subclassed by an arbitrary Java class. Nashorn will
+ * This interface can be implemented by an arbitrary Java class. Nashorn will
  * treat objects of such classes just like nashorn script objects. Usual nashorn
  * operations like obj[i], obj.foo, obj.func(), delete obj.foo will be glued
- * to appropriate method call of this class.
+ * to appropriate method call of this interface.
  */
-public abstract class JSObject {
+public interface JSObject {
     /**
      * Call this object as a JavaScript function. This is equivalent to
      * 'func.apply(thiz, args)' in JavaScript.
@@ -46,9 +44,7 @@ public abstract class JSObject {
      * @param args arguments to method
      * @return result of call
      */
-    public Object call(final Object thiz, final Object... args) {
-        throw new UnsupportedOperationException("call");
-    }
+    public Object call(final Object thiz, final Object... args);
 
     /**
      * Call this 'constructor' JavaScript function to create a new object.
@@ -57,9 +53,7 @@ public abstract class JSObject {
      * @param args arguments to method
      * @return result of constructor call
      */
-    public Object newObject(final Object... args) {
-        throw new UnsupportedOperationException("newObject");
-    }
+    public Object newObject(final Object... args);
 
     /**
      * Evaluate a JavaScript expression.
@@ -67,20 +61,7 @@ public abstract class JSObject {
      * @param s JavaScript expression to evaluate
      * @return evaluation result
      */
-    public Object eval(final String s) {
-        throw new UnsupportedOperationException("eval");
-    }
-
-    /**
-     * Call a JavaScript function member of this object.
-     *
-     * @param name name of the member function to call
-     * @param args arguments to be passed to the member function
-     * @return result of call
-     */
-    public Object callMember(final String name, final Object... args) {
-        throw new UnsupportedOperationException("call");
-    }
+    public Object eval(final String s);
 
     /**
      * Retrieves a named member of this JavaScript object.
@@ -88,9 +69,7 @@ public abstract class JSObject {
      * @param name of member
      * @return member
      */
-    public Object getMember(final String name) {
-        return null;
-    }
+    public Object getMember(final String name);
 
     /**
      * Retrieves an indexed member of this JavaScript object.
@@ -98,9 +77,7 @@ public abstract class JSObject {
      * @param index index slot to retrieve
      * @return member
      */
-    public Object getSlot(final int index) {
-        return null;
-    }
+    public Object getSlot(final int index);
 
     /**
      * Does this object have a named member?
@@ -108,9 +85,7 @@ public abstract class JSObject {
      * @param name name of member
      * @return true if this object has a member of the given name
      */
-    public boolean hasMember(final String name) {
-        return false;
-    }
+    public boolean hasMember(final String name);
 
     /**
      * Does this object have a indexed property?
@@ -118,18 +93,14 @@ public abstract class JSObject {
      * @param slot index to check
      * @return true if this object has a slot
      */
-    public boolean hasSlot(final int slot) {
-        return false;
-    }
+    public boolean hasSlot(final int slot);
 
     /**
      * Remove a named member from this JavaScript object
      *
      * @param name name of the member
      */
-    public void removeMember(final String name) {
-        //empty
-    }
+    public void removeMember(final String name);
 
     /**
      * Set a named member in this JavaScript object
@@ -137,9 +108,7 @@ public abstract class JSObject {
      * @param name  name of the member
      * @param value value of the member
      */
-    public void setMember(final String name, final Object value) {
-        //empty
-    }
+    public void setMember(final String name, final Object value);
 
     /**
      * Set an indexed member in this JavaScript object
@@ -147,9 +116,7 @@ public abstract class JSObject {
      * @param index index of the member slot
      * @param value value of the member
      */
-    public void setSlot(final int index, final Object value) {
-        //empty
-    }
+    public void setSlot(final int index, final Object value);
 
     // property and value iteration
 
@@ -158,20 +125,14 @@ public abstract class JSObject {
      *
      * @return set of property names
      */
-    @SuppressWarnings("unchecked")
-    public Set<String> keySet() {
-        return Collections.EMPTY_SET;
-    }
+    public Set<String> keySet();
 
     /**
      * Returns the set of all property values of this object.
      *
      * @return set of property values.
      */
-    @SuppressWarnings("unchecked")
-    public Collection<Object> values() {
-        return Collections.EMPTY_SET;
-    }
+    public Collection<Object> values();
 
     // JavaScript instanceof check
 
@@ -181,9 +142,7 @@ public abstract class JSObject {
      * @param instance instace to check
      * @return true if the given 'instance' is an instance of this 'function' object
      */
-    public boolean isInstance(final Object instance) {
-        return false;
-    }
+    public boolean isInstance(final Object instance);
 
     /**
      * Checking whether this object is an instance of the given 'clazz' object.
@@ -191,56 +150,40 @@ public abstract class JSObject {
      * @param clazz clazz to check
      * @return true if this object is an instance of the given 'clazz'
      */
-    public boolean isInstanceOf(final Object clazz) {
-        if (clazz instanceof JSObject) {
-            return ((JSObject)clazz).isInstance(this);
-        }
-
-        return false;
-    }
+    public boolean isInstanceOf(final Object clazz);
 
     /**
      * ECMA [[Class]] property
      *
      * @return ECMA [[Class]] property value of this object
      */
-    public String getClassName() {
-        return getClass().getName();
-    }
+    public String getClassName();
 
     /**
      * Is this a function object?
      *
      * @return if this mirror wraps a ECMAScript function instance
      */
-    public boolean isFunction() {
-        return false;
-    }
+    public boolean isFunction();
 
     /**
      * Is this a 'use strict' function object?
      *
      * @return true if this mirror represents a ECMAScript 'use strict' function
      */
-    public boolean isStrictFunction() {
-        return false;
-    }
+    public boolean isStrictFunction();
 
     /**
      * Is this an array object?
      *
      * @return if this mirror wraps a ECMAScript array object
      */
-    public boolean isArray() {
-        return false;
-    }
+    public boolean isArray();
 
     /**
      * Returns this object's numeric value.
      *
      * @return this object's numeric value.
      */
-    public double toNumber() {
-        return Double.NaN;
-    }
+    public double toNumber();
 }

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

@@ -51,7 +51,7 @@ import jdk.nashorn.internal.runtime.ScriptRuntime;
 /**
  * Mirror object that wraps a given Nashorn Script object.
  */
-public final class ScriptObjectMirror extends JSObject implements Bindings {
+public final class ScriptObjectMirror extends AbstractJSObject implements Bindings {
     private static AccessControlContext getContextAccCtxt() {
         final Permissions perms = new Permissions();
         perms.add(new RuntimePermission(Context.NASHORN_GET_CONTEXT));
@@ -162,7 +162,6 @@ public final class ScriptObjectMirror extends JSObject implements Bindings {
         });
     }
 
-    @Override
     public Object callMember(final String functionName, final Object... args) {
         functionName.getClass(); // null check
         final ScriptObject oldGlobal = Context.getGlobal();

src/jdk/nashorn/internal/runtime/JSType.java

@@ -1043,6 +1043,10 @@ public enum JSType {
             return toNumber((ScriptObject)obj);
         }
 
+        if (obj instanceof JSObject) {
+            return ((JSObject)obj).toNumber();
+        }
+
         return Double.NaN;
     }
 

src/jdk/nashorn/internal/runtime/linker/JSObjectLinker.java

@@ -107,8 +107,6 @@ final class JSObjectLinker implements TypeBasedGuardingDynamicLinker, GuardingTy
                 return c > 2 ? findSetMethod(desc) : findSetIndexMethod();
             case "call":
                 return findCallMethod(desc);
-            case "callMethod":
-                return findCallMethodMethod(desc);
             case "new":
                 return findNewMethod(desc);
             default:
@@ -134,13 +132,6 @@ final class JSObjectLinker implements TypeBasedGuardingDynamicLinker, GuardingTy
         return new GuardedInvocation(JSOBJECTLINKER_PUT, null, IS_JSOBJECT_GUARD);
     }
 
-    private static GuardedInvocation findCallMethodMethod(final CallSiteDescriptor desc) {
-        final String methodName = desc.getNameToken(2);
-        MethodHandle func = MH.insertArguments(JSOBJECT_CALLMEMBER, 1, methodName);
-        func = MH.asCollector(func, Object[].class, desc.getMethodType().parameterCount() - 1);
-        return new GuardedInvocation(func, null, IS_JSOBJECT_GUARD);
-    }
-
     private static GuardedInvocation findCallMethod(final CallSiteDescriptor desc) {
         final MethodHandle func = MH.asCollector(JSOBJECT_CALL, Object[].class, desc.getMethodType().parameterCount() - 2);
         return new GuardedInvocation(func, null, IS_JSOBJECT_GUARD);
@@ -216,7 +207,6 @@ final class JSObjectLinker implements TypeBasedGuardingDynamicLinker, GuardingTy
     // method handles of JSObject class
     private static final MethodHandle JSOBJECT_GETMEMBER  = findJSObjectMH("getMember", Object.class, String.class);
     private static final MethodHandle JSOBJECT_SETMEMBER  = findJSObjectMH("setMember", Void.TYPE, String.class, Object.class);
-    private static final MethodHandle JSOBJECT_CALLMEMBER = findJSObjectMH("callMember", Object.class, String.class, Object[].class);
     private static final MethodHandle JSOBJECT_CALL       = findJSObjectMH("call", Object.class, Object.class, Object[].class);
     private static final MethodHandle JSOBJECT_NEW        = findJSObjectMH("newObject", Object.class, Object[].class);
 

test/script/basic/JDK-8024847.js

@@ -100,3 +100,9 @@ var obj = new JSObject() {
 var jlist = Java.to(obj, java.util.List);
 print(jlist instanceof java.util.List);
 print(jlist);
+
+var obj = new JSObject() {
+    toNumber: function() { return 42; }
+};
+
+print(32 + obj);

test/script/basic/JDK-8024847.js.EXPECTED

@@ -10,3 +10,4 @@ true
 [hello, world]
 true
 [nashorn, js]
+74

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

@@ -46,7 +46,7 @@ import org.testng.annotations.Test;
  * JSObject implementations.
  */
 public class PluggableJSObjectTest {
-    public static class MapWrapperObject extends JSObject {
+    public static class MapWrapperObject extends AbstractJSObject {
         private final HashMap<String, Object> map = new LinkedHashMap<>();
 
         public HashMap<String, Object> getMap() {
@@ -109,7 +109,7 @@ public class PluggableJSObjectTest {
         }
     }
 
-    public static class BufferObject extends JSObject {
+    public static class BufferObject extends AbstractJSObject {
         private final IntBuffer buf;
 
         public BufferObject(int size) {
@@ -170,7 +170,7 @@ public class PluggableJSObjectTest {
         }
     }
 
-    public static class Adder extends JSObject {
+    public static class Adder extends AbstractJSObject {
         @Override
         public Object call(Object thiz, Object... args) {
             double res = 0.0;
@@ -202,7 +202,7 @@ public class PluggableJSObjectTest {
         }
     }
 
-    public static class Factory extends JSObject {
+    public static class Factory extends AbstractJSObject {
         @Override
         public Object newObject(Object... args) {
             return new HashMap<Object, Object>();

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

@@ -129,7 +129,7 @@ public class ScriptObjectMirrorTest {
         final ScriptEngine e = m.getEngineByName("nashorn");
         try {
             e.eval("var obj = { '1': 'world', func: function() { return this.bar; }, bar: 'hello' }");
-            JSObject obj = (JSObject) e.get("obj");
+            ScriptObjectMirror obj = (ScriptObjectMirror) e.get("obj");
 
             // try basic get on existing properties
             if (!obj.getMember("bar").equals("hello")) {