util: fix util.getCallSites plurality

`util.getCallSite` returns an array of call site objects. Rename the function to reflect that it returns a given count of frames captured as an array of call site object. Renames the first parameter `frames` to be `frameCount` to indicate that it specifies the count of returned call sites. PR-URL: nodejs#55626 Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Marco Ippolito <[email protected]>
tpoisseau · Nov 21, 2024 · 6ea7cc0 · 6ea7cc0
1 parent 4aedd9f
commit 6ea7cc0
Show file tree

Hide file tree

Showing 10 changed files with 89 additions and 61 deletions.
diff --git a/benchmark/util/get-callsite.js b/benchmark/util/get-callsite.js
@@ -1,15 +1,15 @@
 'use strict';
 
 const common = require('../common');
-const { getCallSite } = require('node:util');
+const { getCallSites } = require('node:util');
 const assert = require('node:assert');
 
 const bench = common.createBenchmark(main, {
   n: [1e6],
-  method: ['ErrorCallSite', 'ErrorCallSiteSerialized', 'CPP'],
+  method: ['ErrorCallSites', 'ErrorCallSitesSerialized', 'CPP'],
 });
 
-function ErrorGetCallSite() {
+function ErrorGetCallSites() {
   const originalStackFormatter = Error.prepareStackTrace;
   Error.prepareStackTrace = (_err, stack) => {
     if (stack && stack.length > 1) {
@@ -25,15 +25,15 @@ function ErrorGetCallSite() {
   return err.stack;
 }
 
-function ErrorCallSiteSerialized() {
-  const callsite = ErrorGetCallSite();
+function ErrorCallSitesSerialized() {
+  const callSites = ErrorGetCallSites();
   const serialized = [];
-  for (let i = 0; i < callsite.length; ++i) {
+  for (let i = 0; i < callSites.length; ++i) {
     serialized.push({
-      functionName: callsite[i].getFunctionName(),
-      scriptName: callsite[i].getFileName(),
-      lineNumber: callsite[i].getLineNumber(),
-      column: callsite[i].getColumnNumber(),
+      functionName: callSites[i].getFunctionName(),
+      scriptName: callSites[i].getFileName(),
+      lineNumber: callSites[i].getLineNumber(),
+      column: callSites[i].getColumnNumber(),
     });
   }
   return serialized;
@@ -42,14 +42,14 @@ function ErrorCallSiteSerialized() {
 function main({ n, method }) {
   let fn;
   switch (method) {
-    case 'ErrorCallSite':
-      fn = ErrorGetCallSite;
+    case 'ErrorCallSites':
+      fn = ErrorGetCallSites;
       break;
-    case 'ErrorCallSiteSerialized':
-      fn = ErrorCallSiteSerialized;
+    case 'ErrorCallSitesSerialized':
+      fn = ErrorCallSitesSerialized;
       break;
     case 'CPP':
-      fn = getCallSite;
+      fn = getCallSites;
       break;
   }
   let lastStack = {};

diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md
@@ -3761,6 +3761,19 @@ Instantiating classes without the `new` qualifier exported by the `node:repl` mo
 It is recommended to use the `new` qualifier instead. This applies to all REPL classes, including
 `REPLServer` and `Recoverable`.
 
+### DEP0186: `util.getCallSite`
+
+<!-- YAML
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/55626
+    description: Runtime deprecation.
+-->
+
+Type: Runtime
+
+The `util.getCallSite` API has been removed. Please use [`util.getCallSites()`][] instead.
+
 [NIST SP 800-38D]: https://nvlpubs.nist.gov/nistpubs/Legacy/SP/nistspecialpublication800-38d.pdf
 [RFC 6066]: https://tools.ietf.org/html/rfc6066#section-3
 [RFC 8247 Section 2.4]: https://www.rfc-editor.org/rfc/rfc8247#section-2.4
@@ -3886,6 +3899,7 @@ It is recommended to use the `new` qualifier instead. This applies to all REPL c
 [`url.parse()`]: url.md#urlparseurlstring-parsequerystring-slashesdenotehost
 [`url.resolve()`]: url.md#urlresolvefrom-to
 [`util._extend()`]: util.md#util_extendtarget-source
+[`util.getCallSites()`]: util.md#utilgetcallsitesframecount
 [`util.getSystemErrorName()`]: util.md#utilgetsystemerrornameerr
 [`util.inspect()`]: util.md#utilinspectobject-options
 [`util.inspect.custom`]: util.md#utilinspectcustom

diff --git a/doc/api/util.md b/doc/api/util.md
@@ -364,31 +364,31 @@ util.formatWithOptions({ colors: true }, 'See object %O', { foo: 42 });
 // when printed to a terminal.
 ```
 
-## `util.getCallSite(frames)`
+## `util.getCallSites(frameCount)`
 
 > Stability: 1.1 - Active development
 
 <!-- YAML
 added: v22.9.0
 -->
 
-* `frames` {number} Number of frames returned in the stacktrace.
+* `frameCount` {number} Number of frames to capture as call site objects.
   **Default:** `10`. Allowable range is between 1 and 200.
-* Returns: {Object\[]} An array of stacktrace objects
-  * `functionName` {string} Returns the name of the function associated with this stack frame.
+* Returns: {Object\[]} An array of call site objects
+  * `functionName` {string} Returns the name of the function associated with this call site.
   * `scriptName` {string} Returns the name of the resource that contains the script for the
-    function for this StackFrame.
+    function for this call site.
   * `lineNumber` {number} Returns the number, 1-based, of the line for the associate function call.
   * `column` {number} Returns the 1-based column offset on the line for the associated function call.
 
-Returns an array of stacktrace objects containing the stack of
+Returns an array of call site objects containing the stack of
 the caller function.
 
 ```js
 const util = require('node:util');
 
 function exampleFunction() {
-  const callSites = util.getCallSite();
+  const callSites = util.getCallSites();
 
   console.log('Call Sites:');
   callSites.forEach((callSite, index) => {

diff --git a/lib/util.js b/lib/util.js
@@ -330,13 +330,13 @@ function parseEnv(content) {
 
 /**
  * Returns the callSite
- * @param {number} frames
+ * @param {number} frameCount
  * @returns {object}
  */
-function getCallSite(frames = 10) {
+function getCallSites(frameCount = 10) {
   // Using kDefaultMaxCallStackSizeToCapture as reference
-  validateNumber(frames, 'frames', 1, 200);
-  return binding.getCallSite(frames);
+  validateNumber(frameCount, 'frameCount', 1, 200);
+  return binding.getCallSites(frameCount);
 };
 
 // Keep the `exports =` so that various functions can still be monkeypatched
@@ -353,7 +353,12 @@ module.exports = {
   format,
   styleText,
   formatWithOptions,
-  getCallSite,
+  // Deprecated getCallSite.
+  // This API can be removed in next semver-minor release.
+  getCallSite: deprecate(getCallSites,
+                         'The `util.getCallSite` API is deprecated. Please use `util.getCallSites()` instead.',
+                         'DEP0186'),
+  getCallSites,
   getSystemErrorMap,
   getSystemErrorName,
   getSystemErrorMessage,

diff --git a/src/node_util.cc b/src/node_util.cc
@@ -247,7 +247,7 @@ static void ParseEnv(const FunctionCallbackInfo<Value>& args) {
   args.GetReturnValue().Set(dotenv.ToObject(env));
 }
 
-static void GetCallSite(const FunctionCallbackInfo<Value>& args) {
+static void GetCallSites(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
   Isolate* isolate = env->isolate();
 
@@ -345,7 +345,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
   registry->Register(GetProxyDetails);
   registry->Register(GetCallerLocation);
   registry->Register(PreviewEntries);
-  registry->Register(GetCallSite);
+  registry->Register(GetCallSites);
   registry->Register(GetOwnNonIndexProperties);
   registry->Register(GetConstructorName);
   registry->Register(GetExternalValue);
@@ -451,7 +451,7 @@ void Initialize(Local<Object> target,
   SetMethodNoSideEffect(
       context, target, "getConstructorName", GetConstructorName);
   SetMethodNoSideEffect(context, target, "getExternalValue", GetExternalValue);
-  SetMethodNoSideEffect(context, target, "getCallSite", GetCallSite);
+  SetMethodNoSideEffect(context, target, "getCallSites", GetCallSites);
   SetMethod(context, target, "sleep", Sleep);
   SetMethod(context, target, "parseEnv", ParseEnv);
 

diff --git a/test/common/index.js b/test/common/index.js
@@ -30,7 +30,7 @@ const net = require('net');
 // Do not require 'os' until needed so that test-os-checked-function can
 // monkey patch it. If 'os' is required here, that test will fail.
 const path = require('path');
-const { inspect, getCallSite } = require('util');
+const { inspect, getCallSites } = require('util');
 const { isMainThread } = require('worker_threads');
 const { isModuleNamespaceObject } = require('util/types');
 
@@ -550,7 +550,7 @@ function canCreateSymLink() {
 }
 
 function mustNotCall(msg) {
-  const callSite = getCallSite()[1];
+  const callSite = getCallSites()[1];
   return function mustNotCall(...args) {
     const argsInfo = args.length > 0 ?
       `\ncalled with arguments: ${args.map((arg) => inspect(arg)).join(', ')}` : '';

diff --git a/test/fixtures/get-call-site.js b/test/fixtures/get-call-site.js
diff --git a/test/fixtures/get-call-sites.js b/test/fixtures/get-call-sites.js
@@ -0,0 +1,4 @@
+const util = require('node:util');
+const assert = require('node:assert');
+assert.ok(util.getCallSites().length > 1);
+process.stdout.write(util.getCallSites()[0].scriptName);
diff --git a/test/parallel/test-util-getcallsite.js b/test/parallel/test-util-getcallsite.js
@@ -0,0 +1,9 @@
+'use strict';
+
+require('../common');
+const { getCallSite } = require('node:util');
+const { expectWarning } = require('../common');
+
+const warning = 'The `util.getCallSite` API is deprecated. Please use `util.getCallSites()` instead.';
+expectWarning('DeprecationWarning', warning, 'DEP0186');
+getCallSite();
diff --git a/test/parallel/test-util-getCallSite.js → test/parallel/test-util-getcallsites.js b/test/parallel/test-util-getCallSite.js → test/parallel/test-util-getcallsites.js
@@ -3,68 +3,68 @@
 const common = require('../common');
 
 const fixtures = require('../common/fixtures');
-const file = fixtures.path('get-call-site.js');
+const file = fixtures.path('get-call-sites.js');
 
-const { getCallSite } = require('node:util');
+const { getCallSites } = require('node:util');
 const { spawnSync } = require('node:child_process');
 const assert = require('node:assert');
 
 {
-  const callsite = getCallSite();
-  assert.ok(callsite.length > 1);
+  const callSites = getCallSites();
+  assert.ok(callSites.length > 1);
   assert.match(
-    callsite[0].scriptName,
-    /test-util-getCallSite/,
+    callSites[0].scriptName,
+    /test-util-getcallsites/,
     'node:util should be ignored',
   );
 }
 
 {
-  const callsite = getCallSite(3);
-  assert.strictEqual(callsite.length, 3);
+  const callSites = getCallSites(3);
+  assert.strictEqual(callSites.length, 3);
   assert.match(
-    callsite[0].scriptName,
-    /test-util-getCallSite/,
+    callSites[0].scriptName,
+    /test-util-getcallsites/,
     'node:util should be ignored',
   );
 }
 
 // Guarantee dot-left numbers are ignored
 {
-  const callsite = getCallSite(3.6);
-  assert.strictEqual(callsite.length, 3);
+  const callSites = getCallSites(3.6);
+  assert.strictEqual(callSites.length, 3);
 }
 
 {
-  const callsite = getCallSite(3.4);
-  assert.strictEqual(callsite.length, 3);
+  const callSites = getCallSites(3.4);
+  assert.strictEqual(callSites.length, 3);
 }
 
 {
   assert.throws(() => {
     // Max than kDefaultMaxCallStackSizeToCapture
-    getCallSite(201);
+    getCallSites(201);
   }, common.expectsError({
     code: 'ERR_OUT_OF_RANGE'
   }));
   assert.throws(() => {
-    getCallSite(-1);
+    getCallSites(-1);
   }, common.expectsError({
     code: 'ERR_OUT_OF_RANGE'
   }));
   assert.throws(() => {
-    getCallSite({});
+    getCallSites({});
   }, common.expectsError({
     code: 'ERR_INVALID_ARG_TYPE'
   }));
 }
 
 {
-  const callsite = getCallSite(1);
-  assert.strictEqual(callsite.length, 1);
+  const callSites = getCallSites(1);
+  assert.strictEqual(callSites.length, 1);
   assert.match(
-    callsite[0].scriptName,
-    /test-util-getCallSite/,
+    callSites[0].scriptName,
+    /test-util-getcallsites/,
     'node:util should be ignored',
   );
 }
@@ -77,8 +77,8 @@ const assert = require('node:assert');
       '-e',
       `const util = require('util');
        const assert = require('assert');
-       assert.ok(util.getCallSite().length > 1);
-       process.stdout.write(util.getCallSite()[0].scriptName);
+       assert.ok(util.getCallSites().length > 1);
+       process.stdout.write(util.getCallSites()[0].scriptName);
       `,
     ],
   );
@@ -100,7 +100,7 @@ const assert = require('node:assert');
 {
   const originalStackTraceLimit = Error.stackTraceLimit;
   Error.stackTraceLimit = 0;
-  const callsite = getCallSite();
-  assert.notStrictEqual(callsite.length, 0);
+  const callSites = getCallSites();
+  assert.notStrictEqual(callSites.length, 0);
   Error.stackTraceLimit = originalStackTraceLimit;
 }