src/goEnvironmentStatus.ts: clear pre-installed terminal PATH mutation

Changes to vscode.EnvironmentVariableCollection persist across
vscode sessions, so when we decide not to mutate PATH, we need to clear
the preexisting changes. The new function 'clearGoRuntimeBaseFromPATH'
reverses almost all of what addGoRuntimeBaseToPATH did on the persisted
state.

Manually tested by setting/unsetting "go.alternateTools".

Also, fixes the case where 'go.alternateTools' is set as an alternate
tool name rather than an absolute path - in that case, we search the
alternate tool from PATH. In this case, the reason shouldn't be 'path'.

Manually tested by setting
   "go.alternateTools": { "go": "mygo.sh" }

where "mygo.sh" is another binary that exists in PATH and shells out go.

In addition to this, this change fixes an exception thrown while
calling updateIntegratedTerminal when no 'go' executable is found from
the default PATH. In that case, getBinPathFromEnvVar returns null,
and path.dirname throws an error for non-string params.
This results in the failure of updating terminal's PATH change and
the users will not see the picked go from the terminal.
This is a bug preexisted before 0.17.1

Manually tested by launching code without go in PATH.

Updates #679
Fixes #713

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

Author: hyangah

src/goEnvironmentStatus.ts

@@ -303,19 +303,25 @@ async function downloadGo(goOption: GoEnvironmentOption) {
 // PATH value cached before addGoRuntimeBaseToPath modified.
 let defaultPathEnv = '';
 
+function pathEnvVarName(): string|undefined {
+	if (process.env.hasOwnProperty('PATH')) {
+		return 'PATH';
+	} else if (process.platform === 'win32' && process.env.hasOwnProperty('Path')) {
+		return 'Path';
+	} else {
+		return;
+	}
+}
+
 // addGoRuntimeBaseToPATH adds the given path to the front of the PATH environment variable.
 // It removes duplicates.
 // TODO: can we avoid changing PATH but utilize toolExecutionEnv?
 export function addGoRuntimeBaseToPATH(newGoRuntimeBase: string) {
 	if (!newGoRuntimeBase) {
 		return;
 	}
-	let pathEnvVar: string;
-	if (process.env.hasOwnProperty('PATH')) {
-		pathEnvVar = 'PATH';
-	} else if (process.platform === 'win32' && process.env.hasOwnProperty('Path')) {
-		pathEnvVar = 'Path';
-	} else {
+	const pathEnvVar = pathEnvVarName();
+	if (!pathEnvVar) {
 		logVerbose(`couldn't find PATH property in process.env`);
 		return;
 	}
@@ -359,14 +365,33 @@ export function addGoRuntimeBaseToPATH(newGoRuntimeBase: string) {
 	process.env[pathEnvVar] = pathVars.join(path.delimiter);
 }
 
+// Clear terminal PATH environment modification previously installed
+// using addGoRuntimeBaseToPATH.
+// In particular, changes to vscode.EnvironmentVariableCollection persist across
+// vscode sessions, so when we decide not to mutate PATH, we need to clear
+// the preexisting changes.
+export function clearGoRuntimeBaseFromPATH() {
+	if (terminalCreationListener) {
+		const l = terminalCreationListener;
+		terminalCreationListener = undefined;
+		l.dispose();
+	}
+	const pathEnvVar = pathEnvVarName();
+	if (!pathEnvVar) {
+		logVerbose(`couldn't find PATH property in process.env`);
+		return;
+	}
+	environmentVariableCollection?.delete(pathEnvVar);
+}
+
 /**
  * update the PATH variable in the given terminal to default to the currently selected Go
  */
 export async function updateIntegratedTerminal(terminal: vscode.Terminal): Promise<void> {
 	if (!terminal) { return; }
 	const gorootBin = path.join(getCurrentGoRoot(), 'bin');
-	const defaultGoRuntimeBin = path.dirname(getBinPathFromEnvVar('go', defaultPathEnv, false));
-	if (gorootBin === defaultGoRuntimeBin) {
+	const defaultGoRuntime = getBinPathFromEnvVar('go', defaultPathEnv, false);
+	if (defaultGoRuntime && gorootBin === path.dirname(defaultGoRuntime)) {
 		return;
 	}
 

src/goInstallTools.ts

@@ -12,7 +12,7 @@ import { SemVer } from 'semver';
 import util = require('util');
 import vscode = require('vscode');
 import { toolExecutionEnvironment, toolInstallationEnvironment } from './goEnv';
-import { addGoRuntimeBaseToPATH, initGoStatusBar } from './goEnvironmentStatus';
+import { addGoRuntimeBaseToPATH, clearGoRuntimeBaseFromPATH, initGoStatusBar } from './goEnvironmentStatus';
 import { getLanguageServerToolPath } from './goLanguageServer';
 import { logVerbose } from './goLogging';
 import { restartLanguageServer } from './goMain';
@@ -392,6 +392,9 @@ export function updateGoVarsFromConfig(): Promise<void> {
 				// version of go than the system default found from PATH (or Path).
 				if (why !== 'path') {
 					addGoRuntimeBaseToPATH(path.join(getCurrentGoRoot(), 'bin'));
+				} else {
+					// clear pre-existing terminal PATH mutation logic set up by this extension.
+					clearGoRuntimeBaseFromPATH();
 				}
 				initGoStatusBar();
 				// TODO: restart language server or synchronize with language server update.

src/utils/pathUtils.ts

@@ -19,7 +19,8 @@ let binPathCache: { [bin: string]: string } = {};
 
 export const envPath = process.env['PATH'] || (process.platform === 'win32' ? process.env['Path'] : null);
 
-export function getBinPathFromEnvVar(toolName: string, envVarValue: string, appendBinToPath: boolean): string {
+// find the tool's path from the given PATH env var, or null if the tool is not found.
+export function getBinPathFromEnvVar(toolName: string, envVarValue: string, appendBinToPath: boolean): string|null {
 	toolName = correctBinname(toolName);
 	if (envVarValue) {
 		const paths = envVarValue.split(path.delimiter);
@@ -45,7 +46,7 @@ export function getBinPathWithPreferredGopathGoroot(
 	return r.binPath;
 }
 
-// Is same as getBinPAthWithPreferredGopathGoroot, but returns why the
+// Is same as getBinPathWithPreferredGopathGoroot, but returns why the
 // returned path was chosen.
 export function getBinPathWithPreferredGopathGorootWithExplanation(
 	toolName: string,
@@ -65,10 +66,11 @@ export function getBinPathWithPreferredGopathGorootWithExplanation(
 	}
 
 	const binname = alternateTool && !path.isAbsolute(alternateTool) ? alternateTool : toolName;
+	const found = (why: string) => binname === toolName ? why : 'alternateTool';
 	const pathFromGoBin = getBinPathFromEnvVar(binname, process.env['GOBIN'], false);
 	if (pathFromGoBin) {
 		binPathCache[toolName] = pathFromGoBin;
-		return {binPath: pathFromGoBin, why: 'gobin'};
+		return {binPath: pathFromGoBin, why: binname === toolName ? 'gobin' : 'alternateTool'};
 	}
 
 	for (const preferred of preferredGopaths) {
@@ -77,7 +79,7 @@ export function getBinPathWithPreferredGopathGorootWithExplanation(
 			const pathFrompreferredGoPath = getBinPathFromEnvVar(binname, preferred, true);
 			if (pathFrompreferredGoPath) {
 				binPathCache[toolName] = pathFrompreferredGoPath;
-				return {binPath: pathFrompreferredGoPath, why: 'gopath'};
+				return {binPath: pathFrompreferredGoPath, why: found('gopath')};
 			}
 		}
 	}
@@ -86,14 +88,14 @@ export function getBinPathWithPreferredGopathGorootWithExplanation(
 	const pathFromGoRoot = getBinPathFromEnvVar(binname, preferredGoroot || getCurrentGoRoot(), true);
 	if (pathFromGoRoot) {
 		binPathCache[toolName] = pathFromGoRoot;
-		return {binPath: pathFromGoRoot, why: 'goroot'};
+		return {binPath: pathFromGoRoot, why: found('goroot')};
 	}
 
 	// Finally search PATH parts
 	const pathFromPath = getBinPathFromEnvVar(binname, envPath, false);
 	if (pathFromPath) {
 		binPathCache[toolName] = pathFromPath;
-		return {binPath: pathFromPath, why: 'path'};
+		return {binPath: pathFromPath, why: found('path')};
 	}
 
 	// Check default path for go