Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Object.getOwnPropertyNames() #4

Closed
drsm opened this issue Apr 19, 2018 · 10 comments
Closed

Object.getOwnPropertyNames() #4

drsm opened this issue Apr 19, 2018 · 10 comments

Comments

@drsm
Copy link
Contributor

drsm commented Apr 19, 2018

No description provided.

@drsm
Copy link
Contributor Author

drsm commented Mar 25, 2019

Here is the patch for the first one (update 2):

# HG changeset patch
# User Artem S. Povalyukhin <artem.povaluhin@gmail.com>
# Date 1553615690 -10800
#      Tue Mar 26 18:54:50 2019 +0300
# Node ID 95c25197c73a41c3d9949221ea82c6797281d4b9
# Parent  5dae5875b22e1829cc7b4565aa8db41cb67b1aa8
Added Object.getOwnPropertyNames().

diff -r 5dae5875b22e -r 95c25197c73a njs/njs_object.c
--- a/njs/njs_object.c	Tue Mar 26 16:58:43 2019 +0300
+++ b/njs/njs_object.c	Tue Mar 26 18:54:50 2019 +0300
@@ -954,6 +954,7 @@
 njs_object_enumerate(njs_vm_t *vm, const njs_value_t *value,
     njs_object_enum_t kind)
 {
+    nxt_bool_t         enum_all, exotic_length;
     u_char             *dst;
     uint32_t           i, length, size, items_length, properties;
     njs_value_t        *string, *item;
@@ -964,11 +965,20 @@
     njs_string_prop_t  string_prop;
     nxt_lvlhsh_each_t  lhe;
 
+    static const njs_value_t  njs_string_length = njs_string("length");
+
+    enum_all = kind < NJS_ENUM_KEYS;
+    exotic_length = 0;
     array = NULL;
     length = 0;
     items_length = 0;
 
     switch (value->type) {
+    case NJS_FUNCTION:
+        exotic_length = enum_all && (value->data.u.function->native == 0);
+
+        break;
+
     case NJS_ARRAY:
         array = value->data.u.array;
         length = array->length;
@@ -978,6 +988,7 @@
                 items_length++;
             }
         }
+        exotic_length = enum_all;
 
         break;
 
@@ -992,6 +1003,8 @@
 
         length = njs_string_prop(&string_prop, string);
         items_length += length;
+        exotic_length = enum_all;
+
         break;
 
     default:
@@ -1013,15 +1026,32 @@
                 break;
             }
 
-            if (prop->type != NJS_WHITEOUT && prop->enumerable) {
+            if (prop->type != NJS_WHITEOUT && (prop->enumerable || enum_all)) {
                 properties++;
             }
         }
 
+        if (nxt_slow_path(enum_all)) {
+            nxt_lvlhsh_each_init(&lhe, &njs_object_hash_proto);
+            hash = &value->data.u.object->shared_hash;
+
+            for ( ;; ) {
+                prop = nxt_lvlhsh_each(hash, &lhe);
+
+                if (prop == NULL) {
+                    break;
+                }
+
+                properties++;
+            }
+
+            hash = &value->data.u.object->hash;
+        }
+
         items_length += properties;
     }
 
-    items = njs_array_alloc(vm, items_length, NJS_ARRAY_SPARE);
+    items = njs_array_alloc(vm, items_length + exotic_length, NJS_ARRAY_SPARE);
     if (nxt_slow_path(items == NULL)) {
         return NULL;
     }
