Ensure we block on autoselection when no interpreter is explictly set by user (#16723)

* Ensure we block on autoselection when no interpreter is explictly set by user

* Added tests

* News entry

Author: Kartik Raj

news/2 Fixes/16723.md

@@ -0,0 +1 @@
+Ensure we block on autoselection when no interpreter is explictly set by user.

src/client/common/interpreterPathProxyService.ts

@@ -0,0 +1,33 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+'use strict';
+
+import { inject, injectable } from 'inversify';
+import { IWorkspaceService } from './application/types';
+import { DeprecatePythonPath } from './experiments/groups';
+import { IExperimentService, IInterpreterPathProxyService, IInterpreterPathService, Resource } from './types';
+import { SystemVariables } from './variables/systemVariables';
+
+@injectable()
+export class InterpreterPathProxyService implements IInterpreterPathProxyService {
+    constructor(
+        @inject(IInterpreterPathService) private readonly interpreterPathService: IInterpreterPathService,
+        @inject(IExperimentService) private readonly experiment: IExperimentService,
+        @inject(IWorkspaceService) private readonly workspace: IWorkspaceService,
+    ) {}
+
+    public get(resource: Resource): string {
+        const systemVariables = new SystemVariables(
+            undefined,
+            this.workspace.getWorkspaceFolder(resource)?.uri.fsPath,
+            this.workspace,
+        );
+        const pythonSettings = this.workspace.getConfiguration('python', resource);
+        return systemVariables.resolveAny(
+            this.experiment.inExperimentSync(DeprecatePythonPath.experiment)
+                ? this.interpreterPathService.get(resource)
+                : pythonSettings.get<string>('pythonPath'),
+        )!;
+    }
+}

src/client/common/serviceRegistry.ts

@@ -20,6 +20,7 @@ import {
     IToolExecutionPath,
     IsWindows,
     ToolExecutionPath,
+    IInterpreterPathProxyService,
 } from './types';
 import { IServiceManager } from '../ioc/types';
 import { JupyterExtensionDependencyManager } from '../jupyter/jupyterExtensionDependencyManager';
@@ -118,11 +119,16 @@ import { IMultiStepInputFactory, MultiStepInputFactory } from './utils/multiStep
 import { Random } from './utils/random';
 import { JupyterNotInstalledNotificationHelper } from '../jupyter/jupyterNotInstalledNotificationHelper';
 import { IJupyterNotInstalledNotificationHelper } from '../jupyter/types';
+import { InterpreterPathProxyService } from './interpreterPathProxyService';
 
 export function registerTypes(serviceManager: IServiceManager): void {
     serviceManager.addSingletonInstance<boolean>(IsWindows, IS_WINDOWS);
 
     serviceManager.addSingleton<IActiveResourceService>(IActiveResourceService, ActiveResourceService);
+    serviceManager.addSingleton<IInterpreterPathProxyService>(
+        IInterpreterPathProxyService,
+        InterpreterPathProxyService,
+    );
     serviceManager.addSingleton<IInterpreterPathService>(IInterpreterPathService, InterpreterPathService);
     serviceManager.addSingleton<IExtensions>(IExtensions, Extensions);
     serviceManager.addSingleton<IRandom>(IRandom, Random);

src/client/common/types.ts

@@ -557,6 +557,14 @@ export interface IInterpreterPathService {
     copyOldInterpreterStorageValuesToNew(resource: Uri | undefined): Promise<void>;
 }
 
+/**
+ * Interface used to access current Interpreter Path
+ */
+export const IInterpreterPathProxyService = Symbol('IInterpreterPathProxyService');
+export interface IInterpreterPathProxyService {
+    get(resource: Resource): string;
+}
+
 export type DefaultLSType = LanguageServerType.Jedi | LanguageServerType.JediLSP | LanguageServerType.Node;
 
 /**

src/client/interpreter/display/index.ts

@@ -3,7 +3,13 @@ import { Disposable, OutputChannel, StatusBarAlignment, StatusBarItem, Uri } fro
 import { IApplicationShell, IWorkspaceService } from '../../common/application/types';
 import { STANDARD_OUTPUT_CHANNEL } from '../../common/constants';
 import '../../common/extensions';
-import { IConfigurationService, IDisposableRegistry, IOutputChannel, IPathUtils, Resource } from '../../common/types';
+import {
+    IDisposableRegistry,
+    IInterpreterPathProxyService,
+    IOutputChannel,
+    IPathUtils,
+    Resource,
+} from '../../common/types';
 import { Interpreters } from '../../common/utils/localize';
 import { IServiceContainer } from '../../ioc/types';
 import { PythonEnvironment } from '../../pythonEnvironments/info';
@@ -22,7 +28,7 @@ export class InterpreterDisplay implements IInterpreterDisplay {
     private readonly workspaceService: IWorkspaceService;
     private readonly pathUtils: IPathUtils;
     private readonly interpreterService: IInterpreterService;
-    private readonly configService: IConfigurationService;
+    private readonly interpreterPathExpHelper: IInterpreterPathProxyService;
     private currentlySelectedInterpreterPath?: string;
     private currentlySelectedWorkspaceFolder: Resource;
     private readonly autoSelection: IInterpreterAutoSelectionService;
@@ -39,7 +45,9 @@ export class InterpreterDisplay implements IInterpreterDisplay {
 
         const application = serviceContainer.get<IApplicationShell>(IApplicationShell);
         const disposableRegistry = serviceContainer.get<Disposable[]>(IDisposableRegistry);
-        this.configService = serviceContainer.get<IConfigurationService>(IConfigurationService);
+        this.interpreterPathExpHelper = serviceContainer.get<IInterpreterPathProxyService>(
+            IInterpreterPathProxyService,
+        );
 
         this.statusBar = application.createStatusBarItem(StatusBarAlignment.Left, 100);
         this.statusBar.command = 'python.setInterpreter';
@@ -75,7 +83,7 @@ export class InterpreterDisplay implements IInterpreterDisplay {
         }
     }
     private async updateDisplay(workspaceFolder?: Uri) {
-        const interpreterPath = this.configService.getSettings(workspaceFolder)?.pythonPath;
+        const interpreterPath = this.interpreterPathExpHelper.get(workspaceFolder);
         if (!interpreterPath || interpreterPath === 'python') {
             await this.autoSelection.autoSelectInterpreter(workspaceFolder); // Block on this only if no interpreter selected.
         }

src/test/common/interpreterPathProxyService.unit.test.ts

@@ -0,0 +1,54 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+'use strict';
+
+import { Uri, WorkspaceConfiguration } from 'vscode';
+import * as TypeMoq from 'typemoq';
+import { expect } from 'chai';
+import { InterpreterPathProxyService } from '../../client/common/interpreterPathProxyService';
+import { IExperimentService, IInterpreterPathProxyService, IInterpreterPathService } from '../../client/common/types';
+import { IWorkspaceService } from '../../client/common/application/types';
+import { DeprecatePythonPath } from '../../client/common/experiments/groups';
+
+suite('Interpreter Path Proxy Service', async () => {
+    let interpreterPathProxyService: IInterpreterPathProxyService;
+    let workspaceService: TypeMoq.IMock<IWorkspaceService>;
+    let experiments: TypeMoq.IMock<IExperimentService>;
+    let interpreterPathService: TypeMoq.IMock<IInterpreterPathService>;
+    const resource = Uri.parse('a');
+    const interpreterPath = 'path/to/interpreter';
+    setup(() => {
+        workspaceService = TypeMoq.Mock.ofType<IWorkspaceService>();
+        experiments = TypeMoq.Mock.ofType<IExperimentService>();
+        interpreterPathService = TypeMoq.Mock.ofType<IInterpreterPathService>();
+        workspaceService
+            .setup((w) => w.getWorkspaceFolder(resource))
+            .returns(() => ({
+                uri: resource,
+                name: 'Workspacefolder',
+                index: 0,
+            }));
+        interpreterPathProxyService = new InterpreterPathProxyService(
+            interpreterPathService.object,
+            experiments.object,
+            workspaceService.object,
+        );
+    });
+
+    test('When in experiment, use interpreter path service to get setting value', () => {
+        experiments.setup((e) => e.inExperimentSync(DeprecatePythonPath.experiment)).returns(() => true);
+        interpreterPathService.setup((i) => i.get(resource)).returns(() => interpreterPath);
+        const value = interpreterPathProxyService.get(resource);
+        expect(value).to.equal(interpreterPath);
+    });
+
+    test('When not in experiment, use workspace service to get setting value', () => {
+        experiments.setup((e) => e.inExperimentSync(DeprecatePythonPath.experiment)).returns(() => false);
+        const workspaceConfig = TypeMoq.Mock.ofType<WorkspaceConfiguration>();
+        workspaceService.setup((i) => i.getConfiguration('python', resource)).returns(() => workspaceConfig.object);
+        workspaceConfig.setup((w) => w.get('pythonPath')).returns(() => interpreterPath);
+        const value = interpreterPathProxyService.get(resource);
+        expect(value).to.equal(interpreterPath);
+    });
+});

src/test/interpreters/display.unit.test.ts

@@ -16,11 +16,10 @@ import { IApplicationShell, IWorkspaceService } from '../../client/common/applic
 import { STANDARD_OUTPUT_CHANNEL } from '../../client/common/constants';
 import { IFileSystem } from '../../client/common/platform/types';
 import {
-    IConfigurationService,
+    IInterpreterPathProxyService,
     IDisposableRegistry,
     IOutputChannel,
     IPathUtils,
-    IPythonSettings,
     ReadWrite,
 } from '../../client/common/types';
 import { Interpreters } from '../../client/common/utils/localize';
@@ -57,8 +56,7 @@ suite('Interpreters Display', () => {
     let fileSystem: TypeMoq.IMock<IFileSystem>;
     let disposableRegistry: Disposable[];
     let statusBar: TypeMoq.IMock<StatusBarItem>;
-    let pythonSettings: TypeMoq.IMock<IPythonSettings>;
-    let configurationService: TypeMoq.IMock<IConfigurationService>;
+    let interpreterPathProxyService: TypeMoq.IMock<IInterpreterPathProxyService>;
     let interpreterDisplay: IInterpreterDisplay;
     let interpreterHelper: TypeMoq.IMock<IInterpreterHelper>;
     let pathUtils: TypeMoq.IMock<IPathUtils>;
@@ -73,8 +71,7 @@ suite('Interpreters Display', () => {
         interpreterHelper = TypeMoq.Mock.ofType<IInterpreterHelper>();
         disposableRegistry = [];
         statusBar = TypeMoq.Mock.ofType<StatusBarItem>();
-        pythonSettings = TypeMoq.Mock.ofType<IPythonSettings>();
-        configurationService = TypeMoq.Mock.ofType<IConfigurationService>();
+        interpreterPathProxyService = TypeMoq.Mock.ofType<IInterpreterPathProxyService>();
         pathUtils = TypeMoq.Mock.ofType<IPathUtils>();
         output = TypeMoq.Mock.ofType<IOutputChannel>();
         autoSelection = mock(InterpreterAutoSelectionService);
@@ -94,8 +91,8 @@ suite('Interpreters Display', () => {
         serviceContainer.setup((c) => c.get(TypeMoq.It.isValue(IFileSystem))).returns(() => fileSystem.object);
         serviceContainer.setup((c) => c.get(TypeMoq.It.isValue(IDisposableRegistry))).returns(() => disposableRegistry);
         serviceContainer
-            .setup((c) => c.get(TypeMoq.It.isValue(IConfigurationService)))
-            .returns(() => configurationService.object);
+            .setup((c) => c.get(TypeMoq.It.isValue(IInterpreterPathProxyService)))
+            .returns(() => interpreterPathProxyService.object);
         serviceContainer
             .setup((c) => c.get(TypeMoq.It.isValue(IInterpreterHelper)))
             .returns(() => interpreterHelper.object);
@@ -215,8 +212,7 @@ suite('Interpreters Display', () => {
         interpreterService
             .setup((i) => i.getActiveInterpreter(TypeMoq.It.isValue(workspaceFolder)))
             .returns(() => Promise.resolve(undefined));
-        configurationService.setup((c) => c.getSettings(TypeMoq.It.isAny())).returns(() => pythonSettings.object);
-        pythonSettings.setup((p) => p.pythonPath).returns(() => pythonPath);
+        interpreterPathProxyService.setup((c) => c.get(TypeMoq.It.isAny())).returns(() => pythonPath);
         fileSystem.setup((f) => f.fileExists(TypeMoq.It.isValue(pythonPath))).returns(() => Promise.resolve(false));
         interpreterHelper
             .setup((v) => v.getInterpreterInformation(TypeMoq.It.isValue(pythonPath)))