Skip to content

Commit 7b6b842

Browse files
author
Kartik Raj
authored
Do not trigger any commands which requires an interpreter if an invalid interpreter is selected (#19481)
* Do not trigger any commands which requires an interpreter if an invalid interpreter is selected * Do not trigger the prompt automatically * Fix tests * Fix single workspace linter tests * Fix other tests * Fix testS
1 parent 9929f76 commit 7b6b842

File tree

18 files changed

+128
-19
lines changed

18 files changed

+128
-19
lines changed

‎src/client/debugger/extension/configuration/resolvers/base.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,8 @@ export abstract class BaseConfigurationResolver<T extends DebugConfiguration>
121121
);
122122
if (debugConfiguration.pythonPath === '${command:python.interpreterPath}' || !debugConfiguration.pythonPath) {
123123
const interpreterPath =
124-
(await this.interpreterService.getActiveInterpreter(workspaceFolder))?.path ?? 'python';
124+
(await this.interpreterService.getActiveInterpreter(workspaceFolder))?.path ??
125+
this.configurationService.getSettings(workspaceFolder).pythonPath;
125126
debugConfiguration.pythonPath = interpreterPath;
126127
this.pythonPathSource = PythonPathSource.settingsJson;
127128
} else {

‎src/client/debugger/extension/debugCommands.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ import { sendTelemetryEvent } from '../../telemetry';
1212
import { EventName } from '../../telemetry/constants';
1313
import { ILaunchJsonReader } from './configuration/types';
1414
import { DebugPurpose, LaunchRequestArguments } from '../types';
15+
import { IInterpreterService } from '../../interpreter/contracts';
16+
import { noop } from '../../common/utils/misc';
1517

1618
@injectable()
1719
export class DebugCommands implements IExtensionSingleActivationService {
@@ -22,12 +24,18 @@ export class DebugCommands implements IExtensionSingleActivationService {
2224
@inject(IDebugService) private readonly debugService: IDebugService,
2325
@inject(ILaunchJsonReader) private readonly launchJsonReader: ILaunchJsonReader,
2426
@inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry,
27+
@inject(IInterpreterService) private readonly interpreterService: IInterpreterService,
2528
) {}
2629

2730
public activate(): Promise<void> {
2831
this.disposables.push(
2932
this.commandManager.registerCommand(Commands.Debug_In_Terminal, async (file?: Uri) => {
3033
sendTelemetryEvent(EventName.DEBUG_IN_TERMINAL_BUTTON);
34+
const interpreter = await this.interpreterService.getActiveInterpreter(file);
35+
if (!interpreter) {
36+
this.commandManager.executeCommand(Commands.TriggerEnvironmentSelection, file).then(noop, noop);
37+
return;
38+
}
3139
const config = await this.getDebugConfiguration(file);
3240
this.debugService.startDebugging(undefined, config);
3341
}),

‎src/client/linters/linterCommands.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ export class LinterCommands implements IDisposable {
112112

113113
public runLinting(): Promise<DiagnosticCollection> {
114114
const engine = this.serviceContainer.get<ILintingEngine>(ILintingEngine);
115-
return engine.lintOpenPythonFiles();
115+
return engine.lintOpenPythonFiles('manual');
116116
}
117117

118118
private get settingsUri(): Uri | undefined {

‎src/client/linters/lintingEngine.ts

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,13 @@ import { inject, injectable } from 'inversify';
77
import { Minimatch } from 'minimatch';
88
import * as path from 'path';
99
import * as vscode from 'vscode';
10-
import { IDocumentManager, IWorkspaceService } from '../common/application/types';
10+
import { ICommandManager, IDocumentManager, IWorkspaceService } from '../common/application/types';
11+
import { Commands } from '../common/constants';
1112
import { IFileSystem } from '../common/platform/types';
1213
import { IConfigurationService } from '../common/types';
13-
import { isNotebookCell } from '../common/utils/misc';
14+
import { isNotebookCell, noop } from '../common/utils/misc';
1415
import { StopWatch } from '../common/utils/stopWatch';
16+
import { IInterpreterService } from '../interpreter/contracts';
1517
import { IServiceContainer } from '../ioc/types';
1618
import { sendTelemetryWhenDone } from '../telemetry';
1719
import { EventName } from '../telemetry/constants';
@@ -61,9 +63,9 @@ export class LintingEngine implements ILintingEngine {
6163
}
6264
}
6365

64-
public async lintOpenPythonFiles(): Promise<vscode.DiagnosticCollection> {
66+
public async lintOpenPythonFiles(trigger: LinterTrigger = 'auto'): Promise<vscode.DiagnosticCollection> {
6567
this.diagnosticCollection.clear();
66-
const promises = this.documents.textDocuments.map(async (document) => this.lintDocument(document, 'auto'));
68+
const promises = this.documents.textDocuments.map(async (document) => this.lintDocument(document, trigger));
6769
await Promise.all(promises);
6870
return this.diagnosticCollection;
6971
}
@@ -75,7 +77,7 @@ export class LintingEngine implements ILintingEngine {
7577
this.diagnosticCollection.set(document.uri, []);
7678

7779
// Check if we need to lint this document
78-
if (!(await this.shouldLintDocument(document))) {
80+
if (!(await this.shouldLintDocument(document, trigger))) {
7981
return;
8082
}
8183

@@ -164,7 +166,16 @@ export class LintingEngine implements ILintingEngine {
164166
return diagnostic;
165167
}
166168

167-
private async shouldLintDocument(document: vscode.TextDocument): Promise<boolean> {
169+
private async shouldLintDocument(document: vscode.TextDocument, trigger: LinterTrigger): Promise<boolean> {
170+
const interpreterService = this.serviceContainer.get<IInterpreterService>(IInterpreterService);
171+
const interpreter = await interpreterService.getActiveInterpreter(document.uri);
172+
if (!interpreter && trigger === 'manual') {
173+
this.serviceContainer
174+
.get<ICommandManager>(ICommandManager)
175+
.executeCommand(Commands.TriggerEnvironmentSelection, document.uri)
176+
.then(noop, noop);
177+
return false;
178+
}
168179
if (!(await this.linterManager.isLintingEnabled(document.uri))) {
169180
this.diagnosticCollection.set(document.uri, []);
170181
return false;

‎src/client/linters/types.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ export enum LintMessageSeverity {
7474
export const ILintingEngine = Symbol('ILintingEngine');
7575
export interface ILintingEngine {
7676
readonly diagnostics: vscode.DiagnosticCollection;
77-
lintOpenPythonFiles(): Promise<vscode.DiagnosticCollection>;
77+
lintOpenPythonFiles(trigger?: LinterTrigger): Promise<vscode.DiagnosticCollection>;
7878
lintDocument(document: vscode.TextDocument, trigger: LinterTrigger): Promise<void>;
7979
clearDiagnostics(document: vscode.TextDocument): void;
8080
}

‎src/client/providers/importSortProvider.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
import { createDeferred, createDeferredFromPromise, Deferred } from '../common/utils/async';
1818
import { Common, Diagnostics } from '../common/utils/localize';
1919
import { noop } from '../common/utils/misc';
20+
import { IInterpreterService } from '../interpreter/contracts';
2021
import { IServiceContainer } from '../ioc/types';
2122
import { traceError } from '../logging';
2223
import { captureTelemetry } from '../telemetry';
@@ -108,6 +109,15 @@ export class SortImportsEditingProvider implements ISortImportsEditingProvider {
108109
}
109110

110111
public async sortImports(uri?: Uri): Promise<void> {
112+
const interpreterService = this.serviceContainer.get<IInterpreterService>(IInterpreterService);
113+
const interpreter = await interpreterService.getActiveInterpreter(uri);
114+
if (!interpreter) {
115+
this.serviceContainer
116+
.get<ICommandManager>(ICommandManager)
117+
.executeCommand(Commands.TriggerEnvironmentSelection, uri)
118+
.then(noop, noop);
119+
return;
120+
}
111121
const extension = this.extensions.getExtension('ms-python.isort');
112122
if (extension && extension.isActive) {
113123
// Don't run isort from python extension if isort extension is active.

‎src/client/providers/replProvider.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import { Disposable } from 'vscode';
22
import { IActiveResourceService, ICommandManager } from '../common/application/types';
33
import { Commands } from '../common/constants';
4+
import { noop } from '../common/utils/misc';
5+
import { IInterpreterService } from '../interpreter/contracts';
46
import { IServiceContainer } from '../ioc/types';
57
import { captureTelemetry } from '../telemetry';
68
import { EventName } from '../telemetry/constants';
@@ -29,6 +31,15 @@ export class ReplProvider implements Disposable {
2931
@captureTelemetry(EventName.REPL)
3032
private async commandHandler() {
3133
const resource = this.activeResourceService.getActiveResource();
34+
const interpreterService = this.serviceContainer.get<IInterpreterService>(IInterpreterService);
35+
const interpreter = await interpreterService.getActiveInterpreter(resource);
36+
if (!interpreter) {
37+
this.serviceContainer
38+
.get<ICommandManager>(ICommandManager)
39+
.executeCommand(Commands.TriggerEnvironmentSelection, resource)
40+
.then(noop, noop);
41+
return;
42+
}
3243
const replProvider = this.serviceContainer.get<ICodeExecutionService>(ICodeExecutionService, 'repl');
3344
await replProvider.initializeRepl(resource);
3445
}

‎src/client/telemetry/types.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import { EventName } from './constants';
88

99
export type EditorLoadTelemetry = IEventNamePropertyMapping[EventName.EDITOR_LOAD];
1010

11-
export type LinterTrigger = 'auto' | 'save';
11+
export type LinterTrigger = 'auto' | 'save' | 'manual';
1212

1313
export type LintingTelemetry = IEventNamePropertyMapping[EventName.LINTING];
1414

‎src/client/terminals/codeExecution/codeExecutionManager.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import '../../common/extensions';
1212
import { IFileSystem } from '../../common/platform/types';
1313
import { IDisposableRegistry, IConfigurationService, Resource } from '../../common/types';
1414
import { noop } from '../../common/utils/misc';
15+
import { IInterpreterService } from '../../interpreter/contracts';
1516
import { IServiceContainer } from '../../ioc/types';
1617
import { traceError } from '../../logging';
1718
import { captureTelemetry, sendTelemetryEvent } from '../../telemetry';
@@ -38,6 +39,12 @@ export class CodeExecutionManager implements ICodeExecutionManager {
3839
[Commands.Exec_In_Terminal, Commands.Exec_In_Terminal_Icon].forEach((cmd) => {
3940
this.disposableRegistry.push(
4041
this.commandManager.registerCommand(cmd as any, async (file: Resource) => {
42+
const interpreterService = this.serviceContainer.get<IInterpreterService>(IInterpreterService);
43+
const interpreter = await interpreterService.getActiveInterpreter(file);
44+
if (!interpreter) {
45+
this.commandManager.executeCommand(Commands.TriggerEnvironmentSelection, file).then(noop, noop);
46+
return;
47+
}
4148
const trigger = cmd === Commands.Exec_In_Terminal ? 'command' : 'icon';
4249
await this.executeFileInTerminal(file, trigger)
4350
.then(() => {
@@ -50,6 +57,12 @@ export class CodeExecutionManager implements ICodeExecutionManager {
5057
});
5158
this.disposableRegistry.push(
5259
this.commandManager.registerCommand(Commands.Exec_Selection_In_Terminal as any, async (file: Resource) => {
60+
const interpreterService = this.serviceContainer.get<IInterpreterService>(IInterpreterService);
61+
const interpreter = await interpreterService.getActiveInterpreter(file);
62+
if (!interpreter) {
63+
this.commandManager.executeCommand(Commands.TriggerEnvironmentSelection, file).then(noop, noop);
64+
return;
65+
}
5366
await this.executeSelectionInTerminal().then(() => {
5467
if (this.shouldTerminalFocusOnStart(file))
5568
this.commandManager.executeCommand('workbench.action.terminal.focus');
@@ -60,6 +73,12 @@ export class CodeExecutionManager implements ICodeExecutionManager {
6073
this.commandManager.registerCommand(
6174
Commands.Exec_Selection_In_Django_Shell as any,
6275
async (file: Resource) => {
76+
const interpreterService = this.serviceContainer.get<IInterpreterService>(IInterpreterService);
77+
const interpreter = await interpreterService.getActiveInterpreter(file);
78+
if (!interpreter) {
79+
this.commandManager.executeCommand(Commands.TriggerEnvironmentSelection, file).then(noop, noop);
80+
return;
81+
}
6382
await this.executeSelectionInDjangoShell().then(() => {
6483
if (this.shouldTerminalFocusOnStart(file))
6584
this.commandManager.executeCommand('workbench.action.terminal.focus');

‎src/client/testing/main.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ export class UnitTestManagementService implements IExtensionActivationService {
149149
const interpreterService = this.serviceContainer.get<IInterpreterService>(IInterpreterService);
150150
const commandManager = this.serviceContainer.get<ICommandManager>(ICommandManager);
151151
if (!(await interpreterService.getActiveInterpreter(wkspace))) {
152-
commandManager.executeCommand('python.triggerEnvSelection', wkspace);
152+
commandManager.executeCommand(constants.Commands.TriggerEnvironmentSelection, wkspace);
153153
return;
154154
}
155155
const configurationService = this.serviceContainer.get<ITestConfigurationService>(ITestConfigurationService);

‎src/client/testing/testController/controller.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,7 @@ export class PythonTestController implements ITestController, IExtensionSingleAc
256256
workspaces.map(async (workspace) => {
257257
if (!(await this.interpreterService.getActiveInterpreter(workspace.uri))) {
258258
this.commandManager
259-
.executeCommand('python.triggerEnvSelection', workspace.uri)
259+
.executeCommand(constants.Commands.TriggerEnvironmentSelection, workspace.uri)
260260
.then(noop, noop);
261261
return;
262262
}
@@ -321,7 +321,7 @@ export class PythonTestController implements ITestController, IExtensionSingleAc
321321
workspaces.map(async (workspace) => {
322322
if (!(await this.interpreterService.getActiveInterpreter(workspace.uri))) {
323323
this.commandManager
324-
.executeCommand('python.triggerEnvSelection', workspace.uri)
324+
.executeCommand(constants.Commands.TriggerEnvironmentSelection, workspace.uri)
325325
.then(noop, noop);
326326
return undefined;
327327
}

‎src/test/debugger/extension/debugCommands.unit.test.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,16 +13,22 @@ import { DebugCommands } from '../../../client/debugger/extension/debugCommands'
1313
import { EXTENSION_ROOT_DIR_FOR_TESTS } from '../../constants';
1414
import * as telemetry from '../../../client/telemetry';
1515
import { ILaunchJsonReader } from '../../../client/debugger/extension/configuration/types';
16+
import { IInterpreterService } from '../../../client/interpreter/contracts';
17+
import { PythonEnvironment } from '../../../client/pythonEnvironments/info';
1618

1719
suite('Debugging - commands', () => {
1820
let commandManager: typemoq.IMock<ICommandManager>;
1921
let debugService: typemoq.IMock<IDebugService>;
2022
let disposables: typemoq.IMock<IDisposableRegistry>;
2123
let launchJsonReader: typemoq.IMock<ILaunchJsonReader>;
24+
let interpreterService: typemoq.IMock<IInterpreterService>;
2225
let debugCommands: IExtensionSingleActivationService;
2326

2427
setup(() => {
2528
commandManager = typemoq.Mock.ofType<ICommandManager>();
29+
commandManager
30+
.setup((c) => c.executeCommand(typemoq.It.isAny(), typemoq.It.isAny()))
31+
.returns(() => Promise.resolve());
2632
debugService = typemoq.Mock.ofType<IDebugService>();
2733
launchJsonReader = typemoq.Mock.ofType<ILaunchJsonReader>();
2834
launchJsonReader
@@ -31,6 +37,10 @@ suite('Debugging - commands', () => {
3137
.verifiable(typemoq.Times.once());
3238

3339
disposables = typemoq.Mock.ofType<IDisposableRegistry>();
40+
interpreterService = typemoq.Mock.ofType<IInterpreterService>();
41+
interpreterService
42+
.setup((i) => i.getActiveInterpreter(typemoq.It.isAny()))
43+
.returns(() => Promise.resolve(({ path: 'ps' } as unknown) as PythonEnvironment));
3444
sinon.stub(telemetry, 'sendTelemetryEvent').callsFake(() => {
3545
/** noop */
3646
});
@@ -53,6 +63,7 @@ suite('Debugging - commands', () => {
5363
debugService.object,
5464
launchJsonReader.object,
5565
disposables.object,
66+
interpreterService.object,
5667
);
5768
await debugCommands.activate();
5869
commandManager.verifyAll();
@@ -74,6 +85,7 @@ suite('Debugging - commands', () => {
7485
debugService.object,
7586
launchJsonReader.object,
7687
disposables.object,
88+
interpreterService.object,
7789
);
7890
await debugCommands.activate();
7991

‎src/test/format/extension.sort.test.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,19 @@ import * as assert from 'assert';
22
import { expect } from 'chai';
33
import * as fs from 'fs';
44
import { EOL } from 'os';
5+
import * as TypeMoq from 'typemoq';
56
import * as path from 'path';
67
import { instance, mock } from 'ts-mockito';
78
import { commands, ConfigurationTarget, Position, Range, Uri, window, workspace } from 'vscode';
89
import { Commands } from '../../client/common/constants';
910
import { ICondaService, IInterpreterService } from '../../client/interpreter/contracts';
10-
import { InterpreterService } from '../../client/interpreter/interpreterService';
1111
import { SortImportsEditingProvider } from '../../client/providers/importSortProvider';
1212
import { ISortImportsEditingProvider } from '../../client/providers/types';
1313
import { CondaService } from '../../client/pythonEnvironments/common/environmentManagers/condaService';
1414
import { updateSetting } from '../common';
1515
import { closeActiveWindows, initialize, initializeTest, IS_MULTI_ROOT_TEST, TEST_TIMEOUT } from '../initialize';
1616
import { UnitTestIocContainer } from '../testing/serviceRegistry';
17+
import { PythonEnvironment } from '../../client/pythonEnvironments/info';
1718

1819
const sortingPath = path.join(__dirname, '..', '..', '..', 'src', 'test', 'pythonFiles', 'sorting');
1920
const fileToFormatWithoutConfig = path.join(sortingPath, 'noconfig', 'before.py');
@@ -65,8 +66,12 @@ suite('Sorting', () => {
6566
ioc.registerProcessTypes();
6667
ioc.registerInterpreterStorageTypes();
6768
await ioc.registerMockInterpreterTypes();
69+
const interpreterService = TypeMoq.Mock.ofType<IInterpreterService>();
70+
interpreterService
71+
.setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny()))
72+
.returns(() => Promise.resolve(({ path: 'ps' } as unknown) as PythonEnvironment));
6873
ioc.serviceManager.rebindInstance<ICondaService>(ICondaService, instance(mock(CondaService)));
69-
ioc.serviceManager.rebindInstance<IInterpreterService>(IInterpreterService, instance(mock(InterpreterService)));
74+
ioc.serviceManager.rebindInstance<IInterpreterService>(IInterpreterService, interpreterService.object);
7075
}
7176
test('Without Config', async () => {
7277
const textDocument = await workspace.openTextDocument(fileToFormatWithoutConfig);

‎src/test/linters/lintengine.test.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,11 @@ import { PYTHON_LANGUAGE, STANDARD_OUTPUT_CHANNEL } from '../../client/common/co
1010
import '../../client/common/extensions';
1111
import { IFileSystem } from '../../client/common/platform/types';
1212
import { IConfigurationService, ILintingSettings, IOutputChannel, IPythonSettings } from '../../client/common/types';
13+
import { IInterpreterService } from '../../client/interpreter/contracts';
1314
import { IServiceContainer } from '../../client/ioc/types';
1415
import { LintingEngine } from '../../client/linters/lintingEngine';
1516
import { ILinterManager, ILintingEngine } from '../../client/linters/types';
17+
import { PythonEnvironment } from '../../client/pythonEnvironments/info';
1618
import { initialize } from '../initialize';
1719

1820
suite('Linting - LintingEngine', () => {
@@ -67,6 +69,12 @@ suite('Linting - LintingEngine', () => {
6769
serviceContainer
6870
.setup((c) => c.get(TypeMoq.It.isValue(ILintingEngine), TypeMoq.It.isAny()))
6971
.returns(() => lintingEngine);
72+
73+
const interpreterService = TypeMoq.Mock.ofType<IInterpreterService>();
74+
interpreterService
75+
.setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny()))
76+
.returns(() => Promise.resolve(({ path: 'ps' } as unknown) as PythonEnvironment));
77+
serviceContainer.setup((c) => c.get(IInterpreterService)).returns(() => interpreterService.object);
7078
});
7179

7280
test('Ensure document.uri is passed into isLintingEnabled', () => {

0 commit comments

Comments
 (0)