Refactor

Author: Kartik Raj

src/client/application/diagnostics/checks/pythonInterpreter.ts

@@ -6,7 +6,7 @@ import { inject, injectable } from 'inversify';
 import { DiagnosticSeverity, l10n } from 'vscode';
 import '../../../common/extensions';
 import * as path from 'path';
-import { IDisposableRegistry, IInterpreterPathService, Resource } from '../../../common/types';
+import { IConfigurationService, IDisposableRegistry, IInterpreterPathService, Resource } from '../../../common/types';
 import { IInterpreterService } from '../../../interpreter/contracts';
 import { IServiceContainer } from '../../../ioc/types';
 import { BaseDiagnostic, BaseDiagnosticsService } from '../base';
@@ -28,10 +28,11 @@ import { EventName } from '../../../telemetry/constants';
 import { IExtensionSingleActivationService } from '../../../activation/types';
 import { cache } from '../../../common/utils/decorators';
 import { noop } from '../../../common/utils/misc';
-import { IPythonExecutionFactory } from '../../../common/process/types';
 import { getOSType, OSType } from '../../../common/utils/platform';
 import { IFileSystem } from '../../../common/platform/types';
 import { traceError } from '../../../logging';
+import { getExecutable } from '../../../common/process/internal/python';
+import { shellExec } from '../../../common/process/rawProcessApis';
 
 const messages = {
     [DiagnosticCodes.NoPythonInterpretersDiagnostic]: l10n.t(
@@ -41,7 +42,7 @@ const messages = {
         'An Invalid Python interpreter is selected{0}, please try changing it to enable features such as IntelliSense, linting, and debugging. See output for more details regarding why the interpreter is invalid.',
     ),
     [DiagnosticCodes.InvalidComspecDiagnostic]: l10n.t(
-        "The environment variable 'Comspec' seems to be set to an invalid value. Please correct it to carry valid path to Command Prompt to enable features such as IntelliSense, linting, and debugging. See instructions which might help.",
+        'We detected an issue with one of your environment variables that breaks features such as IntelliSense, linting and debugging. Try setting the "ComSpec" to a valid Command Prompt path in your system to fix it.',
     ),
 };
 
@@ -112,20 +113,16 @@ export class InvalidPythonInterpreterService extends BaseDiagnosticsService
         );
     }
 
