Merge pull request #1307 from microsoft/feat/step-in-targets

feat: implement step in targets

Author: connor4312

CHANGELOG.md

@@ -8,6 +8,7 @@ This changelog records changes to stable releases since 1.50.2. "TBA" changes he
 - feat: add a new **Debug: Save Diagnostic JS Debug Logs** command ([#1301](https://github.com/microsoft/vscode-js-debug/issues/1301))
 - feat: use custom `toString()` methods to generate object descriptions ([#1284](https://github.com/microsoft/vscode-js-debug/issues/1284))
 - feat: allow easy toggling between compiled and sourcemapped sources ([vscode#151412](https://github.com/microsoft/vscode/issues/151412))
+- feat: implement step in targets ([vscode#123879](https://github.com/microsoft/vscode/issues/123879))
 - fix: debugged child processes in ext host causing teardown ([#1289](https://github.com/microsoft/vscode-js-debug/issues/1289))
 - fix: errors thrown in process tree lookup not being visible ([vscode#150754](https://github.com/microsoft/vscode/issues/150754))
 - fix: extension debugging not working with two ipv6 interfaces ([vscode#144315](https://github.com/microsoft/vscode/issues/144315))

src/adapter/breakpoints.ts

@@ -6,6 +6,7 @@ import { inject, injectable } from 'inversify';
 import Cdp from '../cdp/api';
 import { ILogger, LogTag } from '../common/logging';
 import { bisectArray, flatten } from '../common/objUtils';
+import { IPosition } from '../common/positions';
 import { delay } from '../common/promiseUtil';
 import { SourceMap } from '../common/sourceMaps/sourceMap';
 import * as urlUtils from '../common/urlUtils';
@@ -73,6 +74,11 @@ export const enum EntryBreakpointMode {
   Greedy,
 }
 
+export interface IPossibleBreakLocation {
+  uiLocations: IUiLocation[];
+  breakLocation: Cdp.Debugger.BreakLocation;
+}
+
 @injectable()
 export class BreakpointManager {
   _dap: Dap.Api;
@@ -287,25 +293,22 @@ export class BreakpointManager {
    */
   public async getBreakpointLocations(
     thread: Thread,
-    request: Dap.BreakpointLocationsParams,
-  ): Promise<Dap.BreakpointLocation[]> {
-    // Find the source we're querying in, then resolve all possibly sourcemapped
-    // locations for that script.
-    const source = this._sourceContainer.source(request.source);
-    if (!source) {
-      return [];
-    }
-
+    source: Source,
+    start: IPosition,
+    end: IPosition,
+  ) {
+    const start1 = start.base1;
     const startLocations = this._sourceContainer.currentSiblingUiLocations({
       source,
-      lineNumber: request.line,
-      columnNumber: request.column === undefined ? 1 : request.column,
+      lineNumber: start1.lineNumber,
+      columnNumber: start1.columnNumber,
     });
 
+    const end1 = end.base1;
     const endLocations = this._sourceContainer.currentSiblingUiLocations({
       source,
-      lineNumber: request.endLine === undefined ? request.line + 1 : request.endLine,
-      columnNumber: request.endColumn === undefined ? 1 : request.endColumn,
+      lineNumber: end1.lineNumber,
+      columnNumber: end1.columnNumber,
     });
 
     // As far as I know the number of start and end locations should be the
@@ -321,7 +324,8 @@ export class BreakpointManager {
     // For each viable location, attempt to identify its script ID and then ask
     // Chrome for the breakpoints in the given range. For almost all scripts
     // we'll only every find one viable location with a script.
-    const todo: Promise<Dap.BreakpointLocation[]>[] = [];
+    const todo: Promise<void>[] = [];
+    const result: IPossibleBreakLocation[] = [];
     for (let i = 0; i < startLocations.length; i++) {
       const start = startLocations[i];
       const end = endLocations[i];
@@ -354,39 +358,28 @@ export class BreakpointManager {
           })
           .then(r => {
             if (!r) {
-              return [];
+              return;
             }
 
             // Map the locations from CDP back to their original source positions.
             // Discard any that map outside of the source we're interested in,
             // which is possible (e.g. if a section of code from one source is
             // inlined amongst the range we request).
-            const result: Dap.BreakpointLocation[] = [];
-            for (const location of r.locations) {
-              const { lineNumber, columnNumber = 0 } = location;
-              const sourceLocations = this._sourceContainer.currentSiblingUiLocations(
-                {
-                  source: lsrc,
-                  ...rawToUiOffset(
-                    base0To1({ lineNumber, columnNumber }),
-                    lsrc.runtimeScriptOffset,
-                  ),
-                },
-                source,
-              );
-
-              for (const srcLocation of sourceLocations) {
-                result.push({ line: srcLocation.lineNumber, column: srcLocation.columnNumber });
-              }
+            for (const breakLocation of r.locations) {
+              const { lineNumber, columnNumber = 0 } = breakLocation;
+              const uiLocations = this._sourceContainer.currentSiblingUiLocations({
+                source: lsrc,
+                ...rawToUiOffset(base0To1({ lineNumber, columnNumber }), lsrc.runtimeScriptOffset),
+              });
+
+              result.push({ breakLocation, uiLocations });
             }
-
-            return result;
           }),
       );
     }
+    await Promise.all(todo);
 
-    // Gather our results and flatten the array.
-    return (await Promise.all(todo)).reduce((acc, r) => [...acc, ...r], []);
+    return result;
   }
 
   /**

src/adapter/debugAdapter.ts

@@ -7,6 +7,8 @@ import * as nls from 'vscode-nls';
 import { Cdp } from '../cdp/api';
 import { DisposableList, IDisposable } from '../common/disposable';
 import { ILogger, LogTag } from '../common/logging';
+import { posInt32Counter, truthy } from '../common/objUtils';
+import { Base1Position } from '../common/positions';
 import { getDeferred, IDeferred } from '../common/promiseUtil';
 import { IRenameProvider } from '../common/sourceMaps/renameProvider';
 import * as sourceUtils from '../common/sourceUtils';
@@ -50,7 +52,7 @@ export class DebugAdapter implements IDisposable {
   private _thread: Thread | undefined;
   private _threadDeferred = getDeferred<Thread>();
   private _configurationDoneDeferred: IDeferred<void>;
-  private lastBreakpointId = 0;
+  private breakpointIdCounter = posInt32Counter();
   private readonly _cdpProxyProvider = this._services.get<ICdpProxyProvider>(ICdpProxyProvider);
 
   constructor(
@@ -92,7 +94,7 @@ export class DebugAdapter implements IDisposable {
     this.dap.on('continue', () => this._withThread(thread => thread.resume()));
     this.dap.on('pause', () => this._withThread(thread => thread.pause()));
     this.dap.on('next', () => this._withThread(thread => thread.stepOver()));
-    this.dap.on('stepIn', () => this._withThread(thread => thread.stepInto()));
+    this.dap.on('stepIn', params => this._withThread(thread => thread.stepInto(params.targetId)));
     this.dap.on('stepOut', () => this._withThread(thread => thread.stepOut()));
     this.dap.on('restartFrame', params => this._withThread(thread => thread.restartFrame(params)));
     this.dap.on('scopes', params => this._withThread(thread => thread.scopes(params)));
@@ -107,16 +109,47 @@ export class DebugAdapter implements IDisposable {
     this.dap.on('getPerformance', () =>
       this._withThread(thread => performanceProvider.retrieve(thread.cdp())),
     );
-    this.dap.on('breakpointLocations', params =>
-      this._withThread(async thread => ({
-        breakpoints: await this.breakpointManager.getBreakpointLocations(thread, params),
-      })),
-    );
+    this.dap.on('breakpointLocations', params => this._breakpointLocations(params));
     this.dap.on('createDiagnostics', params => this._dumpDiagnostics(params));
     this.dap.on('requestCDPProxy', () => this._requestCDPProxy());
     this.dap.on('setExcludedCallers', params => this._onSetExcludedCallers(params));
     this.dap.on('saveDiagnosticLogs', ({ toFile }) => this._saveDiagnosticLogs(toFile));
     this.dap.on('setSourceMapStepping', params => this._setSourceMapStepping(params));
+    this.dap.on('stepInTargets', params => this._stepInTargets(params));
+  }
+
+  private _breakpointLocations(
+    params: Dap.BreakpointLocationsParams,
+  ): Promise<Dap.BreakpointLocationsResult> {
+    return this._withThread(async thread => {
+      const source = this.sourceContainer.source(params.source);
+      if (!source) {
+        return { breakpoints: [] };
+      }
+
+      const possibleBps = await this.breakpointManager.getBreakpointLocations(
+        thread,
+        source,
+        new Base1Position(params.line, params.column || 1),
+        new Base1Position(
+          params.endLine || params.line + 1,
+          params.endColumn || params.column || 1,
+        ),
+      );
+
+      return {
+        breakpoints: possibleBps
+          .map(bp => bp.uiLocations.find(l => l.source === source))
+          .filter(truthy)
+          .map(bp => ({ line: bp.lineNumber, column: bp.columnNumber })),
+      };
+    });
+  }
+
+  private _stepInTargets(params: Dap.StepInTargetsParams): Promise<Dap.StepInTargetsResult> {
+    return this._withThread(async thread => ({
+      targets: await thread.getStepInTargets(params.frameId),
+    }));
   }
 
   private _setSourceMapStepping({
@@ -194,7 +227,7 @@ export class DebugAdapter implements IDisposable {
       supportsSetVariable: true,
       supportsRestartFrame: true,
       supportsGotoTargetsRequest: false,
-      supportsStepInTargetsRequest: false,
+      supportsStepInTargetsRequest: true,
       supportsCompletionsRequest: true,
       supportsModulesRequest: false,
       additionalModuleColumns: [],
@@ -224,7 +257,7 @@ export class DebugAdapter implements IDisposable {
   ): Promise<Dap.SetBreakpointsResult | Dap.Error> {
     return this.breakpointManager.setBreakpoints(
       params,
-      params.breakpoints?.map(() => ++this.lastBreakpointId) ?? [],
+      params.breakpoints?.map(() => this.breakpointIdCounter()) ?? [],
     );
   }
 

src/adapter/stackTrace.ts

@@ -4,7 +4,7 @@
 
 import * as nls from 'vscode-nls';
 import Cdp from '../cdp/api';
-import { once } from '../common/objUtils';
+import { once, posInt32Counter } from '../common/objUtils';
 import { Base0Position } from '../common/positions';
 import Dap from '../dap/api';
 import { asyncScopesNotAvailable } from '../dap/errors';
@@ -17,6 +17,8 @@ import { IExtraProperty, IScopeRef, IVariableContainer } from './variableStore';
 const localize = nls.loadMessageBundle();
 
 export interface IFrameElement {
+  /** DAP stack frame ID */
+  frameId: number;
   /** Formats the stack element as V8 would format it */
   formatAsNative(): Promise<string>;
   /** Pretty formats the stack element as text */
@@ -146,7 +148,7 @@ export class StackTrace {
   _appendFrame(frame: FrameElement) {
     this.frames.push(frame);
     if (frame instanceof StackFrame) {
-      this._frameById.set(frame._id, frame);
+      this._frameById.set(frame.frameId, frame);
     }
   }
 
@@ -200,7 +202,11 @@ interface IScope {
   callFrameId: string;
 }
 
+const frameIdCounter = posInt32Counter();
+
 export class AsyncSeparator implements IFrameElement {
+  public readonly frameId = frameIdCounter();
+
   constructor(private readonly label = 'async') {}
 
   public async toDap(): Promise<Dap.StackFrame> {
@@ -217,9 +223,8 @@ export class AsyncSeparator implements IFrameElement {
 }
 
 export class StackFrame implements IFrameElement {
-  private static _lastFrameId = 0;
+  public readonly frameId = frameIdCounter();
 
-  _id: number;
   private _name: string;
   private _rawLocation: RawLocation;
   public readonly uiLocation: () =>
@@ -262,7 +267,6 @@ export class StackFrame implements IFrameElement {
     rawLocation: RawLocation,
     private readonly isAsync = false,
   ) {
-    this._id = ++StackFrame._lastFrameId;
     this._name = callFrame.functionName || '<anonymous>';
     this._rawLocation = rawLocation;
     this.uiLocation = once(() => thread.rawLocationToUiLocation(rawLocation));
@@ -403,7 +407,7 @@ export class StackFrame implements IFrameElement {
     }
 
     return {
-      id: this._id,
+      id: this.frameId,
       name: formattedName, // TODO: Use params to format the name
       line,
       column,