From 3cb02e5088c523c24dd2b2d4a63c7de9ef162308 Mon Sep 17 00:00:00 2001
From: Gabriel Schulhof <gabriel.schulhof@intel.com>
Date: Thu, 23 Jan 2020 15:52:49 -0800
Subject: [PATCH 1/3] test: cover property n-api null cases

Add test coverage for passing `NULL` to each parameter of
`napi.*(propert|element)` and `napi_set_prototype`. In the case of
`napi_define_properties` also test setting various initializer fields
to `NULL`.

PR-URL: https://github.com/nodejs/node/pull/31488
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
---
 test/js-native-api/test_object/binding.gyp   |   1 +
 test/js-native-api/test_object/test_null.c   | 401 +++++++++++++++++++
 test/js-native-api/test_object/test_null.h   |   8 +
 test/js-native-api/test_object/test_null.js  |  52 +++
 test/js-native-api/test_object/test_object.c |   3 +
 5 files changed, 465 insertions(+)
 create mode 100644 test/js-native-api/test_object/test_null.c
 create mode 100644 test/js-native-api/test_object/test_null.h
 create mode 100644 test/js-native-api/test_object/test_null.js

diff --git a/test/js-native-api/test_object/binding.gyp b/test/js-native-api/test_object/binding.gyp
index 1cb35d974dece9..e681f98f73ace6 100644
--- a/test/js-native-api/test_object/binding.gyp
+++ b/test/js-native-api/test_object/binding.gyp
@@ -5,6 +5,7 @@
       "sources": [
         "../common.c",
         "../entry_point.c",
+        "test_null.c",
         "test_object.c"
       ]
     }
