src/goDebugConfiguration: remove user set '--gcflags' from config

When building the executable for debugging, delve sets '--gcflags'.
When the user also provides this flag, there is an error in the Go
build system, as the flag can only be specified once. If a user
tries to set this flag, remove it and display a warning.

Fixes #117

Change-Id: I84a1e718674b4bafaf31da91711d4b2f06f8b280
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/265580
Trust: Suzy Mueller <[email protected]>
Trust: Hyang-Ah Hana Kim <[email protected]>
Run-TryBot: Suzy Mueller <[email protected]>
TryBot-Result: kokoro <[email protected]>
Reviewed-by: Hyang-Ah Hana Kim <[email protected]>

Author: suzmue

docs/debugging.md

@@ -9,6 +9,7 @@ This document explains how to debug your Go programs in VS Code. The Go debugger
   * [Configuration](#configuration)
 * [Launch Configurations](#launch-configurations)
   * [Specifying build tags](#specifying-build-tags)
+  * [Specifying other build flags](#specifying-other-build-flags)
   * [Using VS Code Variables](#using-vs-code-variables)
   * [Snippets](#snippets)
 * [Debugging on Windows Subsystem for Linux (WSL)](#debugging-on-windows-subsystem-for-linux-wsl)
@@ -135,6 +136,22 @@ in your launch configuration. This property supports multiple tags, which you ca
 
 <!--TODO(rstambler): Confirm that the extension works with a comma (not space) separated list.-->
 
+### Specifying other build flags
+
+The flags specified in `buildFlags` and `env.GOFLAGS` are passed to the Go compiler when building your program for debugging. Delve adds `--gcflags='all=-N -l'` to the list of build flags to disable optimizations. User specified buildFlags conflict with this setting, so the extension removes them ([Issue #117](https://github.com/golang/vscode-go/issues/117)). If you wish to debug a program using custom `--gcflags`, build the program using `go build` and launch using `exec` mode:
+
+```json
+{
+    "name": "Launch executable",
+    "type": "go",
+    "request": "launch",
+    "mode": "exec",
+    "program": "/absolute/path/to/executable"
+}
+```
+
+Note that it is not recommended to debug optimized executables as Delve may not have the information necessary to properly debug your program.
+
 ### Using [VS Code variables]
 
 Any property in the launch configuration that requires a file path can be specified in terms of [VS Code variables]. Here are some useful ones to know:

package-lock.json

@@ -46,6 +46,7 @@
   },
   "extensionDependencies": [],
   "dependencies": {
+    "@types/yargs-parser": "^15.0.0",
     "deep-equal": "^2.0.2",
     "diff": "^4.0.2",
     "glob": "^7.1.6",

package.json

@@ -7,6 +7,8 @@
 
 import path = require('path');
 import vscode = require('vscode');
+import parse = require('yargs-parser');
+import unparse = require('yargs-unparser');
 import { toolExecutionEnvironment } from './goEnv';
 import { promptForMissingTool } from './goInstallTools';
 import { packagePathToGoModPathMap } from './goModules';
@@ -92,6 +94,28 @@ export class GoDebugConfigurationProvider implements vscode.DebugConfigurationPr
 			debugConfiguration['cwd'] = resolvePath(debugConfiguration['cwd']);
 		}
 
+		// Remove any '--gcflags' entries and show a warning
+		if (debugConfiguration['buildFlags']) {
+			const resp = this.removeFlag(debugConfiguration['buildFlags'], 'gcflags');
+			if (resp.removed) {
+				debugConfiguration['buildFlags'] = resp.args;
+				this.showWarning(
+					'ignoreDebugGCFlagsWarning',
+					`User specified build flag '--gcflags' in 'buildFlags' is being ignored (see [debugging with build flags](https://github.com/golang/vscode-go/blob/master/docs/debugging.md#specifying-other-build-flags) documentation)`
+				);
+			}
+		}
+		if (debugConfiguration['env'] && debugConfiguration['env']['GOFLAGS']) {
+			const resp = this.removeFlag(debugConfiguration['env']['GOFLAGS'], 'gcflags');
+			if (resp.removed) {
+				debugConfiguration['env']['GOFLAGS'] = resp.args;
+				this.showWarning(
+					'ignoreDebugGCFlagsWarning',
+					`User specified build flag '--gcflags' in 'GOFLAGS' is being ignored (see [debugging with build flags](https://github.com/golang/vscode-go/blob/master/docs/debugging.md#specifying-other-build-flags) documentation)`
+				);
+			}
+		}
+
 		debugConfiguration['dlvToolPath'] = getBinPath('dlv');
 		if (!path.isAbsolute(debugConfiguration['dlvToolPath'])) {
 			promptForMissingTool('dlv');
@@ -156,4 +180,13 @@ export class GoDebugConfigurationProvider implements vscode.DebugConfigurationPr
 			}
 		});
 	}
+
+	private removeFlag(args: string, flag: string): {args: string, removed: boolean} {
+		const argv = parse(args, {configuration: {'short-option-groups': false}});
+		if (argv[flag]) {
+			delete argv[flag];
+			return { args: unparse(argv).join(' '), removed: true };
+		}
+		return {args, removed: false};
+	}
 }

src/goDebugConfiguration.ts

@@ -4,6 +4,7 @@ import os = require('os');
 import path = require('path');
 import sinon = require('sinon');
 import vscode = require('vscode');
+import parse = require('yargs-parser');
 import { GoDebugConfigurationProvider } from '../../src/goDebugConfiguration';
 import goEnv = require('../../src/goEnv');
 import { updateGoVarsFromConfig } from '../../src/goInstallTools';
@@ -279,3 +280,136 @@ suite('Debug Configuration Merge User Settings', () => {
 		});
 	});
 });
