Refactor proxies to be callable objects only if their initial target is callable

* Refactor `NativeProxy` so only proxies of callables are callable.
* Fix checking of proxy functions being constructors.

Author: aardvark179

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

@@ -531,6 +531,10 @@ The abstract operation IsConstructor takes argument argument (an ECMAScript lang
         if (argument instanceof LambdaFunction) {
             return false;
         }
+        if (argument instanceof NativeProxy.NativeProxyFunction) {
+            var f = ((NativeProxy) argument).getTargetThrowIfRevoked();
+            return isConstructor(cx, f);
+        }
 
         return argument instanceof Constructable;
     }

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

@@ -16,7 +16,7 @@
  *
  * @author Ronald Brill
  */
-final class NativeProxy extends ScriptableObject implements Function {
+class NativeProxy extends ScriptableObject {
     private static final long serialVersionUID = 6676871870513494844L;
 
     private static final String PROXY_TAG = "Proxy";
@@ -102,41 +102,6 @@ public String getClassName() {
         return target.getClassName();
     }
 
-    /**
-     * see <a
-     * href="https://262.ecma-international.org/12.0/#sec-proxy-object-internal-methods-and-internal-slots-construct-argumentslist-newtarget">10.5.13
-     * [[Construct]] (argumentsList, newTarget)</a>
-     */
-    @Override
-    public Scriptable construct(Context cx, Scriptable scope, Object[] args) {
-        /*
-         * 1. Let handler be O.[[ProxyHandler]].
-         * 2. If handler is null, throw a TypeError exception.
-         * 3. Assert: Type(handler) is Object.
-         * 4. Let target be O.[[ProxyTarget]].
-         * 5. Assert: IsConstructor(target) is true.
-         * 6. Let trap be ? GetMethod(handler, "construct").
-         * 7. If trap is undefined, then
-         *     a. Return ? Construct(target, argumentsList, newTarget).
-         * 8. Let argArray be ! CreateArrayFromList(argumentsList).
-         * 9. Let newObj be ? Call(trap, handler, « target, argArray, newTarget »).
-         * 10. If Type(newObj) is not Object, throw a TypeError exception.
-         * 11. Return newObj.
-         */
-        ScriptableObject target = getTargetThrowIfRevoked();
-
-        Function trap = getTrap(TRAP_CONSTRUCT);
-        if (trap != null) {
-            Object result = callTrap(trap, new Object[] {target, args, this});
-            if (!(result instanceof Scriptable) || ScriptRuntime.isSymbol(result)) {
-                throw ScriptRuntime.typeError("Constructor trap has to return a scriptable.");
-            }
-            return (ScriptableObject) result;
-        }
-
-        return ((Constructable) target).construct(cx, scope, args);
-    }
-
     /**
      * <a
      * href="https://262.ecma-international.org/12.0/#sec-proxy-object-internal-methods-and-internal-slots-hasproperty-p">10.5.7
@@ -1259,47 +1224,6 @@ public void setPrototype(Scriptable prototype) {
         target.setPrototype(prototype);
     }
 
-    /**
-     * see <a
-     * href="https://262.ecma-international.org/12.0/#sec-proxy-object-internal-methods-and-internal-slots-call-thisargument-argumentslist">10.5.12
-     * [[Call]] (thisArgument, argumentsList)</a>
-     */
-    @Override
-    public Object call(Context cx, Scriptable scope, Scriptable thisObj, Object[] args) {
-        /*
-         * 1. Let handler be O.[[ProxyHandler]].
-         * 2. If handler is null, throw a TypeError exception.
-         * 3. Assert: Type(handler) is Object.
-         * 4. Let target be O.[[ProxyTarget]].
-         * 5. Let trap be ? GetMethod(handler, "apply").
-         * 6. If trap is undefined, then
-         *     a. Return ? Call(target, thisArgument, argumentsList).
-         * 7. Let argArray be ! CreateArrayFromList(argumentsList).
-         * 8. Return ? Call(trap, handler, « target, thisArgument, argArray »).
-         */
-        ScriptableObject target = getTargetThrowIfRevoked();
-
-        Scriptable argumentsList = cx.newArray(scope, args);
-
-        Function trap = getTrap(TRAP_APPLY);
-        if (trap != null) {
-            return callTrap(trap, new Object[] {target, thisObj, argumentsList});
-        }
-
-        return ScriptRuntime.applyOrCall(
-                true, cx, scope, target, new Object[] {thisObj, argumentsList});
-    }
-
-    @Override
-    public Scriptable getDeclarationScope() {
-        ScriptableObject target = getTargetThrowIfRevoked();
-        if (target instanceof Function) {
-            return ((Function) target).getDeclarationScope();
-        }
-        Kit.codeBug();
-        return null;
-    }
-
     private static NativeProxy constructor(Context cx, Scriptable scope, Object[] args) {
         if (args.length < 2) {
             throw ScriptRuntime.typeErrorById(
@@ -1311,7 +1235,13 @@ private static NativeProxy constructor(Context cx, Scriptable scope, Object[] ar
         ScriptableObject target = ensureScriptableObjectButNotSymbol(args[0]);
         ScriptableObject handler = ensureScriptableObjectButNotSymbol(args[1]);
 
-        NativeProxy proxy = new NativeProxy(target, handler);
+        NativeProxy proxy;
+        if (target instanceof Function) {
+            proxy = new NativeProxyFunction(target, handler);
+        } else {
+            proxy = new NativeProxy(target, handler);
+        }
+
         proxy.setPrototypeDirect(ScriptableObject.getClassPrototype(scope, PROXY_TAG));
         proxy.setParentScope(scope);
         return proxy;
@@ -1332,7 +1262,7 @@ private static Object revocable(
         return revocable;
     }
 
-    private Function getTrap(String trapName) {
+    protected final Function getTrap(String trapName) {
         Object handlerProp = ScriptableObject.getProperty(handlerObj, trapName);
         if (Scriptable.NOT_FOUND == handlerProp) {
             return null;
@@ -1347,7 +1277,7 @@ private Function getTrap(String trapName) {
         return (Function) handlerProp;
     }
 
-    private Object callTrap(Function trap, Object[] args) {
+    protected final Object callTrap(Function trap, Object[] args) {
         return trap.call(Context.getContext(), trap.getDeclarationScope(), handlerObj, args);
     }
 
@@ -1357,4 +1287,87 @@ ScriptableObject getTargetThrowIfRevoked() {
         }
         return targetObj;
     }
+
+    static class NativeProxyFunction extends NativeProxy implements Function {
+
+        NativeProxyFunction(ScriptableObject target, Scriptable handler) {
+            super(target, handler);
+        }
+
+        /**
+         * see <a href=
+         * "https://262.ecma-international.org/12.0/#sec-proxy-object-internal-methods-and-internal-slots-construct-argumentslist-newtarget">10.5.13
+         * [[Construct]] (argumentsList, newTarget)</a>
+         */
+        @Override
+        public Scriptable construct(Context cx, Scriptable scope, Object[] args) {
+            /*
+             * 1. Let handler be O.[[ProxyHandler]].
+             * 2. If handler is null, throw a TypeError exception.
+             * 3. Assert: Type(handler) is Object.
+             * 4. Let target be O.[[ProxyTarget]].
+             * 5. Assert: IsConstructor(target) is true.
+             * 6. Let trap be ? GetMethod(handler, "construct").
+             * 7. If trap is undefined, then
+             * a. Return ? Construct(target, argumentsList, newTarget).
+             * 8. Let argArray be ! CreateArrayFromList(argumentsList).
+             * 9. Let newObj be ? Call(trap, handler, « target, argArray, newTarget »).
+             * 10. If Type(newObj) is not Object, throw a TypeError exception.
+             * 11. Return newObj.
+             */
+            ScriptableObject target = getTargetThrowIfRevoked();
+
+            Function trap = getTrap(TRAP_CONSTRUCT);
+            if (trap != null) {
+                Object result = callTrap(trap, new Object[] {target, args, this});
+                if (!(result instanceof Scriptable) || ScriptRuntime.isSymbol(result)) {
+                    throw ScriptRuntime.typeError("Constructor trap has to return a scriptable.");
+                }
+                return (ScriptableObject) result;
+            }
+
+            return ((Constructable) target).construct(cx, scope, args);
+        }
+
+        /**
+         * see <a href=
+         * "https://262.ecma-international.org/12.0/#sec-proxy-object-internal-methods-and-internal-slots-call-thisargument-argumentslist">10.5.12
+         * [[Call]] (thisArgument, argumentsList)</a>
+         */
+        @Override
+        public Object call(Context cx, Scriptable scope, Scriptable thisObj, Object[] args) {
+            /*
+             * 1. Let handler be O.[[ProxyHandler]].
+             * 2. If handler is null, throw a TypeError exception.
+             * 3. Assert: Type(handler) is Object.
+             * 4. Let target be O.[[ProxyTarget]].
+             * 5. Let trap be ? GetMethod(handler, "apply").
+             * 6. If trap is undefined, then
+             * a. Return ? Call(target, thisArgument, argumentsList).
+             * 7. Let argArray be ! CreateArrayFromList(argumentsList).
+             * 8. Return ? Call(trap, handler, « target, thisArgument, argArray »).
+             */
+            ScriptableObject target = getTargetThrowIfRevoked();
+
+            Scriptable argumentsList = cx.newArray(scope, args);
+
+            Function trap = getTrap(TRAP_APPLY);
+            if (trap != null) {
+                return callTrap(trap, new Object[] {target, thisObj, argumentsList});
+            }
+
+            return ScriptRuntime.applyOrCall(
+                    true, cx, scope, target, new Object[] {thisObj, argumentsList});
+        }
+
+        @Override
+        public Scriptable getDeclarationScope() {
+            ScriptableObject target = getTargetThrowIfRevoked();
+            if (target instanceof Function) {
+                return ((Function) target).getDeclarationScope();
+            }
+            Kit.codeBug();
+            return null;
+        }
+    }
 }

tests/testsrc/test262.properties

@@ -1466,14 +1466,14 @@ built-ins/JSON 42/165 (25.45%)
     rawJSON 10/10 (100.0%)
     stringify/replacer-array-abrupt.js
     stringify/replacer-array-proxy.js
-    stringify/replacer-array-wrong-type.js
+    stringify/replacer-array-proxy-revoked.js
+    stringify/replacer-array-proxy-revoked-realm.js
     stringify/replacer-function-arguments.js
     stringify/replacer-function-object-deleted-property.js
     stringify/replacer-function-result.js
     stringify/value-array-abrupt.js
     stringify/value-array-proxy.js
     stringify/value-bigint-tojson-receiver.js
-    stringify/value-object-proxy.js
 
 built-ins/Map 35/204 (17.16%)
     prototype/getOrInsertComputed 19/19 (100.0%)
@@ -2014,7 +2014,7 @@ built-ins/Promise 383/639 (59.94%)
     resolve-thenable-deferred.js {unsupported: [async]}
     resolve-thenable-immed.js {unsupported: [async]}
 
-built-ins/Proxy 66/311 (21.22%)
+built-ins/Proxy 65/311 (20.9%)
     construct/arguments-realm.js
     construct/call-parameters.js
     construct/call-parameters-new-target.js
@@ -2078,7 +2078,6 @@ built-ins/Proxy 66/311 (21.22%)
     set/trap-is-null-receiver.js
     set/trap-is-null-target-is-proxy.js
     set/trap-is-undefined-target-is-proxy.js
-    create-target-is-not-a-constructor.js
     get-fn-realm.js
     get-fn-realm-recursive.js