[release] prepare v0.20.0 release (2)

6c7bfb3 src/debugAdapter: report that next is automatically cancelled if interrupted
f38e348 src/goLanguageServer: propagate go.buildFlags,buildTags to gopls
dbc8da9 src/goDebug: fix drive casing when substituting paths
2f72921 test/integration/goDebug: disable a substitutePath test that times out

Change-Id: I1f808808aca0160ad51f1b8a0f809365d3f8513d

Author: hyangah

docs/settings.md

@@ -79,7 +79,7 @@ Default: `false`
 
 ### `go.buildFlags`
 
-Flags to `go build`/`go test` used during build-on-save or running tests. (e.g. ["-ldflags='-s'"])
+Flags to `go build`/`go test` used during build-on-save or running tests. (e.g. ["-ldflags='-s'"]) This is propagated to the language server if `gopls.buildFlags` is not specified.
 
 ### `go.buildOnSave`
 
@@ -91,7 +91,7 @@ Default: `package`
 
 ### `go.buildTags`
 
-The Go build tags to use for all commands, that support a `-tags '...'` argument. When running tests, go.testTags will be used instead if it was set.
+The Go build tags to use for all commands, that support a `-tags '...'` argument. When running tests, go.testTags will be used instead if it was set. This is propagated to the language server if `gopls.buildFlags` is not specified.
 
 Default: ``
 
@@ -617,6 +617,8 @@ buildFlags is the set of flags passed on to the build system when invoked.
 It is applied to queries like `go list`, which is used when discovering files.
 The most common use is to set `-tags`.
 
+If unspecified, values of `go.buildFlags, go.buildTags` will be propagated.
+
 
 #### `codelenses`
 codelenses overrides the enabled/disabled state of code lenses. See the "Code Lenses"
