lib: add lint rules for reflective function calls

PR-URL: https://github.com/nodejs/node/pull/60825
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Jordan Harband <ljharb@gmail.com>
Reviewed-By: LiviaMedeiros <livia@cirno.name>

lib/_http_server.js

@@ -672,8 +672,9 @@ assignFunctionName(EE.captureRejectionSymbol, function(err, event, ...args) {
       break;
     }
     default:
-      net.Server.prototype[SymbolFor('nodejs.rejection')]
+      ReflectApply(
-        .apply(this, arguments);
+        net.Server.prototype[SymbolFor('nodejs.rejection')],
+        this, arguments);
   }
 });
 

lib/eslint.config_partial.mjs

@@ -38,6 +38,10 @@ const noRestrictedSyntax = [
     selector: "ThrowStatement > NewExpression[callee.name=/^ERR_[A-Z_]+$/] > ObjectExpression:first-child:not(:has([key.name='message']):has([key.name='code']):has([key.name='syscall']))",
     message: 'The context passed into the SystemError constructor must include .code, .syscall, and .message properties.',
   },
+  {
+    selector: "CallExpression:matches([callee.type='Identifier'][callee.name='FunctionPrototypeApply'], [callee.type='MemberExpression'][callee.property.type='Identifier'][callee.property.name='apply'][arguments.length=2])",
+    message: 'Use `ReflectApply` instead of %Function.prototype.apply%',
+  },
 ];
 
 export default [
@@ -57,7 +61,13 @@ export default [
     rules: {
       'prefer-object-spread': 'error',
       'no-buffer-constructor': 'error',
-      'no-restricted-syntax': noRestrictedSyntax,
+      'no-restricted-syntax': [
+        ...noRestrictedSyntax,
+        {
+          selector: "CallExpression[callee.type='Identifier'][callee.name='ReflectApply'][arguments.2.type='ArrayExpression']",
+          message: 'Use `FunctionPrototypeCall` to avoid creating an ad-hoc array',
+        },
+      ],
       'no-restricted-globals': [
         'error',
         {

lib/events.js

@@ -505,7 +505,7 @@ EventEmitter.prototype.emit = function emit(type, ...args) {
     return false;
 
   if (typeof handler === 'function') {
-    const result = handler.apply(this, args);
+    const result = ReflectApply(handler, this, args);
 
     // We check if result is undefined first because that
     // is the most common case so we do not pay any perf
@@ -517,7 +517,7 @@ EventEmitter.prototype.emit = function emit(type, ...args) {
     const len = handler.length;
     const listeners = arrayClone(handler);
     for (let i = 0; i < len; ++i) {
-      const result = listeners[i].apply(this, args);
+      const result = ReflectApply(listeners[i], this, args);
 
       // We check if result is undefined first because that
       // is the most common case so we do not pay any perf
@@ -620,7 +620,7 @@ function onceWrapper() {
     this.fired = true;
     if (arguments.length === 0)
       return this.listener.call(this.target);
-    return this.listener.apply(this.target, arguments);
+    return ReflectApply(this.listener, this.target, arguments);
   }
 }
 

lib/internal/async_hooks.js

@@ -5,6 +5,7 @@ const {
   ErrorCaptureStackTrace,
   ObjectDefineProperty,
   ObjectPrototypeHasOwnProperty,
+  ReflectApply,
   Symbol,
 } = primordials;
 
@@ -125,9 +126,9 @@ function callbackTrampoline(asyncId, resource, cb, ...args) {
   let result;
   if (asyncId === 0 && typeof domain_cb === 'function') {
     args.unshift(cb);
-    result = domain_cb.apply(this, args);
+    result = ReflectApply(domain_cb, this, args);
   } else {
-    result = cb.apply(this, args);
+    result = ReflectApply(cb, this, args);
   }
 
   if (asyncId !== 0 && hasHooks(kAfter))
@@ -462,14 +463,14 @@ function clearDefaultTriggerAsyncId() {
  */
 function defaultTriggerAsyncIdScope(triggerAsyncId, block, ...args) {
   if (triggerAsyncId === undefined)
-    return block.apply(null, args);
+    return ReflectApply(block, null, args);
   // CHECK(NumberIsSafeInteger(triggerAsyncId))
   // CHECK(triggerAsyncId > 0)
   const oldDefaultTriggerAsyncId = async_id_fields[kDefaultTriggerAsyncId];
   async_id_fields[kDefaultTriggerAsyncId] = triggerAsyncId;
 
   try {
-    return block.apply(null, args);
+    return ReflectApply(block, null, args);
   } finally {
     async_id_fields[kDefaultTriggerAsyncId] = oldDefaultTriggerAsyncId;
   }

lib/internal/modules/esm/translators.js

@@ -6,7 +6,6 @@ const {
   JSONParse,
   ObjectAssign,
   ObjectPrototypeHasOwnProperty,
-  ReflectApply,
   SafeArrayIterator,
   SafeMap,
   SafeSet,
@@ -198,8 +197,8 @@ function loadCJSModule(module, source, url, filename, isMain) {
   });
   setOwnProperty(requireFn, 'main', process.mainModule);
 
-  ReflectApply(compiledWrapper, module.exports,
+  FunctionPrototypeCall(compiledWrapper, module.exports,
-               [module.exports, requireFn, module, filename, __dirname]);
+                        module.exports, requireFn, module, filename, __dirname);
   setOwnProperty(module, 'loaded', true);
 }
 

lib/internal/streams/end-of-stream.js

@@ -6,6 +6,7 @@
 const {
   Promise,
   PromisePrototypeThen,
+  ReflectApply,
   SymbolDispose,
 } = primordials;
 
@@ -280,7 +281,7 @@ function eos(stream, options, callback) {
       const originalCallback = callback;
       callback = once((...args) => {
         disposable[SymbolDispose]();
-        originalCallback.apply(stream, args);
+        ReflectApply(originalCallback, stream, args);
       });
     }
   }
@@ -304,13 +305,13 @@ function eosWeb(stream, options, callback) {
       const originalCallback = callback;
       callback = once((...args) => {
         disposable[SymbolDispose]();
-        originalCallback.apply(stream, args);
+        ReflectApply(originalCallback, stream, args);
       });
     }
   }
   const resolverFn = (...args) => {
     if (!isAborted) {
-      process.nextTick(() => callback.apply(stream, args));
+      process.nextTick(() => ReflectApply(callback, stream, args));
     }
   };
   PromisePrototypeThen(

lib/internal/streams/readable.js

@@ -30,6 +30,7 @@ const {
   ObjectKeys,
   ObjectSetPrototypeOf,
   Promise,
+  ReflectApply,
   SafeSet,
   Symbol,
   SymbolAsyncDispose,
@@ -1182,8 +1183,7 @@ Readable.prototype.removeListener = function(ev, fn) {
 Readable.prototype.off = Readable.prototype.removeListener;
 
 Readable.prototype.removeAllListeners = function(ev) {
-  const res = Stream.prototype.removeAllListeners.apply(this,
+  const res = ReflectApply(Stream.prototype.removeAllListeners, this, arguments);
-                                                        arguments);
 
   if (ev === 'readable' || ev === undefined) {
     // We need to check if there is someone still listening to

lib/internal/test_runner/mock/mock_timers.js

@@ -5,7 +5,6 @@ const {
   ArrayPrototypeIncludes,
   DatePrototypeGetTime,
   DatePrototypeToString,
-  FunctionPrototypeApply,
   FunctionPrototypeBind,
   FunctionPrototypeToString,
   NumberIsNaN,
@@ -14,6 +13,7 @@ const {
   ObjectGetOwnPropertyDescriptor,
   ObjectGetOwnPropertyDescriptors,
   PromiseWithResolvers,
+  ReflectApply,
   Symbol,
   SymbolDispose,
   globalThis,
@@ -645,7 +645,7 @@ class MockTimers {
     let timer = this.#executionQueue.peek();
     while (timer) {
       if (timer.runAt > this.#now) break;
-      FunctionPrototypeApply(timer.callback, undefined, timer.args);
+      ReflectApply(timer.callback, undefined, timer.args);
 
       // Check if the timeout was cleared by calling clearTimeout inside its own callback
       const afterCallback = this.#executionQueue.peek();