@@ -1031,6 +1061,7 @@
     if (array != NULL) {
 
         switch (kind) {
+        case NJS_ENUM_PROP_NAMES:
         case NJS_ENUM_KEYS:
             for (i = 0; i < length; i++) {
                 if (njs_is_valid(&array->start[i])) {
@@ -1077,6 +1108,7 @@
     } else if (length != 0) {
 
         switch (kind) {
+        case NJS_ENUM_PROP_NAMES:
         case NJS_ENUM_KEYS:
             for (i = 0; i < length; i++) {
                 njs_uint32_to_string(item++, i);
@@ -1179,11 +1211,16 @@
         }
     }
 
+    if (nxt_slow_path(exotic_length != 0)) {
+        *item++ = njs_string_length;
+    }
+
     if (nxt_fast_path(properties != 0)) {
         nxt_lvlhsh_each_init(&lhe, &njs_object_hash_proto);
 
         switch (kind) {
 
+        case NJS_ENUM_PROP_NAMES:
         case NJS_ENUM_KEYS:
             for ( ;; ) {
                 prop = nxt_lvlhsh_each(hash, &lhe);
@@ -1192,7 +1229,22 @@
                     break;
                 }
 
-                if (prop->type != NJS_WHITEOUT && prop->enumerable) {
+                if (prop->type != NJS_WHITEOUT && (prop->enumerable || enum_all)) {
+                    njs_string_copy(item++, &prop->name);
+                }
+            }
+
+            if (nxt_slow_path(enum_all)) {
+                nxt_lvlhsh_each_init(&lhe, &njs_object_hash_proto);
+                hash = &value->data.u.object->shared_hash;
+
+                for ( ;; ) {
+                    prop = nxt_lvlhsh_each(hash, &lhe);
+
+                    if (prop == NULL) {
+                        break;
+                    }
+
                     njs_string_copy(item++, &prop->name);
                 }
             }
@@ -1721,6 +1773,35 @@
 
 
 static njs_ret_t
+njs_object_get_own_property_names(njs_vm_t *vm, njs_value_t *args, nxt_uint_t nargs,
+    njs_index_t unused)
+{
+    njs_array_t        *names;
+    const njs_value_t  *value;
+
+    value = njs_arg(args, nargs, 1);
+
+    if (njs_is_null_or_undefined(value)) {
+        njs_type_error(vm, "cannot convert %s argument to object",
+                       njs_type_string(value->type));
+
+        return NXT_ERROR;
+    }
+
+    names = njs_object_enumerate(vm, value, NJS_ENUM_PROP_NAMES);
+    if (names == NULL) {
+        return NXT_ERROR;
+    }
+
+    vm->retval.data.u.array = names;
+    vm->retval.type = NJS_ARRAY;
+    vm->retval.data.truth = 1;
+
+    return NXT_OK;
+}
+
+
+static njs_ret_t
 njs_object_get_prototype_of(njs_vm_t *vm, njs_value_t *args, nxt_uint_t nargs,
     njs_index_t unused)
 {
@@ -2152,6 +2233,14 @@
                                      NJS_STRING_ARG),
     },
 
+    /* Object.getOwnPropertyNames(). */
+    {
+        .type = NJS_METHOD,
+        .name = njs_long_string("getOwnPropertyNames"),
+        .value = njs_native_function(njs_object_get_own_property_names, 0,
+                                     NJS_SKIP_ARG, NJS_OBJECT_ARG),
+    },
+
     /* Object.getPrototypeOf(). */
     {
         .type = NJS_METHOD,
diff -r 5dae5875b22e -r 95c25197c73a njs/njs_object.h
--- a/njs/njs_object.h	Tue Mar 26 16:58:43 2019 +0300
+++ b/njs/njs_object.h	Tue Mar 26 18:54:50 2019 +0300
@@ -18,7 +18,8 @@
 
 
 typedef enum {
-    NJS_ENUM_KEYS = 0,
+    NJS_ENUM_PROP_NAMES = 0,
+    NJS_ENUM_KEYS,
     NJS_ENUM_VALUES,
     NJS_ENUM_BOTH,
 } njs_object_enum_t;
diff -r 5dae5875b22e -r 95c25197c73a njs/test/njs_unit_test.c
--- a/njs/test/njs_unit_test.c	Tue Mar 26 16:58:43 2019 +0300
+++ b/njs/test/njs_unit_test.c	Tue Mar 26 18:54:50 2019 +0300
@@ -8673,6 +8673,41 @@
     { nxt_string("var o = {}; o[void 0] = 'a'; Object.getOwnPropertyDescriptor(o, undefined).value"),
       nxt_string("a") },
 
+    { nxt_string("Object.getOwnPropertyNames()"),
+      nxt_string("TypeError: cannot convert undefined argument to object") },
+
+    { nxt_string("Array.isArray(Object.getOwnPropertyNames({}))"),
+      nxt_string("true") },
+
+    { nxt_string("Object.getOwnPropertyNames({a:1, b:1, c:1})"),
+      nxt_string("a,b,c") },
+
+    { nxt_string("Object.getOwnPropertyNames(Object.defineProperty({a:1}, 'b', {}))"),
+      nxt_string("a,b") },
+
+    { nxt_string("Object.getOwnPropertyNames(Object.defineProperty([], 'b', {}))"),
+      nxt_string("length,b") },
+
+    { nxt_string("Object.getOwnPropertyNames(Object.defineProperty(new String(), 'b', {}))"),
+      nxt_string("length,b") },
+
+    { nxt_string("Object.getOwnPropertyNames([1,2,3])"),
+      nxt_string("0,1,2,length") },
+
+    { nxt_string("Object.getOwnPropertyNames('abc')"),
+      nxt_string("0,1,2,length") },
+
+    { nxt_string("Object.getOwnPropertyNames(function() {})"),
+      nxt_string("length,prototype") },
+
+    { nxt_string("Object.getOwnPropertyNames(Array)"),
+      nxt_string("name,length,prototype,isArray,of") },
+
+#if 0
+    { nxt_string("Object.getOwnPropertyNames(Array.isArray)"),
+      nxt_string("length") },
+#endif
+
     { nxt_string("Object.defineProperty(Object.freeze({}), 'b', {})"),
       nxt_string("TypeError: object is not extensible") },
 

@drsm
Copy link
Contributor Author

drsm commented Mar 26, 2019

the patch updated to support functions. not sure it is correct.

@xeioex
Copy link
Contributor

xeioex commented Mar 26, 2019

@drsm

Looks good. I plan to commit the patch with minor improvements: https://gist.github.com/xeioex/2d200ac7594e9ae2c834dbce5420d00f

@xeioex
Copy link
Contributor

xeioex commented Mar 26, 2019

@drsm

Object.getOwnPropertyDescriptors()

BTW, I think we need a separate ticket for it.

@xeioex xeioex changed the title Object.getOwnPropertyNames() / Object.getOwnPropertyDescriptors() Object.getOwnPropertyNames() Mar 26, 2019
@drsm
Copy link
Contributor Author

drsm commented Mar 26, 2019

@xeioex

Looks good. I plan to commit the patch with minor improvements: https://gist.github.com/xeioex/2d200ac7594e9ae2c834dbce5420d00f

+ NJS_ENUM_OWN_KEYS = 0,
no ).
NJS_ENUM_KEYS|VALUES|BOTH - enum all enumerable own properties (it doesn't scan the prototype chain)
NJS_ENUM_PROP_NAMES or maybe NJS_ENUM_PROPERTIES enum all own properties including non-enumerable.

and maybe NJS_ENUM_BOTH has to be renamed to NJS_ENUM_ENTRIES,
as there will be NJS_ENUM_DESCRIPTORS before NJS_ENUM_KEYS for Object.getOwnPropertyDescriptors()

@xeioex
Copy link
Contributor

xeioex commented Mar 26, 2019

@drsm

Thanks for catching.

NJS_ENUM_OWN_KEYS -> NJS_ENUM_ALL_KEYS?

NJS_ENUM_DESCRIPTORS -> NJS_ENUM_ALL_DESCRIPTORS?

I am trying to make it easier to understand.
So, by default, only enumerable properties are enumerated. All properties are enumerated if ALL is present in the flag name.

@xeioex
Copy link
Contributor

xeioex commented Mar 26, 2019

@drsm

Alternatively, we can use an additional argument for the function:

njs_object_enumerate(vm, value, NJS_ENUM_KEYS, 1 /* all */);

@drsm
Copy link
Contributor Author

drsm commented Mar 26, 2019

@xeioex

Alternatively, we can use an additional argument for the function:
njs_object_enumerate(vm, value, NJS_ENUM_KEYS, 1 /* all */);

i think no, this will cover the KEYS case, but not the DESCRIPTORS, and there is no API where non-enumerable VALUES|ENTRIES are used.

@xeioex
Copy link
Contributor

xeioex commented Mar 26, 2019

@drsm
Take a look: https://gist.github.com/xeioex/f67d80cbcfcb267104e74a0da42b59d5

and there is no API where non-enumerable VALUES|ENTRIES are used.

I think it is more or less OK. Because many internal functions, in fact, work only for special combinations of arguments.

@drsm
Copy link
Contributor Author

drsm commented Mar 26, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants