process: use frozen array for process.allowedNodeEnvironmentFlags

doc/api/process.md

@@ -575,17 +575,21 @@ This feature is not available in [`Worker`][] threads.
 ## `process.allowedNodeEnvironmentFlags`
 <!-- YAML
 added: v10.10.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/37171
+    description: Use a frozen array instead of a subclass of `Set`.
 -->
 
-* {Set}
+* {string[]}
 
 The `process.allowedNodeEnvironmentFlags` property is a special,
-read-only `Set` of flags allowable within the [`NODE_OPTIONS`][]
+read-only `Array` of flags allowable within the [`NODE_OPTIONS`][]
 environment variable.
 
-`process.allowedNodeEnvironmentFlags` extends `Set`, but overrides
-`Set.prototype.has` to recognize several different possible flag
-representations. `process.allowedNodeEnvironmentFlags.has()` will
+`process.allowedNodeEnvironmentFlags` is a frozen `Array`, but overrides
+`Array.prototype.includes` to recognize several different possible flag
+representations. `process.allowedNodeEnvironmentFlags.includes()` will
 return `true` in the following cases:
 
 * Flags may omit leading single (`-`) or double (`--`) dashes; e.g.,
@@ -613,10 +617,6 @@ process.allowedNodeEnvironmentFlags.forEach((flag) => {
 });
 ```
 
-The methods `add()`, `clear()`, and `delete()` of
-`process.allowedNodeEnvironmentFlags` do nothing, and will fail
-silently.
-
 If Node.js was compiled *without* [`NODE_OPTIONS`][] support (shown in
 [`process.config`][]), `process.allowedNodeEnvironmentFlags` will
 contain what *would have* been allowable.

lib/internal/process/per_thread.js

@@ -6,17 +6,18 @@
 
 const {
   ArrayPrototypeEvery,
+  ArrayPrototypeIncludes,
   ArrayPrototypeMap,
   ArrayPrototypePush,
   ArrayPrototypeSplice,
   BigUint64Array,
   Float64Array,
+  FunctionPrototype,
   NumberMAX_SAFE_INTEGER,
-  ObjectDefineProperty,
   ObjectFreeze,
   ReflectApply,
+  ReflectGet,
   RegExpPrototypeTest,
-  SafeSet,
   StringPrototypeEndsWith,
   StringPrototypeReplace,
   StringPrototypeSlice,
@@ -293,55 +294,53 @@ function buildAllowedFlags() {
 
   // Save these for comparison against flags provided to
   // process.allowedNodeEnvironmentFlags.has() which lack leading dashes.
-  const nodeFlags = new SafeSet(ArrayPrototypeMap(allowedNodeEnvironmentFlags,
-                                                  trimLeadingDashes));
-
-  class NodeEnvironmentFlagsSet extends SafeSet {
-    constructor(...args) {
-      super(...args);
-
-      // The super constructor consumes `add`, but
-      // disallow any future adds.
-      ObjectDefineProperty(this, 'add', {
-        value: () => this
-      });
-    }
-
-    delete() {
-      // No-op, `Set` API compatible
-      return false;
-    }
-
-    clear() {
-      // No-op
-    }
-
-    has(key) {
-      // This will return `true` based on various possible
-      // permutations of a flag, including present/missing leading
-      // dash(es) and/or underscores-for-dashes.
-      // Strips any values after `=`, inclusive.
-      // TODO(addaleax): It might be more flexible to run the option parser
-      // on a dummy option set and see whether it rejects the argument or
-      // not.
-      if (typeof key === 'string') {
-        key = StringPrototypeReplace(key, replaceUnderscoresRegex, '-');
-        if (RegExpPrototypeTest(leadingDashesRegex, key)) {
-          key = StringPrototypeReplace(key, trailingValuesRegex, '');
-          return super.has(key);
-        }
-        return nodeFlags.has(key);
+  const nodeFlags =
+    ArrayPrototypeMap(allowedNodeEnvironmentFlags, trimLeadingDashes);
+
+  return new Proxy(ObjectFreeze(allowedNodeEnvironmentFlags), {
+    get(target, key, receiver) {
+      switch (key) {
+        case 'add':
+          // No-op, `Set` API compatible
+          return () => receiver;
+
+        case 'delete':
+          // No-op, `Set` API compatible
+          return () => false;
+
+        case 'clear':
+          // No-op, `Set` API compatible
+          return FunctionPrototype;
+
+        case 'has':
+        case 'includes':
+          return (key) => {
+            // This will return `true` based on various possible
+            // permutations of a flag, including present/missing leading
+            // dash(es) and/or underscores-for-dashes.
+            // Strips any values after `=`, inclusive.
+            // TODO(addaleax): It might be more flexible to run the option
+            // parser on a dummy option set and see whether it rejects the
+            // argument or not.
+            if (typeof key === 'string') {
+              key = StringPrototypeReplace(key, replaceUnderscoresRegex, '-');
+              if (RegExpPrototypeTest(leadingDashesRegex, key)) {
+                key = StringPrototypeReplace(key, trailingValuesRegex, '');
+                return ArrayPrototypeIncludes(target, key);
+              }
+              return ArrayPrototypeIncludes(nodeFlags, key);
+            }
+            return false;
+          };
+
+        case 'size':
+          return target.length;
+
+        default:
+          return ReflectGet(target, key);
       }
-      return false;
-    }
-  }
-
-  ObjectFreeze(NodeEnvironmentFlagsSet.prototype.constructor);
-  ObjectFreeze(NodeEnvironmentFlagsSet.prototype);
-
-  return ObjectFreeze(new NodeEnvironmentFlagsSet(
-    allowedNodeEnvironmentFlags
-  ));
+    },
+  });
 }
 
 // Lazy load internal/trace_events_async_hooks only if the async_hooks

test/parallel/test-process-env-allowed-flags.js

@@ -1,6 +1,6 @@
 'use strict';
 
-require('../common');
+const common = require('../common');
 const assert = require('assert');
 
 // Assert legit flags are allowed, and bogus flags are disallowed
@@ -62,15 +62,58 @@ const assert = require('assert');
                      true);
 
   process.allowedNodeEnvironmentFlags.add('foo');
+  assert.throws(
+    () => Set.prototype.add.call(process.allowedNodeEnvironmentFlags, 'foo'),
+    /Set\.prototype\.add called on incompatible receiver/);
   assert.strictEqual(process.allowedNodeEnvironmentFlags.has('foo'), false);
-  process.allowedNodeEnvironmentFlags.forEach((flag) => {
-    assert.strictEqual(flag === 'foo', false);
-  });
 
-  process.allowedNodeEnvironmentFlags.clear();
-  assert.strictEqual(process.allowedNodeEnvironmentFlags.size > 0, true);
+  assert.throws(
+    () => process.allowedNodeEnvironmentFlags.push('foo'),
+    /object is not extensible/);
+  assert.throws(
+    () => Array.prototype.push.call(process.allowedNodeEnvironmentFlags, 'foo'),
+    /object is not extensible/);
+  assert.strictEqual(
+    process.allowedNodeEnvironmentFlags.includes('foo'),
+    false);
+  assert.strictEqual(
+    Array.prototype.includes.call(process.allowedNodeEnvironmentFlags, 'foo'),
+    false);
+
+  const thisArg = {};
+  process.allowedNodeEnvironmentFlags.forEach(
+    common.mustCallAtLeast(function(flag, _, arr) {
+      assert.notStrictEqual(flag, 'foo');
+      assert.strictEqual(this, thisArg);
+      assert.strictEqual(arr, process.allowedNodeEnvironmentFlags);
+    }),
+    thisArg
+  );
+
+  for (const flag of process.allowedNodeEnvironmentFlags.keys()) {
+    assert.notStrictEqual(flag, 'foo');
+  }
+  for (const flag of process.allowedNodeEnvironmentFlags.values()) {
+    assert.notStrictEqual(flag, 'foo');
+  }
+  for (const flag of process.allowedNodeEnvironmentFlags) {
+    assert.notStrictEqual(flag, 'foo');
+  }
+  for (const [flag] of process.allowedNodeEnvironmentFlags.entries()) {
+    assert.notStrictEqual(flag, 'foo');
+  }
 
   const size = process.allowedNodeEnvironmentFlags.size;
+
+  assert.strictEqual(process.allowedNodeEnvironmentFlags.length, size);
+
+  process.allowedNodeEnvironmentFlags.clear();
+  assert.strictEqual(process.allowedNodeEnvironmentFlags.size, size);
+
   process.allowedNodeEnvironmentFlags.delete('-r');
   assert.strictEqual(process.allowedNodeEnvironmentFlags.size, size);
+
+  assert.throws(() => process.allowedNodeEnvironmentFlags.splice(0),
+                /Cannot delete property/);
+  assert.strictEqual(process.allowedNodeEnvironmentFlags.size, size);
 }