8148379: jdk.nashorn.api.scripting spec. adjustments, clarifications

Reviewed-by: hannesw, mhaupt

nashorn/make/build.xml

@@ -242,6 +242,7 @@
     <javadoc destdir="${dist.javadoc.dir}" use="yes" overview="${nashorn.module.src.dir}/overview.html"
         extdirs="${nashorn.ext.path}" windowtitle="${nashorn.product.name} ${nashorn.version}"
         additionalparam="-quiet" failonerror="true" useexternalfile="true">
+      <arg value="${javadoc.option}"/>
       <classpath>
         <pathelement location="${build.classes.dir}"/>
       </classpath>
@@ -261,6 +262,7 @@
     <javadoc destdir="${dist.javadoc.dir}" use="yes" overview="${nashorn.module.src.dir}/overview.html"
         extdirs="${nashorn.ext.path}" windowtitle="${nashorn.product.name} ${nashorn.version}"
         additionalparam="-quiet" failonerror="true" useexternalfile="true">
+      <arg value="${javadoc.option}"/>
       <classpath>
         <pathelement location="${build.classes.dir}"/>
       </classpath>
@@ -276,6 +278,7 @@
     <javadoc destdir="${dist.javadoc.dir}" use="yes" overview="${nashorn.module.src.dir}/overview.html"
         extdirs="${nashorn.ext.path}" windowtitle="${nashorn.product.name} ${nashorn.version}"
         additionalparam="-quiet" failonerror="true" useexternalfile="true">
+      <arg value="${javadoc.option}"/>
       <classpath>
         <pathelement location="${build.classes.dir}"/>
       </classpath>
@@ -289,6 +292,7 @@
     <javadoc destdir="${dist.javadoc.dir}" use="yes"
         windowtitle="Dynalink"
         additionalparam="-quiet" failonerror="true" useexternalfile="true">
+      <arg value="${javadoc.option}"/>
       <classpath>
         <pathelement location="${build.classes.dir}"/>
       </classpath>

nashorn/make/project.properties

@@ -34,6 +34,8 @@ build.compiler=modern
 javac.source=1.8
 javac.target=1.8
 
+javadoc.option=-tag "implSpec:a:Implementation Requirements:"
+
 # nashorn version information
 nashorn.version=0.1
 nashorn.fullversion=0.1

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

@@ -27,6 +27,7 @@ package jdk.nashorn.api.scripting;
 
 import java.util.Collection;
 import java.util.Collections;
