From 330f08ca9833221619433286d0cc89cfb43c512a Mon Sep 17 00:00:00 2001 From: Hyang-Ah Hana Kim Date: Wed, 11 Mar 2020 12:32:52 -0400 Subject: [PATCH 1/6] telemetry.ts: send telemetry only if aiKey is not an empty string(#3091) --- src/telemetry.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/telemetry.ts b/src/telemetry.ts index 5f789aad3..b06ae390f 100644 --- a/src/telemetry.ts +++ b/src/telemetry.ts @@ -183,6 +183,9 @@ function sendTelemetryEvent( properties?: { [key: string]: string }, measures?: { [key: string]: number } ): void { + if (!aiKey) { + return; // cannot enable telemetry + } telemtryReporter = telemtryReporter ? telemtryReporter : new TelemetryReporter(extensionId, extensionVersion, aiKey); From 5dc0357d681f05a7d7c78869a751728e7e86579a Mon Sep 17 00:00:00 2001 From: Alan Fregtman <941331+darkvertex@users.noreply.github.com> Date: Sat, 14 Mar 2020 17:16:23 -0400 Subject: [PATCH 2/6] bug_report.md: Fix "architecture" typo. (#3095) --- .github/ISSUE_TEMPLATE/bug_report.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md index 466cf56ea..2e9768d5c 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.md +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -27,7 +27,7 @@ Please answer these questions before submitting your issue. Thanks! - - Check your installed extensions to get the version of the VS Code Go extension - -- Run `go env GOOS GOARCH` to get the operating system and processor arhcitecture details +- Run `go env GOOS GOARCH` to get the operating system and processor architecture details - ### Share the Go related settings you have added/edited From 6d1bf5c7999d5af0adcc15141ca012c34a9ca71c Mon Sep 17 00:00:00 2001 From: Hyang-Ah Hana Kim Date: Sun, 15 Mar 2020 23:35:24 -0400 Subject: [PATCH 3/6] Include the link to release note/package overview in the update prompt, and update gopls default version (#3041) --- src/goInstallTools.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/goInstallTools.ts b/src/goInstallTools.ts index bbe06d3b8..9c9071438 100644 --- a/src/goInstallTools.ts +++ b/src/goInstallTools.ts @@ -361,11 +361,18 @@ export async function promptForUpdatingTool(toolName: string) { } const goVersion = await getGoVersion(); const updateMsg = `Your version of ${tool.name} appears to be out of date. Please update for an improved experience.`; - vscode.window.showInformationMessage(updateMsg, 'Update').then((selected) => { + const choices: string[] = ['Update']; + if (toolName === `gopls`) { + choices.push('Release Notes'); // TODO(hyangah): pass more info such as version, release note location. + } + vscode.window.showInformationMessage(updateMsg, ...choices).then((selected) => { switch (selected) { case 'Update': installTools([tool], goVersion); break; + case 'Release Notes': + vscode.commands.executeCommand('vscode.open', vscode.Uri.parse('https://github.com/golang/go/issues/33030#issuecomment-510151934')); + break; default: declinedUpdates.push(tool); break; From 7da50774f84578b5ec6c46b27c0a3446d485f4a6 Mon Sep 17 00:00:00 2001 From: Quoc Truong Date: Thu, 19 Mar 2020 12:55:24 -0700 Subject: [PATCH 4/6] Address mismatch on path separators in debug config (#2010) (#3108) --- src/debugAdapter/goDebug.ts | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/src/debugAdapter/goDebug.ts b/src/debugAdapter/goDebug.ts index c83a2a037..35447a552 100644 --- a/src/debugAdapter/goDebug.ts +++ b/src/debugAdapter/goDebug.ts @@ -305,9 +305,16 @@ function logError(...args: any[]) { logger.error(logArgsToString(args)); } +function findPathSeparator(filePath: string) { + return filePath.includes('/') ? '/' : '\\'; +} + function normalizePath(filePath: string) { if (process.platform === 'win32') { + const pathSeparator = findPathSeparator(filePath); filePath = path.normalize(filePath); + // Normalize will replace everything with backslash on Windows. + filePath = filePath.replace(/\\/g, pathSeparator); return fixDriveCasingInWindows(filePath); } return filePath; @@ -754,13 +761,6 @@ class GoDebugSession extends LoggingDebugSession { log('InitializeResponse'); } - protected findPathSeperator(filePath: string) { - if (/^(\w:[\\/]|\\\\)/.test(filePath)) { - return '\\'; - } - return filePath.includes('/') ? '/' : '\\'; - } - protected launchRequest(response: DebugProtocol.LaunchResponse, args: LaunchRequestArguments): void { if (!args.program) { this.sendErrorResponse( @@ -835,10 +835,12 @@ class GoDebugSession extends LoggingDebugSession { if (this.delve.remotePath.length === 0) { return this.convertClientPathToDebugger(filePath); } + // The filePath may have a different path separator than the localPath + // So, update it to use the same separator as the remote path to ease + // in replacing the local path in it with remote path + filePath = filePath.replace(/\/|\\/g, this.remotePathSeparator); return filePath - .replace(this.delve.program, this.delve.remotePath) - .split(this.localPathSeparator) - .join(this.remotePathSeparator); + .replace(this.delve.program.replace(/\/|\\/g, this.remotePathSeparator), this.delve.remotePath); } protected toLocalPath(pathToConvert: string): string { @@ -1392,8 +1394,8 @@ class GoDebugSession extends LoggingDebugSession { } if (args.remotePath.length > 0) { - this.localPathSeparator = this.findPathSeperator(localPath); - this.remotePathSeparator = this.findPathSeperator(args.remotePath); + this.localPathSeparator = findPathSeparator(localPath); + this.remotePathSeparator = findPathSeparator(args.remotePath); const llist = localPath.split(/\/|\\/).reverse(); const rlist = args.remotePath.split(/\/|\\/).reverse(); From 00c318b30ba15773987851af42586c72c625c7a8 Mon Sep 17 00:00:00 2001 From: Luis GG Date: Fri, 20 Mar 2020 01:57:37 -0300 Subject: [PATCH 5/6] Add stacktrace dump and better error messages on EXC_BAD_ACCESS panics (#2904) --- src/debugAdapter/goDebug.ts | 119 ++++++++++++++++++++++++++++++------ 1 file changed, 100 insertions(+), 19 deletions(-) diff --git a/src/debugAdapter/goDebug.ts b/src/debugAdapter/goDebug.ts index 35447a552..811cf4a67 100644 --- a/src/debugAdapter/goDebug.ts +++ b/src/debugAdapter/goDebug.ts @@ -132,6 +132,7 @@ interface DebugThread { id: number; line: number; pc: number; + goroutineID: number; function?: DebugFunction; } @@ -153,6 +154,7 @@ interface DebugFunction { goType: number; args: DebugVariable[]; locals: DebugVariable[]; + optimized: boolean; } interface ListVarsOut { @@ -700,7 +702,7 @@ class Delve { await this.callPromise('Detach', [this.isApiV1 ? true : { Kill: isLocalDebugging }]); } catch (err) { log('DetachResponse'); - logError(`Failed to detach - ${err.toString() || ''}`); + logError(err, 'Failed to detach'); shouldForceClean = isLocalDebugging; } } @@ -874,7 +876,7 @@ class GoDebugSession extends LoggingDebugSession { // We use NonBlocking so the call would return immediately. this.debugState = await this.delve.getDebugState(); } catch (error) { - logError(`Failed to get state ${String(error)}`); + this.logDelveError(error, 'Failed to get state'); } if (!this.debugState.Running && !this.continueRequestRunning) { @@ -885,15 +887,13 @@ class GoDebugSession extends LoggingDebugSession { () => { return this.setBreakPoints(response, args).then(() => { return this.continue(true).then(null, (err) => { - logError( - `Failed to continue delve after halting it to set breakpoints: "${err.toString()}"` - ); + this.logDelveError(err, 'Failed to continue delve after halting it to set breakpoints'); }); }); }, (err) => { this.skipStopEventOnce = false; - logError(err); + this.logDelveError(err, 'Failed to halt delve before attempting to set breakpoint'); return this.sendErrorResponse( response, 2008, @@ -921,7 +921,7 @@ class GoDebugSession extends LoggingDebugSession { } if (err) { - logError('Failed to get threads - ' + err.toString()); + this.logDelveError(err, 'Failed to get threads'); return this.sendErrorResponse(response, 2003, 'Unable to display threads: "{e}"', { e: err.toString() }); @@ -963,7 +963,7 @@ class GoDebugSession extends LoggingDebugSession { [stackTraceIn], (err, out) => { if (err) { - logError('Failed to produce stack trace!'); + this.logDelveError(err, 'Failed to produce stacktrace'); return this.sendErrorResponse(response, 2004, 'Unable to produce stack trace: "{e}"', { e: err.toString() }); @@ -1004,7 +1004,7 @@ class GoDebugSession extends LoggingDebugSession { this.delve.isApiV1 ? [listLocalVarsIn] : [{ scope: listLocalVarsIn, cfg: this.delve.loadConfig }], (err, out) => { if (err) { - logError('Failed to list local variables - ' + err.toString()); + this.logDelveError(err, 'Failed to get list local variables'); return this.sendErrorResponse(response, 2005, 'Unable to list locals: "{e}"', { e: err.toString() }); @@ -1020,7 +1020,7 @@ class GoDebugSession extends LoggingDebugSession { : [{ scope: listLocalFunctionArgsIn, cfg: this.delve.loadConfig }], (listFunctionErr, outArgs) => { if (listFunctionErr) { - logError('Failed to list function args - ' + listFunctionErr.toString()); + this.logDelveError(listFunctionErr, 'Failed to list function args'); return this.sendErrorResponse(response, 2006, 'Unable to list args: "{e}"', { e: listFunctionErr.toString() }); @@ -1100,7 +1100,7 @@ class GoDebugSession extends LoggingDebugSession { this.delve.isApiV1 ? [filter] : [{ filter, cfg: this.delve.loadConfig }], (listPkgVarsErr, listPkgVarsOut) => { if (listPkgVarsErr) { - logError('Failed to list global vars - ' + listPkgVarsErr.toString()); + this.logDelveError(listPkgVarsErr, 'Failed to list global vars'); return this.sendErrorResponse( response, 2007, @@ -1172,7 +1172,7 @@ class GoDebugSession extends LoggingDebugSession { const variable = this.delve.isApiV1 ? result : (result).Variable; v.children = variable.children; }, - (err) => logError('Failed to evaluate expression - ' + err.toString()) + (err) => this.logDelveError(err, 'Failed to evaluate expression') ); } }; @@ -1251,7 +1251,7 @@ class GoDebugSession extends LoggingDebugSession { log('NextRequest'); this.delve.call('Command', [{ name: 'next' }], (err, out) => { if (err) { - logError('Failed to next - ' + err.toString()); + this.logDelveError(err, 'Failed to next'); } const state = this.delve.isApiV1 ? out : (out).State; log('next state', state); @@ -1266,7 +1266,7 @@ class GoDebugSession extends LoggingDebugSession { log('StepInRequest'); this.delve.call('Command', [{ name: 'step' }], (err, out) => { if (err) { - logError('Failed to step - ' + err.toString()); + this.logDelveError(err, 'Failed to step in'); } const state = this.delve.isApiV1 ? out : (out).State; log('stop state', state); @@ -1281,7 +1281,7 @@ class GoDebugSession extends LoggingDebugSession { log('StepOutRequest'); this.delve.call('Command', [{ name: 'stepOut' }], (err, out) => { if (err) { - logError('Failed to stepout - ' + err.toString()); + this.logDelveError(err, 'Failed to step out'); } const state = this.delve.isApiV1 ? out : (out).State; log('stepout state', state); @@ -1296,7 +1296,7 @@ class GoDebugSession extends LoggingDebugSession { log('PauseRequest'); this.delve.call('Command', [{ name: 'halt' }], (err, out) => { if (err) { - logError('Failed to halt - ' + err.toString()); + this.logDelveError(err, 'Failed to halt'); return this.sendErrorResponse(response, 2010, 'Unable to halt execution: "{e}"', { e: err.toString() }); @@ -1345,7 +1345,7 @@ class GoDebugSession extends LoggingDebugSession { this.delve.call(this.delve.isApiV1 ? 'SetSymbol' : 'Set', [setSymbolArgs], (err) => { if (err) { const errMessage = `Failed to set variable: ${err.toString()}`; - logError(errMessage); + this.logDelveError(err, 'Failed to set variable'); return this.sendErrorResponse(response, 2010, errMessage); } response.body = { value: args.value }; @@ -1743,7 +1743,7 @@ class GoDebugSession extends LoggingDebugSession { // [TODO] Can we avoid doing this? https://github.com/Microsoft/vscode/issues/40#issuecomment-161999881 this.delve.call('ListGoroutines', [], (err, out) => { if (err) { - logError('Failed to get threads - ' + err.toString()); + this.logDelveError(err, 'Failed to get threads'); } const goroutines = this.delve.isApiV1 ? out : (out).Goroutines; this.updateGoroutinesList(goroutines); @@ -1783,7 +1783,7 @@ class GoDebugSession extends LoggingDebugSession { if (!calledWhenSettingBreakpoint) { errorCallback = (err: any) => { if (err) { - logError('Failed to continue - ' + err.toString()); + this.logDelveError(err, 'Failed to continue'); } this.handleReenterDebug('breakpoint'); throw err; @@ -1840,6 +1840,87 @@ class GoDebugSession extends LoggingDebugSession { }); }); } + + private logDelveError(err: any, message: string) { + if (err === undefined) { + return; + } + + let errorMessage = err.toString(); + // Handle unpropagated fatalpanic errors with a more user friendly message: + // https://github.com/microsoft/vscode-go/issues/1903#issuecomment-460126884 + // https://github.com/go-delve/delve/issues/852 + // This affects macOS only although we're agnostic of the OS at this stage, only handle the error + if (errorMessage === 'bad access') { + errorMessage = 'unpropagated fatalpanic: signal SIGSEGV (EXC_BAD_ACCESS). This fatalpanic is not traceable on macOS, see https://github.com/go-delve/delve/issues/852'; + } + + logError(message + ' - ' + errorMessage); + + if (errorMessage === 'bad access') { + logError('WARNING: this stack might not be from the expected active goroutine'); + } + + this.dumpStacktrace(); + } + + private async dumpStacktrace() { + // Get current goroutine + // Debugger may be stopped at this point but we still can (and need) to obtain state and stacktrace + let goroutineId = 0; + try { + const stateCallResult = await this.delve.getDebugState(); + // In some fault scenarios there may not be a currentGoroutine available from the debugger state + // Use the current thread + if (!stateCallResult.currentGoroutine) { + goroutineId = stateCallResult.currentThread.goroutineID; + } else { + goroutineId = stateCallResult.currentGoroutine.id; + } + } catch (error) { + logError('dumpStacktrace - Failed to get debugger state ' + error); + } + + // Get goroutine stacktrace + const stackTraceIn = { id: goroutineId, depth: this.delve.stackTraceDepth }; + if (!this.delve.isApiV1) { + Object.assign(stackTraceIn, { full: false, cfg: this.delve.loadConfig }); + } + this.delve.call( + this.delve.isApiV1 ? + 'StacktraceGoroutine' : 'Stacktrace', [stackTraceIn], (err, out) => { + if (err) { + logError('dumpStacktrace: Failed to produce stack trace' + err); + return; + } + const locations = this.delve.isApiV1 ? out : (out).Locations; + log('locations', locations); + const stackFrames = locations.map((location, frameId) => { + const uniqueStackFrameId = this.stackFrameHandles.create([goroutineId, frameId]); + return new StackFrame( + uniqueStackFrameId, + location.function ? location.function.name : '', + location.file === '' ? null : new Source( + path.basename(location.file), + this.toLocalPath(location.file) + ), + location.line, + 0 + ); + }); + + // Dump stacktrace into error logger + logError(`Last known immediate stacktrace (goroutine id ${goroutineId}):`); + let output = ''; + stackFrames.forEach((stackFrame) => { + output = output.concat(`\t${stackFrame.source.path}:${stackFrame.line}\n`); + if (stackFrame.name) { + output = output.concat(`\t\t${stackFrame.name}\n`); + } + }); + logError(output); + }); + } } function random(low: number, high: number): number { From d53b1b3a29676fe326d35e42b3caf3b2da650e72 Mon Sep 17 00:00:00 2001 From: Hyang-Ah Hana Kim Date: Fri, 20 Mar 2020 01:42:50 -0400 Subject: [PATCH 6/6] goLanguageServer: set completion follow up command from middleware (#3084) --- src/goLanguageServer.ts | 50 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/src/goLanguageServer.ts b/src/goLanguageServer.ts index 56cc72fff..d6ebd730f 100644 --- a/src/goLanguageServer.ts +++ b/src/goLanguageServer.ts @@ -12,6 +12,7 @@ import semver = require('semver'); import util = require('util'); import vscode = require('vscode'); import { + Command, FormattingOptions, HandleDiagnosticsSignature, LanguageClient, @@ -143,6 +144,55 @@ export async function registerLanguageFeatures(ctx: vscode.ExtensionContext) { return null; } return next(document, token); + }, + provideCompletionItem: ( + document: vscode.TextDocument, + position: vscode.Position, + context: vscode.CompletionContext, + token: vscode.CancellationToken, + next: ProvideCompletionItemsSignature + ) => { + // TODO(hyangah): when v1.42+ api is available, we can simplify + // language-specific configuration lookup using the new + // ConfigurationScope. + // const paramHintsEnabled = vscode.workspace.getConfiguration( + // 'editor.parameterHints', + // { languageId: 'go', uri: document.uri }); + + const editorParamHintsEnabled = vscode.workspace.getConfiguration( + 'editor.parameterHints', document.uri)['enabled']; + const goParamHintsEnabled = vscode.workspace.getConfiguration( + '[go]', document.uri)['editor.parameterHints.enabled']; + + let paramHintsEnabled: boolean = false; + if (typeof goParamHintsEnabled === 'undefined') { + paramHintsEnabled = editorParamHintsEnabled; + } else { + paramHintsEnabled = goParamHintsEnabled; + } + let cmd: Command; + if (paramHintsEnabled) { + cmd = { title: 'triggerParameterHints', command: 'editor.action.triggerParameterHints' }; + } + + function configureCommands( + r: vscode.CompletionItem[] | vscode.CompletionList | null | undefined): + vscode.CompletionItem[] | vscode.CompletionList | null | undefined { + if (r) { + (Array.isArray(r) ? r : r.items).forEach((i: vscode.CompletionItem) => { + i.command = cmd; + }); + } + return r; + } + const ret = next(document, position, context, token); + + const isThenable = (obj: vscode.ProviderResult): obj is Thenable => obj && (obj)['then']; + if (isThenable(ret)) { + return ret.then(configureCommands); + } + return configureCommands(ret); + } } }