Expand variables in pythonPath before validating it (microsoft#3629)

For microsoft#3392

news/2 Fixes/3392.md

@@ -0,0 +1 @@
+Expand variables in `pythonPath` before validating it.

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

@@ -5,9 +5,11 @@
 
 import { inject, injectable } from 'inversify';
 import { DiagnosticSeverity, Uri } from 'vscode';
+import { IWorkspaceService } from '../../../common/application/types';
 import '../../../common/extensions';
-import { Logger } from '../../../common/logger';
+import { Logger, traceError } from '../../../common/logger';
 import { IConfigurationService } from '../../../common/types';
+import { SystemVariables } from '../../../common/variables/systemVariables';
 import { IInterpreterHelper } from '../../../interpreter/contracts';
 import { IServiceContainer } from '../../../ioc/types';
 import { BaseDiagnostic, BaseDiagnosticsService } from '../base';
@@ -32,7 +34,11 @@ const CommandName = 'python.setInterpreter';
 @injectable()
 export class InvalidPythonPathInDebuggerService extends BaseDiagnosticsService implements IInvalidPythonPathInDebuggerService {
     protected readonly messageService: IDiagnosticHandlerService<MessageCommandPrompt>;
-    constructor(@inject(IServiceContainer) serviceContainer: IServiceContainer) {
+    constructor(@inject(IServiceContainer) serviceContainer: IServiceContainer,
+        @inject(IWorkspaceService) private readonly workspace: IWorkspaceService,
+        @inject(IDiagnosticsCommandFactory) private readonly commandFactory: IDiagnosticsCommandFactory,
+        @inject(IInterpreterHelper) private readonly interpreterHelper: IInterpreterHelper,
+        @inject(IConfigurationService) private readonly configService: IConfigurationService) {
         super([DiagnosticCodes.InvalidPythonPathInDebuggerDiagnostic], serviceContainer);
         this.messageService = serviceContainer.get<IDiagnosticHandlerService<MessageCommandPrompt>>(IDiagnosticHandlerService, DiagnosticCommandPromptHandlerServiceId);
     }
@@ -45,30 +51,33 @@ export class InvalidPythonPathInDebuggerService extends BaseDiagnosticsService i
             return;
         }
         const diagnostic = diagnostics[0];
-        const commandFactory = this.serviceContainer.get<IDiagnosticsCommandFactory>(IDiagnosticsCommandFactory);
         const options = [
             {
                 prompt: 'Select Python Interpreter',
-                command: commandFactory.createCommand(diagnostic, { type: 'executeVSCCommand', options: CommandName })
+                command: this.commandFactory.createCommand(diagnostic, { type: 'executeVSCCommand', options: CommandName })
             }
         ];
 
         await this.messageService.handle(diagnostic, { commandPrompts: options });
     }
     public async validatePythonPath(pythonPath?: string, resource?: Uri) {
+        pythonPath = pythonPath ? this.resolveVariables(pythonPath, resource) : undefined;
         // tslint:disable-next-line:no-invalid-template-strings
         if (pythonPath === '${config:python.pythonPath}' || !pythonPath) {
-            const configService = this.serviceContainer.get<IConfigurationService>(IConfigurationService);
-            pythonPath = configService.getSettings(resource).pythonPath;
+            pythonPath = this.configService.getSettings(resource).pythonPath;
         }
-        const helper = this.serviceContainer.get<IInterpreterHelper>(IInterpreterHelper);
-        if (await helper.getInterpreterInformation(pythonPath).catch(() => undefined)) {
+        if (await this.interpreterHelper.getInterpreterInformation(pythonPath).catch(() => undefined)) {
             return true;
         }
-
+        traceError(`Invalid Python Path '${pythonPath}'`);
         this.handle([new InvalidPythonPathInDebuggerDiagnostic()])
             .catch(ex => Logger.error('Failed to handle invalid python path in debugger', ex))
             .ignoreErrors();
         return false;
     }
+    protected resolveVariables(pythonPath: string, resource: Uri | undefined): string {
+        const workspaceFolder = resource ? this.workspace.getWorkspaceFolder(resource) : undefined;
+        const systemVariables = new SystemVariables(workspaceFolder ? workspaceFolder.uri.fsPath : undefined);
+        return systemVariables.resolveAny(pythonPath);
+    }
 }

src/client/debugger/extension/configProviders/baseProvider.ts

@@ -40,7 +40,7 @@ export abstract class BaseConfigurationProvider implements DebugConfigurationPro
             }
 
             await this.provideLaunchDefaults(workspaceFolder, config);
-            const isValid = await this.validateLaunchConfiguration(config);
+            const isValid = await this.validateLaunchConfiguration(folder, config);
             if (!isValid) {
                 return;
             }
@@ -90,9 +90,9 @@ export abstract class BaseConfigurationProvider implements DebugConfigurationPro
         // Pass workspace folder so we can get this when we get debug events firing.
         debugConfiguration.workspaceFolder = workspaceFolder ? workspaceFolder.fsPath : undefined;
     }
