lib: refactor primordials.makeSafe to use more primordials

PR-URL: https://github.com/nodejs/node/pull/36865
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

lib/internal/per_context/primordials.js

@@ -6,6 +6,12 @@
 // so that Node.js's builtin modules do not need to later look these up from
 // the global proxy, which can be mutated by users.
 
+const {
+  defineProperty: ReflectDefineProperty,
+  getOwnPropertyDescriptor: ReflectGetOwnPropertyDescriptor,
+  ownKeys: ReflectOwnKeys,
+} = Reflect;
+
 // TODO(joyeecheung): we can restrict access to these globals in builtin
 // modules through the JS linter, for example: ban access such as `Object`
 // (which falls back to a lookup in the global proxy) in favor of
@@ -19,17 +25,6 @@ const { bind, call } = Function.prototype;
 const uncurryThis = bind.bind(call);
 primordials.uncurryThis = uncurryThis;
 
-function copyProps(src, dest) {
-  for (const key of Reflect.ownKeys(src)) {
-    if (!Reflect.getOwnPropertyDescriptor(dest, key)) {
-      Reflect.defineProperty(
-        dest,
-        key,
-        Reflect.getOwnPropertyDescriptor(src, key));
-    }
-  }
-}
-
 function getNewKey(key) {
   return typeof key === 'symbol' ?
     `Symbol${key.description[7].toUpperCase()}${key.description.slice(8)}` :
@@ -37,12 +32,12 @@ function getNewKey(key) {
 }
 
 function copyAccessor(dest, prefix, key, { enumerable, get, set }) {
-  Reflect.defineProperty(dest, `${prefix}Get${key}`, {
+  ReflectDefineProperty(dest, `${prefix}Get${key}`, {
     value: uncurryThis(get),
     enumerable
   });
   if (set !== undefined) {
-    Reflect.defineProperty(dest, `${prefix}Set${key}`, {
+    ReflectDefineProperty(dest, `${prefix}Set${key}`, {
       value: uncurryThis(set),
       enumerable
     });
@@ -50,129 +45,47 @@ function copyAccessor(dest, prefix, key, { enumerable, get, set }) {
 }
 
 function copyPropsRenamed(src, dest, prefix) {
-  for (const key of Reflect.ownKeys(src)) {
+  for (const key of ReflectOwnKeys(src)) {
     const newKey = getNewKey(key);
-    const desc = Reflect.getOwnPropertyDescriptor(src, key);
+    const desc = ReflectGetOwnPropertyDescriptor(src, key);
     if ('get' in desc) {
       copyAccessor(dest, prefix, newKey, desc);
     } else {
-      Reflect.defineProperty(dest, `${prefix}${newKey}`, desc);
+      ReflectDefineProperty(dest, `${prefix}${newKey}`, desc);
     }
   }
 }
 
 function copyPropsRenamedBound(src, dest, prefix) {
-  for (const key of Reflect.ownKeys(src)) {
+  for (const key of ReflectOwnKeys(src)) {
     const newKey = getNewKey(key);
-    const desc = Reflect.getOwnPropertyDescriptor(src, key);
+    const desc = ReflectGetOwnPropertyDescriptor(src, key);
     if ('get' in desc) {
       copyAccessor(dest, prefix, newKey, desc);
     } else {
       if (typeof desc.value === 'function') {
         desc.value = desc.value.bind(src);
       }
-      Reflect.defineProperty(dest, `${prefix}${newKey}`, desc);
+      ReflectDefineProperty(dest, `${prefix}${newKey}`, desc);
     }
   }
 }
 
 function copyPrototype(src, dest, prefix) {
-  for (const key of Reflect.ownKeys(src)) {
+  for (const key of ReflectOwnKeys(src)) {
     const newKey = getNewKey(key);
-    const desc = Reflect.getOwnPropertyDescriptor(src, key);
+    const desc = ReflectGetOwnPropertyDescriptor(src, key);
     if ('get' in desc) {
       copyAccessor(dest, prefix, newKey, desc);
     } else {
       if (typeof desc.value === 'function') {
         desc.value = uncurryThis(desc.value);
       }
-      Reflect.defineProperty(dest, `${prefix}${newKey}`, desc);
+      ReflectDefineProperty(dest, `${prefix}${newKey}`, desc);
     }
   }
 }
 
-const createSafeIterator = (factory, next) => {
-  class SafeIterator {
-    constructor(iterable) {
-      this._iterator = factory(iterable);
-    }
-    next() {
-      return next(this._iterator);
-    }
-    [Symbol.iterator]() {
-      return this;
-    }
-  }
-  Object.setPrototypeOf(SafeIterator.prototype, null);
-  Object.freeze(SafeIterator.prototype);
-  Object.freeze(SafeIterator);
-  return SafeIterator;
-};
-
-function makeSafe(unsafe, safe) {
-  if (Symbol.iterator in unsafe.prototype) {
-    const dummy = new unsafe();
-    let next; // We can reuse the same `next` method.
-
-    for (const key of Reflect.ownKeys(unsafe.prototype)) {
-      if (!Reflect.getOwnPropertyDescriptor(safe.prototype, key)) {
-        const desc = Reflect.getOwnPropertyDescriptor(unsafe.prototype, key);
-        if (
-          typeof desc.value === 'function' &&
-          desc.value.length === 0 &&
-          Symbol.iterator in (desc.value.call(dummy) ?? {})
-        ) {
-          const createIterator = uncurryThis(desc.value);
-          next ??= uncurryThis(createIterator(dummy).next);
-          const SafeIterator = createSafeIterator(createIterator, next);
-          desc.value = function() {
-            return new SafeIterator(this);
-          };
-        }
-        Reflect.defineProperty(safe.prototype, key, desc);
-      }
-    }
-  } else {
-    copyProps(unsafe.prototype, safe.prototype);
-  }
-  copyProps(unsafe, safe);
-
-  Object.setPrototypeOf(safe.prototype, null);
-  Object.freeze(safe.prototype);
-  Object.freeze(safe);
-  return safe;
-}
-primordials.makeSafe = makeSafe;
-
-// Subclass the constructors because we need to use their prototype
-// methods later.
-// Defining the `constructor` is necessary here to avoid the default
-// constructor which uses the user-mutable `%ArrayIteratorPrototype%.next`.
-primordials.SafeMap = makeSafe(
-  Map,
-  class SafeMap extends Map {
-    constructor(i) { super(i); } // eslint-disable-line no-useless-constructor
-  }
-);
-primordials.SafeWeakMap = makeSafe(
-  WeakMap,
-  class SafeWeakMap extends WeakMap {
-    constructor(i) { super(i); } // eslint-disable-line no-useless-constructor
-  }
-);
-primordials.SafeSet = makeSafe(
-  Set,
-  class SafeSet extends Set {
-    constructor(i) { super(i); } // eslint-disable-line no-useless-constructor
-  }
-);
-primordials.SafeWeakSet = makeSafe(
-  WeakSet,
-  class SafeWeakSet extends WeakSet {
-    constructor(i) { super(i); } // eslint-disable-line no-useless-constructor
-  }
-);
-
 // Create copies of the namespace objects
 [
   'JSON',
@@ -256,6 +169,41 @@ primordials.SafeWeakSet = makeSafe(
   copyPrototype(original.prototype, primordials, `${name}Prototype`);
 });
 
+/* eslint-enable node-core/prefer-primordials */
+
+const {
+  ArrayPrototypeForEach,
+  FunctionPrototypeCall,
+  Map,
+  ObjectFreeze,
+  ObjectSetPrototypeOf,
+  Set,
+  SymbolIterator,
+  WeakMap,
+  WeakSet,
+} = primordials;
+
+// Because these functions are used by `makeSafe`, which is exposed
+// on the `primordials` object, it's important to use const references
+// to the primordials that they use:
+const createSafeIterator = (factory, next) => {
+  class SafeIterator {
+    constructor(iterable) {
+      this._iterator = factory(iterable);
+    }
+    next() {
+      return next(this._iterator);
+    }
+    [SymbolIterator]() {
+      return this;
+    }
+  }
+  ObjectSetPrototypeOf(SafeIterator.prototype, null);
+  ObjectFreeze(SafeIterator.prototype);
+  ObjectFreeze(SafeIterator);
+  return SafeIterator;
+};
+
 primordials.SafeArrayIterator = createSafeIterator(
   primordials.ArrayPrototypeSymbolIterator,
   primordials.ArrayIteratorPrototypeNext
@@ -265,5 +213,80 @@ primordials.SafeStringIterator = createSafeIterator(
   primordials.StringIteratorPrototypeNext
 );
 
-Object.setPrototypeOf(primordials, null);
-Object.freeze(primordials);
+const copyProps = (src, dest) => {
+  ArrayPrototypeForEach(ReflectOwnKeys(src), (key) => {
+    if (!ReflectGetOwnPropertyDescriptor(dest, key)) {
+      ReflectDefineProperty(
+        dest,
+        key,
+        ReflectGetOwnPropertyDescriptor(src, key));
+    }
+  });
+};
+
+const makeSafe = (unsafe, safe) => {
+  if (SymbolIterator in unsafe.prototype) {
+    const dummy = new unsafe();
+    let next; // We can reuse the same `next` method.
+
+    ArrayPrototypeForEach(ReflectOwnKeys(unsafe.prototype), (key) => {
+      if (!ReflectGetOwnPropertyDescriptor(safe.prototype, key)) {
+        const desc = ReflectGetOwnPropertyDescriptor(unsafe.prototype, key);
+        if (
+          typeof desc.value === 'function' &&
+          desc.value.length === 0 &&
+          SymbolIterator in (FunctionPrototypeCall(desc.value, dummy) ?? {})
+        ) {
+          const createIterator = uncurryThis(desc.value);
+          next ??= uncurryThis(createIterator(dummy).next);
+          const SafeIterator = createSafeIterator(createIterator, next);
+          desc.value = function() {
+            return new SafeIterator(this);
+          };
+        }
+        ReflectDefineProperty(safe.prototype, key, desc);
+      }
+    });
+  } else {
+    copyProps(unsafe.prototype, safe.prototype);
+  }
+  copyProps(unsafe, safe);
+
+  ObjectSetPrototypeOf(safe.prototype, null);
+  ObjectFreeze(safe.prototype);
+  ObjectFreeze(safe);
+  return safe;
+};
+primordials.makeSafe = makeSafe;
+
+// Subclass the constructors because we need to use their prototype
+// methods later.
+// Defining the `constructor` is necessary here to avoid the default
+// constructor which uses the user-mutable `%ArrayIteratorPrototype%.next`.
+primordials.SafeMap = makeSafe(
+  Map,
+  class SafeMap extends Map {
+    constructor(i) { super(i); } // eslint-disable-line no-useless-constructor
+  }
+);
+primordials.SafeWeakMap = makeSafe(
+  WeakMap,
+  class SafeWeakMap extends WeakMap {
+    constructor(i) { super(i); } // eslint-disable-line no-useless-constructor
+  }
+);
+primordials.SafeSet = makeSafe(
+  Set,
+  class SafeSet extends Set {
+    constructor(i) { super(i); } // eslint-disable-line no-useless-constructor
+  }
+);
+primordials.SafeWeakSet = makeSafe(
+  WeakSet,
+  class SafeWeakSet extends WeakSet {
+    constructor(i) { super(i); } // eslint-disable-line no-useless-constructor
+  }
+);
+
+ObjectSetPrototypeOf(primordials, null);
+ObjectFreeze(primordials);