further improve 'arguments' support

* Use the existing ScriptRuntime.typeErrorThrower(cx) instead of having a separate ThrowTypeError in Arguments.
This also fixes the setup of the ThrowTypeError returned by ScriptRuntime.typeErrorThrower(cx).
* test cases as reference and for checking backward compatibility
All test passing with the code base before the first 'argument' changes (commit 3f4e1a7)
* migrate Arguments to lambda and fix two more cases
* arguments accessed from the function itself are readonly

Author: rbri

rhino/src/main/java/org/mozilla/javascript/Arguments.java

@@ -14,32 +14,25 @@
  * @see org.mozilla.javascript.NativeCall
  * @author Norris Boyd
  */
-final class Arguments extends IdScriptableObject {
+class Arguments extends ScriptableObject {
     private static final long serialVersionUID = 4275508002492040609L;
 
     private static final String CLASS_NAME = "Arguments";
 
     // Fields to hold caller, callee and length properties,
     // where NOT_FOUND value tags deleted properties.
-    // In addition if callerObj == NULL_VALUE, it tags null for scripts, as
-    // initial callerObj == null means access to caller arguments available
-    // only in JS <= 1.3 scripts
-    private Object callerObj;
+    // In addition, the 'caller' arguments is only available in JS <= 1.3 scripts
     private Object calleeObj;
     private Object lengthObj;
 
-    private int callerAttr = DONTENUM;
-    private int calleeAttr = DONTENUM;
-    private int lengthAttr = DONTENUM;
-
     private NativeCall activation;
 
     // Initially args holds activation.getOriginalArgs(), but any modification
     // of its elements triggers creation of a copy. If its element holds NOT_FOUND,
     // it indicates deleted index, in which case super class is queried.
     private Object[] args;
 
-    public Arguments(NativeCall activation) {
+    public Arguments(NativeCall activation, Context cx) {
         this.activation = activation;
 
         Scriptable parent = activation.getParentScope();
@@ -52,19 +45,13 @@ public Arguments(NativeCall activation) {
         JSFunction f = activation.function;
         calleeObj = f;
 
-        int version = f.getLanguageVersion();
-        if (version <= Context.VERSION_1_3 && version != Context.VERSION_DEFAULT) {
-            callerObj = null;
-        } else {
-            callerObj = NOT_FOUND;
-        }
-
         defineProperty(
                 SymbolKey.ITERATOR,
                 TopLevel.getBuiltinPrototype(
                                 ScriptableObject.getTopLevelScope(parent), TopLevel.Builtins.Array)
                         .get("values", parent),
                 ScriptableObject.DONTENUM);
+        defineProperty("length", lengthObj, ScriptableObject.DONTENUM);
 
         if (activation.isStrict) {
             // ECMAScript2015
@@ -75,14 +62,28 @@ public Arguments(NativeCall activation) {
             //   9. Perform DefinePropertyOrThrow(obj, "callee", PropertyDescriptor {[[Get]]:
             // %ThrowTypeError%,
             //      [[Set]]: %ThrowTypeError%, [[Enumerable]]: false, [[Configurable]]: false}).
-            setGetterOrSetter("caller", 0, new ThrowTypeError(parent, "caller"), true);
-            setGetterOrSetter("caller", 0, new ThrowTypeError(parent, "caller"), false);
-            setGetterOrSetter("callee", 0, new ThrowTypeError(parent, "callee"), true);
-            setGetterOrSetter("callee", 0, new ThrowTypeError(parent, "callee"), false);
-            setAttributes("caller", DONTENUM | PERMANENT);
-            setAttributes("callee", DONTENUM | PERMANENT);
-            callerObj = null;
+            BaseFunction typeErrorThrower = ScriptRuntime.typeErrorThrower(cx);
+            int version = cx.getLanguageVersion();
+            if (version <= Context.VERSION_1_8) {
+                setGetterOrSetter("caller", 0, typeErrorThrower, true);
+                setGetterOrSetter("caller", 0, typeErrorThrower, false);
+                setGetterOrSetter("callee", 0, typeErrorThrower, true);
+                setGetterOrSetter("callee", 0, typeErrorThrower, false);
+                setAttributes("caller", DONTENUM | PERMANENT);
+                setAttributes("callee", DONTENUM | PERMANENT);
+            } else {
+                setGetterOrSetter("callee", 0, typeErrorThrower, true);
+                setGetterOrSetter("callee", 0, typeErrorThrower, false);
+                setAttributes("callee", DONTENUM | PERMANENT);
+            }
             calleeObj = null;
+        } else {
+            defineProperty("callee", calleeObj, ScriptableObject.DONTENUM);
+
+            int version = cx.getLanguageVersion();
+            if (version <= Context.VERSION_1_3 && version != Context.VERSION_DEFAULT) {
+                defineProperty("caller", (Object) null, ScriptableObject.DONTENUM);
+            }
         }
     }
 
@@ -196,6 +197,11 @@ public void put(String name, Scriptable start, Object value) {
         super.put(name, start, value);
     }
 
+    @Override
+    public void put(Symbol key, Scriptable start, Object value) {
+        super.put(key, start, value);
+    }
+
     @Override
     public void delete(int index) {
         if (0 <= index && index < args.length) {
@@ -204,128 +210,6 @@ public void delete(int index) {
         super.delete(index);
     }
 
-    private static final int Id_callee = 1,
-            Id_length = 2,
-            Id_caller = 3,
-            MAX_INSTANCE_ID = Id_caller;
-
-    @Override
-    protected int getMaxInstanceId() {
-        return MAX_INSTANCE_ID;
-    }
-
-    @Override
-    protected int findInstanceIdInfo(String s) {
-        int id;
-        switch (s) {
-            case "callee":
-                id = Id_callee;
-                break;
-            case "length":
-                id = Id_length;
-                break;
-            case "caller":
-                id = Id_caller;
-                break;
-            default:
-                id = 0;
-                break;
-        }
-        Context cx = Context.getContext();
-        if (cx.isStrictMode()) {
-            if (id == Id_callee || id == Id_caller) {
-                return super.findInstanceIdInfo(s);
-            }
-        }
-
-        if (id == 0) return super.findInstanceIdInfo(s);
-
-        int attr;
-        switch (id) {
-            case Id_callee:
-                attr = calleeAttr;
-                break;
-            case Id_caller:
-                attr = callerAttr;
-                break;
-            case Id_length:
-                attr = lengthAttr;
-                break;
-            default:
-                throw new IllegalStateException();
-        }
-        return instanceIdInfo(attr, id);
-    }
-
-    @Override
-    protected String getInstanceIdName(int id) {
-        switch (id) {
-            case Id_callee:
-                return "callee";
-            case Id_length:
-                return "length";
-            case Id_caller:
-                return "caller";
-        }
-        return null;
-    }
-
-    @Override
-    protected Object getInstanceIdValue(int id) {
-        switch (id) {
-            case Id_callee:
-                return calleeObj;
-            case Id_length:
-                return lengthObj;
-            case Id_caller:
-                {
-                    Object value = callerObj;
-                    if (value == UniqueTag.NULL_VALUE) {
-                        value = null;
-                    } else if (value == null) {
-                        NativeCall caller = activation.parentActivationCall;
-                        if (caller != null) {
-                            value = caller.get("arguments", caller);
-                        }
-                    }
-                    return value;
-                }
-        }
-        return super.getInstanceIdValue(id);
-    }
-
-    @Override
-    protected void setInstanceIdValue(int id, Object value) {
-        switch (id) {
-            case Id_callee:
-                calleeObj = value;
-                return;
-            case Id_length:
-                lengthObj = value;
-                return;
-            case Id_caller:
-                callerObj = (value != null) ? value : UniqueTag.NULL_VALUE;
-                return;
-        }
-        super.setInstanceIdValue(id, value);
-    }
-
-    @Override
-    protected void setInstanceIdAttributes(int id, int attr) {
-        switch (id) {
-            case Id_callee:
-                calleeAttr = attr;
-                return;
-            case Id_length:
-                lengthAttr = attr;
-                return;
-            case Id_caller:
-                callerAttr = attr;
-                return;
-        }
-        super.setInstanceIdAttributes(id, attr);
-    }
-
     @Override
     Object[] getIds(CompoundOperationMap map, boolean getNonEnumerable, boolean getSymbols) {
         Object[] ids = super.getIds(map, getNonEnumerable, getSymbols);
@@ -428,28 +312,60 @@ protected boolean defineOwnProperty(
         return true;
     }
 
-    private static final class ThrowTypeError extends BaseFunction {
-        private static final long serialVersionUID = -744615873947395749L;
-        private String propertyName;
+    static final class ReadonlyArguments extends Arguments {
+        private boolean initialized;
 
-        ThrowTypeError(Scriptable scope, String propertyName) {
-            this.propertyName = propertyName;
-            setPrototype(ScriptableObject.getFunctionPrototype(scope));
+        public ReadonlyArguments(Arguments arguments, Context cx) {
+            super(arguments.activation, cx);
+            initialized = true;
+        }
 
-            setAttributes("length", DONTENUM | PERMANENT | READONLY);
-            setAttributes("name", DONTENUM | PERMANENT | READONLY);
+        @Override
+        public void put(int index, Scriptable start, Object value) {
+            if (initialized) {
+                return;
+            }
+            super.put(index, start, value);
+        }
 
-            setAttributes("arity", DONTENUM);
-            delete("arity");
-            setAttributes("arguments", DONTENUM);
-            delete("arguments");
+        @Override
+        public void put(String name, Scriptable start, Object value) {
+            if (initialized) {
+                return;
+            }
+            super.put(name, start, value);
+        }
+
+        @Override
+        public void put(Symbol key, Scriptable start, Object value) {
+            if (initialized) {
+                return;
+            }
+            super.put(key, start, value);
+        }
+
+        @Override
+        public void delete(int index) {
+            if (initialized) {
+                return;
+            }
+            super.delete(index);
+        }
 
-            preventExtensions();
+        @Override
+        public void delete(String name) {
+            if (initialized) {
+                return;
+            }
+            super.delete(name);
         }
 
         @Override
-        public Object call(Context cx, Scriptable scope, Scriptable thisObj, Object[] args) {
-            throw ScriptRuntime.typeErrorById("msg.arguments.not.access.strict", propertyName);
+        public void delete(Symbol key) {
+            if (initialized) {
+                return;
+            }
+            super.delete(key);
         }
     }
 }

rhino/src/main/java/org/mozilla/javascript/BaseFunction.java

@@ -713,7 +713,15 @@ private Object getArguments() {
         }
         Context cx = Context.getContext();
         NativeCall activation = ScriptRuntime.findFunctionActivation(cx, this);
-        return (activation == null) ? null : activation.get("arguments", activation);
+        // return (activation == null) ? null : activation.get("arguments", activation);
+        if (activation == null) {
+            return null;
+        }
+        Object arguments = activation.get("arguments", activation);
+        if (arguments instanceof Arguments && cx.getLanguageVersion() >= Context.VERSION_ES6) {
+            return new Arguments.ReadonlyArguments((Arguments) arguments, cx);
+        }
+        return arguments;
     }
 
     private static Scriptable jsConstructor(

rhino/src/main/java/org/mozilla/javascript/NativeCall.java

@@ -79,7 +79,7 @@ static void init(Scriptable scope, boolean sealed) {
             // initialize "arguments" property but only if it was not overridden by
             // the parameter with the same name
             if (!super.has("arguments", this) && !isArrow) {
-                arguments = new Arguments(this);
+                arguments = new Arguments(this, cx);
                 defineProperty("arguments", arguments, PERMANENT);
             }
         }

rhino/src/main/java/org/mozilla/javascript/ScriptRuntime.java

@@ -68,28 +68,43 @@ public static BaseFunction typeErrorThrower() {
     /** Returns representation of the [[ThrowTypeError]] object. See ECMA 5 spec, 13.2.3 */
     public static BaseFunction typeErrorThrower(Context cx) {
         if (cx.typeErrorThrower == null) {
-            BaseFunction thrower =
-                    new BaseFunction() {
-                        private static final long serialVersionUID = -5891740962154902286L;
-
-                        @Override
-                        public Object call(
-                                Context cx, Scriptable scope, Scriptable thisObj, Object[] args) {
-                            throw typeErrorById("msg.op.not.allowed");
-                        }
-
-                        @Override
-                        public int getLength() {
-                            return 0;
-                        }
-                    };
-            ScriptRuntime.setFunctionProtoAndParent(thrower, cx, cx.topCallScope, false);
-            thrower.preventExtensions();
+            BaseFunction thrower = new ThrowTypeError(cx.topCallScope);
             cx.typeErrorThrower = thrower;
         }
         return cx.typeErrorThrower;
     }
 
+    private static final class ThrowTypeError extends BaseFunction {
+        private static final long serialVersionUID = -5891740962154902286L;
+
+        ThrowTypeError(Scriptable scope) {
+            setPrototype(ScriptableObject.getFunctionPrototype(scope));
+
+            setAttributes("length", DONTENUM | PERMANENT | READONLY);
+            setAttributes("name", DONTENUM | PERMANENT | READONLY);
+
+            // delete arity and arguments (without further checking)
+            getMap().compute(this, "arity", 0, ThrowTypeError::removeWithoutChecking);
+            getMap().compute(this, "arguments", 0, ThrowTypeError::removeWithoutChecking);
+
+            preventExtensions();
+        }
+
+        private static Slot removeWithoutChecking(
+                Object key,
+                int index,
+                Slot slot,
+                CompoundOperationMap compoundOp,
+                SlotMapOwner owner) {
+            return null;
+        }
+
+        @Override
+        public Object call(Context cx, Scriptable scope, Scriptable thisObj, Object[] args) {
+            throw typeErrorById("msg.op.not.allowed");
+        }
+    }
+
     public static Object concat(Object lhs, Object rhs) {
         String rhsString = ScriptRuntime.toString(rhs);
         String lhsString = ScriptRuntime.toString(lhs);