-    // eslint-disable-next-line class-methods-use-this
-    public async diagnose(_resource: Resource): Promise<IDiagnostic[]> {
-        return [];
+    public async diagnose(resource: Resource): Promise<IDiagnostic[]> {
+        return this.diagnoseDefaultShell(resource);
     }
 
     public async _manualDiagnose(resource: Resource): Promise<IDiagnostic[]> {
         const workspaceService = this.serviceContainer.get<IWorkspaceService>(IWorkspaceService);
         const interpreterService = this.serviceContainer.get<IInterpreterService>(IInterpreterService);
-        const currentInterpreter = await interpreterService.getActiveInterpreter(resource);
-        if (!currentInterpreter) {
-            const diagnostics = await this.diagnoseDefaultShell(resource);
-            if (diagnostics.length) {
-                return diagnostics;
-            }
+        const diagnostics = await this.diagnoseDefaultShell(resource);
+        if (diagnostics.length) {
+            return diagnostics;
         }
         const hasInterpreters = await interpreterService.hasInterpreters();
         const interpreterPathService = this.serviceContainer.get<IInterpreterPathService>(IInterpreterPathService);
@@ -142,6 +139,7 @@ export class InvalidPythonInterpreterService extends BaseDiagnosticsService
             ];
         }
 
+        const currentInterpreter = await interpreterService.getActiveInterpreter(resource);
         if (!currentInterpreter) {
             return [
                 new InvalidPythonInterpreterDiagnostic(
@@ -189,7 +187,7 @@ export class InvalidPythonInterpreterService extends BaseDiagnosticsService
         if (diagnostic.code === DiagnosticCodes.InvalidComspecDiagnostic) {
             return [
                 {
-                    prompt: Common.instructions,
+                    prompt: Common.seeInstructions,
                     command: commandFactory.createCommand(diagnostic, {
                         type: 'launch',
                         options: 'https://aka.ms/AAk3djo',
@@ -222,12 +220,16 @@ export class InvalidPythonInterpreterService extends BaseDiagnosticsService
         if (getOSType() !== OSType.Windows) {
             return [];
         }
-        const executionFactory = this.serviceContainer.get<IPythonExecutionFactory>(IPythonExecutionFactory);
-        const executionService = await executionFactory.create({ resource });
+        const interpreterService = this.serviceContainer.get<IInterpreterService>(IInterpreterService);
+        const currentInterpreter = await interpreterService.getActiveInterpreter(resource);
+        if (currentInterpreter) {
+            return [];
+        }
         try {
-            await executionService.getExecutablePath({ throwOnError: true });
+            await this.shellExecPython();
         } catch (ex) {
-            if ((ex as Error).message?.includes('4058')) {
+            // eslint-disable-next-line @typescript-eslint/no-explicit-any
+            if ((ex as any).errno === -4058) {
                 // ENOENT (-4058) error is thrown by Node when the default shell is invalid.
                 if (await this.isComspecInvalid()) {
                     traceError('ComSpec is set to an invalid value', process.env.ComSpec);
@@ -241,7 +243,22 @@ export class InvalidPythonInterpreterService extends BaseDiagnosticsService
     private async isComspecInvalid() {
         const comSpec = process.env.ComSpec ?? '';
         const fs = this.serviceContainer.get<IFileSystem>(IFileSystem);
-        return fs.fileExists(comSpec);
+        return fs.fileExists(comSpec).then((exists) => !exists);
+    }
+
+    @cache(-1, true)
+    // eslint-disable-next-line class-methods-use-this
+    private async shellExecPython() {
+        const configurationService = this.serviceContainer.get<IConfigurationService>(IConfigurationService);
+        const { pythonPath } = configurationService.getSettings();
+        const [args] = getExecutable();
+        const argv = [pythonPath, ...args];
+        // Concat these together to make a set of quoted strings
+        const quoted = argv.reduce(
+            (p, c) => (p ? `${p} ${c.toCommandArgumentForPythonExt()}` : `${c.toCommandArgumentForPythonExt()}`),
+            '',
+        );
+        return shellExec(quoted, { timeout: 15000 });
     }
 }
 

src/client/common/process/pythonEnvironment.ts

@@ -47,7 +47,7 @@ class PythonEnvironment implements IPythonEnvironment {
         return this.cachedInterpreterInformation;
     }
 
-    public async getExecutablePath(options?: { throwOnError?: boolean }): Promise<string | undefined> {
+    public async getExecutablePath(): Promise<string | undefined> {
         // If we've passed the python file, then return the file.
         // This is because on mac if using the interpreter /usr/bin/python2.7 we can get a different value for the path
         if (await this.deps.isValidExecutable(this.pythonPath)) {
@@ -59,7 +59,7 @@ class PythonEnvironment implements IPythonEnvironment {
             return result;
         }
         const python = this.getExecutionInfo();
-        const promise = getExecutablePath(python, this.deps.shellExec, options);
+        const promise = getExecutablePath(python, this.deps.shellExec);
         cachedExecutablePath.set(this.pythonPath, promise);
         return promise;
     }

src/client/common/process/pythonExecutionFactory.ts

@@ -145,7 +145,7 @@ function createPythonService(procService: IProcessService, env: IPythonEnvironme
     const procs = createPythonProcessService(procService, env);
     return {
         getInterpreterInformation: () => env.getInterpreterInformation(),
-        getExecutablePath: (options?: { throwOnError?: boolean }) => env.getExecutablePath(options),
+        getExecutablePath: () => env.getExecutablePath(),
         isModuleInstalled: (m) => env.isModuleInstalled(m),
         getModuleVersion: (m) => env.getModuleVersion(m),
         getExecutionInfo: (a) => env.getExecutionInfo(a),

src/client/common/process/types.ts

@@ -84,7 +84,7 @@ export const IPythonExecutionService = Symbol('IPythonExecutionService');
 
 export interface IPythonExecutionService {
     getInterpreterInformation(): Promise<InterpreterInformation | undefined>;
-    getExecutablePath(options?: { throwOnError?: boolean }): Promise<string | undefined>;
+    getExecutablePath(): Promise<string | undefined>;
     isModuleInstalled(moduleName: string): Promise<boolean>;
     getModuleVersion(moduleName: string): Promise<string | undefined>;
     getExecutionInfo(pythonArgs?: string[]): PythonExecInfo;
@@ -100,7 +100,7 @@ export interface IPythonExecutionService {
 export interface IPythonEnvironment {
     getInterpreterInformation(): Promise<InterpreterInformation | undefined>;
     getExecutionObservableInfo(pythonArgs?: string[], pythonExecutable?: string): PythonExecInfo;
-    getExecutablePath(options?: { throwOnError?: boolean }): Promise<string | undefined>;
+    getExecutablePath(): Promise<string | undefined>;
     isModuleInstalled(moduleName: string): Promise<boolean>;
     getModuleVersion(moduleName: string): Promise<string | undefined>;
     getExecutionInfo(pythonArgs?: string[], pythonExecutable?: string): PythonExecInfo;

src/client/common/utils/localize.ts

@@ -49,7 +49,7 @@ export namespace Diagnostics {
 
 export namespace Common {
     export const allow = l10n.t('Allow');
-    export const instructions = l10n.t('Instructions');
+    export const seeInstructions = l10n.t('See Instructions');
     export const close = l10n.t('Close');
     export const bannerLabelYes = l10n.t('Yes');
     export const bannerLabelNo = l10n.t('No');

src/client/pythonEnvironments/info/executable.ts

@@ -14,11 +14,7 @@ import { copyPythonExecInfo, PythonExecInfo } from '../exec';
  * @param python - the information to use when running Python
  * @param shellExec - the function to use to run Python
  */
-export async function getExecutablePath(
-    python: PythonExecInfo,
-    shellExec: ShellExecFunc,
-    options?: { throwOnError?: boolean },
-): Promise<string | undefined> {
+export async function getExecutablePath(python: PythonExecInfo, shellExec: ShellExecFunc): Promise<string | undefined> {
     try {
         const [args, parse] = getExecutable();
         const info = copyPythonExecInfo(python, args);