Skip to content

Commit 93524ff

Browse files
DonJayamanneDonJayamanne
authored
Expand variables in pythonPath before validating it (microsoft#3629)
For microsoft#3392
1 parent 926e915 commit 93524ff

4 files changed

Lines changed: 87 additions & 13 deletions

File tree

‎news/2 Fixes/3392.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Expand variables in `pythonPath` before validating it.

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

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,11 @@
55

66
import { inject, injectable } from 'inversify';
77
import { DiagnosticSeverity, Uri } from 'vscode';
8+
import { IWorkspaceService } from '../../../common/application/types';
89
import '../../../common/extensions';
9-
import { Logger } from '../../../common/logger';
10+
import { Logger, traceError } from '../../../common/logger';
1011
import { IConfigurationService } from '../../../common/types';
12+
import { SystemVariables } from '../../../common/variables/systemVariables';
1113
import { IInterpreterHelper } from '../../../interpreter/contracts';
1214
import { IServiceContainer } from '../../../ioc/types';
1315
import { BaseDiagnostic, BaseDiagnosticsService } from '../base';
@@ -32,7 +34,11 @@ const CommandName = 'python.setInterpreter';
3234
@injectable()
3335
export class InvalidPythonPathInDebuggerService extends BaseDiagnosticsService implements IInvalidPythonPathInDebuggerService {
3436
protected readonly messageService: IDiagnosticHandlerService<MessageCommandPrompt>;
35-
constructor(@inject(IServiceContainer) serviceContainer: IServiceContainer) {
37+
constructor(@inject(IServiceContainer) serviceContainer: IServiceContainer,
38+
@inject(IWorkspaceService) private readonly workspace: IWorkspaceService,
39+
@inject(IDiagnosticsCommandFactory) private readonly commandFactory: IDiagnosticsCommandFactory,
40+
@inject(IInterpreterHelper) private readonly interpreterHelper: IInterpreterHelper,
41+
@inject(IConfigurationService) private readonly configService: IConfigurationService) {
3642
super([DiagnosticCodes.InvalidPythonPathInDebuggerDiagnostic], serviceContainer);
3743
this.messageService = serviceContainer.get<IDiagnosticHandlerService<MessageCommandPrompt>>(IDiagnosticHandlerService, DiagnosticCommandPromptHandlerServiceId);
3844
}
@@ -45,30 +51,33 @@ export class InvalidPythonPathInDebuggerService extends BaseDiagnosticsService i
4551
return;
4652
}
4753
const diagnostic = diagnostics[0];
48-
const commandFactory = this.serviceContainer.get<IDiagnosticsCommandFactory>(IDiagnosticsCommandFactory);
4954
const options = [
5055
{
5156
prompt: 'Select Python Interpreter',
52-
command: commandFactory.createCommand(diagnostic, { type: 'executeVSCCommand', options: CommandName })
57+
command: this.commandFactory.createCommand(diagnostic, { type: 'executeVSCCommand', options: CommandName })
5358
}
5459
];
5560

5661
await this.messageService.handle(diagnostic, { commandPrompts: options });
5762
}
5863
public async validatePythonPath(pythonPath?: string, resource?: Uri) {
64+
pythonPath = pythonPath ? this.resolveVariables(pythonPath, resource) : undefined;
5965
// tslint:disable-next-line:no-invalid-template-strings
6066
if (pythonPath === '${config:python.pythonPath}' || !pythonPath) {
61-
const configService = this.serviceContainer.get<IConfigurationService>(IConfigurationService);
62-
pythonPath = configService.getSettings(resource).pythonPath;
67+
pythonPath = this.configService.getSettings(resource).pythonPath;
6368
}
64-
const helper = this.serviceContainer.get<IInterpreterHelper>(IInterpreterHelper);
65-
if (await helper.getInterpreterInformation(pythonPath).catch(() => undefined)) {
69+
if (await this.interpreterHelper.getInterpreterInformation(pythonPath).catch(() => undefined)) {
6670
return true;
6771
}
68-
72+
traceError(`Invalid Python Path '${pythonPath}'`);
6973
this.handle([new InvalidPythonPathInDebuggerDiagnostic()])
7074
.catch(ex => Logger.error('Failed to handle invalid python path in debugger', ex))
7175
.ignoreErrors();
7276
return false;
7377
}
78+
protected resolveVariables(pythonPath: string, resource: Uri | undefined): string {
79+
const workspaceFolder = resource ? this.workspace.getWorkspaceFolder(resource) : undefined;
80+
const systemVariables = new SystemVariables(workspaceFolder ? workspaceFolder.uri.fsPath : undefined);
81+
return systemVariables.resolveAny(pythonPath);
82+
}
7483
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ export abstract class BaseConfigurationProvider implements DebugConfigurationPro
4040
}
4141

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

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

Lines changed: 65 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,13 @@
88
import { expect } from 'chai';
99
import * as path from 'path';
1010
import * as typemoq from 'typemoq';
11+
import { Uri } from 'vscode';
1112
import { InvalidPythonPathInDebuggerService } from '../../../../client/application/diagnostics/checks/invalidPythonPathInDebugger';
1213
import { CommandOption, IDiagnosticsCommandFactory } from '../../../../client/application/diagnostics/commands/types';
1314
import { DiagnosticCodes } from '../../../../client/application/diagnostics/constants';
1415
import { DiagnosticCommandPromptHandlerServiceId, MessageCommandPrompt } from '../../../../client/application/diagnostics/promptHandler';
1516
import { IDiagnostic, IDiagnosticCommand, IDiagnosticHandlerService, IInvalidPythonPathInDebuggerService } from '../../../../client/application/diagnostics/types';
17+
import { IWorkspaceService } from '../../../../client/common/application/types';
1618
import { IConfigurationService, IPythonSettings } from '../../../../client/common/types';
1719
import { IInterpreterHelper } from '../../../../client/interpreter/contracts';
1820
import { IServiceContainer } from '../../../../client/ioc/types';
@@ -23,6 +25,7 @@ suite('Application Diagnostics - Checks Python Path in debugger', () => {
2325
let commandFactory: typemoq.IMock<IDiagnosticsCommandFactory>;
2426
let configService: typemoq.IMock<IConfigurationService>;
2527
let helper: typemoq.IMock<IInterpreterHelper>;
28+
let workspaceService: typemoq.IMock<IWorkspaceService>;
2629
setup(() => {
2730
const serviceContainer = typemoq.Mock.ofType<IServiceContainer>();
2831
messageHandler = typemoq.Mock.ofType<IDiagnosticHandlerService<MessageCommandPrompt>>();
@@ -37,8 +40,11 @@ suite('Application Diagnostics - Checks Python Path in debugger', () => {
3740
helper = typemoq.Mock.ofType<IInterpreterHelper>();
3841
serviceContainer.setup(s => s.get(typemoq.It.isValue(IInterpreterHelper)))
3942
.returns(() => helper.object);
43+
workspaceService = typemoq.Mock.ofType<IWorkspaceService>();
44+
serviceContainer.setup(s => s.get(typemoq.It.isValue(IWorkspaceService)))
45+
.returns(() => workspaceService.object);
4046

41-
diagnosticService = new InvalidPythonPathInDebuggerService(serviceContainer.object);
47+
diagnosticService = new InvalidPythonPathInDebuggerService(serviceContainer.object, workspaceService.object, commandFactory.object, helper.object, configService.object);
4248
});
4349

4450
test('Can handle InvalidPythonPathInDebugger diagnostics', async () => {
@@ -108,6 +114,64 @@ suite('Application Diagnostics - Checks Python Path in debugger', () => {
108114
helper.verifyAll();
109115
expect(valid).to.be.equal(true, 'not valid');
110116
});
117+
test('Ensure ${workspaceFolder} is not expanded when a resource is not passed', async () => {
118+
const pythonPath = '${workspaceFolder}/venv/bin/python';
119+
120+
workspaceService
121+
.setup(c => c.getWorkspaceFolder(typemoq.It.isAny()))
122+
.returns(() => undefined)
123+
.verifiable(typemoq.Times.never());
124+
helper
125+
.setup(h => h.getInterpreterInformation(typemoq.It.isAny()))
126+
.returns(() => Promise.resolve({}))
127+
.verifiable(typemoq.Times.once());
128+
129+
await diagnosticService.validatePythonPath(pythonPath);
130+
131+
configService.verifyAll();
132+
helper.verifyAll();
133+
});
134+
test('Ensure ${workspaceFolder} is expanded', async () => {
135+
const pythonPath = '${workspaceFolder}/venv/bin/python';
136+
137+
const workspaceFolder = { uri: Uri.parse('full/path/to/workspace'), name: '', index: 0 };
138+
const expectedPath = `${workspaceFolder.uri.fsPath}/venv/bin/python`;
139+
140+
workspaceService
141+
.setup(c => c.getWorkspaceFolder(typemoq.It.isAny()))
142+
.returns(() => workspaceFolder)
143+
.verifiable(typemoq.Times.once());
144+
helper
145+
.setup(h => h.getInterpreterInformation(typemoq.It.isValue(expectedPath)))
146+
.returns(() => Promise.resolve({}))
147+
.verifiable(typemoq.Times.once());
148+
149+
const valid = await diagnosticService.validatePythonPath(pythonPath, Uri.parse('something'));
150+
151+
configService.verifyAll();
152+
helper.verifyAll();
153+
expect(valid).to.be.equal(true, 'not valid');
154+
});
155+
test('Ensure ${env:XYZ123} is expanded', async () => {
156+
const pythonPath = '${env:XYZ123}/venv/bin/python';
157+
158+
process.env.XYZ123 = 'something/else';
159+
const expectedPath = `${process.env.XYZ123}/venv/bin/python`;
160+
workspaceService
161+
.setup(c => c.getWorkspaceFolder(typemoq.It.isAny()))
162+
.returns(() => undefined)
163+
.verifiable(typemoq.Times.once());
164+
helper
165+
.setup(h => h.getInterpreterInformation(typemoq.It.isValue(expectedPath)))
166+
.returns(() => Promise.resolve({}))
167+
.verifiable(typemoq.Times.once());
168+
169+
const valid = await diagnosticService.validatePythonPath(pythonPath);
170+
171+
configService.verifyAll();
172+
helper.verifyAll();
173+
expect(valid).to.be.equal(true, 'not valid');
174+
});
111175
test('Ensure we get python path from config when path = undefined', async () => {
112176
const pythonPath = undefined;
113177

0 commit comments

Comments
 (0)