From 9a58f734aa37d706159d9870f7edf5e70c31de1c Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Mon, 23 Sep 2019 20:00:36 -0700 Subject: [PATCH] Fix microsoft/vscode-node-debug2#242 Fix microsoft/vscode-chrome-debug#934 --- package-lock.json | 65 +++++--------- package.json | 4 +- src/chrome/breakpoints.ts | 86 ++++++++++++++++++- src/chrome/chromeDebugAdapter.ts | 6 +- src/transformers/basePathTransformer.ts | 6 +- src/transformers/baseSourceMapTransformer.ts | 13 ++- src/transformers/urlPathTransformer.ts | 26 +++--- test/chrome/chromeDebugAdapter.test.ts | 2 +- .../baseSourceMapTransformer.test.ts | 6 +- test/transformers/urlPathTransformer.test.ts | 4 +- 10 files changed, 145 insertions(+), 73 deletions(-) diff --git a/package-lock.json b/package-lock.json index c25b25345..4cc9f9d21 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2073,8 +2073,7 @@ "ansi-regex": { "version": "2.1.1", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "aproba": { "version": "1.2.0", @@ -2095,14 +2094,12 @@ "balanced-match": { "version": "1.0.0", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "brace-expansion": { "version": "1.1.11", "bundled": true, "dev": true, - "optional": true, "requires": { "balanced-match": "^1.0.0", "concat-map": "0.0.1" @@ -2117,20 +2114,17 @@ "code-point-at": { "version": "1.1.0", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "concat-map": { "version": "0.0.1", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "console-control-strings": { "version": "1.1.0", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "core-util-is": { "version": "1.0.2", @@ -2247,8 +2241,7 @@ "inherits": { "version": "2.0.3", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "ini": { "version": "1.3.5", @@ -2260,7 +2253,6 @@ "version": "1.0.0", "bundled": true, "dev": true, - "optional": true, "requires": { "number-is-nan": "^1.0.0" } @@ -2275,7 +2267,6 @@ "version": "3.0.4", "bundled": true, "dev": true, - "optional": true, "requires": { "brace-expansion": "^1.1.7" } @@ -2283,14 +2274,12 @@ "minimist": { "version": "0.0.8", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "minipass": { "version": "2.3.5", "bundled": true, "dev": true, - "optional": true, "requires": { "safe-buffer": "^5.1.2", "yallist": "^3.0.0" @@ -2309,7 +2298,6 @@ "version": "0.5.1", "bundled": true, "dev": true, - "optional": true, "requires": { "minimist": "0.0.8" } @@ -2390,8 +2378,7 @@ "number-is-nan": { "version": "1.0.1", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "object-assign": { "version": "4.1.1", @@ -2403,7 +2390,6 @@ "version": "1.4.0", "bundled": true, "dev": true, - "optional": true, "requires": { "wrappy": "1" } @@ -2489,8 +2475,7 @@ "safe-buffer": { "version": "5.1.2", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "safer-buffer": { "version": "2.1.2", @@ -2526,7 +2511,6 @@ "version": "1.0.2", "bundled": true, "dev": true, - "optional": true, "requires": { "code-point-at": "^1.0.0", "is-fullwidth-code-point": "^1.0.0", @@ -2546,7 +2530,6 @@ "version": "3.0.1", "bundled": true, "dev": true, - "optional": true, "requires": { "ansi-regex": "^2.0.0" } @@ -2590,14 +2573,12 @@ "wrappy": { "version": "1.0.2", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "yallist": { "version": "3.0.3", "bundled": true, - "dev": true, - "optional": true + "dev": true } } }, @@ -5924,7 +5905,7 @@ }, "process-nextick-args": { "version": "1.0.7", - "resolved": "https://registry.npmjs.org/process-nextick-args/-/process-nextick-args-1.0.7.tgz", + "resolved": false, "integrity": "sha1-FQ4gt1ZZCtP5EJPyWk8q2L/zC6M=", "dev": true }, @@ -7692,25 +7673,25 @@ } }, "vscode-debugadapter": { - "version": "1.34.0", - "resolved": "https://registry.npmjs.org/vscode-debugadapter/-/vscode-debugadapter-1.34.0.tgz", - "integrity": "sha512-ye11QX9K4QA4TOuJHVusJenTuqh7GrCsLZJ7Gs86spOJ4WTTPd8wFLJBjL5ivkCUln/Tyo4HMkxlLBlTBtHdog==", + "version": "1.37.1", + "resolved": "https://registry.npmjs.org/vscode-debugadapter/-/vscode-debugadapter-1.37.1.tgz", + "integrity": "sha512-g/xNsUdkrd0DifaoRJ4+wypSMsMbK47fGpetpmIOnOQiDFjtYKvqxsgyUZ4BOj2jKP2Xa40B4YXfUUJpqreWJg==", "requires": { "mkdirp": "^0.5.1", - "vscode-debugprotocol": "1.34.0" + "vscode-debugprotocol": "1.37.0" }, "dependencies": { "vscode-debugprotocol": { - "version": "1.34.0", - "resolved": "https://registry.npmjs.org/vscode-debugprotocol/-/vscode-debugprotocol-1.34.0.tgz", - "integrity": "sha512-tcMThtgk9TUtE8zzAIwPvHZfgnEYnVa7cI3YaQk/o54Q9cme+TLd/ao60a6ycj5rCrI/B5r/mAfeK5EKSItm7g==" + "version": "1.37.0", + "resolved": "https://registry.npmjs.org/vscode-debugprotocol/-/vscode-debugprotocol-1.37.0.tgz", + "integrity": "sha512-ppZLEBbFRVNsK0YpfgFi/x2CDyihx0F+UpdKmgeJcvi05UgSXYdO0n9sDVYwoGvvYQPvlpDQeWuY0nloOC4mPA==" } } }, "vscode-debugprotocol": { - "version": "1.34.0", - "resolved": "https://registry.npmjs.org/vscode-debugprotocol/-/vscode-debugprotocol-1.34.0.tgz", - "integrity": "sha512-tcMThtgk9TUtE8zzAIwPvHZfgnEYnVa7cI3YaQk/o54Q9cme+TLd/ao60a6ycj5rCrI/B5r/mAfeK5EKSItm7g==" + "version": "1.37.0", + "resolved": "https://registry.npmjs.org/vscode-debugprotocol/-/vscode-debugprotocol-1.37.0.tgz", + "integrity": "sha512-ppZLEBbFRVNsK0YpfgFi/x2CDyihx0F+UpdKmgeJcvi05UgSXYdO0n9sDVYwoGvvYQPvlpDQeWuY0nloOC4mPA==" }, "vscode-nls": { "version": "4.0.0", @@ -7825,7 +7806,7 @@ }, "wrap-ansi": { "version": "2.1.0", - "resolved": "https://registry.npmjs.org/wrap-ansi/-/wrap-ansi-2.1.0.tgz", + "resolved": "http://registry.npmjs.org/wrap-ansi/-/wrap-ansi-2.1.0.tgz", "integrity": "sha1-2Pw9KE3QV5T+hJc8rs3Rz4JP3YU=", "dev": true, "requires": { diff --git a/package.json b/package.json index 26b43a6e4..f5b976f30 100644 --- a/package.json +++ b/package.json @@ -18,8 +18,8 @@ "glob": "^7.1.3", "noice-json-rpc": "^1.2.0", "source-map": "^0.6.1", - "vscode-debugadapter": "^1.34.0", - "vscode-debugprotocol": "^1.34.0", + "vscode-debugadapter": "^1.37.1", + "vscode-debugprotocol": "^1.37.0", "vscode-nls": "^4.0.0", "vscode-uri": "^2.0.2", "ws": "^6.0.0" diff --git a/src/chrome/breakpoints.ts b/src/chrome/breakpoints.ts index e564bbacb..163042cae 100644 --- a/src/chrome/breakpoints.ts +++ b/src/chrome/breakpoints.ts @@ -100,7 +100,7 @@ export class Breakpoints { if (sourceMapTransformerResponse && sourceMapTransformerResponse.ids) { ids = sourceMapTransformerResponse.ids; } - args = this.adapter.pathTransformer.setBreakpoints(args); + args.source = this.adapter.pathTransformer.setBreakpoints(args.source); // Get the target url of the script let targetScriptUrl: string; @@ -146,7 +146,7 @@ export class Breakpoints { // We need to send the original args to avoid adjusting the line and column numbers twice here return this.unverifiedBpResponseForBreakpoints(originalArgs, requestSeq, targetScriptUrl, body.breakpoints, localize('bp.fail.unbound', 'Breakpoint set but not yet bound')); } - this.adapter.sourceMapTransformer.setBreakpointsResponse(body, requestSeq); + body.breakpoints = this.adapter.sourceMapTransformer.setBreakpointsResponse(body.breakpoints, true, requestSeq) || body.breakpoints; this.adapter.lineColTransformer.setBreakpointsResponse(body); return body; }); @@ -264,6 +264,88 @@ export class Breakpoints { }; } + /** + * Using the request object from the DAP, set all breakpoints on the target + * @param args The setBreakpointRequest arguments from the DAP client + * @param scripts The script container associated with this instance of the adapter + * @param requestSeq The request sequence number from the DAP + * @param ids IDs passed in for previously unverified breakpoints + */ + public async getBreakpointsLocations(args: DebugProtocol.BreakpointLocationsArguments, scripts: ScriptContainer, requestSeq: number): Promise { + + if (args.source.path) { + args.source.path = this.adapter.displayPathToRealPath(args.source.path); + args.source.path = utils.canonicalizeUrl(args.source.path); + } + + await this.validateBreakpointsPath(args); + + // Deep copy the args that we are going to modify, and keep the original values in originalArgs + args = JSON.parse(JSON.stringify(args)); + args.endLine = this.adapter.lineColTransformer.convertClientLineToDebugger(typeof args.endLine === 'number' ? args.endLine : args.line + 1); + args.endColumn = this.adapter.lineColTransformer.convertClientLineToDebugger(args.endColumn || 1); + args.line = this.adapter.lineColTransformer.convertClientLineToDebugger(args.line); + args.column = this.adapter.lineColTransformer.convertClientColumnToDebugger(args.column || 1); + + if (args.source.path) { + const source1 = JSON.parse(JSON.stringify(args.source)); + const startArgs = this.adapter.sourceMapTransformer.setBreakpoints({ breakpoints: [{ line: args.line, column: args.column }], source: source1 }, requestSeq); + args.line = startArgs.args.breakpoints[0].line; + args.column = startArgs.args.breakpoints[0].column; + + const endArgs = this.adapter.sourceMapTransformer.setBreakpoints({ breakpoints: [{ line: args.endLine, column: args.endColumn }], source: args.source }, requestSeq); + args.endLine = endArgs.args.breakpoints[0].line; + args.endColumn = endArgs.args.breakpoints[0].column; + } + + args.source = this.adapter.pathTransformer.setBreakpoints(args.source); + + // Get the target url of the script + let targetScriptUrl: string; + if (args.source.sourceReference) { + const handle = scripts.getSource(args.source.sourceReference); + if ((!handle || !handle.scriptId) && args.source.path) { + // A sourcemapped script with inline sources won't have a scriptId here, but the + // source.path has been fixed. + targetScriptUrl = args.source.path; + } else { + const targetScript = scripts.getScriptById(handle.scriptId); + if (targetScript) { + targetScriptUrl = targetScript.url; + } + } + } else if (args.source.path) { + targetScriptUrl = args.source.path; + } + + if (targetScriptUrl) { + const script = scripts.getScriptByUrl(targetScriptUrl); + if (script) { + const end = typeof args.endLine === 'number' ? + { scriptId: script.scriptId, lineNumber: args.endLine, columnNumber: args.endColumn || 0 } : + { scriptId: script.scriptId, lineNumber: args.line + 1, columnNumber: 0 }; + + const possibleBpResponse = await this.chrome.Debugger.getPossibleBreakpoints({ + start: { scriptId: script.scriptId, lineNumber: args.line, columnNumber: args.column || 0 }, + end, + restrictToFunction: false + }); + let breakpoints = possibleBpResponse.locations.map(loc => { + return { + line: loc.lineNumber, + column: loc.columnNumber + }; + }); + breakpoints = this.adapter.sourceMapTransformer.setBreakpointsResponse(breakpoints, false, requestSeq); + const response = { breakpoints }; + this.adapter.lineColTransformer.setBreakpointsResponse(response); + return response as DebugProtocol.BreakpointLocationsResponse['body']; + } + } + + return null; + } + /** * Transform breakpoint responses from the chrome-devtools target to the DAP response * @param url The URL of the script for which we are translating breakpoint responses diff --git a/src/chrome/chromeDebugAdapter.ts b/src/chrome/chromeDebugAdapter.ts index 938c725a3..b77f28081 100644 --- a/src/chrome/chromeDebugAdapter.ts +++ b/src/chrome/chromeDebugAdapter.ts @@ -272,7 +272,8 @@ export abstract class ChromeDebugAdapter implements IDebugAdapter { supportsDelayedStackTraceLoading: true, supportsValueFormattingOptions: true, supportsEvaluateForHovers: true, - supportsLoadedSourcesRequest: true + supportsLoadedSourcesRequest: true, + supportsBreakpointLocationsRequest: true }; } @@ -1679,4 +1680,7 @@ export abstract class ChromeDebugAdapter implements IDebugAdapter { return this._breakpoints.validateBreakpointsPath(args); } + public breakpointLocations(args: DebugProtocol.BreakpointLocationsArguments, _telemetryPropertyCollector?: ITelemetryPropertyCollector, requestSeq?: number): Promise { + return this._breakpoints.getBreakpointsLocations(args, this._scriptContainer, requestSeq); + } } diff --git a/src/transformers/basePathTransformer.ts b/src/transformers/basePathTransformer.ts index 1b1b8c71b..6133be018 100644 --- a/src/transformers/basePathTransformer.ts +++ b/src/transformers/basePathTransformer.ts @@ -4,7 +4,7 @@ import { DebugProtocol } from 'vscode-debugprotocol'; -import { ISetBreakpointsArgs, ILaunchRequestArgs, IAttachRequestArgs, IStackTraceResponseBody } from '../debugAdapterInterfaces'; +import { ILaunchRequestArgs, IAttachRequestArgs, IStackTraceResponseBody } from '../debugAdapterInterfaces'; /** * Converts a local path from Code to a path on the target. @@ -18,8 +18,8 @@ export class BasePathTransformer { return Promise.resolve(); } - public setBreakpoints(args: ISetBreakpointsArgs): ISetBreakpointsArgs { - return args; + public setBreakpoints(source: DebugProtocol.Source): DebugProtocol.Source { + return source; } public clearTargetContext(): void { diff --git a/src/transformers/baseSourceMapTransformer.ts b/src/transformers/baseSourceMapTransformer.ts index e761d0fb8..16332271f 100644 --- a/src/transformers/baseSourceMapTransformer.ts +++ b/src/transformers/baseSourceMapTransformer.ts @@ -6,7 +6,7 @@ import * as path from 'path'; import { DebugProtocol } from 'vscode-debugprotocol'; import { ISetBreakpointsArgs, ILaunchRequestArgs, IAttachRequestArgs, - ISetBreakpointsResponseBody, IInternalStackTraceResponseBody, IScopesResponseBody, IInternalStackFrame } from '../debugAdapterInterfaces'; + IInternalStackTraceResponseBody, IScopesResponseBody, IInternalStackFrame } from '../debugAdapterInterfaces'; import { MappedPosition, ISourcePathDetails } from '../sourceMaps/sourceMap'; import { SourceMaps } from '../sourceMaps/sourceMaps'; import * as utils from '../utils'; @@ -175,14 +175,17 @@ export class BaseSourceMapTransformer { /** * Apply sourcemapping back to authored files from the response */ - public setBreakpointsResponse(response: ISetBreakpointsResponseBody, requestSeq: number): void { + public setBreakpointsResponse(breakpoints: DebugProtocol.Breakpoint[], shouldFilter: boolean, requestSeq: number): DebugProtocol.Breakpoint[] { if (this._sourceMaps && this._requestSeqToSetBreakpointsArgs.has(requestSeq)) { const args = this._requestSeqToSetBreakpointsArgs.get(requestSeq); if (args.authoredPath) { // authoredPath is set, so the file was mapped to source. // Remove breakpoints from files that map to the same file, and map back to source. - response.breakpoints = response.breakpoints.filter((_, i) => i < args.originalBPs.length); - response.breakpoints.forEach((bp, i) => { + if (shouldFilter) { + breakpoints = breakpoints.filter((_, i) => i < args.originalBPs.length); + } + + breakpoints.forEach((bp, i) => { const mapped = this._sourceMaps.mapToAuthored(args.generatedPath, bp.line, bp.column); if (mapped) { logger.log(`SourceMaps.setBP: Mapped ${args.generatedPath}:${bp.line + 1}:${bp.column + 1} to ${mapped.source}:${mapped.line + 1}`); @@ -198,6 +201,8 @@ export class BaseSourceMapTransformer { }); } } + + return breakpoints; } /** diff --git a/src/transformers/urlPathTransformer.ts b/src/transformers/urlPathTransformer.ts index f2f236279..398e92396 100644 --- a/src/transformers/urlPathTransformer.ts +++ b/src/transformers/urlPathTransformer.ts @@ -4,7 +4,7 @@ import { BasePathTransformer } from './basePathTransformer'; -import { ISetBreakpointsArgs, ILaunchRequestArgs, IAttachRequestArgs, IStackTraceResponseBody, IPathMapping } from '../debugAdapterInterfaces'; +import { ILaunchRequestArgs, IAttachRequestArgs, IStackTraceResponseBody, IPathMapping } from '../debugAdapterInterfaces'; import * as utils from '../utils'; import { logger } from 'vscode-debugadapter'; import { DebugProtocol } from 'vscode-debugprotocol'; @@ -30,28 +30,28 @@ export class UrlPathTransformer extends BasePathTransformer { return super.attach(args); } - public setBreakpoints(args: ISetBreakpointsArgs): ISetBreakpointsArgs { - if (!args.source.path) { + public setBreakpoints(source: DebugProtocol.Source): DebugProtocol.Source { + if (!source.path) { // sourceReference script, nothing to do - return args; + return source; } - if (utils.isURL(args.source.path)) { + if (utils.isURL(source.path)) { // already a url, use as-is - logger.log(`Paths.setBP: ${args.source.path} is already a URL`); - return args; + logger.log(`Paths.setBP: ${source.path} is already a URL`); + return source; } - const path = utils.canonicalizeUrl(args.source.path); + const path = utils.canonicalizeUrl(source.path); const url = this.getTargetPathFromClientPath(path); if (url) { - args.source.path = url; - logger.log(`Paths.setBP: Resolved ${path} to ${args.source.path}`); - return args; + source.path = url; + logger.log(`Paths.setBP: Resolved ${path} to ${source.path}`); + return source; } else { logger.log(`Paths.setBP: No target url cached yet for client path: ${path}.`); - args.source.path = path; - return args; + source.path = path; + return source; } } diff --git a/test/chrome/chromeDebugAdapter.test.ts b/test/chrome/chromeDebugAdapter.test.ts index b99e0cb71..e986b6d42 100644 --- a/test/chrome/chromeDebugAdapter.test.ts +++ b/test/chrome/chromeDebugAdapter.test.ts @@ -400,7 +400,7 @@ suite('ChromeDebugAdapter', () => { .returns(() => Promise.resolve(generatedScriptPath)); mockSourceMapTransformer.setup(x => x.setBreakpoints(It.isAny(), It.isAnyNumber(), It.isAny())) - .returns(( args: ISetBreakpointsArgs, ids: number[]) => { + .returns((args: ISetBreakpointsArgs, ids: number[]) => { args.source.path = generatedScriptPath; return { args, ids }; }); diff --git a/test/transformers/baseSourceMapTransformer.test.ts b/test/transformers/baseSourceMapTransformer.test.ts index 581f759cc..d2342e5ff 100644 --- a/test/transformers/baseSourceMapTransformer.test.ts +++ b/test/transformers/baseSourceMapTransformer.test.ts @@ -215,7 +215,7 @@ suite('BaseSourceMapTransformer', () => { source: { path: AUTHORED_PATH }, breakpoints: AUTHORED_BPS() }, 0); - transformer.setBreakpointsResponse(response, 0); + response.breakpoints = transformer.setBreakpointsResponse(response.breakpoints, true, 0); assert.deepEqual(response, expected); }); @@ -228,7 +228,7 @@ suite('BaseSourceMapTransformer', () => { source: { path: RUNTIME_PATH }, breakpoints: RUNTIME_BPS() }, 0); - transformer.setBreakpointsResponse(response, 0); + response.breakpoints = transformer.setBreakpointsResponse(response.breakpoints, true, 0); assert.deepEqual(response, expected); }); @@ -248,7 +248,7 @@ suite('BaseSourceMapTransformer', () => { const transformer = getTransformer(/*sourceMaps=*/true, /*suppressDefaultMock=*/true); transformer.setBreakpoints(setBPArgs, /*requestSeq=*/0); transformer.setBreakpoints(setBPArgs2, /*requestSeq=*/1); - transformer.setBreakpointsResponse(response, /*requestSeq=*/1); + response.breakpoints = transformer.setBreakpointsResponse(response.breakpoints, true, /*requestSeq=*/1); assert.deepEqual(response, expected); mock.verifyAll(); diff --git a/test/transformers/urlPathTransformer.test.ts b/test/transformers/urlPathTransformer.test.ts index 90f6b43ce..162ba417e 100644 --- a/test/transformers/urlPathTransformer.test.ts +++ b/test/transformers/urlPathTransformer.test.ts @@ -56,13 +56,13 @@ suite('UrlPathTransformer', () => { .returns(() => Promise.resolve(CLIENT_PATH)).verifiable(); await transformer.scriptParsed(TARGET_URL); - transformer.setBreakpoints(SET_BP_ARGS); + SET_BP_ARGS.source = transformer.setBreakpoints(SET_BP_ARGS.source); assert.deepEqual(SET_BP_ARGS, EXPECTED_SET_BP_ARGS); }); test(`doesn't modify the args when it can't map the client script to the target script`, () => { const origArgs = JSON.parse(JSON.stringify(SET_BP_ARGS)); - transformer.setBreakpoints(SET_BP_ARGS); + SET_BP_ARGS.source = transformer.setBreakpoints(SET_BP_ARGS.source); assert.deepEqual(SET_BP_ARGS, origArgs); });