+
+suite('Debug Configuration Modify User Config', () => {
+	const debugConfigProvider = new GoDebugConfigurationProvider();
+
+	function checkBuildFlags(input: string, expected: { [key: string]: any }) {
+		// Parse the string result.
+		const actual = parse(input, {configuration: {'short-option-groups': false}} );
+
+		// Delete the empty entry that is created by parse.
+		delete actual['_'];
+
+		// Compare the two maps.
+		assert.strictEqual(actual.size, expected.size);
+
+		const expectedKeys = [];
+		for (const key in expected) {
+			if (expected.hasOwnProperty(key)) {
+				expectedKeys.push(key);
+			}
+		}
+		expectedKeys.sort();
+
+		const actualKeys = [];
+		for (const key in actual) {
+			if (actual.hasOwnProperty(key)) {
+				actualKeys.push(key);
+			}
+		}
+		actualKeys.sort();
+
+		for (let i = 0; i < expectedKeys.length; i ++) {
+			assert.strictEqual(actualKeys[i], expectedKeys[i]);
+			assert.strictEqual(actual[actualKeys[i]], expected[expectedKeys[i]]);
+		}
+	}
+
+	suite('remove gcflags', () => {
+		test('remove user set --gcflags in buildFlags', () => {
+			const config = {
+				name: 'Launch',
+				type: 'go',
+				request: 'launch',
+				mode: 'auto',
+				program: '${fileDirname}',
+				env: {},
+				buildFlags: '--gcflags=all=-l'
+			};
+
+			debugConfigProvider.resolveDebugConfiguration(undefined, config);
+
+			checkBuildFlags(config.buildFlags, {});
+		});
+
+		test('remove user set -gcflags in buildFlags', () => {
+			const config = {
+				name: 'Launch',
+				type: 'go',
+				request: 'launch',
+				mode: 'auto',
+				program: '${fileDirname}',
+				env: {},
+				buildFlags: `-gcflags all=-l`
+			};
+
+			debugConfigProvider.resolveDebugConfiguration(undefined, config);
+
+			checkBuildFlags(config.buildFlags, {});
+		});
+
+		test('remove user set --gcflags while preserving other build flags in buildFlags', () => {
+			const config = {
+				name: 'Launch',
+				type: 'go',
+				request: 'launch',
+				mode: 'auto',
+				program: '${fileDirname}',
+				env: {},
+				buildFlags: '-race --gcflags=all=-l --mod=mod'
+			};
+
+			debugConfigProvider.resolveDebugConfiguration(undefined, config);
+
+			checkBuildFlags(config.buildFlags, {race: true, mod: 'mod'});
+		});
+
+		test('preserve empty buildFlags', () => {
+			const config = {
+				name: 'Launch',
+				type: 'go',
+				request: 'launch',
+				mode: 'auto',
+				program: '${fileDirname}',
+				env: {},
+				buildFlags: ''
+			};
+
+			debugConfigProvider.resolveDebugConfiguration(undefined, config);
+
+			checkBuildFlags(config.buildFlags, {});
+		});
+
+		test('preserve buildFlags', () => {
+			const config = {
+				name: 'Launch',
+				type: 'go',
+				request: 'launch',
+				mode: 'auto',
+				program: '${fileDirname}',
+				env: {},
+				buildFlags: '-race --mod=mod'
+			};
+
+			debugConfigProvider.resolveDebugConfiguration(undefined, config);
+
+			checkBuildFlags(config.buildFlags, {race: true, mod: 'mod'});
+		});
+
+		test('remove user set --gcflags in GOFLAGS', () => {
+			const config = {
+				name: 'Launch',
+				type: 'go',
+				request: 'launch',
+				mode: 'auto',
+				program: '${fileDirname}',
+				env: {GOFLAGS: '-race --gcflags=-l --mod=mod'},
+			};
+
+			debugConfigProvider.resolveDebugConfiguration(undefined, config);
+
+			checkBuildFlags(config.env.GOFLAGS, {race: true, mod: 'mod'});
+		});
+	});
+});

test/integration/goDebugConfiguration.test.ts

@@ -0,0 +1,25 @@
+declare module 'yargs-unparser' {
+	// Modified from './node_modules/@types/yargs-parser/index.d.ts'.
+	namespace yargsUnparser {
+		interface Arguments {
+			/** Non-option arguments */
+			_: string[];
+			/** The script name or node command */
+			$0: string;
+			/** All remaining options */
+			[argName: string]: any;
+		}
+
+		interface Options {
+			alias?: { [key: string]: string | string[] };
+			default?: { [key: string]: any };
+			command?: string;
+		}
+
+		interface Unparser {
+			(argv: Arguments, opts?: Options): string[];
+		}
+	}
+	var yargsUnparser: yargsUnparser.Unparser;
+	export = yargsUnparser;
+}