NativeReflect: re-implement Reflect.set()

Author: duonglaiquang

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

@@ -387,41 +387,165 @@ private static Object preventExtensions(
         return target.preventExtensions();
     }
 
+    /*
+     * https://tc39.es/ecma262/#sec-reflect.set
+     * 1. If target is not an Object, throw a TypeError exception.
+     * 2. Let key be ? ToPropertyKey(propertyKey).
+     * 3. If receiver is not present, then
+     *        a. Set receiver to target.
+     * 4. Return ? target.[[Set]](key, V, receiver).
+     */
     private static Object set(Context cx, Scriptable scope, Scriptable thisObj, Object[] args) {
-        ScriptableObject target = checkTarget(args);
-        if (args.length < 2) {
-            return true;
+        final ScriptableObject target = checkTarget(args);
+        final Object propertyKey = args.length > 1 ? args[1] : Undefined.instance;
+        final Object value = args.length > 2 ? args[2] : Undefined.instance;
+        final Object receiver = args.length > 3 ? args[3] : target;
+
+        // If target is a proxy, delegate to the proxy handler
+        if (target instanceof NativeProxy) {
+            final NativeProxy proxy = (NativeProxy) target;
+            final Function trap = proxy.getTrap("set");
+            if (trap != null) {
+                final ScriptableObject proxyTarget = proxy.getTargetThrowIfRevoked();
+                final Object[] trapArgs = {proxyTarget, propertyKey, value, receiver};
+                final boolean booleanTrapResult = ScriptRuntime.toBoolean(proxy.callTrap(trap, trapArgs));
+                if (!booleanTrapResult) {
+                    return false;
+                }
+
+                // checks for non-configurable properties
+                // https://tc39.es/ecma262/#sec-proxy-object-internal-methods-and-internal-slots-set-p-v-receiver steps 10
+                final DescriptorInfo targetDesc = proxyTarget.getOwnPropertyDescriptor(cx, propertyKey);
+                if (targetDesc != null && targetDesc.isConfigurable(false)) {
+                    if (targetDesc.isDataDescriptor() && targetDesc.isWritable(false)) {
+                        if (!Objects.equals(value, targetDesc.value)) {
+                            throw ScriptRuntime.typeError(
+                                    "proxy can't successfully set a non-writable,"
+                                            + " non-configurable property '\"" + propertyKey + "\"'");
+                        }
+                    }
+                    if (targetDesc.isAccessorDescriptor()
+                            && (targetDesc.setter == null
+                                    || targetDesc.setter == Scriptable.NOT_FOUND
+                                    || Undefined.isUndefined(targetDesc.setter))) {
+                        throw ScriptRuntime.typeError(
+                                "proxy can't successfully set a non-writable,"
+                                        + " non-configurable property '\"" + propertyKey + "\"'");
+                    }
+                }
+                return true;
+            }
         }
 
-        ScriptableObject receiver =
-                args.length > 3 ? ScriptableObject.ensureScriptableObject(args[3]) : target;
-        if (receiver != target) {
-            DescriptorInfo descriptor = target.getOwnPropertyDescriptor(cx, args[1]);
-            if (descriptor != null) {
-                Object setter = descriptor.setter;
-                if (setter != null && setter != NOT_FOUND) {
-                    ((Function) setter).call(cx, scope, receiver, new Object[] {args[2]});
-                    return true;
+        return internalSet(cx, target, propertyKey, value, receiver);
+    }
+
+    /*
+     * https://tc39.es/ecma262/#sec-ordinary-object-internal-methods-and-internal-slots-set-p-v-receiver
+     * 1. Let ownDesc be ? O.[[GetOwnProperty]](P).
+     * 2. If ownDesc is undefined, then
+     *        a. Let parent be ? O.[[GetPrototypeOf]]().
+     *        b. If parent is not null, then
+     *               i. Return ? parent.[[Set]](P, V, Receiver).
+     *        c. Else,
+     *               i. Set ownDesc to the PropertyDescriptor
+     *                  { [[Value]]: undefined, [[Writable]]: true,
+     *                    [[Enumerable]]: true, [[Configurable]]: true }.
+     * 3. If IsDataDescriptor(ownDesc) is true, then
+     *        a. If ownDesc.[[Writable]] is false, return false.
+     *        b. If Receiver is not an Object, return false.
+     *        c. Let existingDescriptor be ? Receiver.[[GetOwnProperty]](P).
+     *        d. If existingDescriptor is not undefined, then
+     *               i. If IsAccessorDescriptor(existingDescriptor) is true, return false.
+     *               ii. If existingDescriptor.[[Writable]] is false, return false.
+     *               iii. Let valueDesc be the PropertyDescriptor { [[Value]]: V }.
+     *               iv. Return ? Receiver.[[DefineOwnProperty]](P, valueDesc).
+     *        e. Else,
+     *               i. Assert: Receiver does not currently have a property P.
+     *               ii. Return ? CreateDataProperty(Receiver, P, V).
+     * 4. Assert: IsAccessorDescriptor(ownDesc) is true.
+     * 5. Let setter be ownDesc.[[Set]].
+     * 6. If setter is undefined, return false.
+     * 7. Perform ? Call(setter, Receiver, « V »).
+     * 8. Return true.
+     */
+    private static boolean internalSet(Context cx, ScriptableObject target, Object propertyKey,
+            Object value, Object receiver) {
+        try {
+            DescriptorInfo ownDesc = target.getOwnPropertyDescriptor(cx, propertyKey);
+            if (ownDesc == null) {
+                final Scriptable parent = target.getPrototype();
+                if (parent != null) {
+                    return internalSet(cx, ScriptableObject.ensureScriptableObject(parent), propertyKey, value, receiver);
+                }
+                ownDesc = new DescriptorInfo(true, true, true, Undefined.instance);
+            }
+
+            if (ownDesc.isDataDescriptor()) {
+                if (ownDesc.isWritable(false)) {
+                    return false;
+                }
+                if (!ScriptRuntime.isObject(receiver)) {
+                    return false;
                 }
 
-                if (descriptor.isConfigurable(false)) {
+                final ScriptableObject receiverObj = ScriptableObject.ensureScriptableObject(receiver);
+                final DescriptorInfo existingDescriptor = receiverObj.getOwnPropertyDescriptor(cx, propertyKey);
+                if (existingDescriptor != null) {
+                    if (existingDescriptor.isAccessorDescriptor()) {
+                        return false;
+                    }
+                    if (existingDescriptor.isWritable(false)) {
+                        return false;
+                    }
+                } else if (!receiverObj.isExtensible()) {
                     return false;
                 }
+
+                // If receiver is a proxy, set property directly on the proxy's target
+                // to avoid recursion (reflect <-> proxy)
+                final ScriptableObject realReceiverObj = receiverObj instanceof NativeProxy
+                        ? ((NativeProxy) receiverObj).getTargetThrowIfRevoked()
+                        : receiverObj;
+
+                if (ScriptRuntime.isSymbol(propertyKey)) {
+                    realReceiverObj.put((Symbol) propertyKey, realReceiverObj, value);
+                } else {
+                    final StringIdOrIndex s = ScriptRuntime.toStringIdOrIndex(propertyKey);
+                    if (s.stringId == null) {
+                        realReceiverObj.put(s.index, realReceiverObj, value);
+                    } else {
+                        realReceiverObj.put(s.stringId, realReceiverObj, value);
+                    }
+                }
+
+                return true;
             }
-        }
 
-        if (ScriptRuntime.isSymbol(args[1])) {
-            receiver.put((Symbol) args[1], receiver, args[2]);
-        } else {
-            StringIdOrIndex s = ScriptRuntime.toStringIdOrIndex(args[1]);
-            if (s.stringId == null) {
-                receiver.put(s.index, receiver, args[2]);
-            } else {
-                receiver.put(s.stringId, receiver, args[2]);
+            if (ownDesc.isAccessorDescriptor()) {
+                final Object setter = ownDesc.setter;
+                if (setter == null
+                        || setter == Scriptable.NOT_FOUND
+                        || Undefined.isUndefined(setter)) {
+                    return false;
+                }
+                final Scriptable receiverForCall;
+                if (receiver == null || Undefined.isUndefined(receiver)) {
+                    receiverForCall = cx.isStrictMode()
+                            ? null
+                            : ScriptableObject.getTopLevelScope(target);
+                } else {
+                    receiverForCall = ScriptableObject.ensureScriptable(receiver);
+                }
+
+                ((Function) setter).call(cx, target, receiverForCall, new Object[] {value});
             }
-        }
 
-        return true;
+            return true;
+
+        } catch (EcmaError e) {
+            return false;
+        }
     }
 
     private static Object setPrototypeOf(

tests/src/test/java/org/mozilla/javascript/tests/es6/NativeReflectTest.java

@@ -496,6 +496,40 @@ public void getWithReceiver() {
         Utils.assertWithAllModes_ES6("42 hello 10", js);
     }
 
+    @Test
+    public void setWithReceiver() {
+        String js =
+                // accessor: receiver is used as 'this' in setter
+                "var target = {};\n"
+                        + "Object.defineProperty(target, 'x', {\n"
+                        + "  set: function(v) { this.result = v; },\n"
+                        + "  get: function() { return this.result; }\n"
+                        + "});\n"
+                        + "var receiver = {};\n"
+                        + "Reflect.set(target, 'x', 42, receiver);\n"
+                        + "var accessorResult = receiver.result + ' ' + (target.result === undefined);\n"
+                        // non-writable target: returns false
+                        + "var nonWritableTarget = {};\n"
+                        + "Object.defineProperty(nonWritableTarget, 'x',"
+                        + " { value: 1, writable: false, configurable: true });\n"
+                        + "var nonWritableResult = Reflect.set(nonWritableTarget, 'x', 2);\n"
+                        // receiver constraints: accessor, non-writable, non-extensible all return false
+                        + "var accessorReceiver = {};\n"
+                        + "Object.defineProperty(accessorReceiver, 'x',"
+                        + " { get: function() {}, set: function() {} });\n"
+                        + "var readonlyReceiver = {};\n"
+                        + "Object.defineProperty(readonlyReceiver, 'x',"
+                        + " { value: 99, writable: false });\n"
+                        + "var sealedReceiver = {}; Object.preventExtensions(sealedReceiver);\n"
+                        + "var receiverResults = Reflect.set({ x: 1 }, 'x', 2, accessorReceiver)"
+                        + " + ' ' + Reflect.set({ x: 1 }, 'x', 2, readonlyReceiver)"
+                        + " + ' ' + Reflect.set(Object.create({ x: 1 }), 'x', 2, sealedReceiver);\n"
+                        + "accessorResult"
+                        + " + ' ' + nonWritableResult"
+                        + " + ' ' + receiverResults";
+        Utils.assertWithAllModes_ES6("42 true false false false false", js);
+    }
+
     @Test
     public void getWithProxyTarget() {
         String js =
@@ -519,6 +553,33 @@ public void getWithProxyTarget() {
         Utils.assertWithAllModes_ES6("trapped:x threw", js);
     }
 
+    @Test
+    public void setWithProxyTarget() {
+        String js =
+                "var trapLog = '';\n"
+                        + "var target = {};\n"
+                        + "var proxy = new Proxy(target, {\n"
+                        + "  set: function(target, prop, value, receiver) {\n"
+                        + "    trapLog = prop + '=' + value;\n"
+                        + "    target[prop] = value;\n"
+                        + "    return true;\n"
+                        + "  }\n"
+                        + "});\n"
+                        + "var trapResult = Reflect.set(proxy, 'x', 42);\n"
+                        + "var frozenTarget = {};\n"
+                        + "Object.defineProperty(frozenTarget, 'x',"
+                        + " { value: 42, writable: false, configurable: false });\n"
+                        + "var nonConfigurableProxy = new Proxy(frozenTarget, {\n"
+                        + "  set: function(target, prop, value, receiver) { return true; }\n"
+                        + "});\n"
+                        + "var nonConfigurableResult;\n"
+                        + "try { Reflect.set(nonConfigurableProxy, 'x', 99); nonConfigurableResult = 'no error'; }"
+                        + " catch (e) { nonConfigurableResult = 'threw'; }\n"
+                        + "trapResult + ' ' + trapLog + ' ' + target.x"
+                        + " + ' ' + nonConfigurableResult";
+        Utils.assertWithAllModes_ES6("true x=42 42 threw", js);
+    }
+
     @Test
     public void proxyTrapForwardsViaReflect() {
         String js =
@@ -531,11 +592,18 @@ public void proxyTrapForwardsViaReflect() {
                         + "  get: function(target, key, receiver) {\n"
                         + "    accessLog.push('get:' + key);\n"
                         + "    return Reflect.get(target, key, receiver);\n"
+                        + "  },\n"
+                        + "  set: function(target, key, value, receiver) {\n"
+                        + "    accessLog.push('set:' + key);\n"
+                        + "    return Reflect.set(target, key, value, receiver);\n"
                         + "  }\n"
                         + "});\n"
                         + "var getResult = proxy.x;\n"
                         + "var accessorResult = Reflect.get(proxy, 'context', { id: 'custom' });\n"
-                        + "getResult + ' ' + accessorResult + ' | ' + accessLog";
-        Utils.assertWithAllModes_ES6("hello context-custom | get:x,get:context", js);
+                        + "proxy.y = 99;\n"
+                        + "var setResult = target.y;\n"
+                        + "getResult + ' ' + accessorResult + ' ' + setResult"
+                        + " + ' | ' + accessLog";
+        Utils.assertWithAllModes_ES6("hello context-custom 99 | get:x,get:context,set:y", js);
     }
 }