-    protected async validateLaunchConfiguration(debugConfiguration: LaunchRequestArguments): Promise<boolean> {
+    protected async validateLaunchConfiguration(folder: WorkspaceFolder | undefined, debugConfiguration: LaunchRequestArguments): Promise<boolean> {
         const diagnosticService = this.serviceContainer.get<IInvalidPythonPathInDebuggerService>(IDiagnosticsService, InvalidPythonPathInDebuggerServiceId);
-        return diagnosticService.validatePythonPath(debugConfiguration.pythonPath);
+        return diagnosticService.validatePythonPath(debugConfiguration.pythonPath, folder ? folder.uri : undefined);
     }
     private getWorkspaceFolder(folder: WorkspaceFolder | undefined): Uri | undefined {
         if (folder) {

src/test/application/diagnostics/checks/invalidPythonPathInDebugger.unit.test.ts

@@ -8,11 +8,13 @@
 import { expect } from 'chai';
 import * as path from 'path';
 import * as typemoq from 'typemoq';
+import { Uri } from 'vscode';
 import { InvalidPythonPathInDebuggerService } from '../../../../client/application/diagnostics/checks/invalidPythonPathInDebugger';
 import { CommandOption, IDiagnosticsCommandFactory } from '../../../../client/application/diagnostics/commands/types';
 import { DiagnosticCodes } from '../../../../client/application/diagnostics/constants';
 import { DiagnosticCommandPromptHandlerServiceId, MessageCommandPrompt } from '../../../../client/application/diagnostics/promptHandler';
 import { IDiagnostic, IDiagnosticCommand, IDiagnosticHandlerService, IInvalidPythonPathInDebuggerService } from '../../../../client/application/diagnostics/types';
+import { IWorkspaceService } from '../../../../client/common/application/types';
 import { IConfigurationService, IPythonSettings } from '../../../../client/common/types';
 import { IInterpreterHelper } from '../../../../client/interpreter/contracts';
 import { IServiceContainer } from '../../../../client/ioc/types';
@@ -23,6 +25,7 @@ suite('Application Diagnostics - Checks Python Path in debugger', () => {
     let commandFactory: typemoq.IMock<IDiagnosticsCommandFactory>;
     let configService: typemoq.IMock<IConfigurationService>;
     let helper: typemoq.IMock<IInterpreterHelper>;
+    let workspaceService: typemoq.IMock<IWorkspaceService>;
     setup(() => {
         const serviceContainer = typemoq.Mock.ofType<IServiceContainer>();
         messageHandler = typemoq.Mock.ofType<IDiagnosticHandlerService<MessageCommandPrompt>>();
@@ -37,8 +40,11 @@ suite('Application Diagnostics - Checks Python Path in debugger', () => {
         helper = typemoq.Mock.ofType<IInterpreterHelper>();
         serviceContainer.setup(s => s.get(typemoq.It.isValue(IInterpreterHelper)))
             .returns(() => helper.object);
+        workspaceService = typemoq.Mock.ofType<IWorkspaceService>();
+        serviceContainer.setup(s => s.get(typemoq.It.isValue(IWorkspaceService)))
+            .returns(() => workspaceService.object);
 
-        diagnosticService = new InvalidPythonPathInDebuggerService(serviceContainer.object);
+        diagnosticService = new InvalidPythonPathInDebuggerService(serviceContainer.object, workspaceService.object, commandFactory.object, helper.object, configService.object);
     });
 
     test('Can handle InvalidPythonPathInDebugger diagnostics', async () => {
@@ -108,6 +114,64 @@ suite('Application Diagnostics - Checks Python Path in debugger', () => {
         helper.verifyAll();
         expect(valid).to.be.equal(true, 'not valid');
     });
+    test('Ensure ${workspaceFolder} is not expanded when a resource is not passed', async () => {
+        const pythonPath = '${workspaceFolder}/venv/bin/python';
+
+        workspaceService
+            .setup(c => c.getWorkspaceFolder(typemoq.It.isAny()))
+            .returns(() => undefined)
+            .verifiable(typemoq.Times.never());
+        helper
+            .setup(h => h.getInterpreterInformation(typemoq.It.isAny()))
+            .returns(() => Promise.resolve({}))
+            .verifiable(typemoq.Times.once());
+
+        await diagnosticService.validatePythonPath(pythonPath);
+
+        configService.verifyAll();
+        helper.verifyAll();
+    });
+    test('Ensure ${workspaceFolder} is expanded', async () => {
+        const pythonPath = '${workspaceFolder}/venv/bin/python';
+
+        const workspaceFolder = { uri: Uri.parse('full/path/to/workspace'), name: '', index: 0 };
+        const expectedPath = `${workspaceFolder.uri.fsPath}/venv/bin/python`;
+
+        workspaceService
+            .setup(c => c.getWorkspaceFolder(typemoq.It.isAny()))
+            .returns(() => workspaceFolder)
+            .verifiable(typemoq.Times.once());
+        helper
+            .setup(h => h.getInterpreterInformation(typemoq.It.isValue(expectedPath)))
+            .returns(() => Promise.resolve({}))
+            .verifiable(typemoq.Times.once());
+
+        const valid = await diagnosticService.validatePythonPath(pythonPath, Uri.parse('something'));
+
+        configService.verifyAll();
+        helper.verifyAll();
+        expect(valid).to.be.equal(true, 'not valid');
+    });
+    test('Ensure ${env:XYZ123} is expanded', async () => {
+        const pythonPath = '${env:XYZ123}/venv/bin/python';
+
+        process.env.XYZ123 = 'something/else';
+        const expectedPath = `${process.env.XYZ123}/venv/bin/python`;
+        workspaceService
+            .setup(c => c.getWorkspaceFolder(typemoq.It.isAny()))
+            .returns(() => undefined)
+            .verifiable(typemoq.Times.once());
+        helper
+            .setup(h => h.getInterpreterInformation(typemoq.It.isValue(expectedPath)))
+            .returns(() => Promise.resolve({}))
+            .verifiable(typemoq.Times.once());
+
+        const valid = await diagnosticService.validatePythonPath(pythonPath);
+
+        configService.verifyAll();
+        helper.verifyAll();
+        expect(valid).to.be.equal(true, 'not valid');
+    });
     test('Ensure we get python path from config when path = undefined', async () => {
         const pythonPath = undefined;