@@ -626,7 +628,7 @@ Example Usage:
 ```json5
 "gopls": {
 ...
-  "codelenses": {
+  "codelens": {
     "generate": false,  // Don't show the `go generate` lens.
     "gc_details": true  // Show a code lens toggling the display of gc's choices.
   }

package.json

@@ -847,13 +847,13 @@
             "type": "string"
           },
           "default": [],
-          "description": "Flags to `go build`/`go test` used during build-on-save or running tests. (e.g. [\"-ldflags='-s'\"])",
+          "description": "Flags to `go build`/`go test` used during build-on-save or running tests. (e.g. [\"-ldflags='-s'\"]) This is propagated to the language server if `gopls.buildFlags` is not specified.",
           "scope": "resource"
         },
         "go.buildTags": {
           "type": "string",
           "default": "",
-          "description": "The Go build tags to use for all commands, that support a `-tags '...'` argument. When running tests, go.testTags will be used instead if it was set.",
+          "description": "The Go build tags to use for all commands, that support a `-tags '...'` argument. When running tests, go.testTags will be used instead if it was set. This is propagated to the language server if `gopls.buildFlags` is not specified.",
           "scope": "resource"
         },
         "go.testTags": {
@@ -1693,7 +1693,7 @@
           "properties": {
             "buildFlags": {
               "type": "array",
-              "markdownDescription": "buildFlags is the set of flags passed on to the build system when invoked.\nIt is applied to queries like `go list`, which is used when discovering files.\nThe most common use is to set `-tags`.\n",
+              "markdownDescription": "buildFlags is the set of flags passed on to the build system when invoked.\nIt is applied to queries like `go list`, which is used when discovering files.\nThe most common use is to set `-tags`.\n\nIf unspecified, values of `go.buildFlags, go.buildTags` will be propagated.\n",
               "default": [],
               "scope": "resource"
             },
@@ -1755,7 +1755,7 @@
             },
             "codelenses": {
               "type": "object",
-              "markdownDescription": "codelenses overrides the enabled/disabled state of code lenses. See the \"Code Lenses\"\nsection of settings.md for the list of supported lenses.\n\nExample Usage:\n```json5\n\"gopls\": {\n...\n  \"codelenses\": {\n    \"generate\": false,  // Don't show the `go generate` lens.\n    \"gc_details\": true  // Show a code lens toggling the display of gc's choices.\n  }\n...\n}\n```\n",
+              "markdownDescription": "codelenses overrides the enabled/disabled state of code lenses. See the \"Code Lenses\"\nsection of settings.md for the list of supported lenses.\n\nExample Usage:\n```json5\n\"gopls\": {\n...\n  \"codelens\": {\n    \"generate\": false,  // Don't show the `go generate` lens.\n    \"gc_details\": true  // Show a code lens toggling the display of gc's choices.\n  }\n...\n}\n```\n",
               "default": {
                 "gc_details": false,
                 "generate": true,

src/debugAdapter/goDebug.ts

@@ -90,6 +90,7 @@ interface DebuggerState {
 	currentGoroutine: DebugGoroutine;
 	Running: boolean;
 	Threads: DebugThread[];
+	NextInProgress: boolean;
 }
 
 export interface PackageBuildInfo {
@@ -371,7 +372,18 @@ function normalizePath(filePath: string) {
 	return filePath;
 }
 
-function normalizeSeparators(filePath: string): string {
+// normalizeSeparators will prepare the filepath for comparison in mapping from
+// local to debugger path and from debugger path to local path. All separators are
+// replaced with '/', and the drive name is capitalized for windows paths.
+// Exported for testing
+export function normalizeSeparators(filePath: string): string {
+	// Although the current machine may not be running windows,
+	// the remote machine may be and we need to fix the drive
+	// casing.
+	// This is a workaround for issue in https://github.com/Microsoft/vscode/issues/9448#issuecomment-244804026
+	if (filePath.indexOf(':') === 1) {
+		filePath = filePath.substr(0, 1).toUpperCase() + filePath.substr(1);
+	}
 	return filePath.replace(/\/|\\/g, '/');
 }
 
@@ -846,6 +858,7 @@ export class GoDebugSession extends LoggingDebugSession {
 	private breakpoints: Map<string, DebugBreakpoint[]>;
 	// Editing breakpoints requires halting delve, skip sending Stop Event to VS Code in such cases
 	private skipStopEventOnce: boolean;
+	private overrideStopReason: string;
 	private debugState: DebuggerState;
 	private delve: Delve;
 	private localPathSeparator: string;
@@ -866,13 +879,16 @@ export class GoDebugSession extends LoggingDebugSession {
 
 	private continueEpoch = 0;
 	private continueRequestRunning = false;
+	private nextEpoch = 0;
+	private nextRequestRunning = false;
 	public constructor(
 		debuggerLinesStartAt1: boolean,
 		isServer: boolean = false,
 		readonly fileSystem = fs) {
 		super('', debuggerLinesStartAt1, isServer);
 		this.variableHandles = new Handles<DebugVariable>();
 		this.skipStopEventOnce = false;
+		this.overrideStopReason = '';
 		this.stopOnEntry = false;
 		this.debugState = null;
 		this.delve = null;
@@ -1292,11 +1308,25 @@ export class GoDebugSession extends LoggingDebugSession {
 			log('Debuggee is not running. Setting breakpoints without halting.');
 			await this.setBreakPoints(response, args);
 		} else {
+			// Skip stop event if a continue request is running.
 			this.skipStopEventOnce = this.continueRequestRunning;
+			const haltedDuringNext = this.nextRequestRunning;
+			if (haltedDuringNext) {
+				this.overrideStopReason = 'next cancelled';
+			}
+
 			log(`Halting before setting breakpoints. SkipStopEventOnce is ${this.skipStopEventOnce}.`);
 			this.delve.callPromise('Command', [{ name: 'halt' }]).then(
 				() => {
 					return this.setBreakPoints(response, args).then(() => {
+						// We do not want to continue if it was running a next request, since the
+						// request was automatically cancelled.
+						if (haltedDuringNext) {
+							// Send an output event containing a warning that next was cancelled.
+							const warning = `Setting breakpoints during 'next', 'step in' or 'step out' halted delve and cancelled the next request`;
+							this.sendEvent(new OutputEvent(warning, 'stderr'));
+							return;
+						}
 						return this.continue(true).then(null, (err) => {
 							this.logDelveError(err, 'Failed to continue delve after halting it to set breakpoints');
 						});
@@ -1676,8 +1706,16 @@ export class GoDebugSession extends LoggingDebugSession {
 	}
 
 	protected nextRequest(response: DebugProtocol.NextResponse): void {
+		this.nextEpoch++;
+		const closureEpoch = this.nextEpoch;
+		this.nextRequestRunning = true;
+
 		log('NextRequest');
 		this.delve.call<DebuggerState | CommandOut>('Command', [{ name: 'next' }], (err, out) => {
+			if (closureEpoch === this.continueEpoch) {
+				this.nextRequestRunning = false;
+			}
+
 			if (err) {
 				this.logDelveError(err, 'Failed to next');
 			}
@@ -1691,8 +1729,16 @@ export class GoDebugSession extends LoggingDebugSession {
 	}
 
 	protected stepInRequest(response: DebugProtocol.StepInResponse): void {
+		this.nextEpoch++;
+		const closureEpoch = this.nextEpoch;
+		this.nextRequestRunning = true;
+
 		log('StepInRequest');
 		this.delve.call<DebuggerState | CommandOut>('Command', [{ name: 'step' }], (err, out) => {
+			if (closureEpoch === this.continueEpoch) {
+				this.nextRequestRunning = false;
+			}
+
 			if (err) {
 				this.logDelveError(err, 'Failed to step in');
 			}
@@ -1706,8 +1752,16 @@ export class GoDebugSession extends LoggingDebugSession {
 	}
 
 	protected stepOutRequest(response: DebugProtocol.StepOutResponse): void {
+		this.nextEpoch++;
+		const closureEpoch = this.nextEpoch;
+		this.nextRequestRunning = true;
+
 		log('StepOutRequest');
 		this.delve.call<DebuggerState | CommandOut>('Command', [{ name: 'stepOut' }], (err, out) => {
+			if (closureEpoch === this.continueEpoch) {
+				this.nextRequestRunning = false;
+			}
+
 			if (err) {
 				this.logDelveError(err, 'Failed to step out');
 			}
@@ -2341,6 +2395,11 @@ export class GoDebugSession extends LoggingDebugSession {
 					return;
 				}
 
+				if (this.overrideStopReason?.length > 0) {
+					reason = this.overrideStopReason;
+					this.overrideStopReason = '';
+				}
+
 				const stoppedEvent = new StoppedEvent(reason, this.debugState.currentGoroutine.id);
 				(<any>stoppedEvent.body).allThreadsStopped = true;
 				this.sendEvent(stoppedEvent);
@@ -2364,7 +2423,7 @@ export class GoDebugSession extends LoggingDebugSession {
 		} catch (error) {
 			this.logDelveError(error, 'Failed to get state');
 			// Fall back to the internal tracking.
-			return this.continueRequestRunning;
+			return this.continueRequestRunning || this.nextRequestRunning;
 		}
 	}
 

src/goLanguageServer.ts

@@ -290,9 +290,7 @@ function buildLanguageClientOption(cfg: LanguageServerConfig): BuildLanguageClie
 // buildLanguageClient returns a language client built using the given language server config.
 // The returned language client need to be started before use.
 export async function buildLanguageClient(cfg: BuildLanguageClientOption): Promise<LanguageClient> {
-	let goplsWorkspaceConfig = getGoplsConfig() as any;
-	goplsWorkspaceConfig = filterDefaultConfigValues(goplsWorkspaceConfig, 'gopls', undefined);
-	goplsWorkspaceConfig = await adjustGoplsWorkspaceConfiguration(cfg, goplsWorkspaceConfig);
+	const goplsWorkspaceConfig = await adjustGoplsWorkspaceConfiguration(cfg, getGoplsConfig(), 'gopls', undefined);
 	const c = new LanguageClient(
 		'go',  // id
 		cfg.serverName,  // name e.g. gopls
@@ -505,8 +503,7 @@ export async function buildLanguageClient(cfg: BuildLanguageClientOption): Promi
 								const scopeUri = params.items[i].scopeUri;
 								const resource = scopeUri ? vscode.Uri.parse(scopeUri) : undefined;
 								const section = params.items[i].section;
-								workspaceConfig = filterDefaultConfigValues(workspaceConfig, section, resource);
-								workspaceConfig = await adjustGoplsWorkspaceConfiguration(cfg, workspaceConfig);
+								workspaceConfig = await adjustGoplsWorkspaceConfiguration(cfg, workspaceConfig, section, resource);
 							}
 							ret.push(workspaceConfig);
 						}
@@ -519,19 +516,15 @@ export async function buildLanguageClient(cfg: BuildLanguageClientOption): Promi
 	return c;
 }
 
-// filterDefaultConfigValues removes the entries filled based on the default values
+// filterGoplsDefaultConfigValues removes the entries filled based on the default values
 // and selects only those the user explicitly specifies in their settings.
-// This assumes workspaceConfig is a non-null(undefined) object type.
+// This returns a new object created based on the filtered properties of workspaceConfig.
 // Exported for testing.
-export function filterDefaultConfigValues(workspaceConfig: any, section: string, resource: vscode.Uri): any {
+export function filterGoplsDefaultConfigValues(workspaceConfig: any, resource: vscode.Uri): any {
 	if (!workspaceConfig) {
-		return workspaceConfig;
+		workspaceConfig = {};
 	}
-
-	const dot = section?.lastIndexOf('.') || -1;
-	const sectionKey = dot >= 0 ? section.substr(0, dot) : section;  // e.g. 'gopls'
-
-	const cfg = vscode.workspace.getConfiguration(sectionKey, resource);
+	const cfg = getGoplsConfig(resource);
 	const filtered = {} as { [key: string]: any };
 	for (const [key, value] of Object.entries(workspaceConfig)) {
 		if (typeof value === 'function') {
@@ -556,31 +549,63 @@ export function filterDefaultConfigValues(workspaceConfig: any, section: string,
 	return filtered;
 }
 
-// adjustGoplsWorkspaceConfiguration adds any extra options to the gopls
-// config. Right now, the only extra option is enabling experiments for the
-// Nightly extension.
-async function adjustGoplsWorkspaceConfiguration(cfg: LanguageServerConfig, config: any): Promise<any> {
-	if (!config) {
-		return config;
+// passGoConfigToGoplsConfigValues passes some of the relevant 'go.' settings to gopls settings.
+// This assumes `goplsWorkspaceConfig` is an output of filterGoplsDefaultConfigValues,
+// so it is modifiable and doesn't contain properties that are not explicitly set.
+//   - go.buildTags and go.buildFlags are passed as gopls.buildFlags
+//     if goplsWorkspaceConfig doesn't explicitly set it yet.
+// Exported for testing.
+export function passGoConfigToGoplsConfigValues(goplsWorkspaceConfig: any, goWorkspaceConfig: any): any {
+	if (!goplsWorkspaceConfig) {
+		goplsWorkspaceConfig = {};
+	}
+
+	// If gopls.buildFlags is set, don't touch it.
+	if (goplsWorkspaceConfig.buildFlags === undefined) {
+		const buildFlags = [] as string[];
+		if (goWorkspaceConfig?.buildFlags) {
+			buildFlags.push(...goWorkspaceConfig?.buildFlags);
+		}
+		if (goWorkspaceConfig?.buildTags && buildFlags.indexOf('-tags') === -1) {
+			buildFlags.push('-tags', goWorkspaceConfig?.buildTags);
+		}
+		if (buildFlags.length > 0) {
+			goplsWorkspaceConfig.buildFlags = buildFlags;
+		}
 	}
+	return goplsWorkspaceConfig;
+}
+
+// adjustGoplsWorkspaceConfiguration filters unnecessary options and adds any necessary, additional
+// options to the gopls config. See filterGoplsDefaultConfigValues, passGoConfigToGoplsConfigValues.
+// If this is for the nightly extension, we also request to activate features under experiments.
+async function adjustGoplsWorkspaceConfiguration(cfg: LanguageServerConfig, workspaceConfig: any, section: string, resource: vscode.Uri): Promise<any> {
+	// We process only gopls config
+	if (section !== 'gopls') {
+		return workspaceConfig;
+	}
+
+	workspaceConfig = filterGoplsDefaultConfigValues(workspaceConfig, resource);
+	// note: workspaceConfig is a modifiable, valid object.
+	workspaceConfig = passGoConfigToGoplsConfigValues(workspaceConfig, getGoConfig(resource));
+
 	// Only modify the user's configurations for the Nightly.
 	if (extensionId !== 'golang.go-nightly') {
-		return config;
+		return workspaceConfig;
 	}
 	// allExperiments is only available with gopls/v0.5.2 and above.
 	const version = await getLocalGoplsVersion(cfg);
 	if (!version) {
-		return config;
+		return workspaceConfig;
 	}
 	const sv = semver.parse(version, true);
 	if (!sv || semver.lt(sv, 'v0.5.2')) {
-		return config;
+		return workspaceConfig;
 	}
-	const newConfig = Object.assign({}, config);
-	if (!config['allExperiments']) {
-		newConfig['allExperiments'] = true;
+	if (!workspaceConfig['allExperiments']) {
+		workspaceConfig['allExperiments'] = true;
 	}
-	return newConfig;
+	return workspaceConfig;
 }
 
 // createTestCodeLens adds the go.test.cursor and go.debug.cursor code lens

src/util.ts

@@ -158,12 +158,12 @@ let toolsGopath: string;
 
 // getGoConfig is declared as an exported const rather than a function, so it can be stubbbed in testing.
 export const getGoConfig = (uri?: vscode.Uri) => {
-	return getConfig('go');
+	return getConfig('go', uri);
 };
 
 // getGoplsConfig returns the user's gopls configuration.
-export function getGoplsConfig() {
-	return getConfig('gopls');
+export function getGoplsConfig(uri?: vscode.Uri) {
+	return getConfig('gopls', uri);
 }
 
 // getCheckForToolsUpdatesConfig returns go.toolsManagement.checkForUpdates configuration.