Document resolver works for both env path and executable

Author: Kartik Raj

src/client/interpreter/interpreterService.ts

@@ -62,7 +62,7 @@ export class InterpreterService implements Disposable, IInterpreterService {
         return this.didChangeInterpreterConfigurationEmitter.event;
     }
 
-    public _pythonPathSetting: string | undefined = '';
+    public _currentInterpreterDisplay: string | undefined = '';
 
     private readonly didChangeInterpreterConfigurationEmitter = new EventEmitter<Uri | undefined>();
 
@@ -181,8 +181,11 @@ export class InterpreterService implements Disposable, IInterpreterService {
         this.didChangeInterpreterConfigurationEmitter.fire(resource);
         // Check if we actually changed our python path
         const interpreter = await this.getActiveInterpreter(resource);
-        if (this._pythonPathSetting === '' || this._pythonPathSetting !== interpreter?.detailedDisplayName) {
-            this._pythonPathSetting = interpreter?.detailedDisplayName;
+        if (
+            this._currentInterpreterDisplay === '' ||
+            this._currentInterpreterDisplay !== interpreter?.detailedDisplayName
+        ) {
+            this._currentInterpreterDisplay = interpreter?.detailedDisplayName;
             this.didChangeInterpreterEmitter.fire();
             const pySettings = this.configService.getSettings(resource);
             reportActiveInterpreterChanged({

src/client/pythonEnvironments/base/locator.ts

@@ -161,11 +161,11 @@ export interface ILocator<I = PythonEnvInfo, E extends BasicPythonEnvsChangedEve
 interface IResolver {
     /**
      * Find as much info about the given Python environment as possible.
-     * If executable passed is invalid, then `undefined` is returned.
+     * If path passed is invalid, then `undefined` is returned.
      *
-     * @param env - the Python executable path to resolve more information about
+     * @param path - Python executable path or environment path to resolve more information about
      */
-    resolveEnv(env: string): Promise<PythonEnvInfo | undefined>;
+    resolveEnv(path: string): Promise<PythonEnvInfo | undefined>;
 }
 
 export interface IResolvingLocator<I = PythonEnvInfo> extends IResolver, ILocator<I> {}
@@ -194,9 +194,11 @@ export interface IDiscoveryAPI {
     getEnvs(query?: PythonLocatorQuery): PythonEnvInfo[];
     /**
      * Find as much info about the given Python environment as possible.
-     * If executable passed is invalid, then `undefined` is returned.
+     * If path passed is invalid, then `undefined` is returned.
+     *
+     * @param path - Python executable path or environment path to resolve more information about
      */
-    resolveEnv(executablePath: string): Promise<PythonEnvInfo | undefined>;
+    resolveEnv(path: string): Promise<PythonEnvInfo | undefined>;
 }
 
 interface IEmitter<E extends PythonEnvsChangedEvent> {

src/client/pythonEnvironments/base/locators/composite/envsCollectionCache.ts

@@ -36,8 +36,10 @@ export interface IEnvsCollectionCache {
     addEnv(env: PythonEnvInfo, hasCompleteInfo?: boolean): void;
 
     /**
-     * Return cached environment information for a given interpreter path if it exists and
+     * Return cached environment information for a given path if it exists and
      * has complete info, otherwise return `undefined`.
+     *
+     * @param path - Python executable path or path to environment
      */
     getCompleteInfo(path: string): PythonEnvInfo | undefined;
 
@@ -125,6 +127,7 @@ export class PythonEnvInfoCache extends PythonEnvsWatcher<PythonEnvCollectionCha
     }
 
     public getCompleteInfo(path: string): PythonEnvInfo | undefined {
+        // `path` can either be path to environment or executable path
         let env = this.envs.find((e) => arePathsSame(e.location, path));
         if (env?.hasCompleteInfo) {
             return env;

src/client/pythonEnvironments/base/locators/composite/envsCollectionService.ts

@@ -55,15 +55,15 @@ export class EnvsCollectionService extends PythonEnvsWatcher<PythonEnvCollection
         });
     }
 
-    public async resolveEnv(executablePath: string): Promise<PythonEnvInfo | undefined> {
+    public async resolveEnv(path: string): Promise<PythonEnvInfo | undefined> {
         // Note cache may have incomplete info when a refresh is happening.
         // This API is supposed to return complete info by definition, so
         // only use cache if it has complete info on an environment.
-        const cachedEnv = this.cache.getCompleteInfo(executablePath);
+        const cachedEnv = this.cache.getCompleteInfo(path);
         if (cachedEnv) {
             return cachedEnv;
         }
-        const resolved = await this.locator.resolveEnv(executablePath);
+        const resolved = await this.locator.resolveEnv(path);
         if (resolved) {
             this.cache.addEnv(resolved, true);
         }

src/client/pythonEnvironments/base/locators/composite/envsResolver.ts

@@ -19,11 +19,7 @@ import {
 import { PythonEnvsChangedEvent } from '../../watcher';
 import { resolveBasicEnv } from './resolverUtils';
 import { traceVerbose } from '../../../../logging';
-import {
-    getEnvironmentDirFromPath,
-    getInterpreterPathFromDir,
-    isExecutableOrEnvPath,
-} from '../../../common/commonUtils';
+import { getEnvironmentDirFromPath, getInterpreterPathFromDir, isPythonExecutable } from '../../../common/commonUtils';
 
 /**
  * Calls environment info service which runs `interpreterInfo.py` script on environments received
@@ -161,13 +157,13 @@ function getResolvedEnv(interpreterInfo: InterpreterInformation, environment: Py
 async function getExecutablePathAndEnvPath(path: string) {
     let executablePath: string;
     let envPath: string;
-    const isPathAnExecutable = await isExecutableOrEnvPath(path);
+    const isPathAnExecutable = await isPythonExecutable(path);
     if (isPathAnExecutable) {
         executablePath = path;
-        envPath = getEnvironmentDirFromPath(path);
+        envPath = getEnvironmentDirFromPath(executablePath);
     } else {
-        executablePath = (await getInterpreterPathFromDir(path)) ?? '';
         envPath = path;
+        executablePath = (await getInterpreterPathFromDir(envPath)) ?? '';
     }
     return [executablePath, envPath];
 }

src/client/pythonEnvironments/base/locators/composite/resolverUtils.ts

@@ -29,13 +29,11 @@ import { traceError, traceWarn } from '../../../../logging';
 
 function getResolvers(): Map<PythonEnvKind, (env: BasicEnvInfo) => Promise<PythonEnvInfo>> {
     const resolvers = new Map<PythonEnvKind, (_: BasicEnvInfo) => Promise<PythonEnvInfo>>();
-    const defaultResolver = (k: PythonEnvKind) => (e: BasicEnvInfo) => resolveGloballyInstalledEnv(e, k);
-    const defaultVirtualEnvResolver = (k: PythonEnvKind) => (e: BasicEnvInfo) => resolveSimpleEnv(e, k);
     Object.values(PythonEnvKind).forEach((k) => {
-        resolvers.set(k, defaultResolver(k));
+        resolvers.set(k, resolveGloballyInstalledEnv);
     });
     virtualEnvKinds.forEach((k) => {
-        resolvers.set(k, defaultVirtualEnvResolver(k));
+        resolvers.set(k, resolveSimpleEnv);
     });
     resolvers.set(PythonEnvKind.Conda, resolveCondaEnv);
     resolvers.set(PythonEnvKind.WindowsStore, resolveWindowsStoreEnv);
@@ -110,7 +108,7 @@ async function updateEnvUsingRegistry(env: PythonEnvInfo): Promise<void> {
     }
 }
 
-async function resolveGloballyInstalledEnv(env: BasicEnvInfo, kind: PythonEnvKind): Promise<PythonEnvInfo> {
+async function resolveGloballyInstalledEnv(env: BasicEnvInfo): Promise<PythonEnvInfo> {
     const { executablePath } = env;
     let version;
     try {
@@ -119,17 +117,17 @@ async function resolveGloballyInstalledEnv(env: BasicEnvInfo, kind: PythonEnvKin
         version = UNKNOWN_PYTHON_VERSION;
     }
     const envInfo = buildEnvInfo({
-        kind,
+        kind: env.kind,
         version,
         executable: executablePath,
     });
     return envInfo;
 }
 
-async function resolveSimpleEnv(env: BasicEnvInfo, kind: PythonEnvKind): Promise<PythonEnvInfo> {
+async function resolveSimpleEnv(env: BasicEnvInfo): Promise<PythonEnvInfo> {
     const { executablePath } = env;
     const envInfo = buildEnvInfo({
-        kind,
+        kind: env.kind,
         version: await getPythonVersionFromPath(executablePath),
         executable: executablePath,
     });
@@ -173,7 +171,7 @@ async function resolveCondaEnv(env: BasicEnvInfo): Promise<PythonEnvInfo> {
         `${executablePath} identified as a Conda environment but is not returned via '${conda?.command} info' command`,
     );
     // Environment could still be valid, resolve as a simple env.
-    return resolveSimpleEnv(env, PythonEnvKind.Conda);
+    return resolveSimpleEnv(env);
 }
 
 async function resolvePyenvEnv(env: BasicEnvInfo): Promise<PythonEnvInfo> {

src/client/pythonEnvironments/common/commonUtils.ts

@@ -20,16 +20,18 @@ const matchPythonBinFilename =
 type FileFilterFunc = (filename: string) => boolean;
 
 /**
- * Returns `true` if path provides is likely an executable and not path to environment folder
+ * Returns `true` if path provided is likely a python executable.
  */
-export async function isExecutableOrEnvPath(filePath: string): Promise<boolean> {
-    const matchesExecutableRegex = matchPythonBinFilename(filePath);
-    if (!matchesExecutableRegex) {
+export async function isPythonExecutable(filePath: string): Promise<boolean> {
+    const isMatch = matchPythonBinFilename(filePath);
+    if (!isMatch) {
         return false;
     }
+    // On Windows it's fair to assume a path ending with `.exe` denotes a file.
     if (getOSType() === OSType.Windows) {
         return true;
     }
+    // For other operating systems verify if it's a file.
     if (await isFile(filePath)) {
         return true;
     }

src/test/interpreters/interpreterService.unit.test.ts

@@ -248,7 +248,7 @@ suite('Interpreters service', () => {
     test('If stored setting is an empty string, refresh the interpreter display', async () => {
         const service = new InterpreterService(serviceContainer, pyenvs.object);
         const resource = Uri.parse('a');
-        service._pythonPathSetting = '';
+        service._currentInterpreterDisplay = '';
         configService.reset();
         configService.setup((c) => c.getSettings(resource)).returns(() => ({ pythonPath: 'current path' } as any));
         interpreterDisplay
@@ -266,7 +266,7 @@ suite('Interpreters service', () => {
     test('If stored setting is not equal to current interpreter path setting, refresh the interpreter display', async () => {
         const service = new InterpreterService(serviceContainer, pyenvs.object);
         const resource = Uri.parse('a');
-        service._pythonPathSetting = 'stored setting';
+        service._currentInterpreterDisplay = 'stored setting';
         configService.reset();
         configService.setup((c) => c.getSettings(resource)).returns(() => ({ pythonPath: 'current path' } as any));
         interpreterDisplay
@@ -284,7 +284,7 @@ suite('Interpreters service', () => {
     test('If stored setting is equal to current interpreter path setting, do not refresh the interpreter display', async () => {
         const service = new InterpreterService(serviceContainer, pyenvs.object);
         const resource = Uri.parse('a');
-        service._pythonPathSetting = 'setting';
+        service._currentInterpreterDisplay = 'setting';
         configService.reset();
         configService.setup((c) => c.getSettings(resource)).returns(() => ({ pythonPath: 'setting' } as any));
         interpreterDisplay