Use powerShellDefaultVersion everywhere and stop using powerShellExePath

TylerLeonhardt · TylerLeonhardt · commit c7f97c5ffa0a · 2019-11-13T15:34:15.000-08:00
diff --git a/package.json b/package.json
@@ -499,12 +499,6 @@
           "default": [],
           "description": "Specify array of Modules to exclude from Command Explorer listing."
         },
-        "powershell.powerShellExePath": {
-          "type": "string",
-          "default": "",
-          "scope": "machine",
-          "description": "Specifies the full path to a PowerShell executable. Changes the installation of PowerShell used for language and debugging services."
-        },
         "powershell.powerShellAdditionalExePaths": {
           "type": "array",
           "description": "Specifies an array of versionName / exePath pairs where exePath points to a non-standard install location for PowerShell and versionName can be used to reference this path with the powershell.powerShellDefaultVersion setting.",
@@ -530,7 +524,7 @@
         },
         "powershell.powerShellDefaultVersion": {
           "type": "string",
-          "description": "Specifies the PowerShell version name, as displayed by the 'PowerShell: Show Session Menu' command, used when the extension loads e.g \"Windows PowerShell (x86)\" or \"PowerShell Core 6 (x64)\"."
+          "description": "Specifies the PowerShell version name, as displayed by the 'PowerShell: Show Session Menu' command, used when the extension loads e.g \"Windows PowerShell (x86)\" or \"PowerShell Core 6 (x64)\". You can specify additional PowerShell executables by using the \"powershell.powerShellAdditionalExePaths\" setting."
         },
         "powershell.promptToUpdatePowerShell": {
           "type": "boolean",
diff --git a/src/features/GenerateBugReport.ts b/src/features/GenerateBugReport.ts
@@ -113,7 +113,7 @@ ${tableHeader}\n${table};
 
     private getRuntimeInfo() {
 
-        const powerShellExePath = this.sessionManager.getPowerShellExePath();
+        const powerShellExePath = this.sessionManager.PowerShellExeDetails.exePath;
         const powerShellArgs = [
             "-NoProfile",
             "-Command",
diff --git a/src/session.ts b/src/session.ts
@@ -2,7 +2,6 @@
  * Copyright (C) Microsoft Corporation. All rights reserved.
  *--------------------------------------------------------*/
 
-import cp = require("child_process");
 import fs = require("fs");
 import net = require("net");
 import path = require("path");
@@ -17,13 +16,13 @@ import Settings = require("./settings");
 import utils = require("./utils");
 
 import {
-    CloseAction, DocumentSelector, ErrorAction, Executable, LanguageClient, LanguageClientOptions,
-    Middleware, NotificationType, RequestType, RequestType0,
+    CloseAction, DocumentSelector, ErrorAction, LanguageClient, LanguageClientOptions,
+    Middleware, NotificationType, RequestType0,
     ResolveCodeLensSignature, RevealOutputChannelOn, StreamInfo } from "vscode-languageclient";
 
 import { GitHubReleaseInformation, InvokePowerShellUpdateCheck } from "./features/UpdatePowerShell";
 import {
-    getPlatformDetails, IPlatformDetails,
+    getPlatformDetails, IPlatformDetails, IPowerShellExeDetails,
     OperatingSystem, PowerShellExeFinder } from "./platform";
 
 export enum SessionStatus {
@@ -37,9 +36,9 @@ export enum SessionStatus {
 
 export class SessionManager implements Middleware {
     public HostVersion: string;
+    public PowerShellExeDetails: IPowerShellExeDetails;
     private ShowSessionMenuCommandName = "PowerShell.ShowSessionMenu";
     private editorServicesArgs: string;
-    private powerShellExePath: string = "";
     private sessionStatus: SessionStatus = SessionStatus.NeverStarted;
     private suppressRestartPrompt: boolean;
     private focusConsoleOnExecute: boolean;
@@ -119,15 +118,17 @@ export class SessionManager implements Middleware {
 
         this.createStatusBarItem();
 
+        this.promptPowerShellExeSettingsCleanup();
+
         try {
-            this.powerShellExePath = this.getPowerShellExePath();
+            this.PowerShellExeDetails = this.powershellExeFinder.getFirstAvailablePowerShellInstallation();
         } catch (e) {
             this.log.writeError(`Error occurred while searching for a PowerShell executable:\n${e}`);
         }
 
         this.suppressRestartPrompt = false;
 
-        if (!this.powerShellExePath) {
+        if (!this.PowerShellExeDetails) {
             const message = "Unable to find PowerShell."
                 + " Do you have PowerShell installed?"
                 + " You can also configure custom PowerShell installations"
@@ -230,7 +231,7 @@ export class SessionManager implements Middleware {
 
         this.debugSessionProcess =
             new PowerShellProcess(
-                this.powerShellExePath,
+                this.PowerShellExeDetails.exePath,
                 this.bundledModulesPath,
                 "[TEMP] PowerShell Integrated Console",
                 this.log,
@@ -241,51 +242,6 @@ export class SessionManager implements Middleware {
         return this.debugSessionProcess;
     }
 
-    public getPowerShellExePath(): string {
-        if (!this.sessionSettings.powerShellExePath &&
-            this.sessionSettings.developer.powerShellExePath) {
-            // Show deprecation message with fix action.
-            // We don't need to wait on this to complete
-            // because we can finish gathering the configured
-            // PowerShell path without the fix
-            this.showExePathSettingDeprecationWarning();
-        }
-
-        let powerShellExePath: string = this.getConfiguredPowerShellExePath().trim();
-
-        // New versions of PS Core uninstall the previous version
-        // so make sure the path stored in the settings exists.
-        if (!fs.existsSync(powerShellExePath)) {
-            this.log.write(
-                `Path specified by 'powerShellExePath' setting - '${powerShellExePath}' - not found, ` +
-                "reverting to default PowerShell path.");
-            powerShellExePath = "";
-        }
-
-        if (powerShellExePath) {
-            if (this.platformDetails.operatingSystem === OperatingSystem.Windows) {
-                // Check the path bitness
-                const fixedPath = this.powershellExeFinder.fixWindowsPowerShellPath(
-                    powerShellExePath);
-
-                if (fixedPath !== powerShellExePath) {
-                    // Show deprecation message with fix action.
-                    // We don't need to wait on this to complete
-                    // because we can finish gathering the configured
-                    // PowerShell path without the fix
-                    this.showBitnessPathFixWarning(fixedPath);
-                    powerShellExePath = fixedPath;
-                }
-            }
-
-            return this.resolvePowerShellPath(powerShellExePath);
-        }
-
-        // No need to resolve this path, since the finder guarantees its existence
-        const firstPowerShell = this.powershellExeFinder.getFirstAvailablePowerShellInstallation();
-        return firstPowerShell && firstPowerShell.exePath || null;
-    }
-
     // ----- LanguageClient middleware methods -----
 
     public resolveCodeLens(
@@ -331,64 +287,37 @@ export class SessionManager implements Middleware {
             return resolvedCodeLens;
     }
 
-    private async showExePathSettingDeprecationWarning(): Promise<void> {
-        const choice: string = await vscode.window.showWarningMessage(
-            "The 'powershell.developer.powerShellExePath' setting is deprecated, use " +
-            "'powershell.powerShellExePath' instead.",
-            "Fix Automatically");
-
-        if (!choice) {
-            return;
-        }
-
-        this.suppressRestartPrompt = true;
-        await Settings.change("powerShellExePath", this.sessionSettings.developer.powerShellExePath, true);
-        await Settings.change("developer.powerShellExePath", undefined, true);
-        this.suppressRestartPrompt = false;
-    }
-
-    private async showBitnessPathFixWarning(fixedPath: string): Promise<void> {
-        const bitness = this.platformDetails.isOS64Bit ? "64" : "32";
+    private async promptPowerShellExeSettingsCleanup() {
+        if (this.sessionSettings.powerShellExePath) {
+            let warningMessage = "The 'powerShell.powerShellExePath' setting is no longer used. ";
+            warningMessage += this.sessionSettings.powerShellDefaultVersion
+                ? "We can automatically remove it for you."
+                : "We can remove it from your settings and prompt you for which PowerShell you want to use.";
 
-        const choice = await vscode.window.showWarningMessage(
-            `The specified PowerShell path is incorrect for ${bitness}-bit VS Code, using '${fixedPath}' instead.`,
-            "Fix Setting Automatically");
+            const choice = await vscode.window.showWarningMessage(warningMessage, "Let's do it!");
 
-        if (!choice) {
-            return;
-        }
+            if (choice === "") {
+                // They hit the 'x' to close the dialog.
+                return;
+            }
 
-        this.suppressRestartPrompt = true;
-        await Settings.change("powerShellExePath", this.sessionSettings.developer.powerShellExePath, true);
-        await Settings.change("developer.powerShellExePath", undefined, true);
-        this.suppressRestartPrompt = false;
-    }
+            this.suppressRestartPrompt = true;
+            try {
+                await Settings.change("powerShellExePath", undefined, true);
 
-    private getConfiguredPowerShellExePath(): string {
-        // If powershell.powerShellDefaultVersion specified, attempt to find the PowerShell exe path
-        // of the version specified by the setting.
-        if (this.sessionSettings.powerShellDefaultVersion && this.sessionStatus === SessionStatus.NeverStarted) {
-            for (const pwshExe of this.powershellExeFinder.enumeratePowerShellInstallations()) {
-                if (pwshExe.displayName === this.sessionSettings.powerShellDefaultVersion) {
-                    return pwshExe.exePath;
+                // This has been deprecated for a while so siliently remove it if it's there.
+                if (this.sessionSettings.developer.powerShellExePath) {
+                    await Settings.change("developer.powerShellExePath", undefined, true);
                 }
+            } finally {
+                this.suppressRestartPrompt = false;
             }
 
-            // Default PowerShell version was configured but we didn't find it
-            this.log.writeWarning(
-                `Could not find powerShellDefaultVersion: '${this.sessionSettings.powerShellDefaultVersion}'`);
-        }
-
-        if (this.sessionSettings.powerShellExePath) {
-            return this.sessionSettings.powerShellExePath;
-        }
-
-        if (this.sessionSettings.developer.powerShellExePath) {
-            this.showExePathSettingDeprecationWarning();
-            return this.sessionSettings.developer.powerShellExePath;
+            // Show the session menu at the end if they don't have a PowerShellDefaultVersion.
+            if (!this.sessionSettings.powerShellDefaultVersion) {
+                await vscode.commands.executeCommand(this.ShowSessionMenuCommandName);
+            }
         }
-
-        return "";
     }
 
     private onConfigurationUpdated() {
@@ -398,11 +327,10 @@ export class SessionManager implements Middleware {
 
         // Detect any setting changes that would affect the session
         if (!this.suppressRestartPrompt &&
-            (settings.useX86Host !== this.sessionSettings.useX86Host ||
-             settings.powerShellExePath.toLowerCase() !== this.sessionSettings.powerShellExePath.toLowerCase() ||
-             (settings.developer.powerShellExePath ? settings.developer.powerShellExePath.toLowerCase() : null) !==
-                (this.sessionSettings.developer.powerShellExePath
-                    ? this.sessionSettings.developer.powerShellExePath.toLowerCase() : null) ||
+            (settings.useX86Host !==
+                this.sessionSettings.useX86Host ||
+             settings.powerShellDefaultVersion.toLowerCase() !==
+                this.sessionSettings.powerShellDefaultVersion.toLowerCase() ||
              settings.developer.editorServicesLogLevel.toLowerCase() !==
                 this.sessionSettings.developer.editorServicesLogLevel.toLowerCase() ||
              settings.developer.bundledModulesPath.toLowerCase() !==
@@ -461,7 +389,7 @@ export class SessionManager implements Middleware {
 
         this.languageServerProcess =
             new PowerShellProcess(
-                this.powerShellExePath,
+                this.PowerShellExeDetails.exePath,
                 this.bundledModulesPath,
                 "PowerShell Integrated Console",
                 this.log,
@@ -691,63 +619,20 @@ export class SessionManager implements Middleware {
             SessionStatus.Failed);
     }
 
-    private changePowerShellExePath(exePath: string) {
+    private changePowerShellDefaultVersion(exePath: IPowerShellExeDetails) {
         this.suppressRestartPrompt = true;
         Settings
-            .change("powerShellExePath", exePath, true)
+            .change("powerShellDefaultVersion", exePath.displayName, true)
             .then(() => this.restartSession());
     }
 
-    private resolvePowerShellPath(powerShellExePath: string): string {
-        const resolvedPath = path.resolve(__dirname, powerShellExePath);
-
-        // If the path does not exist, show an error
-        if (!utils.checkIfFileExists(resolvedPath)) {
-            const pwshPath = resolvedPath || powerShellExePath;
-            const pwshExeName = path.basename(pwshPath) || "PowerShell executable";
-
-            this.setSessionFailure(`${pwshExeName} cannot be found or is not accessible at path '${pwshPath}'`);
-
-            return null;
-        }
-
-        return resolvedPath;
-    }
-
-    private getPowerShellVersionLabel(): string {
-        if (this.powerShellExePath) {
-            const powerShellCommandLine = [
-                this.powerShellExePath,
-                "-NoProfile",
-                "-NonInteractive",
-            ];
-
-            // Only add ExecutionPolicy param on Windows
-            if (utils.isWindowsOS()) {
-                powerShellCommandLine.push("-ExecutionPolicy", "Bypass");
-            }
-
-            powerShellCommandLine.push(
-                "-Command",
-                "'$PSVersionTable | ConvertTo-Json'");
-
-            const powerShellOutput = cp.execSync(powerShellCommandLine.join(" "));
-            const versionDetails = JSON.parse(powerShellOutput.toString());
-            return versionDetails.PSVersion.Label;
-        } else {
-            // TODO: throw instead?
-            return null;
-        }
-    }
-
     private showSessionConsole(isExecute?: boolean) {
         if (this.languageServerProcess) {
             this.languageServerProcess.showConsole(isExecute && !this.focusConsoleOnExecute);
         }
     }
 
     private showSessionMenu() {
-        const currentExePath = (this.powerShellExePath || "").toLowerCase();
         const availablePowerShellExes = this.powershellExeFinder.getAllAvailablePowerShellInstallations();
 
         let sessionText: string;
@@ -760,7 +645,7 @@ export class SessionManager implements Middleware {
             case SessionStatus.Stopping:
                 const currentPowerShellExe =
                 availablePowerShellExes
-                    .find((item) => item.exePath.toLowerCase() === currentExePath);
+                    .find((item) => item.displayName.toLowerCase() === this.PowerShellExeDetails.displayName);
 
                 const powerShellSessionName =
                     currentPowerShellExe ?
@@ -782,11 +667,11 @@ export class SessionManager implements Middleware {
 
         const powerShellItems =
             availablePowerShellExes
-                .filter((item) => item.exePath.toLowerCase() !== currentExePath)
+                .filter((item) => item.displayName.toLowerCase() !== this.PowerShellExeDetails.displayName)
                 .map((item) => {
                     return new SessionMenuItem(
                         `Switch to: ${item.displayName}`,
-                        () => { this.changePowerShellExePath(item.exePath); });
+                        () => { this.changePowerShellDefaultVersion(item); });
                 });
 
         const menuItems: SessionMenuItem[] = [
diff --git a/src/settings.ts b/src/settings.ts
@@ -69,6 +69,7 @@ export interface IDebuggingSettings {
 
 export interface IDeveloperSettings {
     featureFlags?: string[];
+    // This setting is no longer used but is here to assist in cleaning up the users settings.
     powerShellExePath?: string;
     bundledModulesPath?: string;
     editorServicesLogLevel?: string;
@@ -79,6 +80,7 @@ export interface IDeveloperSettings {
 export interface ISettings {
     powerShellAdditionalExePaths?: IPowerShellAdditionalExePathSettings[];
     powerShellDefaultVersion?: string;
+    // This setting is no longer used but is here to assist in cleaning up the users settings.
     powerShellExePath?: string;
     promptToUpdatePowerShell?: boolean;
     bundledModulesPath?: string;
diff --git a/test/settings.test.ts b/test/settings.test.ts
@@ -3,6 +3,7 @@
  *--------------------------------------------------------*/
 
 import * as assert from "assert";
+import { IPowerShellExeDetails } from "../src/platform";
 import Settings = require("../src/settings");
 
 suite("Settings module", () => {
@@ -22,10 +23,15 @@ suite("Settings module", () => {
 
     test("Settings that can only be user settings update correctly", async () => {
         // set to false means it's set as a workspace-level setting so this should throw.
-        assert.rejects(async () => await Settings.change("powerShellExePath", "dummyPath", false));
+        const psExeDetails = [{
+            versionName: "My PowerShell",
+            exePath: "dummyPath",
+        }];
+
+        assert.rejects(async () => await Settings.change("powerShellAdditionalExePaths", psExeDetails, false));
 
         // set to true means it's a user-level setting so this should not throw.
-        await Settings.change("powerShellExePath", "dummyPath", true);
-        assert.strictEqual(Settings.load().powerShellExePath, "dummyPath");
+        await Settings.change("powerShellAdditionalExePaths", psExeDetails, true);
+        assert.strictEqual(Settings.load().powerShellAdditionalExePaths[0].versionName, psExeDetails[0].versionName);
     });
 });
