Refactor the Object.defineProperties function

Fix assert and corner cases in Object.defineProperties function.

JerryScript-DCO-1.0-Signed-off-by: Kristof Kosztyo [email protected]

Author: kkristof

jerry-core/ecma/builtin-objects/ecma-builtin-object.cpp

@@ -412,67 +412,134 @@ ecma_builtin_object_object_define_properties (ecma_value_t this_arg __attr_unuse
 {
   ecma_completion_value_t ret_value = ecma_make_empty_completion_value ();
 
+  // 1.
   if (!ecma_is_value_object (arg1))
   {
     ret_value = ecma_make_throw_obj_completion_value (ecma_new_standard_error (ECMA_ERROR_TYPE));
-    return ret_value;
   }
+  else
+  {
+    ecma_object_t *obj_p = ecma_get_object_from_value (arg1);
 
-  ecma_object_t *obj_p = ecma_get_object_from_value (arg1);
-  ecma_object_t *props_p = ecma_get_object_from_value (arg2);
+    // 2.
+    ECMA_TRY_CATCH (props,
+                    ecma_op_to_object (arg2),
+                    ret_value);
 
-  ecma_property_t *property_p;
+    ecma_object_t *props_p = ecma_get_object_from_completion_value (props);
+    ecma_property_t *property_p;
 
-  for (property_p = ecma_get_property_list (props_p);
-       property_p != NULL;
-       property_p = ECMA_GET_POINTER (ecma_property_t, property_p->next_property_p))
-  {
-    ecma_string_t *property_name_p;
+    // First we need to know how many properties should be stored
+    uint32_t property_number = 0;
+    for (property_p = ecma_get_property_list (props_p);
+         property_p != NULL;
+         property_p = ECMA_GET_POINTER (ecma_property_t, property_p->next_property_p))
+    {
+      if ((property_p->type == ECMA_PROPERTY_NAMEDDATA || property_p->type == ECMA_PROPERTY_NAMEDACCESSOR)
+          && ecma_is_property_enumerable (property_p))
+      {
+        property_number++;
+      }
+    }
 
-    if (property_p->type == ECMA_PROPERTY_NAMEDDATA)
+    // 3.
+    MEM_DEFINE_LOCAL_ARRAY (property_names_p, property_number, ecma_string_t*);
+
+    uint32_t index = 0;
+    for (property_p = ecma_get_property_list (props_p);
+         property_p != NULL;
+         property_p = ECMA_GET_POINTER (ecma_property_t, property_p->next_property_p))
     {
-      property_name_p = ECMA_GET_NON_NULL_POINTER (ecma_string_t,
-                                                   property_p->u.named_data_property.name_p);
+      ecma_string_t *property_name_p;
+
+      if (property_p->type == ECMA_PROPERTY_NAMEDDATA)
+      {
+        property_name_p = ECMA_GET_NON_NULL_POINTER (ecma_string_t,
+                                                     property_p->u.named_data_property.name_p);
+      }
+      else if (property_p->type == ECMA_PROPERTY_NAMEDACCESSOR)
+      {
+        property_name_p = ECMA_GET_NON_NULL_POINTER (ecma_string_t,
+                                                     property_p->u.named_accessor_property.name_p);
+      }
+      else
+      {
+        continue;
+      }
+
+      if (ecma_is_property_enumerable (property_p))
+      {
+        property_names_p[index++] = ecma_copy_or_ref_ecma_string (property_name_p);
+      }
     }
-    else if (property_p->type == ECMA_PROPERTY_NAMEDACCESSOR)
+
+    // 4.
+    MEM_DEFINE_LOCAL_ARRAY (property_descriptors, property_number, ecma_property_descriptor_t);
+    uint32_t property_descriptor_number = 0;
+
+    for (index = 0;
+         index < property_number && ecma_is_completion_value_empty (ret_value);
+         index++)
     {
-      property_name_p = ECMA_GET_NON_NULL_POINTER (ecma_string_t,
-                                                   property_p->u.named_accessor_property.name_p);
+      // 5.a
+      ECMA_TRY_CATCH (desc_obj,
+                      ecma_op_object_get (props_p, property_names_p[index]),
+                      ret_value);
+
+      // 5.b
+      ECMA_TRY_CATCH (conv_result,
+                      ecma_op_to_property_descriptor (ecma_get_completion_value_value (desc_obj),
+                                                      &property_descriptors[index]),
+                      ret_value);
+
+      property_descriptor_number++;
+
+      ECMA_FINALIZE (conv_result);
+      ECMA_FINALIZE (desc_obj);
     }
-    else
+
+    // 6.
+    for (index = 0;
+         index < property_number && ecma_is_completion_value_empty (ret_value);
+         index++)
     {
-      continue;
+      ECMA_TRY_CATCH (define_own_prop_ret,
+                      ecma_op_object_define_own_property (obj_p,
+                                                          property_names_p[index],
+                                                          &property_descriptors[index],
+                                                          true),
+                      ret_value);
+
+      ECMA_FINALIZE (define_own_prop_ret);
     }
-    ecma_property_descriptor_t prop_desc;
 
-    ECMA_TRY_CATCH (descObj,
-                    ecma_op_general_object_get (props_p, property_name_p),
-                    ret_value);
+    // Clean up
+    for (index = 0;
+         index < property_descriptor_number;
+         index++)
+    {
+      ecma_free_property_descriptor (&property_descriptors[index]);
+    }
 
-    ECMA_TRY_CATCH (conv_result,
-                    ecma_op_to_property_descriptor (ecma_get_completion_value_value (descObj),
-                                                    &prop_desc),
-                    ret_value);
+    MEM_FINALIZE_LOCAL_ARRAY (property_descriptors);
 
-    ECMA_TRY_CATCH (define_own_prop_ret,
-                    ecma_op_object_define_own_property (obj_p,
-                                                        property_name_p,
-                                                        &prop_desc,
-                                                        true),
-                    ret_value);
+    for (index = 0;
+         index < property_number;
+         index++)
+    {
+      ecma_deref_ecma_string (property_names_p[index]);
+    }
 
-    ECMA_FINALIZE (define_own_prop_ret);
-    ecma_free_property_descriptor (&prop_desc);
-    ECMA_FINALIZE (conv_result);
-    ECMA_FINALIZE (descObj);
+    MEM_FINALIZE_LOCAL_ARRAY (property_names_p);
 
-    if (ecma_is_completion_value_throw (ret_value))
+    // 7.
+    if (ecma_is_completion_value_empty (ret_value))
     {
-      return ret_value;
+      ret_value = ecma_make_normal_completion_value (ecma_copy_value (arg1, true));
     }
-  }
 
-  ret_value = ecma_make_normal_completion_value (ecma_copy_value (arg1, true));
+    ECMA_FINALIZE (props);
+  }
 
   return ret_value;
 } /* ecma_builtin_object_object_define_properties */

tests/jerry/object_define_properties.js

@@ -26,9 +26,179 @@ Object.defineProperties(obj, {
   "Hello": {
     value: "world",
     writable: false
+  },
+  "inner_object": {
+    value : {
+      "a" : 1,
+      "b" : {
+        value: "foo"
+      }
+    }
   }
 });
 
 assert (obj.foo === true);
 assert (obj.bar === "baz");
 assert (obj.Hello === "world");
+assert (obj.inner_object.a === 1);
+assert (obj.inner_object.b.value === "foo");
+
+// These cases should throw TypeError
+try {
+  Object.defineProperties(obj, undefined);
+  assert (false);
+} catch (e) {
+  assert (e instanceof TypeError);
+}
+
+try {
+  Object.defineProperties(obj, null);
+  assert (false);
+} catch (e) {
+  assert (e instanceof TypeError);
+}
+
+try {
+  Object.defineProperties(undefined, {
+    "foo": {
+      value: true,
+      writable: true
+    }
+  });
+  assert (false);
+} catch (e) {
+  assert (e instanceof TypeError);
+}
+
+// Check for internal assert, see issue #131.
+try {
+  Object.defineProperties([], undefined);
+  assert (false);
+} catch (e) {
+  assert (e instanceof TypeError);
+}
+
+// If one of the properties is wrong than it shouldn't update the object.
+var obj2 = {
+  a: 5
+};
+try {
+  Object.defineProperties(obj2, {
+    "foo": {
+      value: true,
+      writable: true
+    },
+    "bar": {
+      value: 3,
+      set: 3
+    },
+    "Hello": {
+      value: "world",
+      writable: false
+    }
+  });
+  assert (false);
+} catch (e) {
+  assert (e instanceof TypeError);
+  assert (obj2.foo === undefined);
+  assert (obj2.set === undefined);
+  assert (obj2.Hello === undefined);
+  assert (obj2.a === 5);
+}
+
+// Define accessors
+var obj = {};
+Object.defineProperties(obj, {
+  "foo": {
+    value: 42,
+    writable: true,
+  },
+  "bar": {
+    get: function() { return this.foo },
+    set: function(v) { this.foo = v }
+  }
+});
+
+assert (obj.bar === 42);
+obj.bar = "baz";
+assert (obj.foo === "baz");
+
+// Define get method which throws error
+var obj = {};
+var props = {
+  prop1: {
+    value: 1,
+    writable: true,
+  },
+  get bar() {
+    throw TypeError;
+    return { value : 2, writable : true };
+  },
+  prop2: {
+    value: 3,
+    writable: true,
+  },
+  prop3: {
+    value: 4,
+    writable: true,
+  }
+};
+
+try {
+  Object.defineProperties(obj, props);
+  assert (false);
+} catch (e) {
+  assert (e instanceof TypeError);
+}
+
+// Define get method which deletes a property
+var obj = {};
+Object.defineProperties(obj, {
+  "foo": {
+    value: 42,
+    writable: true,
+  },
+  "a": {
+    value: "b",
+    configurable: true
+  },
+  "bar": {
+    get: function() {
+      delete this.a;
+      return this.foo;
+    },
+  }
+});
+
+assert (obj.a === "b");
+assert (obj.bar === 42);
+assert (obj.a === undefined);
+
+// This code should throw TypeError
+var obj = {};
+var props = {
+  prop1: {
+    value: 1,
+    writable: true,
+  },
+  get bar() {
+    delete props.prop1;
+    delete props.prop2;
+    return { value : 2, writable : true };
+  },
+  prop2: {
+    value: 3,
+    writable: true,
+  },
+  prop3: {
+    value: 4,
+    writable: true,
+  }
+};
+
+try {
+  Object.defineProperties(obj, props);
+  assert (false);
+} catch (e) {
+  assert (e instanceof TypeError);
+}