src/goDebugConfiguration: combine envFile and env

With the resolveDebugConfigurationWithSubstitutedVariables,
we can read envFile and combine it into the env property
from the extension host.

Delve DAP will handle only the env arg and not have any
special handling for envFile. So, processing it from the
extension side makes more sense for long term.

This move allows us to centralize the .env file read support.
For backwards compatibility, I left the logic in the old DA
but removed it from the new delve DAP DA.

Fixes golang/vscode-go#452
Updates golang/vscode-go#23

Change-Id: I641eb2e62051985ba3486901483ad796256aba2c
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/248659
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Polina Sokolova <polina@google.com>
diff --git a/src/debugAdapter/goDebug.ts b/src/debugAdapter/goDebug.ts
index b6dd86e..f669ac5 100644
--- a/src/debugAdapter/goDebug.ts
+++ b/src/debugAdapter/goDebug.ts
@@ -259,8 +259,6 @@
 	buildFlags?: string;
 	init?: string;
 	trace?: 'verbose' | 'log' | 'error';
-	/** Optional path to .env file. */
-	envFile?: string | string[];
 	backend?: string;
 	output?: string;
 	/** Delve LoadConfig parameters */
@@ -273,6 +271,12 @@
 
 	showGlobalVariables?: boolean;
 	packagePathToGoModPathMap: { [key: string]: string };
+
+	/** Optional path to .env file. */
+	// TODO: deprecate .env file processing from DA.
+	// We expect the extension processes .env files
+	// and send the information to DA using the 'env' property.
+	envFile?: string | string[];
 }
 
 interface AttachRequestArguments extends DebugProtocol.AttachRequestArguments {
diff --git a/src/debugAdapter2/goDlvDebug.ts b/src/debugAdapter2/goDlvDebug.ts
index c861f81..28676fc 100644
--- a/src/debugAdapter2/goDlvDebug.ts
+++ b/src/debugAdapter2/goDlvDebug.ts
@@ -20,7 +20,6 @@
 	TerminatedEvent
 } from 'vscode-debugadapter';
 import { DebugProtocol } from 'vscode-debugprotocol';
-import { parseEnvFiles } from '../utils/envUtils';
 import { envPath, getBinPathWithPreferredGopathGoroot } from '../utils/goPath';
 import { killProcessTree } from '../utils/processUtils';
 
@@ -57,8 +56,6 @@
 	buildFlags?: string;
 	init?: string;
 	trace?: 'verbose' | 'log' | 'error';
-	/** Optional path to .env file. */
-	envFile?: string | string[];
 	backend?: string;
 	output?: string;
 	/** Delve LoadConfig parameters */
@@ -610,10 +607,11 @@
 			goRunArgs.push(...launchArgs.args);
 		}
 
-		// Read env from disk and merge into env variables.
-		const fileEnvs = parseEnvFiles(launchArgs.envFile);
+		// launchArgs.env includes all the environment variables
+		// including vscode-go's toolsExecutionEnvironment (PATH, GOPATH, ...),
+		// and those read from .env files.
 		const launchArgsEnv = launchArgs.env || {};
-		const programEnv = Object.assign({}, process.env, fileEnvs, launchArgsEnv);
+		const programEnv = Object.assign({}, process.env, launchArgsEnv);
 
 		log(`Current working directory: ${dirname}`);
 		const goExe = getBinPathWithPreferredGopathGoroot('go', []);
diff --git a/src/goDebugConfiguration.ts b/src/goDebugConfiguration.ts
index a75b847..983269f 100644
--- a/src/goDebugConfiguration.ts
+++ b/src/goDebugConfiguration.ts
@@ -12,6 +12,7 @@
 import { packagePathToGoModPathMap } from './goModules';
 import { getFromGlobalState, updateGlobalState } from './stateUtils';
 import { getBinPath, getCurrentGoPath, getGoConfig } from './util';
+import { parseEnvFiles } from './utils/envUtils';
 
 export class GoDebugConfigurationProvider implements vscode.DebugConfigurationProvider {
 	constructor(private defaultDebugAdapterType: string = 'go') { }
@@ -60,20 +61,9 @@
 
 		debugConfiguration['packagePathToGoModPathMap'] = packagePathToGoModPathMap;
 
-		const gopath = getCurrentGoPath(folder ? folder.uri : undefined);
-		if (!debugConfiguration['env']) {
-			debugConfiguration['env'] = { GOPATH: gopath };
-		} else if (!debugConfiguration['env']['GOPATH']) {
-			debugConfiguration['env']['GOPATH'] = gopath;
-		}
-
 		const goConfig = getGoConfig(folder && folder.uri);
-		const goToolsEnvVars = toolExecutionEnvironment();
-		Object.keys(goToolsEnvVars).forEach((key) => {
-			if (!debugConfiguration['env'].hasOwnProperty(key)) {
-				debugConfiguration['env'][key] = goToolsEnvVars[key];
-			}
-		});
+
+		combineEnvFilesAndEnv(folder, debugConfiguration);
 
 		const dlvConfig = goConfig.get<any>('delveConfig');
 		let useApiV1 = false;
@@ -146,3 +136,18 @@
 		});
 	}
 }
