From 272c88ddf0b806325a1c723b6369d524d6e9409b Mon Sep 17 00:00:00 2001 From: Connor Peet Date: Tue, 13 Aug 2024 14:40:17 -0700 Subject: [PATCH] feat: adopt location references to link function locations Refs https://github.com/microsoft/debug-adapter-protocol/issues/343 --- CHANGELOG.md | 1 + src/adapter/debugAdapter.ts | 18 ++++ src/adapter/threads.ts | 1 + src/adapter/variableStore.ts | 36 ++++++- src/dap/api.d.ts | 95 ++++++++++++++++++- src/dap/errors.ts | 3 + src/dap/telemetryClassification.d.ts | 2 + .../evaluate-supports-location-lookup.txt | 23 +++++ src/test/evaluate/evaluate.ts | 9 ++ 9 files changed, 185 insertions(+), 3 deletions(-) create mode 100644 src/test/evaluate/evaluate-supports-location-lookup.txt diff --git a/CHANGELOG.md b/CHANGELOG.md index f1514c045..3e38d83f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ This changelog records changes to stable releases since 1.50.2. "TBA" changes he - feat: add basic network view, support experimental networking for node ([#2051](https://github.com/microsoft/vscode-js-debug/issues/2051)) - feat: support "debug url" in terminals created through the `node-terminal` launch type ([#2049](https://github.com/microsoft/vscode-js-debug/issues/2049)) +- feat: adopt location references to link function locations - fix: hover evaluation incorrectly showing undefined ([vscode#221503](https://github.com/microsoft/vscode/issues/221503)) ## v1.92 (July 2024) diff --git a/src/adapter/debugAdapter.ts b/src/adapter/debugAdapter.ts index 4bb01ac46..2e0fda734 100644 --- a/src/adapter/debugAdapter.ts +++ b/src/adapter/debugAdapter.ts @@ -106,6 +106,7 @@ export class DebugAdapter implements IDisposable { this.dap.on('toggleSkipFileStatus', params => this._toggleSkipFileStatus(params)); this.dap.on('toggleSkipFileStatus', params => this._toggleSkipFileStatus(params)); this.dap.on('prettyPrintSource', params => this._prettyPrintSource(params)); + this.dap.on('locations', params => this._onLocations(params)); this.dap.on('revealPage', () => this._withThread(thread => thread.revealPage())); this.dap.on('getPerformance', () => this._withThread(thread => performanceProvider.retrieve(thread.cdp())), @@ -374,6 +375,21 @@ export class DebugAdapter implements IDisposable { return undefined; } + async _onLocations(params: Dap.LocationsParams): Promise { + const variableStore = this.findVariableStore(v => v.hasVariable(params.locationReference)); + if (!variableStore || !this._thread) throw errors.locationNotFound(); + const location = await variableStore.getLocations(params.locationReference); + const uiLocation = await this._thread.rawLocationToUiLocationWithWaiting( + this._thread.rawLocation(location), + ); + if (!uiLocation) throw errors.locationNotFound(); + return { + source: await uiLocation.source.toDap(), + line: uiLocation.lineNumber, + column: uiLocation.columnNumber, + }; + } + async _onVariables(params: Dap.VariablesParams): Promise { const variableStore = this.findVariableStore(v => v.hasVariable(params.variablesReference)); return { variables: (await variableStore?.getVariables(params)) ?? [] }; @@ -425,6 +441,8 @@ export class DebugAdapter implements IDisposable { namedVariables: r.namedVariables, presentationHint: r.presentationHint, type: r.type, + memoryReference: r.memoryReference, + valueLocationReference: r.valueLocationReference, }; } diff --git a/src/adapter/threads.ts b/src/adapter/threads.ts index e0a2da1fa..c52ce5267 100644 --- a/src/adapter/threads.ts +++ b/src/adapter/threads.ts @@ -648,6 +648,7 @@ export class Thread implements IVariableStoreLocationProvider { namedVariables: variable.namedVariables, indexedVariables: variable.indexedVariables, memoryReference: variable.memoryReference, + valueLocationReference: variable.valueLocationReference, }; } diff --git a/src/adapter/variableStore.ts b/src/adapter/variableStore.ts index a98e398bc..e221d58c3 100644 --- a/src/adapter/variableStore.ts +++ b/src/adapter/variableStore.ts @@ -268,7 +268,9 @@ class VariableContext { } if (object.objectId) { - if (object.subtype === 'map' || object.subtype === 'set') { + if (object.type === 'function') { + return this.createVariable(FunctionVariable, ctx, object, customStringRepr); + } else if (object.subtype === 'map' || object.subtype === 'set') { return this.createVariable(SetOrMapVariable, ctx, object, customStringRepr); } else if (!objectPreview.subtypesWithoutPreview.has(object.subtype)) { return this.createVariable(ObjectVariable, ctx, object, customStringRepr); @@ -707,7 +709,7 @@ class StacktraceOutputVariable extends Variable { } class FunctionLocationVariable extends Variable { - private readonly location: Cdp.Debugger.Location; + public readonly location: Cdp.Debugger.Location; constructor(context: VariableContext, remoteObject: Cdp.Runtime.RemoteObject) { super(context, remoteObject); @@ -845,6 +847,20 @@ class ObjectVariable extends Variable implements IMemoryReadable { } } +class FunctionVariable extends ObjectVariable { + private readonly baseChildren = once(() => super.getChildren({ variablesReference: this.id })); + + public override async toDap(previewContext: PreviewContextType): Promise { + const [dap, children] = await Promise.all([super.toDap(previewContext), this.baseChildren()]); + + if (children.some(c => c instanceof FunctionLocationVariable)) { + dap.valueLocationReference = this.id; + } + + return dap; + } +} + const entriesVariableName = '[[Entries]]'; class SetOrMapVariable extends ObjectVariable { @@ -1435,6 +1451,22 @@ export class VariableStore { .map(v => v.dap); } + public async getLocations(variablesReference: number): Promise { + const container = this.vars.get(variablesReference); + if (!container) { + throw errors.locationNotFound(); + } + + // note: is actually free because the container will have cached its children + const children = await container.getChildren({ variablesReference }); + const locationVar = children.find(isInstanceOf(FunctionLocationVariable)); + if (!locationVar) { + throw errors.locationNotFound(); + } + + return locationVar.location; + } + /** Sets a variable */ public async setVariable(params: Dap.SetVariableParams): Promise { const container = this.vars.get(params.variablesReference); diff --git a/src/dap/api.d.ts b/src/dap/api.d.ts index 3e86c8783..78ece5ef9 100644 --- a/src/dap/api.d.ts +++ b/src/dap/api.d.ts @@ -821,6 +821,18 @@ export namespace Dap { */ disassembleRequest(params: DisassembleParams): Promise; + /** + * Looks up information about a location reference previously returned by the debug adapter. + */ + on( + request: 'locations', + handler: (params: LocationsParams) => Promise, + ): () => void; + /** + * Looks up information about a location reference previously returned by the debug adapter. + */ + locationsRequest(params: LocationsParams): Promise; + /** * Sets the enabled custom breakpoints. */ @@ -1670,6 +1682,11 @@ export namespace Dap { */ disassemble(params: DisassembleParams): Promise; + /** + * Looks up information about a location reference previously returned by the debug adapter. + */ + locations(params: LocationsParams): Promise; + /** * Sets the enabled custom breakpoints. */ @@ -2328,6 +2345,13 @@ export namespace Dap { * This attribute may be returned by a debug adapter if corresponding capability `supportsMemoryReferences` is true. */ memoryReference?: string; + + /** + * A reference that allows the client to request the location where the returned value is declared. For example, if a function pointer is returned, the adapter may be able to look up the function's location. This should be present only if the adapter is likely to be able to resolve the location. + * + * This reference shares the same lifetime as the `variablesReference`. See 'Lifetime of Object References' in the Overview section for details. + */ + valueLocationReference?: integer; } export interface EvaluationOptionsParams { @@ -2833,6 +2857,40 @@ export namespace Dap { sources: Source[]; } + export interface LocationsParams { + /** + * Location reference to resolve. + */ + locationReference: integer; + } + + export interface LocationsResult { + /** + * The source containing the location; either `source.path` or `source.sourceReference` must be specified. + */ + source: Source; + + /** + * The line number of the location. The client capability `linesStartAt1` determines whether it is 0- or 1-based. + */ + line: integer; + + /** + * Position of the location within the `line`. It is measured in UTF-16 code units and the client capability `columnsStartAt1` determines whether it is 0- or 1-based. If no column is given, the first position in the start line is assumed. + */ + column?: integer; + + /** + * End line of the location, present if the location refers to a range. The client capability `linesStartAt1` determines whether it is 0- or 1-based. + */ + endLine?: integer; + + /** + * End position of the location within `endLine`, present if the location refers to a range. It is measured in UTF-16 code units and the client capability `columnsStartAt1` determines whether it is 0- or 1-based. + */ + endColumn?: integer; + } + export interface LongPredictionEventParams {} export interface MemoryEventParams { @@ -2982,6 +3040,13 @@ export namespace Dap { * Additional data to report. For the `telemetry` category the data is sent to telemetry, for the other categories the data is shown in JSON format. */ data?: any[] | boolean | integer | null | number | object | string; + + /** + * A reference that allows the client to request the location where the new value is declared. For example, if the logged value is function pointer, the adapter may be able to look up the function's location. This should be present only if the adapter is likely to be able to resolve the location. + * + * This reference shares the same lifetime as the `variablesReference`. See 'Lifetime of Object References' in the Overview section for details. + */ + locationReference?: integer; } export interface PauseParams { @@ -3474,6 +3539,13 @@ export namespace Dap { * This attribute may be returned by a debug adapter if corresponding capability `supportsMemoryReferences` is true. */ memoryReference?: string; + + /** + * A reference that allows the client to request the location where the new value is declared. For example, if the new value is function pointer, the adapter may be able to look up the function's location. This should be present only if the adapter is likely to be able to resolve the location. + * + * This reference shares the same lifetime as the `variablesReference`. See 'Lifetime of Object References' in the Overview section for details. + */ + valueLocationReference?: integer; } export interface SetFunctionBreakpointsParams { @@ -3574,6 +3646,13 @@ export namespace Dap { * This attribute may be returned by a debug adapter if corresponding capability `supportsMemoryReferences` is true. */ memoryReference?: string; + + /** + * A reference that allows the client to request the location where the new value is declared. For example, if the new value is function pointer, the adapter may be able to look up the function's location. This should be present only if the adapter is likely to be able to resolve the location. + * + * This reference shares the same lifetime as the `variablesReference`. See 'Lifetime of Object References' in the Overview section for details. + */ + valueLocationReference?: integer; } export interface SourceParams { @@ -4019,6 +4098,20 @@ export namespace Dap { * This attribute may be returned by a debug adapter if corresponding capability `supportsMemoryReferences` is true. */ memoryReference?: string; + + /** + * A reference that allows the client to request the location where the variable is declared. This should be present only if the adapter is likely to be able to resolve the location. + * + * This reference shares the same lifetime as the `variablesReference`. See 'Lifetime of Object References' in the Overview section for details. + */ + declarationLocationReference?: integer; + + /** + * A reference that allows the client to request the location where the variable's value is declared. For example, if the variable contains a function pointer, the adapter may be able to look up the function's location. This should be present only if the adapter is likely to be able to resolve the location. + * + * This reference shares the same lifetime as the `variablesReference`. See 'Lifetime of Object References' in the Overview section for details. + */ + valueLocationReference?: integer; } /** @@ -4112,7 +4205,7 @@ export namespace Dap { endColumn?: integer; /** - * Indicates whether this frame can be restarted with the `restart` request. Clients should only use this if the debug adapter supports the `restart` request and the corresponding capability `supportsRestartRequest` is true. If a debug adapter has this capability, then `canRestart` defaults to `true` if the property is absent. + * Indicates whether this frame can be restarted with the `restartFrame` request. Clients should only use this if the debug adapter supports the `restart` request and the corresponding capability `supportsRestartFrame` is true. If a debug adapter has this capability, then `canRestart` defaults to `true` if the property is absent. */ canRestart?: boolean; diff --git a/src/dap/errors.ts b/src/dap/errors.ts index 9abf2dae2..f5374ca64 100644 --- a/src/dap/errors.ts +++ b/src/dap/errors.ts @@ -227,6 +227,9 @@ export const noUwpPipeFound = () => ), ); +export const locationNotFound = () => + createUserError(l10n.t('Could not find a location for the variable')); + /** * Returns if the value looks like a DAP error. */ diff --git a/src/dap/telemetryClassification.d.ts b/src/dap/telemetryClassification.d.ts index b8c756c20..4ae529fae 100644 --- a/src/dap/telemetryClassification.d.ts +++ b/src/dap/telemetryClassification.d.ts @@ -94,6 +94,8 @@ interface IDAPOperationClassification { '!writememory.errors': { classification: 'CallstackOrException'; purpose: 'PerformanceAndHealth' }; disassemble: { classification: 'SystemMetaData'; purpose: 'PerformanceAndHealth' }; '!disassemble.errors': { classification: 'CallstackOrException'; purpose: 'PerformanceAndHealth' }; + locations: { classification: 'SystemMetaData'; purpose: 'PerformanceAndHealth' }; + '!locations.errors': { classification: 'CallstackOrException'; purpose: 'PerformanceAndHealth' }; setcustombreakpoints: { classification: 'SystemMetaData'; purpose: 'PerformanceAndHealth' }; '!setcustombreakpoints.errors': { classification: 'CallstackOrException'; purpose: 'PerformanceAndHealth' }; prettyprintsource: { classification: 'SystemMetaData'; purpose: 'PerformanceAndHealth' }; diff --git a/src/test/evaluate/evaluate-supports-location-lookup.txt b/src/test/evaluate/evaluate-supports-location-lookup.txt new file mode 100644 index 000000000..adeb42bb6 --- /dev/null +++ b/src/test/evaluate/evaluate-supports-location-lookup.txt @@ -0,0 +1,23 @@ + +> result: ƒ printArr(arr) { + for (const num of arr) { + console.log(plusTwo(num)); + } +} + > arguments: (...) + > caller: (...) + length: 1 + name: 'printArr' + > prototype: {constructor: ƒ} + [[FunctionLocation]]: @ ${workspaceFolder}/web/basic.ts:5 + > [[Prototype]]: ƒ () + > [[Scopes]]: Scopes[1] +{ + column : 18 + line : 5 + source : { + name : basic.ts + path : ${workspaceFolder}/web/basic.ts + sourceReference : + } +} diff --git a/src/test/evaluate/evaluate.ts b/src/test/evaluate/evaluate.ts index 2721c7466..7d9369516 100644 --- a/src/test/evaluate/evaluate.ts +++ b/src/test/evaluate/evaluate.ts @@ -82,6 +82,15 @@ describe('evaluate', () => { p.assertLog(); }); + itIntegrates('supports location lookup', async ({ r }) => { + const p = await r.launchUrlAndLoad('basic.html'); + const fn = await p.logger.evaluateAndLog('printArr'); + expect(fn.valueLocationReference).to.be.greaterThan(0); + const location = await p.dap.locations({ locationReference: fn.valueLocationReference! }); + p.log(location); + p.assertLog(); + }); + itIntegrates('repl', async ({ r }) => { const p = await r.launchUrlAndLoad('index.html');