feat: show class names of methods in call stack view (#1802)

* feat: show class names of methods in call stack view Uses the same logic as variable preview generation. Closes #1770 * better formatting for static, fix test
microsoft · Sep 13, 2023 · 65f087c · 65f087c
1 parent bf9c6e6
commit 65f087c
Show file tree

Hide file tree

Showing 44 changed files with 223 additions and 114 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,7 +4,7 @@ This changelog records changes to stable releases since 1.50.2. "TBA" changes he
 
 ## Nightly (only)
 
-Nothing, yet
+- feat: show class names of methods in call stack view ([#1770](https://github.com/microsoft/vscode-js-debug/issues/1770))
 
 ## v1.82 (August 2023)
 

diff --git a/src/adapter/stackTrace.ts b/src/adapter/stackTrace.ts
@@ -12,6 +12,7 @@ import { asyncScopesNotAvailable } from '../dap/errors';
 import { ProtocolError } from '../dap/protocolError';
 import { StackFrameStepOverReason, shouldStepOverStackFrame } from './smartStepping';
 import { IPreferredUiLocation } from './sources';
+import { getToStringIfCustom } from './templates/getStringyProps';
 import { RawLocation, Thread } from './threads';
 import { IExtraProperty, IScopeRef, IVariableContainer } from './variableStore';
 
@@ -226,10 +227,70 @@ export class AsyncSeparator implements IFrameElement {
   }
 }
 
+const fallbackName = '<anonymous>';
+const CLASS_CTOR_RE = /^class\s+(.+) {($|\n)/;
+
+async function getEnhancedName(
+  thread: Thread,
+  callFrame: Cdp.Debugger.CallFrame,
+  useCustomName: boolean,
+) {
+  if (!callFrame.functionName) {
+    // 1. if there's no function name, this cannot be a method. Top-level code in
+    //    a .js file will have a generic "object" scope but no name, so this avoids
+    //    misrepresenting it.
+    return fallbackName;
+  }
+
+  if (callFrame.functionName.includes('.')) {
+    // 2. Some object names are formatted nicely and already contain a method
+    //    access, so skip formatting those.
+    return callFrame.functionName;
+  }
+
+  let objName: string | undefined;
+  if (callFrame.this.objectId && useCustomName) {
+    const ret = await thread.cdp().Runtime.callFunctionOn({
+      functionDeclaration: getToStringIfCustom.decl('64', 'null'),
+      objectId: callFrame.this.objectId,
+      returnByValue: true,
+    });
+    objName = ret?.result.value;
+  }
+
+  if (!objName && callFrame.this.description) {
+    objName = callFrame.this.description;
+
+    // Static methods `this` is described like `class Foo {` -- make that just `Foo`
+    const classCtor = CLASS_CTOR_RE.exec(objName);
+    if (classCtor) {
+      objName = classCtor[1];
+    }
+  }
+  if (!objName) {
+    return callFrame.functionName;
+  }
+
+  const idx = objName.indexOf('\n');
+  if (idx !== -1) {
+    objName = objName.slice(0, idx).trim();
+  }
+
+  const fnName = callFrame.functionName;
+  if (objName === fnName) {
+    return `${objName}.constructor`;
+  }
+
+  return objName ? `${objName}.${fnName}` : fnName;
+}
+
+function getDefaultName(callFrame: Cdp.Debugger.CallFrame | Cdp.Runtime.CallFrame) {
+  return callFrame.functionName || fallbackName;
+}
+
 export class StackFrame implements IFrameElement {
   public readonly frameId = frameIdCounter();
 
-  private _name: string;
   private _rawLocation: RawLocation;
   public readonly uiLocation: () =>
     | Promise<IPreferredUiLocation | undefined>
@@ -271,7 +332,6 @@ export class StackFrame implements IFrameElement {
     rawLocation: RawLocation,
     private readonly isAsync = false,
   ) {
-    this._name = callFrame.functionName || '<anonymous>';
     this._rawLocation = rawLocation;
     this.uiLocation = once(() => thread.rawLocationToUiLocation(rawLocation));
     this._thread = thread;
@@ -387,11 +447,27 @@ export class StackFrame implements IFrameElement {
     return { scopes: scopes.filter(truthy) };
   }
 
+  private readonly getLocationInfo = once(async () => {
+    const uiLocation = this.uiLocation();
+    const isSmartStepped = await shouldStepOverStackFrame(this);
+    // only use the relatively expensive custom tostring lookup for frames
+    // that aren't skipped, to avoid unnecessary work e.g. on node_internals
+    const name =
+      'this' in this.callFrame
+        ? await getEnhancedName(
+            this._thread,
+            this.callFrame,
+            isSmartStepped === StackFrameStepOverReason.NotStepped,
+          )
+        : getDefaultName(this.callFrame);
+
+    return { isSmartStepped, name, uiLocation: await uiLocation };
+  });
+
   /** @inheritdoc */
   async toDap(format?: Dap.StackFrameFormat): Promise<Dap.StackFrame> {
-    const uiLocation = await this.uiLocation();
+    const { isSmartStepped, name, uiLocation } = await this.getLocationInfo();
     const source = uiLocation ? await uiLocation.source.toDap() : undefined;
-    const isSmartStepped = await shouldStepOverStackFrame(this);
     const presentationHint = isSmartStepped ? 'deemphasize' : 'normal';
     if (isSmartStepped && source) {
       source.origin =
@@ -403,7 +479,7 @@ export class StackFrame implements IFrameElement {
     const line = (uiLocation || this._rawLocation).lineNumber;
     const column = (uiLocation || this._rawLocation).columnNumber;
 
-    let formattedName = this._name;
+    let formattedName = name;
 
     if (source && format) {
       if (format.module) {
@@ -430,21 +506,21 @@ export class StackFrame implements IFrameElement {
 
   /** @inheritdoc */
   async formatAsNative(): Promise<string> {
-    const uiLocation = await this.uiLocation();
+    const { name, uiLocation } = await this.getLocationInfo();
     const url =
       (await uiLocation?.source.existingAbsolutePath()) ||
       (await uiLocation?.source.prettyName()) ||
       this.callFrame.url;
     const { lineNumber, columnNumber } = uiLocation || this._rawLocation;
-    return `    at ${this._name} (${url}:${lineNumber}:${columnNumber})`;
+    return `    at ${name} (${url}:${lineNumber}:${columnNumber})`;
   }
 
   /** @inheritdoc */
   async format(): Promise<string> {
-    const uiLocation = await this.uiLocation();
+    const { name, uiLocation } = await this.getLocationInfo();
     const prettyName = (await uiLocation?.source.prettyName()) || '<unknown>';
     const anyLocation = uiLocation || this._rawLocation;
-    let text = `${this._name} @ ${prettyName}:${anyLocation.lineNumber}`;
+    let text = `${name} @ ${prettyName}:${anyLocation.lineNumber}`;
     if (anyLocation.columnNumber > 1) text += `:${anyLocation.columnNumber}`;
     return text;
   }

diff --git a/src/adapter/templates/getStringyProps.ts b/src/adapter/templates/getStringyProps.ts
@@ -94,7 +94,10 @@ export const getToStringIfCustom = templateFunction(function (
 
   if (typeof this === 'object' && this) {
     let str: string | undefined;
-    for (const sym of runtimeArgs[0]) {
+    for (const sym of [
+      Symbol.for(DescriptionSymbols.Generic),
+      Symbol.for(DescriptionSymbols.Node),
+    ]) {
       try {
         str = (this as Record<symbol, () => string>)[sym]();
         break;

diff --git a/src/adapter/variableStore.ts b/src/adapter/variableStore.ts
@@ -791,7 +791,6 @@ class ObjectVariable extends Variable implements IMemoryReadable {
             `${customStringReprMaxLength}`,
             this.context.customDescriptionGenerator || 'null',
           ),
-          arguments: [await this.context.getDescriptionSymbols(this.remoteObject.objectId)],
           objectId: this.remoteObject.objectId,
           returnByValue: true,
         });

diff --git a/src/test/breakpoints/breakpoints-breakpoint-placement-end-function-stmt-tsc.txt b/src/test/breakpoints/breakpoints-breakpoint-placement-end-function-stmt-tsc.txt
@@ -4,4 +4,4 @@
     reason : breakpoint
     threadId : <number>
 }
-foo @ ${workspaceFolder}/sourceMapLocations/test.ts:6:3
+global.foo @ ${workspaceFolder}/sourceMapLocations/test.ts:6:3
diff --git a/src/test/breakpoints/breakpoints-breakpoint-placement-first-function-stmt-tsc.txt b/src/test/breakpoints/breakpoints-breakpoint-placement-first-function-stmt-tsc.txt
@@ -4,4 +4,4 @@
     reason : breakpoint
     threadId : <number>
 }
-foo @ ${workspaceFolder}/sourceMapLocations/test.ts:4:3
+global.foo @ ${workspaceFolder}/sourceMapLocations/test.ts:4:3
diff --git a/src/test/breakpoints/breakpoints-can-step-in-when-first-line-of-code-is-function.txt b/src/test/breakpoints/breakpoints-can-step-in-when-first-line-of-code-is-function.txt
@@ -11,5 +11,5 @@
     reason : step
     threadId : <number>
 }
-double @ ${fixturesDir}/test.js:2:3
+global.double @ ${fixturesDir}/test.js:2:3
 <anonymous> @ ${fixturesDir}/test.js:5:1
diff --git a/src/test/breakpoints/breakpoints-configure-absolute-paths-in-source-maps.txt b/src/test/breakpoints/breakpoints-configure-absolute-paths-in-source-maps.txt
@@ -4,10 +4,10 @@
     reason : breakpoint
     threadId : <number>
 }
-bar @ ${workspaceFolder}/web/browserify/module2.ts:3:3
+Object.bar @ ${workspaceFolder}/web/browserify/module2.ts:3:3
 3../module1 @ ${workspaceFolder}/web/browserify/pause.ts:4:4
-o @ ${workspaceFolder}/node_modules/browser-pack/_prelude.js:1:1
-r @ ${workspaceFolder}/node_modules/browser-pack/_prelude.js:1:1
+Window.o @ ${workspaceFolder}/node_modules/browser-pack/_prelude.js:1:1
+Window.r @ ${workspaceFolder}/node_modules/browser-pack/_prelude.js:1:1
 <anonymous> @ ${workspaceFolder}/node_modules/browser-pack/_prelude.js:1:1
 {
     allThreadsStopped : false
@@ -16,8 +16,8 @@ r @ ${workspaceFolder}/node_modules/browser-pack/_prelude.js:1:1
     threadId : <number>
 }
 foo @ ${workspaceFolder}/web/browserify/module1.ts:3:3
-bar @ ${workspaceFolder}/web/browserify/module2.ts:3:3
+Object.bar @ ${workspaceFolder}/web/browserify/module2.ts:3:3
 3../module1 @ ${workspaceFolder}/web/browserify/pause.ts:4:4
-o @ ${workspaceFolder}/node_modules/browser-pack/_prelude.js:1:1
-r @ ${workspaceFolder}/node_modules/browser-pack/_prelude.js:1:1
+Window.o @ ${workspaceFolder}/node_modules/browser-pack/_prelude.js:1:1
+Window.r @ ${workspaceFolder}/node_modules/browser-pack/_prelude.js:1:1
 <anonymous> @ ${workspaceFolder}/node_modules/browser-pack/_prelude.js:1:1
diff --git a/src/test/breakpoints/breakpoints-configure-remove.txt b/src/test/breakpoints/breakpoints-configure-remove.txt
@@ -4,7 +4,7 @@
     reason : breakpoint
     threadId : <number>
 }
-foo @ ${workspaceFolder}/web/script.js:2:3
+Window.foo @ ${workspaceFolder}/web/script.js:2:3
 <anonymous> @ ${workspaceFolder}/web/script.js:9:1
 {
     allThreadsStopped : false

diff --git a/src/test/breakpoints/breakpoints-configure-source-map-predicted.txt b/src/test/breakpoints/breakpoints-configure-source-map-predicted.txt
@@ -4,10 +4,10 @@
     reason : breakpoint
     threadId : <number>
 }
-bar @ ${workspaceFolder}/web/browserify/module2.ts:3:3
+Object.bar @ ${workspaceFolder}/web/browserify/module2.ts:3:3
 3../module1 @ ${workspaceFolder}/web/browserify/pause.ts:4:4
-o @ ${workspaceFolder}/node_modules/browser-pack/_prelude.js:1:1
-r @ ${workspaceFolder}/node_modules/browser-pack/_prelude.js:1:1
+Window.o @ ${workspaceFolder}/node_modules/browser-pack/_prelude.js:1:1
+Window.r @ ${workspaceFolder}/node_modules/browser-pack/_prelude.js:1:1
 <anonymous> @ ${workspaceFolder}/node_modules/browser-pack/_prelude.js:1:1
 {
     allThreadsStopped : false
@@ -16,8 +16,8 @@ r @ ${workspaceFolder}/node_modules/browser-pack/_prelude.js:1:1
     threadId : <number>
 }
 foo @ ${workspaceFolder}/web/browserify/module1.ts:3:3
-bar @ ${workspaceFolder}/web/browserify/module2.ts:3:3
+Object.bar @ ${workspaceFolder}/web/browserify/module2.ts:3:3
 3../module1 @ ${workspaceFolder}/web/browserify/pause.ts:4:4
-o @ ${workspaceFolder}/node_modules/browser-pack/_prelude.js:1:1
-r @ ${workspaceFolder}/node_modules/browser-pack/_prelude.js:1:1
+Window.o @ ${workspaceFolder}/node_modules/browser-pack/_prelude.js:1:1
+Window.r @ ${workspaceFolder}/node_modules/browser-pack/_prelude.js:1:1
 <anonymous> @ ${workspaceFolder}/node_modules/browser-pack/_prelude.js:1:1
diff --git a/src/test/breakpoints/breakpoints-configure-source-map-thats-path-mapped.txt b/src/test/breakpoints/breakpoints-configure-source-map-thats-path-mapped.txt
@@ -4,5 +4,5 @@
     reason : breakpoint
     threadId : <number>
 }
-foo @ ${workspaceFolder}/web/tmp/app.ts:2:3
+Window.foo @ ${workspaceFolder}/web/tmp/app.ts:2:3
 <anonymous> @ ${workspaceFolder}/web/tmp/app.ts:5:1
diff --git a/src/test/breakpoints/breakpoints-configure-source-map.txt b/src/test/breakpoints/breakpoints-configure-source-map.txt
@@ -4,10 +4,10 @@
     reason : breakpoint
     threadId : <number>
 }
-bar @ ${workspaceFolder}/web/browserify/module2.ts:3:3
+Object.bar @ ${workspaceFolder}/web/browserify/module2.ts:3:3
 3../module1 @ ${workspaceFolder}/web/browserify/pause.ts:4:4
-o @ ${workspaceFolder}/node_modules/browser-pack/_prelude.js:1:1
-r @ ${workspaceFolder}/node_modules/browser-pack/_prelude.js:1:1
+Window.o @ ${workspaceFolder}/node_modules/browser-pack/_prelude.js:1:1
+Window.r @ ${workspaceFolder}/node_modules/browser-pack/_prelude.js:1:1
 <anonymous> @ ${workspaceFolder}/node_modules/browser-pack/_prelude.js:1:1
 {
     allThreadsStopped : false
@@ -16,8 +16,8 @@ r @ ${workspaceFolder}/node_modules/browser-pack/_prelude.js:1:1
     threadId : <number>
 }
 foo @ ${workspaceFolder}/web/browserify/module1.ts:3:3
-bar @ ${workspaceFolder}/web/browserify/module2.ts:3:3
+Object.bar @ ${workspaceFolder}/web/browserify/module2.ts:3:3
 3../module1 @ ${workspaceFolder}/web/browserify/pause.ts:4:4
-o @ ${workspaceFolder}/node_modules/browser-pack/_prelude.js:1:1
-r @ ${workspaceFolder}/node_modules/browser-pack/_prelude.js:1:1
+Window.o @ ${workspaceFolder}/node_modules/browser-pack/_prelude.js:1:1
+Window.r @ ${workspaceFolder}/node_modules/browser-pack/_prelude.js:1:1
 <anonymous> @ ${workspaceFolder}/node_modules/browser-pack/_prelude.js:1:1
diff --git a/...reakpoints/breakpoints-does-not-interrupt-stepin-with-instrumentation-breakpoint-1665.txt b/...reakpoints/breakpoints-does-not-interrupt-stepin-with-instrumentation-breakpoint-1665.txt
@@ -5,15 +5,15 @@ Evaluating#1: test()
     reason : pause
     threadId : <number>
 }
-test @ <eval>/VM<xx>:4:13
+Window.test @ <eval>/VM<xx>:4:13
 <anonymous> @ eval1.js:1:1
 {
     allThreadsStopped : false
     description : Paused
     reason : step
     threadId : <number>
 }
-test @ <eval>/VM<xx>:5:13
+Window.test @ <eval>/VM<xx>:5:13
 <anonymous> @ eval1.js:1:1
 {
     allThreadsStopped : false
@@ -22,5 +22,5 @@ test @ <eval>/VM<xx>:5:13
     threadId : <number>
 }
 <anonymous> @ foo.js:2:15
-test @ <eval>/VM<xx>:5:15
+Window.test @ <eval>/VM<xx>:5:15
 <anonymous> @ eval1.js:1:1
diff --git a/...akpoints/breakpoints-does-not-interrupt-stepover-with-instrumentation-breakpoint-1556.txt b/...akpoints/breakpoints-does-not-interrupt-stepover-with-instrumentation-breakpoint-1556.txt
@@ -5,21 +5,21 @@ Evaluating#1: test()
     reason : pause
     threadId : <number>
 }
-test @ <eval>/VM<xx>:4:13
+Window.test @ <eval>/VM<xx>:4:13
 <anonymous> @ eval1.js:1:1
 {
     allThreadsStopped : false
     description : Paused
     reason : step
     threadId : <number>
 }
-test @ <eval>/VM<xx>:5:13
+Window.test @ <eval>/VM<xx>:5:13
 <anonymous> @ eval1.js:1:1
 {
     allThreadsStopped : false
     description : Paused
     reason : step
     threadId : <number>
 }
-test @ <eval>/VM<xx>:13:13
+Window.test @ <eval>/VM<xx>:13:13
 <anonymous> @ eval1.js:1:1
diff --git a/src/test/breakpoints/breakpoints-excludes-callers.txt b/src/test/breakpoints/breakpoints-excludes-callers.txt
@@ -4,24 +4,24 @@
     reason : pause
     threadId : <number>
 }
-bar @ <eval>/VM<xx>:11:11
-foo @ <eval>/VM<xx>:3:11
+Window.bar @ <eval>/VM<xx>:11:11
+Window.foo @ <eval>/VM<xx>:3:11
 <anonymous> @ <eval>/VM<xx>:14:9
 {
     allThreadsStopped : false
     description : Paused on debugger statement
     reason : pause
     threadId : <number>
 }
-bar @ <eval>/VM<xx>:11:11
-baz @ <eval>/VM<xx>:7:11
+Window.bar @ <eval>/VM<xx>:11:11
+Window.baz @ <eval>/VM<xx>:7:11
 <anonymous> @ <eval>/VM<xx>:15:9
 {
     allThreadsStopped : false
     description : Paused on debugger statement
     reason : pause
     threadId : <number>
 }
-bar @ <eval>/VM<xx>:11:11
-baz @ <eval>/VM<xx>:7:11
+Window.bar @ <eval>/VM<xx>:11:11
+Window.baz @ <eval>/VM<xx>:7:11
 <anonymous> @ <eval>/VM<xx>:17:9
diff --git a/src/test/breakpoints/breakpoints-launched-inline.txt b/src/test/breakpoints/breakpoints-launched-inline.txt
@@ -11,5 +11,5 @@
     reason : breakpoint
     threadId : <number>
 }
-foo @ ${workspaceFolder}/web/inlinescriptpause.html:6:7
+Window.foo @ ${workspaceFolder}/web/inlinescriptpause.html:6:7
 <anonymous> @ ${workspaceFolder}/web/inlinescriptpause.html:9:5
diff --git a/src/test/breakpoints/breakpoints-launched-overwrite.txt b/src/test/breakpoints/breakpoints-launched-overwrite.txt
@@ -4,7 +4,7 @@
     reason : breakpoint
     threadId : <number>
 }
-foo @ <eval>/VM<xx>:3:19
+Window.foo @ <eval>/VM<xx>:3:19
 <anonymous> @ localhost꞉8001/test.js:1:1
 {
     allThreadsStopped : false

diff --git a/src/test/breakpoints/breakpoints-launched-ref.txt b/src/test/breakpoints/breakpoints-launched-ref.txt
@@ -5,5 +5,5 @@ Evaluating#1: foo();
     reason : breakpoint
     threadId : <number>
 }
-foo @ <eval>/VM<xx>:3:11
+Window.foo @ <eval>/VM<xx>:3:11
 <anonymous> @ localhost꞉8001/eval1.js:1:1