diff --git a/test/js-native-api/test_object/test_null.c b/test/js-native-api/test_object/test_null.c
new file mode 100644
index 00000000000000..523217f3c0aad2
--- /dev/null
+++ b/test/js-native-api/test_object/test_null.c
@@ -0,0 +1,401 @@
+#define NAPI_EXPERIMENTAL
+#include <js_native_api.h>
+
+#include "../common.h"
+#include "test_null.h"
+
+static napi_value SetProperty(napi_env env, napi_callback_info info) {
+  napi_value return_value, object, key;
+
+  NAPI_CALL(env, napi_create_object(env, &return_value));
+  NAPI_CALL(env, napi_create_object(env, &object));
+  NAPI_CALL(env,
+      napi_create_string_utf8(env, "someString", NAPI_AUTO_LENGTH, &key));
+
+  add_returned_status(env,
+                      "envIsNull",
+                      return_value,
+                      "Invalid argument",
+                      napi_invalid_arg,
+                      napi_set_property(NULL, object, key, object));
+
+  napi_set_property(env, NULL, key, object);
+  add_last_status(env, "objectIsNull", return_value);
+
+  napi_set_property(env, object, NULL, object);
+  add_last_status(env, "keyIsNull", return_value);
+
+  napi_set_property(env, object, key, NULL);
+  add_last_status(env, "valueIsNull", return_value);
+
+  return return_value;
+}
+
+static napi_value GetProperty(napi_env env, napi_callback_info info) {
+  napi_value return_value, object, key, prop;
+
+  NAPI_CALL(env, napi_create_object(env, &return_value));
+  NAPI_CALL(env, napi_create_object(env, &object));
+  NAPI_CALL(env,
+      napi_create_string_utf8(env, "someString", NAPI_AUTO_LENGTH, &key));
+
+  add_returned_status(env,
+                      "envIsNull",
+                      return_value,
+                      "Invalid argument",
+                      napi_invalid_arg,
+                      napi_get_property(NULL, object, key, &prop));
+
+  napi_get_property(env, NULL, key, &prop);
+  add_last_status(env, "objectIsNull", return_value);
+
+  napi_get_property(env, object, NULL, &prop);
+  add_last_status(env, "keyIsNull", return_value);
+
+  napi_get_property(env, object, key, NULL);
+  add_last_status(env, "valueIsNull", return_value);
+
+  return return_value;
+}
+
+static napi_value TestBoolValuedPropApi(napi_env env,
+    napi_status (*api)(napi_env, napi_value, napi_value, bool*)) {
+  napi_value return_value, object, key;
+  bool result;
+
+  NAPI_CALL(env, napi_create_object(env, &return_value));
+  NAPI_CALL(env, napi_create_object(env, &object));
+  NAPI_CALL(env,
+      napi_create_string_utf8(env, "someString", NAPI_AUTO_LENGTH, &key));
+
+  add_returned_status(env,
+                      "envIsNull",
+                      return_value,
+                      "Invalid argument",
+                      napi_invalid_arg,
+                      api(NULL, object, key, &result));
+
+  api(env, NULL, key, &result);
+  add_last_status(env, "objectIsNull", return_value);
+
+  api(env, object, NULL, &result);
+  add_last_status(env, "keyIsNull", return_value);
+
+  api(env, object, key, NULL);
+  add_last_status(env, "valueIsNull", return_value);
+
+  return return_value;
+}
+
+static napi_value HasProperty(napi_env env, napi_callback_info info) {
+  return TestBoolValuedPropApi(env, napi_has_property);
+}
+
+static napi_value HasOwnProperty(napi_env env, napi_callback_info info) {
+  return TestBoolValuedPropApi(env, napi_has_own_property);
+}
+
+static napi_value DeleteProperty(napi_env env, napi_callback_info info) {
+  return TestBoolValuedPropApi(env, napi_delete_property);
+}
+
+static napi_value SetNamedProperty(napi_env env, napi_callback_info info) {
+  napi_value return_value, object;
+
+  NAPI_CALL(env, napi_create_object(env, &return_value));
+  NAPI_CALL(env, napi_create_object(env, &object));
+
+  add_returned_status(env,
+                      "envIsNull",
+                      return_value,
+                      "Invalid argument",
+                      napi_invalid_arg,
+                      napi_set_named_property(NULL, object, "key", object));
+
+  napi_set_named_property(env, NULL, "key", object);
+  add_last_status(env, "objectIsNull", return_value);
+
+  napi_set_named_property(env, object, NULL, object);
+  add_last_status(env, "keyIsNull", return_value);
+
+  napi_set_named_property(env, object, "key", NULL);
+  add_last_status(env, "valueIsNull", return_value);
+
+  return return_value;
+}
+
+static napi_value GetNamedProperty(napi_env env, napi_callback_info info) {
+  napi_value return_value, object, prop;
+
+  NAPI_CALL(env, napi_create_object(env, &return_value));
+  NAPI_CALL(env, napi_create_object(env, &object));
+
+  add_returned_status(env,
+                      "envIsNull",
+                      return_value,
+                      "Invalid argument",
+                      napi_invalid_arg,
+                      napi_get_named_property(NULL, object, "key", &prop));
+
+  napi_get_named_property(env, NULL, "key", &prop);
+  add_last_status(env, "objectIsNull", return_value);
+
+  napi_get_named_property(env, object, NULL, &prop);
+  add_last_status(env, "keyIsNull", return_value);
+
+  napi_get_named_property(env, object, "key", NULL);
+  add_last_status(env, "valueIsNull", return_value);
+
+  return return_value;
+}
+
+static napi_value HasNamedProperty(napi_env env, napi_callback_info info) {
+  napi_value return_value, object;
+  bool result;
+
+  NAPI_CALL(env, napi_create_object(env, &return_value));
+  NAPI_CALL(env, napi_create_object(env, &object));
+
+  add_returned_status(env,
+                      "envIsNull",
+                      return_value,
+                      "Invalid argument",
+                      napi_invalid_arg,
+                      napi_has_named_property(NULL, object, "key", &result));
+
+  napi_has_named_property(env, NULL, "key", &result);
+  add_last_status(env, "objectIsNull", return_value);
+
+  napi_has_named_property(env, object, NULL, &result);
+  add_last_status(env, "keyIsNull", return_value);
+
+  napi_has_named_property(env, object, "key", NULL);
+  add_last_status(env, "valueIsNull", return_value);
+
+  return return_value;
+}
+
+static napi_value SetElement(napi_env env, napi_callback_info info) {
+  napi_value return_value, object;
+
+  NAPI_CALL(env, napi_create_object(env, &return_value));
+  NAPI_CALL(env, napi_create_object(env, &object));
+
+  add_returned_status(env,
+                      "envIsNull",
+                      return_value,
+                      "Invalid argument",
+                      napi_invalid_arg,
+                      napi_set_element(NULL, object, 0, object));
+
+  napi_set_element(env, NULL, 0, object);
+  add_last_status(env, "objectIsNull", return_value);
+
+  napi_set_property(env, object, 0, NULL);
+  add_last_status(env, "valueIsNull", return_value);
+
+  return return_value;
+}
+
+static napi_value GetElement(napi_env env, napi_callback_info info) {
+  napi_value return_value, object, prop;
+
+  NAPI_CALL(env, napi_create_object(env, &return_value));
+  NAPI_CALL(env, napi_create_object(env, &object));
+
+  add_returned_status(env,
+                      "envIsNull",
+                      return_value,
+                      "Invalid argument",
+                      napi_invalid_arg,
+                      napi_get_element(NULL, object, 0, &prop));
+
+  napi_get_property(env, NULL, 0, &prop);
+  add_last_status(env, "objectIsNull", return_value);
+
+  napi_get_property(env, object, 0, NULL);
+  add_last_status(env, "valueIsNull", return_value);
+
+  return return_value;
+}
+
+static napi_value TestBoolValuedElementApi(napi_env env,
+    napi_status (*api)(napi_env, napi_value, uint32_t, bool*)) {
+  napi_value return_value, object;
+  bool result;
+
+  NAPI_CALL(env, napi_create_object(env, &return_value));
+  NAPI_CALL(env, napi_create_object(env, &object));
+
+  add_returned_status(env,
+                      "envIsNull",
+                      return_value,
+                      "Invalid argument",
+                      napi_invalid_arg,
+                      api(NULL, object, 0, &result));
+
+  api(env, NULL, 0, &result);
+  add_last_status(env, "objectIsNull", return_value);
+
+  api(env, object, 0, NULL);
+  add_last_status(env, "valueIsNull", return_value);
+
+  return return_value;
+}
+
+static napi_value HasElement(napi_env env, napi_callback_info info) {
+  return TestBoolValuedElementApi(env, napi_has_element);
+}
+
+static napi_value DeleteElement(napi_env env, napi_callback_info info) {
+  return TestBoolValuedElementApi(env, napi_delete_element);
+}
+
+static napi_value DefineProperties(napi_env env, napi_callback_info info) {
+  napi_value object, return_value;
+
+  napi_property_descriptor desc = {
+    "prop", NULL, DefineProperties, NULL, NULL, NULL, napi_enumerable, NULL
+  };
+
+  NAPI_CALL(env, napi_create_object(env, &object));
+  NAPI_CALL(env, napi_create_object(env, &return_value));
+
+  add_returned_status(env,
+                      "envIsNull",
+                      return_value,
+                      "Invalid argument",
+                      napi_invalid_arg,
+                      napi_define_properties(NULL, object, 1, &desc));
+
+  napi_define_properties(env, NULL, 1, &desc);
+  add_last_status(env, "objectIsNull", return_value);
+
+  napi_define_properties(env, object, 1, NULL);
+  add_last_status(env, "descriptorListIsNull", return_value);
+
+  desc.utf8name = NULL;
+  napi_define_properties(env, object, 1, NULL);
+  add_last_status(env, "utf8nameIsNull", return_value);
+  desc.utf8name = "prop";
+
+  desc.method = NULL;
+  napi_define_properties(env, object, 1, NULL);
+  add_last_status(env, "methodIsNull", return_value);
+  desc.method = DefineProperties;
+
+  return return_value;
+}
+
+static napi_value GetPropertyNames(napi_env env, napi_callback_info info) {
+  napi_value return_value, props;
+
+  NAPI_CALL(env, napi_create_object(env, &return_value));
+
+  add_returned_status(env,
+                      "envIsNull",
+                      return_value,
+                      "Invalid argument",
+                      napi_invalid_arg,
+                      napi_get_property_names(NULL, return_value, &props));
+
+  napi_get_property_names(env, NULL, &props);
+  add_last_status(env, "objectIsNull", return_value);
+
+  napi_get_property_names(env, return_value, NULL);
+  add_last_status(env, "valueIsNull", return_value);
+
+  return return_value;
+}
+
+static napi_value GetAllPropertyNames(napi_env env, napi_callback_info info) {
+  napi_value return_value, props;
+
+  NAPI_CALL(env, napi_create_object(env, &return_value));
+
+  add_returned_status(env,
+                      "envIsNull",
+                      return_value,
+                      "Invalid argument",
+                      napi_invalid_arg,
+                      napi_get_all_property_names(NULL,
+                                                  return_value,
+                                                  napi_key_own_only,
+                                                  napi_key_writable,
+                                                  napi_key_keep_numbers,
+                                                  &props));
+
+  napi_get_all_property_names(env,
+                              NULL,
+                              napi_key_own_only,
+                              napi_key_writable,
+                              napi_key_keep_numbers,
+                              &props);
+  add_last_status(env, "objectIsNull", return_value);
+
+  napi_get_all_property_names(env,
+                              return_value,
+                              napi_key_own_only,
+                              napi_key_writable,
+                              napi_key_keep_numbers,
+                              NULL);
+  add_last_status(env, "valueIsNull", return_value);
+
+  return return_value;
+}
+
+static napi_value GetPrototype(napi_env env, napi_callback_info info) {
+  napi_value return_value, proto;
+
+  NAPI_CALL(env, napi_create_object(env, &return_value));
+
+  add_returned_status(env,
+                      "envIsNull",
+                      return_value,
+                      "Invalid argument",
+                      napi_invalid_arg,
+                      napi_get_prototype(NULL, return_value, &proto));
+
+  napi_get_prototype(env, NULL, &proto);
+  add_last_status(env, "objectIsNull", return_value);
+
+  napi_get_prototype(env, return_value, NULL);
+  add_last_status(env, "valueIsNull", return_value);
+
+  return return_value;
+}
+
+void init_test_null(napi_env env, napi_value exports) {
+  napi_value test_null;
+
+  const napi_property_descriptor test_null_props[] = {
+    DECLARE_NAPI_PROPERTY("setProperty", SetProperty),
+    DECLARE_NAPI_PROPERTY("getProperty", GetProperty),
+    DECLARE_NAPI_PROPERTY("hasProperty", HasProperty),
+    DECLARE_NAPI_PROPERTY("hasOwnProperty", HasOwnProperty),
+    DECLARE_NAPI_PROPERTY("deleteProperty", DeleteProperty),
+    DECLARE_NAPI_PROPERTY("setNamedProperty", SetNamedProperty),
+    DECLARE_NAPI_PROPERTY("getNamedProperty", GetNamedProperty),
+    DECLARE_NAPI_PROPERTY("hasNamedProperty", HasNamedProperty),
+    DECLARE_NAPI_PROPERTY("setElement", SetElement),
+    DECLARE_NAPI_PROPERTY("getElement", GetElement),
+    DECLARE_NAPI_PROPERTY("hasElement", HasElement),
+    DECLARE_NAPI_PROPERTY("deleteElement", DeleteElement),
+    DECLARE_NAPI_PROPERTY("defineProperties", DefineProperties),
+    DECLARE_NAPI_PROPERTY("getPropertyNames", GetPropertyNames),
+    DECLARE_NAPI_PROPERTY("getAllPropertyNames", GetAllPropertyNames),
+    DECLARE_NAPI_PROPERTY("getPrototype", GetPrototype),
+  };
+
+  NAPI_CALL_RETURN_VOID(env, napi_create_object(env, &test_null));
+  NAPI_CALL_RETURN_VOID(env, napi_define_properties(
+      env, test_null, sizeof(test_null_props) / sizeof(*test_null_props),
+      test_null_props));
+
+  const napi_property_descriptor test_null_set = {
+    "testNull", NULL, NULL, NULL, NULL, test_null, napi_enumerable, NULL
+  };
+
+  NAPI_CALL_RETURN_VOID(env,
+      napi_define_properties(env, exports, 1, &test_null_set));
+}
diff --git a/test/js-native-api/test_object/test_null.h b/test/js-native-api/test_object/test_null.h
new file mode 100644
index 00000000000000..e10b708372c3b1
--- /dev/null
+++ b/test/js-native-api/test_object/test_null.h
@@ -0,0 +1,8 @@
+#ifndef TEST_JS_NATIVE_API_TEST_OBJECT_TEST_NULL_H_
+#define TEST_JS_NATIVE_API_TEST_OBJECT_TEST_NULL_H_
+
+#include <js_native_api.h>
+
+void init_test_null(napi_env env, napi_value exports);
+
+#endif  // TEST_JS_NATIVE_API_TEST_OBJECT_TEST_NULL_H_
diff --git a/test/js-native-api/test_object/test_null.js b/test/js-native-api/test_object/test_null.js
new file mode 100644
index 00000000000000..c94aa0fc84f19d
--- /dev/null
+++ b/test/js-native-api/test_object/test_null.js
@@ -0,0 +1,52 @@
+'use strict';
+const common = require('../../common');
+const assert = require('assert');
+
+// Test passing NULL to object-related N-APIs.
+const { testNull } = require(`./build/${common.buildType}/test_object`);
+
+const expectedForProperty = {
+  envIsNull: 'Invalid argument',
+  objectIsNull: 'Invalid argument',
+  keyIsNull: 'Invalid argument',
+  valueIsNull: 'Invalid argument'
+};
+assert.deepStrictEqual(testNull.setProperty(), expectedForProperty);
+assert.deepStrictEqual(testNull.getProperty(), expectedForProperty);
+assert.deepStrictEqual(testNull.hasProperty(), expectedForProperty);
+assert.deepStrictEqual(testNull.hasOwnProperty(), expectedForProperty);
+// It's OK not to want the result of a deletion.
+assert.deepStrictEqual(testNull.deleteProperty(),
+                       Object.assign({},
+                                     expectedForProperty,
+                                     { valueIsNull: 'napi_ok' }));
+assert.deepStrictEqual(testNull.setNamedProperty(), expectedForProperty);
+assert.deepStrictEqual(testNull.getNamedProperty(), expectedForProperty);
+assert.deepStrictEqual(testNull.hasNamedProperty(), expectedForProperty);
+
+const expectedForElement = {
+  envIsNull: 'Invalid argument',
+  objectIsNull: 'Invalid argument',
+  valueIsNull: 'Invalid argument'
+};
+assert.deepStrictEqual(testNull.setElement(), expectedForElement);
+assert.deepStrictEqual(testNull.getElement(), expectedForElement);
+assert.deepStrictEqual(testNull.hasElement(), expectedForElement);
+// It's OK not to want the result of a deletion.
+assert.deepStrictEqual(testNull.deleteElement(),
+                       Object.assign({},
+                                     expectedForElement,
+                                     { valueIsNull: 'napi_ok' }));
+
+assert.deepStrictEqual(testNull.defineProperties(), {
+  envIsNull: 'Invalid argument',
+  objectIsNull: 'Invalid argument',
+  descriptorListIsNull: 'Invalid argument',
+  utf8nameIsNull: 'Invalid argument',
+  methodIsNull: 'Invalid argument',
+});
+
+// `expectedForElement` also works for the APIs below.
+assert.deepStrictEqual(testNull.getPropertyNames(), expectedForElement);
+assert.deepStrictEqual(testNull.getAllPropertyNames(), expectedForElement);
+assert.deepStrictEqual(testNull.getPrototype(), expectedForElement);
diff --git a/test/js-native-api/test_object/test_object.c b/test/js-native-api/test_object/test_object.c
index db2d30c64a7cf4..33819d26b611ef 100644
--- a/test/js-native-api/test_object/test_object.c
+++ b/test/js-native-api/test_object/test_object.c
@@ -1,6 +1,7 @@
 #include <js_native_api.h>
 #include "../common.h"
 #include <string.h>
