move isConstructor() to AbstractEcmaObjectOperations and some improve…

…d comments
mozilla · Sep 22, 2024 · d7f9f4e · d7f9f4e
1 parent 94faac2
commit d7f9f4e
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 21 deletions.
diff --git a/rhino/src/main/java/org/mozilla/javascript/AbstractEcmaObjectOperations.java b/rhino/src/main/java/org/mozilla/javascript/AbstractEcmaObjectOperations.java
@@ -417,9 +417,7 @@ static boolean validateAndApplyPropertyDescriptor(
             return true;
         }
 
-        if (!Objects.equals(
-                ScriptableObject.isDataDescriptor(current),
-                ScriptableObject.isDataDescriptor(desc))) {
+        if (ScriptableObject.isDataDescriptor(current) != ScriptableObject.isDataDescriptor(desc)) {
             if (Boolean.FALSE.equals(current.get("configurable"))) {
                 return false;
             }
@@ -465,4 +463,33 @@ static boolean validateAndApplyPropertyDescriptor(
         }
         return true;
     }
+
+    /**
+     * IsConstructor ( argument )
+     *
+     * <p>https://262.ecma-international.org/12.0/#sec-isconstructor
+     */
+    static boolean isConstructor(Object argument) {
+        /*
+            The abstract operation IsConstructor takes argument argument (an ECMAScript language value).
+            It determines if argument is a function object with a [[Construct]] internal method.
+            It performs the following steps when called:
+
+            1. If Type(argument) is not Object, return false.
+            2. If argument has a [[Construct]] internal method, return true.
+            3. Return false.
+         */
+
+        // Found no good way to implement this based on the spec.
+        // Therefor I did this as first step - this only supports Lambda based method declarations.
+        // see #1376 for more
+        if (argument instanceof LambdaConstructor) {
+            return true;
+        }
+        if (argument instanceof LambdaFunction) {
+            return false;
+        }
+
+        return argument instanceof Constructable;
+    }
 }
diff --git a/rhino/src/main/java/org/mozilla/javascript/BaseFunction.java b/rhino/src/main/java/org/mozilla/javascript/BaseFunction.java
@@ -443,8 +443,8 @@ public Scriptable construct(Context cx, Scriptable scope, Object[] args) {
     }
 
     /**
-     * Creates new script object. The default implementation of {@link #construct} uses the method
-     * to to get the value for <code>thisObj</code> argument when invoking {@link #call}. The methos
+     * Creates new script object. The default implementation of {@link #construct} uses this method
+     * to to get the value for <code>thisObj</code> argument when invoking {@link #call}. The method
      * is allowed to return <code>null</code> to indicate that {@link #call} will create a new
      * object itself. In this case {@link #construct} will set scope and prototype on the result
      * {@link #call} unless they are already set.

diff --git a/rhino/src/main/java/org/mozilla/javascript/NativeReflect.java b/rhino/src/main/java/org/mozilla/javascript/NativeReflect.java
@@ -142,7 +142,7 @@ private static Scriptable construct(
                     Integer.toString(args.length));
         }
 
-        if (!isConstructor(args[0])) {
+        if (!AbstractEcmaObjectOperations.isConstructor(args[0])) {
             throw ScriptRuntime.typeErrorById("msg.not.ctor", ScriptRuntime.typeof(args[0]));
         }
 
@@ -151,7 +151,7 @@ private static Scriptable construct(
             return ctor.construct(cx, scope, ScriptRuntime.emptyArgs);
         }
 
-        if (args.length > 2 && !isConstructor(args[2])) {
+        if (args.length > 2 && !AbstractEcmaObjectOperations.isConstructor(args[2])) {
             throw ScriptRuntime.typeErrorById("msg.not.ctor", ScriptRuntime.typeof(args[2]));
         }
 
@@ -174,7 +174,9 @@ private static Scriptable construct(
             }
         }
 
-        // hack to set the right prototype before calling the ctor
+        // our Constructable interface does not support the newTarget;
+        // therefore we use a cloned implementation that fixes
+        // the prototype before executing call(..).
         if (ctor instanceof BaseFunction && newTargetPrototype != null) {
             BaseFunction ctorBaseFunction = (BaseFunction) ctor;
             Scriptable result = ctorBaseFunction.createObject(cx, scope);
@@ -198,19 +200,6 @@ private static Scriptable construct(
         return newScriptable;
     }
 
-    private static boolean isConstructor(final Object argument) {
-        // Hack for the moment because all Functions are Constructable
-        // see #1376 for more
-        if (argument instanceof LambdaConstructor) {
-            return true;
-        }
-        if (argument instanceof LambdaFunction) {
-            return false;
-        }
-
-        return argument instanceof Constructable;
-    }
-
     private static Object defineProperty(
             Context cx, Scriptable scope, Scriptable thisObj, Object[] args) {
         if (args.length < 3) {