+
+// combineEnvFilesAndEnv reads debugConfiguration.envFile and
+// combines the environment variables from all the env files and
+// debugConfiguration.env, on top of the tools execution environment variables.
+// It also unsets 'envFile' from the user-suppled debugConfiguration
+// because it is already applied.
+function combineEnvFilesAndEnv(
+	folder: vscode.WorkspaceFolder, debugConfiguration: vscode.DebugConfiguration) {
+	const goToolsEnvVars = toolExecutionEnvironment(folder?.uri); // also includes GOPATH: getCurrentGoPath().
+	const fileEnvs = parseEnvFiles(debugConfiguration['envFile']);
+	const env = debugConfiguration['env'] || {};
+
+	debugConfiguration['env'] = Object.assign(goToolsEnvVars, fileEnvs, env);
+	debugConfiguration['envFile'] = undefined;  // unset, since we already processed.
+}
diff --git a/src/goEnv.ts b/src/goEnv.ts
index 0372895..556686a 100644
--- a/src/goEnv.ts
+++ b/src/goEnv.ts
@@ -44,9 +44,9 @@
 
 // toolExecutionEnvironment returns the environment in which tools should
 // be executed. It always returns a new object.
-export function toolExecutionEnvironment(): NodeJS.Dict<string> {
+export function toolExecutionEnvironment(uri?: vscode.Uri): NodeJS.Dict<string> {
 	const env = newEnvironment();
-	const gopath = getCurrentGoPath();
+	const gopath = getCurrentGoPath(uri);
 	if (gopath) {
 		env['GOPATH'] = gopath;
 	}
diff --git a/test/integration/goDebugConfiguration.test.ts b/test/integration/goDebugConfiguration.test.ts
new file mode 100644
index 0000000..595ba45
--- /dev/null
+++ b/test/integration/goDebugConfiguration.test.ts
@@ -0,0 +1,100 @@
+import assert = require('assert');
+import fs = require('fs');
+import os = require('os');
+import path = require('path');
+import sinon = require('sinon');
+import vscode = require('vscode');
+import { GoDebugConfigurationProvider } from '../../src/goDebugConfiguration';
+import goEnv = require('../../src/goEnv');
+import { updateGoVarsFromConfig } from '../../src/goInstallTools';
+import { getCurrentGoPath, rmdirRecursive } from '../../src/util';
+
+suite('Debug Environment Variable Merge Test', () => {
+	const debugConfigProvider = new GoDebugConfigurationProvider();
+
+	suiteSetup(async () => {
+		await updateGoVarsFromConfig();
+
+		// Set up the test fixtures.
+		const fixtureSourcePath = path.join(__dirname, '..', '..', '..', 'test', 'fixtures');
+		const filePath = path.join(fixtureSourcePath, 'baseTest', 'test.go');
+		await vscode.workspace.openTextDocument(vscode.Uri.file(filePath));
+	});
+
+	suiteTeardown(() => {
+		vscode.commands.executeCommand('workbench.action.closeActiveEditor');
+	});
+
+	let sandbox: sinon.SinonSandbox;
+	let tmpDir = '';
+	const toolExecutionEnv: NodeJS.Dict<string> = {};
+	setup(() => {
+		tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'godebugconfig_test'));
+		sandbox = sinon.createSandbox();
+
+	});
+
+	teardown(() => {
+		sandbox.restore();
+		rmdirRecursive(tmpDir);
+	});
+
+	interface Input {
+		env?: { [key: string]: any };
+		envFile?: string|string[];
+		toolsEnv?: { [key: string]: any};
+	}
+
+	function runTest(input: Input,	expected: { [key: string]: any}) {
+		sandbox.stub(goEnv, 'toolExecutionEnvironment').returns(input.toolsEnv || {});
+		const config = debugConfigProvider.resolveDebugConfigurationWithSubstitutedVariables(undefined, {
+			type: 'go',
+			name: 'Launch',
+			request: 'launch',
+			env: input.env,
+			envFile: input.envFile,
+		});
+
+		const actual = config.env;
+		assert.deepStrictEqual(actual, expected);
+	}
+
+	test('works with empty launchArgs', () => {
+		runTest({}, {});
+	});
+
+	test('toolsEnvVars is propagated', () => {
+		const toolsEnv = {
+			GOPATH: '/gopath',
+			GOOS: 'valueFromToolsEnv'};
+
+		runTest({toolsEnv}, {
+			GOPATH: '/gopath',
+			GOOS: 'valueFromToolsEnv'});
+	});
+
+	test('preserves settings from launchArgs.env', () => {
+		const env = {GOPATH: 'valueFromEnv', GOOS: 'valueFromEnv2'};
+		runTest({env}, {
+			GOPATH: 'valueFromEnv',
+			GOOS: 'valueFromEnv2'});
+	});
+
+	test('preserves settings from launchArgs.envFile', () => {
+		const envFile = path.join(tmpDir, 'env');
+		fs.writeFileSync(envFile, 'GOPATH=valueFromEnvFile');
+		runTest({envFile}, {GOPATH: 'valueFromEnvFile'});
+	});
+
+	test('launchArgs.env overwrites launchArgs.envFile', () => {
+		const env = {SOMEVAR1: 'valueFromEnv'};
+		const envFile = path.join(tmpDir, 'env');
+		fs.writeFileSync(envFile, [
+			'SOMEVAR1=valueFromEnvFile1',
+			'SOMEVAR2=valueFromEnvFile2'].join('\n'));
+
+		runTest({ env, envFile }, {
+			SOMEVAR1: 'valueFromEnv',
+			SOMEVAR2: 'valueFromEnvFile2'});
+	});
+});