+#include "test_null.h"
 
 static int test_value = 3;
 
@@ -464,6 +465,8 @@ napi_value Init(napi_env env, napi_value exports) {
     DECLARE_NAPI_PROPERTY("TestGetProperty", TestGetProperty),
   };
 
+  init_test_null(env, exports);
+
   NAPI_CALL(env, napi_define_properties(
       env, exports, sizeof(descriptors) / sizeof(*descriptors), descriptors));
 

From 96cb8642d6e75be873c5232c5775a1f9aaffbdd9 Mon Sep 17 00:00:00 2001
From: himself65 <himself65@outlook.com>
Date: Thu, 17 Oct 2019 14:13:57 +0800
Subject: [PATCH 2/3] n-api: add napi_get_all_property_names

Co-Authored-By: Gabriel Schulhof <gabriel.schulhof@intel.com>

PR-URL: https://github.com/nodejs/node/pull/30006
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
---
 doc/api/n-api.md                             | 91 ++++++++++++++++++++
 src/js_native_api.h                          |  9 ++
 src/js_native_api_types.h                    | 21 +++++
 src/js_native_api_v8.cc                      | 88 ++++++++++++++++---
 src/js_native_api_v8.h                       | 11 +++
 test/js-native-api/test_object/test.js       |  7 +-
 test/js-native-api/test_object/test_object.c | 30 +++++++
 7 files changed, 245 insertions(+), 12 deletions(-)