+import java.util.Objects;
 import java.util.Set;
 
 /**
@@ -34,19 +35,19 @@ import java.util.Set;
  *
  * 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
+ * operations like obj[i], obj.foo, obj.func(), delete obj.foo will be delegated
  * to appropriate method call of this class.
  *
  * @since 1.8u40
  */
 public abstract class AbstractJSObject implements JSObject {
     /**
-     * Call this object as a JavaScript function. This is equivalent to
+     * The default constructor.
-     * 'func.apply(thiz, args)' in JavaScript.
+     */
-     *
+    public AbstractJSObject() {}
-     * @param thiz 'this' object to be passed to the function
+
-     * @param args arguments to method
+    /**
-     * @return result of call
+     * @implSpec This implementation always throws UnsupportedOperationException
      */
     @Override
     public Object call(final Object thiz, final Object... args) {
@@ -54,11 +55,7 @@ public abstract class AbstractJSObject implements JSObject {
     }
 
     /**
-     * Call this 'constructor' JavaScript function to create a new object.
+     * @implSpec This implementation always throws UnsupportedOperationException
-     * 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) {
@@ -66,10 +63,7 @@ public abstract class AbstractJSObject implements JSObject {
     }
 
     /**
-     * Evaluate a JavaScript expression.
+     * @implSpec This imlementation always throws UnsupportedOperationException
-     *
-     * @param s JavaScript expression to evaluate
-     * @return evaluation result
      */
     @Override
     public Object eval(final String s) {
@@ -77,21 +71,16 @@ public abstract class AbstractJSObject implements JSObject {
     }
 
     /**
-     * Retrieves a named member of this JavaScript object.
+     * @implSpec This implementation always returns null
-     *
-     * @param name of member
-     * @return member
      */
     @Override
     public Object getMember(final String name) {
+        Objects.requireNonNull(name);
         return null;
     }
 
     /**
-     * Retrieves an indexed member of this JavaScript object.
+     * @implSpec This implementation always returns null
-     *
-     * @param index index slot to retrieve
-     * @return member
      */
     @Override
     public Object getSlot(final int index) {
@@ -99,21 +88,16 @@ public abstract class AbstractJSObject implements JSObject {
     }
 
     /**
-     * Does this object have a named member?
+     * @implSpec This implementation always returns false
-     *
-     * @param name name of member
-     * @return true if this object has a member of the given name
      */
     @Override
     public boolean hasMember(final String name) {
+        Objects.requireNonNull(name);
         return false;
     }
 
     /**
-     * Does this object have a indexed property?
+     * @implSpec This implementation always returns false
-     *
-     * @param slot index to check
-     * @return true if this object has a slot
      */
     @Override
     public boolean hasSlot(final int slot) {
@@ -121,31 +105,25 @@ public abstract class AbstractJSObject implements JSObject {
     }
 
     /**
-     * Remove a named member from this JavaScript object
+     * @implSpec This implementation is a no-op
-     *
-     * @param name name of the member
      */
     @Override
     public void removeMember(final String name) {
+        Objects.requireNonNull(name);
         //empty
     }
 
     /**
-     * Set a named member in this JavaScript object
+     * @implSpec This implementation is a no-op
-     *
-     * @param name  name of the member
-     * @param value value of the member
      */
     @Override
     public void setMember(final String name, final Object value) {
+        Objects.requireNonNull(name);
         //empty
     }
 
     /**
-     * Set an indexed member in this JavaScript object
+     * @implSpec This implementation is a no-op
-     *
-     * @param index index of the member slot
-     * @param value value of the member
      */
     @Override
     public void setSlot(final int index, final Object value) {
@@ -155,9 +133,7 @@ public abstract class AbstractJSObject implements JSObject {
     // property and value iteration
 
     /**
-     * Returns the set of all property names of this object.
+     * @implSpec This implementation returns empty set
-     *
-     * @return set of property names
      */
     @Override
     public Set<String> keySet() {
@@ -165,9 +141,7 @@ public abstract class AbstractJSObject implements JSObject {
     }
 
     /**
-     * Returns the set of all property values of this object.
+     * @implSpec This implementation returns empty set
-     *
-     * @return set of property values.
      */
     @Override
     public Collection<Object> values() {
@@ -177,22 +151,13 @@ public abstract class AbstractJSObject implements JSObject {
     // JavaScript instanceof check
 
     /**
-     * Checking whether the given object is an instance of 'this' object.
+     * @implSpec This implementation always returns false
-     *
-     * @param instance instance 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) {
@@ -202,20 +167,13 @@ public abstract class AbstractJSObject implements JSObject {
         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?
+     * @implSpec This implementation always returns false
-     *
-     * @return if this mirror wraps a ECMAScript function instance
      */
     @Override
     public boolean isFunction() {
@@ -223,9 +181,7 @@ public abstract class AbstractJSObject implements JSObject {
     }
 
     /**
-     * Is this a 'use strict' function object?
+     * @implSpec This implementation always returns false
-     *
-     * @return true if this mirror represents a ECMAScript 'use strict' function
      */
     @Override
     public boolean isStrictFunction() {
@@ -233,9 +189,7 @@ public abstract class AbstractJSObject implements JSObject {
     }
 
     /**
-     * Is this an array object?
+     * @implSpec This implementation always returns false
-     *
-     * @return if this mirror wraps a ECMAScript array object
      */
     @Override
     public boolean isArray() {

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

@@ -32,7 +32,7 @@ import jdk.nashorn.internal.runtime.JSType;
 /**
  * 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
+ * operations like obj[i], obj.foo, obj.func(), delete obj.foo will be delegated
  * to appropriate method call of this interface.
  *
  * @since 1.8u40
@@ -42,7 +42,7 @@ public interface 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 thiz 'this' object to be passed to the function. This may be null.
      * @param args arguments to method
      * @return result of call
      */
@@ -70,6 +70,7 @@ public interface JSObject {
      *
      * @param name of member
      * @return member
+     * @throws NullPointerException if name is null
      */
     public Object getMember(final String name);
 
@@ -101,6 +102,7 @@ public interface JSObject {
      * Remove a named member from this JavaScript object
      *
      * @param name name of the member
+     * @throws NullPointerException if name is null
      */
     public void removeMember(final String name);
 
@@ -109,6 +111,7 @@ public interface JSObject {
      *
      * @param name  name of the member
      * @param value value of the member
+     * @throws NullPointerException if name is null
      */
     public void setMember(final String name, final Object value);
 

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

@@ -46,6 +46,8 @@ import jdk.nashorn.internal.runtime.ScriptObject;
  */
 @SuppressWarnings("serial")
 public abstract class NashornException extends RuntimeException {
+    private static final long serialVersionUID = 1L;
+
     // script file name
     private String fileName;
     // script line number
@@ -58,7 +60,7 @@ public abstract class NashornException extends RuntimeException {
     private Object ecmaError;
 
     /**
-     * Constructor
+     * Constructor to initialize error message, file name, line and column numbers.
      *
      * @param msg       exception message
      * @param fileName  file name
@@ -70,7 +72,7 @@ public abstract class NashornException extends RuntimeException {
     }
 
     /**
-     * Constructor
+     * Constructor to initialize error message, cause exception, file name, line and column numbers.
      *
      * @param msg       exception message
      * @param cause     exception cause
@@ -86,7 +88,7 @@ public abstract class NashornException extends RuntimeException {
     }
 
     /**
-     * Constructor
+     * Constructor to initialize error message and cause exception.
      *
      * @param msg       exception message
      * @param cause     exception cause

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

@@ -154,7 +154,7 @@ public final class NashornScriptEngineFactory implements ScriptEngineFactory {
     }
 
     /**
-     * Create a new Script engine initialized by given class loader.
+     * Create a new Script engine initialized with the given class loader.
      *
      * @param appLoader class loader to be used as script "app" class loader.
      * @return newly created script engine.
@@ -167,7 +167,7 @@ public final class NashornScriptEngineFactory implements ScriptEngineFactory {
     }
 
     /**
-     * Create a new Script engine initialized by given class filter.
+     * Create a new Script engine initialized with the given class filter.
      *
      * @param classFilter class filter to use.
      * @return newly created script engine.
@@ -181,7 +181,7 @@ public final class NashornScriptEngineFactory implements ScriptEngineFactory {
     }
 
     /**
-     * Create a new Script engine initialized by given arguments.
+     * Create a new Script engine initialized with the given arguments.
      *
      * @param args arguments array passed to script engine.
      * @return newly created script engine.
@@ -195,7 +195,7 @@ public final class NashornScriptEngineFactory implements ScriptEngineFactory {
     }
 
     /**
-     * Create a new Script engine initialized by given arguments.
+     * Create a new Script engine initialized with the given arguments and the given class loader.
      *
      * @param args arguments array passed to script engine.
      * @param appLoader class loader to be used as script "app" class loader.
@@ -210,7 +210,7 @@ public final class NashornScriptEngineFactory implements ScriptEngineFactory {
     }
 
     /**
-     * Create a new Script engine initialized by given arguments.
+     * Create a new Script engine initialized with the given arguments, class loader and class filter.
      *
      * @param args arguments array passed to script engine.
      * @param appLoader class loader to be used as script "app" class loader.

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

@@ -287,22 +287,21 @@ public final class ScriptObjectMirror extends AbstractJSObject implements Bindin
         });
     }
 
-
     @Override
-    public boolean isInstance(final Object obj) {
+    public boolean isInstance(final Object instance) {
-        if (! (obj instanceof ScriptObjectMirror)) {
+        if (! (instance instanceof ScriptObjectMirror)) {
             return false;
         }
 
-        final ScriptObjectMirror instance = (ScriptObjectMirror)obj;
+        final ScriptObjectMirror mirror = (ScriptObjectMirror)instance;
         // if not belongs to my global scope, return false
-        if (global != instance.global) {
+        if (global != mirror.global) {
             return false;
         }
 
         return inGlobal(new Callable<Boolean>() {
             @Override public Boolean call() {
-                return sobj.isInstance(instance.sobj);
+                return sobj.isInstance(mirror.sobj);
             }
         });
     }
@@ -653,10 +652,10 @@ public final class ScriptObjectMirror extends AbstractJSObject implements Bindin
     }
 
     /**
-     * Make a script object mirror on given object if needed. Also converts ConsString instances to Strings.
+     * Make a script object mirror on given object if needed.
      *
      * @param obj object to be wrapped/converted
-     * @param homeGlobal global to which this object belongs. Not used for ConsStrings.
+     * @param homeGlobal global to which this object belongs.
      * @return wrapped/converted object
      */
     public static Object wrap(final Object obj, final Object homeGlobal) {
@@ -664,13 +663,13 @@ public final class ScriptObjectMirror extends AbstractJSObject implements Bindin
     }
 
     /**
-     * Make a script object mirror on given object if needed. Also converts ConsString instances to Strings. The
+     * Make a script object mirror on given object if needed. The created wrapper will implement
-     * created wrapper will implement the Java {@code List} interface if {@code obj} is a JavaScript
+     * the Java {@code List} interface if {@code obj} is a JavaScript {@code Array} object;
-     * {@code Array} object; this is compatible with Java JSON libraries expectations. Arrays retrieved through its
+     * this is compatible with Java JSON libraries expectations. Arrays retrieved through its
      * properties (transitively) will also implement the list interface.
      *
      * @param obj object to be wrapped/converted
-     * @param homeGlobal global to which this object belongs. Not used for ConsStrings.
+     * @param homeGlobal global to which this object belongs.
      * @return wrapped/converted object
      */
     public static Object wrapAsJSONCompatible(final Object obj, final Object homeGlobal) {
@@ -678,10 +677,10 @@ public final class ScriptObjectMirror extends AbstractJSObject implements Bindin
     }
 
     /**
-     * Make a script object mirror on given object if needed. Also converts ConsString instances to Strings.
+     * Make a script object mirror on given object if needed.
      *
      * @param obj object to be wrapped/converted
-     * @param homeGlobal global to which this object belongs. Not used for ConsStrings.
+     * @param homeGlobal global to which this object belongs.
      * @param jsonCompatible if true, the created wrapper will implement the Java {@code List} interface if
      * {@code obj} is a JavaScript {@code Array} object. Arrays retrieved through its properties (transitively)
      * will also implement the list interface.

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

@@ -74,9 +74,15 @@ public final class ScriptUtils {
      * @param func the function to wrap
      * @param sync the object to synchronize on
      * @return a synchronizing wrapper function
+     * @throws IllegalArgumentException if func does not represent a script function
      */
-    public static Object makeSynchronizedFunction(final ScriptFunction func, final Object sync) {
+    public static Object makeSynchronizedFunction(final Object func, final Object sync) {
-        return func.createSynchronized(unwrap(sync));
+        final Object unwrapped = unwrap(func);
+        if (unwrapped instanceof ScriptFunction) {
+            return ((ScriptFunction)unwrapped).createSynchronized(unwrap(sync));
+        }
+
+        throw new IllegalArgumentException();
     }
 
     /**
@@ -84,9 +90,19 @@ public final class ScriptUtils {
      *
      * @param obj object to be wrapped
      * @return wrapped object
+     * @throws IllegalArgumentException if obj cannot be wrapped
      */
-    public static ScriptObjectMirror wrap(final ScriptObject obj) {
+    public static ScriptObjectMirror wrap(final Object obj) {
-        return (ScriptObjectMirror) ScriptObjectMirror.wrap(obj, Context.getGlobal());
+        if (obj instanceof ScriptObjectMirror) {
+            return (ScriptObjectMirror)obj;
+        }
+
+        if (obj instanceof ScriptObject) {
+            final ScriptObject sobj = (ScriptObject)obj;
+            return (ScriptObjectMirror) ScriptObjectMirror.wrap(sobj, Context.getGlobal());
+        }
+
+        throw new IllegalArgumentException();
     }
 
     /**
@@ -135,7 +151,8 @@ public final class ScriptUtils {
      * Convert the given object to the given type.
      *
      * @param obj object to be converted
-     * @param type destination type to convert to
+     * @param type destination type to convert to. type is either a Class
+     * or nashorn representation of a Java type returned by Java.type() call in script.
      * @return converted object
      */
     public static Object convert(final Object obj, final Object type) {

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

@@ -37,10 +37,12 @@ var obj = {
     // Sync called with one argument will synchronize on this-object of invocation
     inc: sync(function(d) {
         this.count += d;
+        Assert.assertTrue(java.lang.Thread.holdsLock(this));
     }),
     // Pass explicit object to synchronize on as second argument
     dec: sync(function(d) {
         this.count -= d;
+        Assert.assertTrue(java.lang.Thread.holdsLock(obj));
     }, obj)
 };