fixes recursive venv assignment and incorrect multiroot result (#558)

fixes #491

Author: eleanorjboyd

src/managers/builtin/venvManager.ts

@@ -103,11 +103,13 @@ export class VenvManager implements EnvironmentManager {
         }
     }
 
+    /**
+     * Returns configuration for quick create in the workspace root, undefined if no suitable Python 3 version is found.
+     */
     quickCreateConfig(): QuickCreateConfig | undefined {
         if (!this.globalEnv || !this.globalEnv.version.startsWith('3.')) {
             return undefined;
         }
-
         return {
             description: l10n.t('Create a virtual environment in workspace root'),
             detail: l10n.t(
@@ -214,6 +216,9 @@ export class VenvManager implements EnvironmentManager {
         }
     }
 
+    /**
+     * Removes the specified Python environment, updates internal collections, and fires change events as needed.
+     */
     async remove(environment: PythonEnvironment): Promise<void> {
         try {
             this.skipWatcherRefresh = true;
@@ -467,6 +472,9 @@ export class VenvManager implements EnvironmentManager {
         await this.loadGlobalEnv(globals);
     }
 
+    /**
+     * Loads and sets the global Python environment from the provided list, resolving if necessary. O(g) where g = globals.length
+     */
     private async loadGlobalEnv(globals: PythonEnvironment[]) {
         this.globalEnv = undefined;
 
@@ -499,30 +507,28 @@ export class VenvManager implements EnvironmentManager {
         }
     }
 
+    /**
+     * Loads and maps Python environments to their corresponding project paths in the workspace. about  O(p × e) where p = projects.len and e = environments.len
+     */
     private async loadEnvMap() {
         const globals = await this.baseManager.getEnvironments('global');
         await this.loadGlobalEnv(globals);
 
         this.fsPathToEnv.clear();
 
         const sorted = sortEnvironments(this.collection);
-        const paths = this.api.getPythonProjects().map((p) => path.normalize(p.uri.fsPath));
+        const projectPaths = this.api.getPythonProjects().map((p) => path.normalize(p.uri.fsPath));
         const events: (() => void)[] = [];
-        for (const p of paths) {
+        // Iterates through all workspace projects
+        for (const p of projectPaths) {
             const env = await getVenvForWorkspace(p);
-
             if (env) {
-                const found = this.findEnvironmentByPath(env, sorted) ?? this.findEnvironmentByPath(env, globals);
-                const previous = this.fsPathToEnv.get(p);
+                // from env path find PythonEnvironment object in the collection.
+                let foundEnv = this.findEnvironmentByPath(env, sorted) ?? this.findEnvironmentByPath(env, globals);
+                const previousEnv = this.fsPathToEnv.get(p);
                 const pw = this.api.getPythonProject(Uri.file(p));
-                if (found) {
-                    this.fsPathToEnv.set(p, found);
-                    if (pw && previous?.envId.id !== found.envId.id) {
-                        events.push(() =>
-                            this._onDidChangeEnvironment.fire({ uri: pw.uri, old: undefined, new: found }),
-                        );
-                    }
-                } else {
+                if (!foundEnv) {
+                    // attempt to resolve
                     const resolved = await resolveVenvPythonEnvironmentPath(
                         env,
                         this.nativeFinder,
@@ -531,39 +537,39 @@ export class VenvManager implements EnvironmentManager {
                         this.baseManager,
                     );
                     if (resolved) {
-                        // If resolved add it to the collection
-                        this.fsPathToEnv.set(p, resolved);
+                        // If resolved; add it to the venvManager collection
                         this.addEnvironment(resolved, false);
-                        if (pw && previous?.envId.id !== resolved.envId.id) {
-                            events.push(() =>
-                                this._onDidChangeEnvironment.fire({ uri: pw.uri, old: undefined, new: resolved }),
-                            );
-                        }
+                        foundEnv = resolved;
                     } else {
                         this.log.error(`Failed to resolve python environment: ${env}`);
+                        return;
                     }
                 }
+                // Given found env, add it to the map and fire the event if needed.
+                this.fsPathToEnv.set(p, foundEnv);
+                if (pw && previousEnv?.envId.id !== foundEnv.envId.id) {
+                    events.push(() =>
+                        this._onDidChangeEnvironment.fire({ uri: pw.uri, old: undefined, new: foundEnv }),
+                    );
+                }
             } else {
-                // There is NO selected venv, then try and choose the venv that is in the workspace.
-                if (sorted.length === 1) {
-                    this.fsPathToEnv.set(p, sorted[0]);
-                } else {
-                    // These are sorted by version and by path length. The assumption is that the user would want to pick
-                    // latest version and the one that is closest to the workspace.
-                    const found = sorted.find((e) => {
-                        const t = this.api.getPythonProject(e.environmentPath)?.uri.fsPath;
-                        return t && path.normalize(t) === p;
-                    });
-                    if (found) {
-                        this.fsPathToEnv.set(p, found);
-                    }
+                // Search through all known environments (e) and check if any are associated with the current project path. If so, add that environment and path in the map.
+                const found = sorted.find((e) => {
+                    const t = this.api.getPythonProject(e.environmentPath)?.uri.fsPath;
+                    return t && path.normalize(t) === p;
+                });
+                if (found) {
+                    this.fsPathToEnv.set(p, found);
                 }
             }
         }
 
         events.forEach((e) => e());
     }
 
+    /**
+     * Finds a PythonEnvironment in the given collection (or all environments) that matches the provided file system path. O(e) where e = environments.len
+     */
     private findEnvironmentByPath(fsPath: string, collection?: PythonEnvironment[]): PythonEnvironment | undefined {
         const normalized = path.normalize(fsPath);
         const envs = collection ?? this.collection;
@@ -573,6 +579,10 @@ export class VenvManager implements EnvironmentManager {
         });
     }
 
+    /**
+     * Returns all Python projects associated with the given environment.
+     * O(p), where p is project.len
+     */
     public getProjectsByEnvironment(environment: PythonEnvironment): PythonProject[] {
         const projects: PythonProject[] = [];
         this.fsPathToEnv.forEach((env, fsPath) => {