diff --git a/doc/api/n-api.md b/doc/api/n-api.md
index 6c4e560ab8be2d..f1e4988114fce2 100644
--- a/doc/api/n-api.md
+++ b/doc/api/n-api.md
@@ -1660,6 +1660,66 @@ However, for better performance, it's better for the caller to make sure that
 the `napi_value` in question is of the JavaScript type expected by the API.
 
 ### Enum types
+#### napi_key_collection_mode
+<!-- YAML
+added: REPLACEME
+-->
+
+> Stability: 1 - Experimental
+
+```C
+typedef enum {
+  napi_key_include_prototypes,
+  napi_key_own_only
+} napi_key_collection_mode;
+```
+
+Describes the `Keys/Properties` filter enums:
+
+`napi_key_collection_mode` limits the range of collected properties.
+
+`napi_key_own_only` limits the collected properties to the given
+object only. `napi_key_include_prototypes` will include all keys
+of the objects's prototype chain as well.
+
+#### napi_key_filter
+<!-- YAML
+added: REPLACEME
+-->
+
+> Stability: 1 - Experimental
+
+```C
+typedef enum {
+  napi_key_all_properties = 0,
+  napi_key_writable = 1,
+  napi_key_enumerable = 1 << 1,
+  napi_key_configurable = 1 << 2,
+  napi_key_skip_strings = 1 << 3,
+  napi_key_skip_symbols = 1 << 4
+} napi_key_filter;
+```
+
+Property filter bits. They can be or'ed to build a composite filter.
+
+#### napi_key_conversion
+<!-- YAML
+added: REPLACEME
+-->
+
+> Stability: 1 - Experimental
+
+```C
+typedef enum {
+  napi_key_keep_numbers,
+  napi_key_numbers_to_strings
+} napi_key_conversion;
+```
+
+`napi_key_numbers_to_strings` will convert integer indices to
+strings. `napi_key_keep_numbers` will return numbers for integer
+indices.
+
 #### napi_valuetype
 
 ```C
@@ -3532,6 +3592,37 @@ This API returns the names of the enumerable properties of `object` as an array
 of strings. The properties of `object` whose key is a symbol will not be
 included.
 
+#### napi_get_all_property_names
+<!-- YAML
+added: REPLACEME
+-->
+
+> Stability: 1 - Experimental
+
+```C
+napi_get_all_property_names(napi_env env,
+                            napi_value object,
+                            napi_key_collection_mode key_mode,
+                            napi_key_filter key_filter,
+                            napi_key_conversion key_conversion,
+                            napi_value* result);
+```
+
+* `[in] env`: The environment that the N-API call is invoked under.
+* `[in] object`: The object from which to retrieve the properties.
+* `[in] key_mode`: Whether to retrieve prototype properties as well.
+* `[in] key_filter`: Which properties to retrieve
+(enumerable/readable/writable).
+* `[in] key_conversion`: Whether to convert numbered property keys to strings.
+* `[out] result`: A `napi_value` representing an array of JavaScript values
+that represent the property names of the object. [`napi_get_array_length`][] and
+[`napi_get_element`][] can be used to iterate over `result`.
+
+Returns `napi_ok` if the API succeeded.
+
+This API returns an array containing the names of the available properties
+of this object.
+
 #### napi_set_property
 <!-- YAML
 added: v8.0.0
diff --git a/src/js_native_api.h b/src/js_native_api.h
index 489549190b663e..cb69fde5d1990e 100644
--- a/src/js_native_api.h
+++ b/src/js_native_api.h
@@ -506,6 +506,15 @@ NAPI_EXTERN napi_status napi_get_value_bigint_words(napi_env env,
                                                     size_t* word_count,
                                                     uint64_t* words);
 
+// Object
+NAPI_EXTERN napi_status
+napi_get_all_property_names(napi_env env,
+                            napi_value object,
+                            napi_key_collection_mode key_mode,
+                            napi_key_filter key_filter,
+                            napi_key_conversion key_conversion,
+                            napi_value* result);
+
 // Instance data
 NAPI_EXTERN napi_status napi_set_instance_data(napi_env env,
                                                void* data,
diff --git a/src/js_native_api_types.h b/src/js_native_api_types.h
index a81e76a64934de..ef44dd457dba6d 100644
--- a/src/js_native_api_types.h
+++ b/src/js_native_api_types.h
@@ -115,4 +115,25 @@ typedef struct {
   napi_status error_code;
 } napi_extended_error_info;
 
+#ifdef NAPI_EXPERIMENTAL
+typedef enum {
+  napi_key_include_prototypes,
+  napi_key_own_only
+} napi_key_collection_mode;
+
+typedef enum {
+  napi_key_all_properties = 0,
+  napi_key_writable = 1,
+  napi_key_enumerable = 1 << 1,
+  napi_key_configurable = 1 << 2,
+  napi_key_skip_strings = 1 << 3,
+  napi_key_skip_symbols = 1 << 4
+} napi_key_filter;
+
+typedef enum {
+  napi_key_keep_numbers,
+  napi_key_numbers_to_strings
+} napi_key_conversion;
+#endif
+
 #endif  // SRC_JS_NATIVE_API_TYPES_H_
diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc
index d26b28dd0df4e8..24a065097a28e1 100644
--- a/src/js_native_api_v8.cc
+++ b/src/js_native_api_v8.cc
@@ -933,6 +933,22 @@ napi_status napi_define_class(napi_env env,
 napi_status napi_get_property_names(napi_env env,
                                     napi_value object,
                                     napi_value* result) {
+  return napi_get_all_property_names(
+      env,
+      object,
+      napi_key_include_prototypes,
+      static_cast<napi_key_filter>(napi_key_enumerable |
+                                   napi_key_skip_symbols),
+      napi_key_numbers_to_strings,
+      result);
+}
+
+napi_status napi_get_all_property_names(napi_env env,
+                                        napi_value object,
+                                        napi_key_collection_mode key_mode,
+                                        napi_key_filter key_filter,
+                                        napi_key_conversion key_conversion,
+                                        napi_value* result) {
   NAPI_PREAMBLE(env);
   CHECK_ARG(env, result);
 
@@ -940,19 +956,69 @@ napi_status napi_get_property_names(napi_env env,
   v8::Local<v8::Object> obj;
   CHECK_TO_OBJECT(env, context, obj, object);
 
-  v8::MaybeLocal<v8::Array> maybe_propertynames = obj->GetPropertyNames(
-    context,
-    v8::KeyCollectionMode::kIncludePrototypes,
-    static_cast<v8::PropertyFilter>(
-        v8::PropertyFilter::ONLY_ENUMERABLE |
-        v8::PropertyFilter::SKIP_SYMBOLS),
-    v8::IndexFilter::kIncludeIndices,
-    v8::KeyConversionMode::kConvertToString);
+  v8::PropertyFilter filter = v8::PropertyFilter::ALL_PROPERTIES;
+  if (key_filter & napi_key_writable) {
+    filter =
+        static_cast<v8::PropertyFilter>(filter |
+                                        v8::PropertyFilter::ONLY_WRITABLE);
+  }
+  if (key_filter & napi_key_enumerable) {
+    filter =
+        static_cast<v8::PropertyFilter>(filter |
+                                        v8::PropertyFilter::ONLY_ENUMERABLE);
+  }
+  if (key_filter & napi_key_configurable) {
+    filter =
+        static_cast<v8::PropertyFilter>(filter |
+                                        v8::PropertyFilter::ONLY_WRITABLE);
+  }
+  if (key_filter & napi_key_skip_strings) {
+    filter =
+        static_cast<v8::PropertyFilter>(filter |
+                                        v8::PropertyFilter::SKIP_STRINGS);
+  }
+  if (key_filter & napi_key_skip_symbols) {
+    filter =
+        static_cast<v8::PropertyFilter>(filter |
+                                        v8::PropertyFilter::SKIP_SYMBOLS);
+  }
+  v8::KeyCollectionMode collection_mode;
+  v8::KeyConversionMode conversion_mode;
+
+  switch (key_mode) {
+    case napi_key_include_prototypes:
+      collection_mode = v8::KeyCollectionMode::kIncludePrototypes;
+      break;
+    case napi_key_own_only:
+      collection_mode = v8::KeyCollectionMode::kOwnOnly;
+      break;
+    default:
+      return napi_set_last_error(env, napi_invalid_arg);
+  }
 
-  CHECK_MAYBE_EMPTY(env, maybe_propertynames, napi_generic_failure);
+  switch (key_conversion) {
+    case napi_key_keep_numbers:
+      conversion_mode = v8::KeyConversionMode::kKeepNumbers;
+      break;
+    case napi_key_numbers_to_strings:
+      conversion_mode = v8::KeyConversionMode::kConvertToString;
+      break;
+    default:
+      return napi_set_last_error(env, napi_invalid_arg);
+  }
 
-  *result = v8impl::JsValueFromV8LocalValue(
-      maybe_propertynames.ToLocalChecked());
+  v8::MaybeLocal<v8::Array> maybe_all_propertynames =
+      obj->GetPropertyNames(context,
+                            collection_mode,
+                            filter,
+                            v8::IndexFilter::kIncludeIndices,
+                            conversion_mode);
+
+  CHECK_MAYBE_EMPTY_WITH_PREAMBLE(
+      env, maybe_all_propertynames, napi_generic_failure);
+
+  *result =
+      v8impl::JsValueFromV8LocalValue(maybe_all_propertynames.ToLocalChecked());
   return GET_RETURN_STATUS(env);
 }
 
diff --git a/src/js_native_api_v8.h b/src/js_native_api_v8.h
index ec397275164c8d..32fc16f155a7fe 100644
--- a/src/js_native_api_v8.h
+++ b/src/js_native_api_v8.h
@@ -199,6 +199,17 @@ napi_status napi_set_last_error(napi_env env, napi_status error_code,
     }                                                              \
   } while (0)
 
+#define RETURN_STATUS_IF_FALSE_WITH_PREAMBLE(env, condition, status)           \
+  do {                                                                         \
+    if (!(condition)) {                                                        \
+      return napi_set_last_error(                                              \
+          (env), try_catch.HasCaught() ? napi_pending_exception : (status));   \
+    }                                                                          \
+  } while (0)
+
+#define CHECK_MAYBE_EMPTY_WITH_PREAMBLE(env, maybe, status)                    \
+  RETURN_STATUS_IF_FALSE_WITH_PREAMBLE((env), !((maybe).IsEmpty()), (status))
+
 namespace v8impl {
 
 //=== Conversion between V8 Handles and napi_value ========================
diff --git a/test/js-native-api/test_object/test.js b/test/js-native-api/test_object/test.js
index 33ab49bb6fb5c5..2cd65af6b34de6 100644
--- a/test/js-native-api/test_object/test.js
+++ b/test/js-native-api/test_object/test.js
@@ -212,8 +212,10 @@ assert.strictEqual(newObject.test_string, 'test string');
     inherited: 1
   });
 
+  const fooSymbol = Symbol('foo');
+
   object.normal = 2;
-  object[Symbol('foo')] = 3;
+  object[fooSymbol] = 3;
   Object.defineProperty(object, 'unenumerable', {
     value: 4,
     enumerable: false,
@@ -224,6 +226,9 @@ assert.strictEqual(newObject.test_string, 'test string');
 
   assert.deepStrictEqual(test_object.GetPropertyNames(object),
                          ['5', 'normal', 'inherited']);
+
+  assert.deepStrictEqual(test_object.GetSymbolNames(object),
+                         [fooSymbol]);
 }
 
 // Verify that passing NULL to napi_set_property() results in the correct
diff --git a/test/js-native-api/test_object/test_object.c b/test/js-native-api/test_object/test_object.c
index 33819d26b611ef..9d9589238d4079 100644
--- a/test/js-native-api/test_object/test_object.c
+++ b/test/js-native-api/test_object/test_object.c
@@ -1,3 +1,5 @@
+#define NAPI_EXPERIMENTAL
+
 #include <js_native_api.h>
 #include "../common.h"
 #include <string.h>
@@ -83,6 +85,33 @@ static napi_value GetPropertyNames(napi_env env, napi_callback_info info) {
   return output;
 }
 
+static napi_value GetSymbolNames(napi_env env, napi_callback_info info) {
+  size_t argc = 1;
+  napi_value args[1];
+  NAPI_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL));
+
+  NAPI_ASSERT(env, argc >= 1, "Wrong number of arguments");
+
+  napi_valuetype value_type0;
+  NAPI_CALL(env, napi_typeof(env, args[0], &value_type0));
+
+  NAPI_ASSERT(env,
+              value_type0 == napi_object,
+              "Wrong type of arguments. Expects an object as first argument.");
+
+  napi_value output;
+  NAPI_CALL(env,
+            napi_get_all_property_names(
+                env,
+                args[0],
+                napi_key_include_prototypes,
+                napi_key_skip_strings,
+                napi_key_numbers_to_strings,
+                &output));
+
+  return output;
+}
+
 static napi_value Set(napi_env env, napi_callback_info info) {
   size_t argc = 3;
   napi_value args[3];
@@ -450,6 +479,7 @@ napi_value Init(napi_env env, napi_value exports) {
     DECLARE_NAPI_PROPERTY("Get", Get),
     DECLARE_NAPI_PROPERTY("GetNamed", GetNamed),
     DECLARE_NAPI_PROPERTY("GetPropertyNames", GetPropertyNames),
+    DECLARE_NAPI_PROPERTY("GetSymbolNames", GetSymbolNames),
     DECLARE_NAPI_PROPERTY("Set", Set),
     DECLARE_NAPI_PROPERTY("SetNamed", SetNamed),
     DECLARE_NAPI_PROPERTY("Has", Has),

From 3a3c011b5507c710c95e1abf1633bc292f83e36a Mon Sep 17 00:00:00 2001
From: Gabriel Schulhof <gabriel.schulhof@intel.com>
Date: Mon, 2 Mar 2020 16:53:06 -0800
Subject: [PATCH 3/3] n-api: define release 6

Mark all N-APIs that have been added since version 5 as stable.

PR-URL: https://github.com/nodejs/node/pull/32058
Fixes: https://github.com/nodejs/abi-stable-node/issues/393
Co-Authored-By: legendecas <legendecas@gmail.com>
Signed-off-by: Gabriel Schulhof <gabriel.schulhof@intel.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
---
 doc/api/n-api.md                              | 32 +++++++------------
 src/js_native_api.h                           |  9 ++++--
 src/js_native_api_types.h                     |  4 +--
 src/node_version.h                            |  2 +-
 test/js-native-api/test_bigint/test_bigint.c  |  2 --
 test/js-native-api/test_general/test.js       |  2 +-
 .../test_instance_data/test_instance_data.c   |  1 -
 test/js-native-api/test_object/test_null.c    |  1 -
 test/js-native-api/test_object/test_object.c  |  2 --
 test/node-api/test_general/test_general.c     |  1 -
 test/node-api/test_instance_data/addon.c      |  1 -
 .../test_instance_data/test_instance_data.c   |  1 -
 .../test_instance_data/test_ref_then_set.c    |  1 -
 .../test_instance_data/test_set_then_ref.c    |  1 -
 14 files changed, 22 insertions(+), 38 deletions(-)

diff --git a/doc/api/n-api.md b/doc/api/n-api.md
index f1e4988114fce2..c3debd29412e95 100644
--- a/doc/api/n-api.md
+++ b/doc/api/n-api.md
@@ -374,6 +374,7 @@ tied to the life cycle of the Agent.
 ### napi_set_instance_data
 <!-- YAML
 added: v12.8.0
+napiVersion: 6
 -->
 
 ```C
@@ -401,6 +402,7 @@ by the previous call, it will not be called.
 ### napi_get_instance_data
 <!-- YAML
 added: v12.8.0
+napiVersion: 6
 -->
 
 ```C
@@ -1663,10 +1665,9 @@ the `napi_value` in question is of the JavaScript type expected by the API.
 #### napi_key_collection_mode
 <!-- YAML
 added: REPLACEME
+napiVersion: 6
 -->
 
-> Stability: 1 - Experimental
-
 ```C
 typedef enum {
   napi_key_include_prototypes,
@@ -1685,10 +1686,9 @@ of the objects's prototype chain as well.
 #### napi_key_filter
 <!-- YAML
 added: REPLACEME
+napiVersion: 6
 -->
 
-> Stability: 1 - Experimental
-
 ```C
 typedef enum {
   napi_key_all_properties = 0,
@@ -1705,10 +1705,9 @@ Property filter bits. They can be or'ed to build a composite filter.
 #### napi_key_conversion
 <!-- YAML
 added: REPLACEME
+napiVersion: 6
 -->
 
-> Stability: 1 - Experimental
-
 ```C
 typedef enum {
   napi_key_keep_numbers,
@@ -2262,10 +2261,9 @@ The JavaScript `Number` type is described in
 #### napi_create_bigint_int64
 <!-- YAML
 added: v10.7.0
+napiVersion: 6
 -->
 
-> Stability: 1 - Experimental
-
 ```C
 napi_status napi_create_bigint_int64(napi_env env,
                                      int64_t value,
@@ -2283,10 +2281,9 @@ This API converts the C `int64_t` type to the JavaScript `BigInt` type.
 #### napi_create_bigint_uint64
 <!-- YAML
 added: v10.7.0
+napiVersion: 6
 -->
 
-> Stability: 1 - Experimental
-
 ```C
 napi_status napi_create_bigint_uint64(napi_env env,
                                       uint64_t value,
@@ -2304,10 +2301,9 @@ This API converts the C `uint64_t` type to the JavaScript `BigInt` type.
 #### napi_create_bigint_words
 <!-- YAML
 added: v10.7.0
+napiVersion: 6
 -->
 
-> Stability: 1 - Experimental
-
 ```C
 napi_status napi_create_bigint_words(napi_env env,
                                      int sign_bit,
@@ -2653,10 +2649,9 @@ This API returns the C double primitive equivalent of the given JavaScript
 #### napi_get_value_bigint_int64
 <!-- YAML
 added: v10.7.0
+napiVersion: 6
 -->
 
-> Stability: 1 - Experimental
-
 ```C
 napi_status napi_get_value_bigint_int64(napi_env env,
                                         napi_value value,
@@ -2680,10 +2675,9 @@ This API returns the C `int64_t` primitive equivalent of the given JavaScript
 #### napi_get_value_bigint_uint64
 <!-- YAML
 added: v10.7.0
+napiVersion: 6
 -->
 
-> Stability: 1 - Experimental
-
 ```C
 napi_status napi_get_value_bigint_uint64(napi_env env,
                                         napi_value value,
@@ -2707,10 +2701,9 @@ This API returns the C `uint64_t` primitive equivalent of the given JavaScript
 #### napi_get_value_bigint_words
 <!-- YAML
 added: v10.7.0
+napiVersion: 6
 -->
 
-> Stability: 1 - Experimental
-
 ```C
 napi_status napi_get_value_bigint_words(napi_env env,
                                         napi_value value,
@@ -3595,10 +3588,9 @@ included.
 #### napi_get_all_property_names
 <!-- YAML
 added: REPLACEME
+napiVersion: 6
 -->
 
-> Stability: 1 - Experimental
-
 ```C
 napi_get_all_property_names(napi_env env,
                             napi_value object,
diff --git a/src/js_native_api.h b/src/js_native_api.h
index cb69fde5d1990e..2675da505c2368 100644
--- a/src/js_native_api.h
+++ b/src/js_native_api.h
@@ -4,7 +4,6 @@
 // This file needs to be compatible with C compilers.
 #include <stddef.h>   // NOLINT(modernize-deprecated-headers)
 #include <stdbool.h>  // NOLINT(modernize-deprecated-headers)
-#include "js_native_api_types.h"
 
 // Use INT_MAX, this should only be consumed by the pre-processor anyway.
 #define NAPI_VERSION_EXPERIMENTAL 2147483647
@@ -18,10 +17,12 @@
 // functions available in a new version of N-API that is not yet ported in all
 // LTS versions, they can set NAPI_VERSION knowing that they have specifically
 // depended on that version.
-#define NAPI_VERSION 5
+#define NAPI_VERSION 6
 #endif
 #endif
 
+#include "js_native_api_types.h"
+
 // If you need __declspec(dllimport), either include <node_api.h> instead, or
 // define NAPI_EXTERN as __declspec(dllimport) on the compiler's command line.
 #ifndef NAPI_EXTERN
@@ -478,7 +479,7 @@ NAPI_EXTERN napi_status napi_add_finalizer(napi_env env,
 
 #endif  // NAPI_VERSION >= 5
 
-#ifdef NAPI_EXPERIMENTAL
+#if NAPI_VERSION >= 6
 
 // BigInt
 NAPI_EXTERN napi_status napi_create_bigint_int64(napi_env env,
@@ -523,7 +524,9 @@ NAPI_EXTERN napi_status napi_set_instance_data(napi_env env,
 
 NAPI_EXTERN napi_status napi_get_instance_data(napi_env env,
                                                void** data);
+#endif  // NAPI_VERSION >= 6
 
+#ifdef NAPI_EXPERIMENTAL
 // ArrayBuffer detaching
 NAPI_EXTERN napi_status napi_detach_arraybuffer(napi_env env,
                                                 napi_value arraybuffer);
diff --git a/src/js_native_api_types.h b/src/js_native_api_types.h
index ef44dd457dba6d..7a49fc9f719b30 100644
--- a/src/js_native_api_types.h
+++ b/src/js_native_api_types.h
@@ -115,7 +115,7 @@ typedef struct {
   napi_status error_code;
 } napi_extended_error_info;
 
-#ifdef NAPI_EXPERIMENTAL
+#if NAPI_VERSION >= 6
 typedef enum {
   napi_key_include_prototypes,
   napi_key_own_only
@@ -134,6 +134,6 @@ typedef enum {
   napi_key_keep_numbers,
   napi_key_numbers_to_strings
 } napi_key_conversion;
-#endif
+#endif  // NAPI_VERSION >= 6
 
 #endif  // SRC_JS_NATIVE_API_TYPES_H_
diff --git a/src/node_version.h b/src/node_version.h
index 91d4cc15a6e7d1..8a9d88b1d4f0f9 100644
--- a/src/node_version.h
+++ b/src/node_version.h
@@ -93,6 +93,6 @@
 
 // The NAPI_VERSION provided by this version of the runtime. This is the version
 // which the Node binary being built supports.
-#define NAPI_VERSION  5
+#define NAPI_VERSION  6
 
 #endif  // SRC_NODE_VERSION_H_
diff --git a/test/js-native-api/test_bigint/test_bigint.c b/test/js-native-api/test_bigint/test_bigint.c
index 4befc171baa5da..c62a0a6a6c2bbc 100644
--- a/test/js-native-api/test_bigint/test_bigint.c
+++ b/test/js-native-api/test_bigint/test_bigint.c
@@ -1,5 +1,3 @@
-#define NAPI_EXPERIMENTAL
-
 #include <inttypes.h>
 #include <stdio.h>
 #include <js_native_api.h>
diff --git a/test/js-native-api/test_general/test.js b/test/js-native-api/test_general/test.js
index 9b847f4f339e49..a4b3df5535a365 100644
--- a/test/js-native-api/test_general/test.js
+++ b/test/js-native-api/test_general/test.js
@@ -33,7 +33,7 @@ assert.notStrictEqual(test_general.testGetPrototype(baseObject),
                       test_general.testGetPrototype(extendedObject));
 
 // Test version management functions. The expected version is currently 4.
-assert.strictEqual(test_general.testGetVersion(), 5);
+assert.strictEqual(test_general.testGetVersion(), 6);
 
 [
   123,
diff --git a/test/js-native-api/test_instance_data/test_instance_data.c b/test/js-native-api/test_instance_data/test_instance_data.c
index a64ebec0c1aba9..5255c3e4a02b94 100644
--- a/test/js-native-api/test_instance_data/test_instance_data.c
+++ b/test/js-native-api/test_instance_data/test_instance_data.c
@@ -1,6 +1,5 @@
 #include <stdio.h>
 #include <stdlib.h>
-#define NAPI_EXPERIMENTAL
 #include <js_native_api.h>
 #include "../common.h"
 
diff --git a/test/js-native-api/test_object/test_null.c b/test/js-native-api/test_object/test_null.c
index 523217f3c0aad2..b6bf4df31cc918 100644
--- a/test/js-native-api/test_object/test_null.c
+++ b/test/js-native-api/test_object/test_null.c
@@ -1,4 +1,3 @@
-#define NAPI_EXPERIMENTAL
 #include <js_native_api.h>
 
 #include "../common.h"
diff --git a/test/js-native-api/test_object/test_object.c b/test/js-native-api/test_object/test_object.c
index 9d9589238d4079..08f619bf7ffc88 100644
--- a/test/js-native-api/test_object/test_object.c
+++ b/test/js-native-api/test_object/test_object.c
@@ -1,5 +1,3 @@
-#define NAPI_EXPERIMENTAL
-
 #include <js_native_api.h>
 #include "../common.h"
 #include <string.h>
diff --git a/test/node-api/test_general/test_general.c b/test/node-api/test_general/test_general.c
index 05bccaf5c2cf4f..be805f782be8d5 100644
--- a/test/node-api/test_general/test_general.c
+++ b/test/node-api/test_general/test_general.c
@@ -1,4 +1,3 @@
-#define NAPI_EXPERIMENTAL
 #include <node_api.h>
 #include <stdlib.h>
 #include "../../js-native-api/common.h"
diff --git a/test/node-api/test_instance_data/addon.c b/test/node-api/test_instance_data/addon.c
index 928b4dfaf8e11e..7cf27bf28ab7eb 100644
--- a/test/node-api/test_instance_data/addon.c
+++ b/test/node-api/test_instance_data/addon.c
@@ -1,6 +1,5 @@
 #include <stdio.h>
 #include <stdlib.h>
-#define NAPI_EXPERIMENTAL
 #include <node_api.h>
 
 static void addon_free(napi_env env, void* data, void* hint) {
diff --git a/test/node-api/test_instance_data/test_instance_data.c b/test/node-api/test_instance_data/test_instance_data.c
index 1a814e91c0665b..24fd502e836176 100644
--- a/test/node-api/test_instance_data/test_instance_data.c
+++ b/test/node-api/test_instance_data/test_instance_data.c
@@ -1,6 +1,5 @@
 #include <stdlib.h>
 #include <uv.h>
-#define NAPI_EXPERIMENTAL
 #include <node_api.h>
 #include "../../js-native-api/common.h"
 
diff --git a/test/node-api/test_instance_data/test_ref_then_set.c b/test/node-api/test_instance_data/test_ref_then_set.c
index a0df1e5b9f8a64..10c779d3241e0e 100644
--- a/test/node-api/test_instance_data/test_ref_then_set.c
+++ b/test/node-api/test_instance_data/test_ref_then_set.c
@@ -1,6 +1,5 @@
 #include <stdio.h>
 #include <stdlib.h>
-#define NAPI_EXPERIMENTAL
 #include <node_api.h>
 
 napi_value addon_new(napi_env env, napi_value exports, bool ref_first);
diff --git a/test/node-api/test_instance_data/test_set_then_ref.c b/test/node-api/test_instance_data/test_set_then_ref.c
index 6ebed2d1e86871..9a1b31aeed390b 100644
--- a/test/node-api/test_instance_data/test_set_then_ref.c
+++ b/test/node-api/test_instance_data/test_set_then_ref.c
@@ -1,6 +1,5 @@
 #include <stdio.h>
 #include <stdlib.h>
-#define NAPI_EXPERIMENTAL
 #include <node_api.h>
 
 napi_value addon_new(napi_env env, napi_value exports, bool ref_first);