Code reviews

Author: Kartik Raj

src/client/pythonEnvironments/common/environmentIdentifier.ts

@@ -4,7 +4,6 @@
 import * as fsapi from 'fs-extra';
 import * as path from 'path';
 import { traceWarning } from '../../common/logger';
-import { Resource } from '../../common/types';
 import { createDeferred } from '../../common/utils/async';
 import { getEnvironmentVariable } from '../../common/utils/platform';
 import { EnvironmentType } from '../info';
@@ -17,6 +16,10 @@ function pathExists(absPath: string): Promise<boolean> {
     return deferred.promise;
 }
 
+function readFile(filePath: string): Promise<string> {
+    return fsapi.readFile(filePath, 'utf-8');
+}
+
 /**
  * Checks if the given interpreter path belongs to a conda environment. Using
  * known folder layout, and presence of 'conda-meta' directory.
@@ -128,23 +131,22 @@ async function isWindowsStoreEnvironment(interpreterPath: string): Promise<boole
  * @param cwd the directory to look into
  * @param lookIntoParentDirectories set to true if we should also search for Pipfile in parent directory
  */
-function checkIfPipFileExists(cwd: string, lookIntoParentDirectories: boolean): boolean {
-    const pipFileName = process.env.PIPENV_PIPFILE;
+async function checkIfPipFileExists(cwd: string, lookIntoParentDirectories: boolean): Promise<boolean> {
+    const pipFileName = getEnvironmentVariable('PIPENV_PIPFILE') || 'Pipfile';
     let depthToSearch = 1;
     if (lookIntoParentDirectories) {
         // PIPENV_MAX_DEPTH tells pipenv the maximum number of directories to recursively search for
         // a Pipfile, defaults to 3: https://pipenv.pypa.io/en/latest/advanced/#pipenv.environments.PIPENV_MAX_DEPTH
-        if (process.env.PIPENV_MAX_DEPTH) {
-            depthToSearch = +process.env.PIPENV_MAX_DEPTH;
+        const maxDepth = getEnvironmentVariable('PIPENV_MAX_DEPTH');
+        if (maxDepth) {
+            depthToSearch = +maxDepth;
         } else {
             depthToSearch = 3;
         }
     }
     while (depthToSearch > 0 && cwd === path.dirname(cwd)) {
-        if (typeof pipFileName === 'string' && pathExists(path.join(cwd, pipFileName))) {
-            return true;
-        }
-        if (pathExists(path.join(cwd, 'Pipfile'))) {
+        // eslint-disable-next-line no-await-in-loop
+        if (await pathExists(path.join(cwd, pipFileName))) {
             return true;
         }
         cwd = path.dirname(cwd);
@@ -157,74 +159,72 @@ function checkIfPipFileExists(cwd: string, lookIntoParentDirectories: boolean):
  * Returns true if interpreter path belongs to a pipenv environment which is located inside a project, false otherwise.
  * @param interpreterPath Absolute path to any python interpreter.
  */
-function isLocalPipenvEnvironment(interpreterPath: string): boolean {
+async function isLocalPipenvEnvironment(interpreterPath: string): Promise<boolean> {
     // Local pipenv environments are created by setting PIPENV_VENV_IN_PROJECT to 1, which always names the environment
     // folder '.venv': https://pipenv.pypa.io/en/latest/advanced/#pipenv.environments.PIPENV_VENV_IN_PROJECT
     // This is the layout we wish to verify.
     // project
     // |__ Pipfile  <--- check if Pipfile exists here
-    // |__ .venv
+    // |__ .venv    <--- check if name of the folder is '.venv'
     //     |__ Scripts/bin
     //         |__ python  <--- interpreterPath
     const venvFolderName = path.basename(path.dirname(path.dirname(interpreterPath)));
     if (venvFolderName !== '.venv') {
         return false;
     }
     const directoryWhereVenvResides = path.dirname(path.dirname(path.dirname(interpreterPath)));
-    if (checkIfPipFileExists(directoryWhereVenvResides, false)) {
+    if (await checkIfPipFileExists(directoryWhereVenvResides, false)) {
         // The directory must contain a Pipfile
         return true;
     }
     return false;
 }
 
 /**
- * Returns true if interpreter path belongs to a global pipenv environment created for a particular project,
- * false otherwise.
+ * Returns true if interpreter path belongs to a global pipenv environment, false otherwise.
  * @param interpreterPath Absolute path to any python interpreter.
- * @param project Absolute path to the project.
  */
-async function isGlobalPipenvEnvironmentRelatedToProject(interpreterPath: string, project: string): Promise<boolean> {
+async function isGlobalPipenvEnvironment(interpreterPath: string): Promise<boolean> {
+    // Global pipenv environments have a .project file with the absolute path to the project.
+    // Also, the name of the project is used as a prefix in the environment folder.
+    // <Environment folder>  <--- check if the name of the project is used as a prefix
+    // |__ .project  <--- check if .project exists here
+    // |__ Scripts/bin
+    //     |__ python  <--- interpreterPath
+    const dotProjectFile = path.join(path.dirname(path.dirname(interpreterPath)), '.project');
+    if (!await pathExists(dotProjectFile)) {
+        return false;
+    }
+
+    const project = await readFile(dotProjectFile);
+    if (!await pathExists(project)) {
+        return false;
+    }
+
+    // The name of the project is used as a prefix in the environment folder.
+    if (interpreterPath.indexOf(`${path.sep}${path.basename(project)}-`) === -1) {
+        return false;
+    }
+
     // PIPENV_NO_INHERIT is used to tell pipenv not to look for Pipfile in parent directories
     // https://pipenv.pypa.io/en/latest/advanced/#pipenv.environments.PIPENV_NO_INHERIT
-    if (!checkIfPipFileExists(project, !process.env.PIPENV_NO_INHERIT)) {
+    if (!checkIfPipFileExists(project, !getEnvironmentVariable('PIPENV_NO_INHERIT'))) {
         return false;
     }
 
-    /**
-     * Global pipenv environments can be stored in 3 possible locations:
-     * * WORKON_HOME environment variable: https://pipenv-fork.readthedocs.io/en/latest/advanced.html#custom-virtual-environment-location
-     * * ~/.virtualenvs - for windows
-     * * ~/.local/share/virtualenvs
-     *
-     * The name of the project is used as a prefix in the virtual env. We're assuming it's unique enough
-     * detail so we don't have further ensure that the environment exists within these locations.
-     */
-    if (interpreterPath.indexOf(`${path.sep}${path.basename(project)}-`) !== -1) {
-        // Note it's still possible that this virtual environment belongs to another project with the same name,
-        // and not this project. But the type of environment would still be pipenv, so we need not care.
-        return true;
-    }
-    return false;
+    return true;
 }
 
 /**
  * Checks if the given interpreter path belongs to a pipenv environment, by locating the Pipfile which was used to
  * create the environment.
  * @param interpreterPath: Absolute path to any python interpreter.
- * @param resource Uri of the project
  */
-async function isPipenvEnvironment(interpreterPath: string, resource: Resource) {
-    if (isLocalPipenvEnvironment(interpreterPath)) {
+async function isPipenvEnvironment(interpreterPath: string) {
+    if (await isLocalPipenvEnvironment(interpreterPath)) {
         return true;
     }
-    /**
-     * Otherwise it's a globally stored environment. Unfortunately, we cannot tell if a global environment is a pipenv
-     * environment just by looking around the interpreter path. Also, because we cannot get project path from the pipenv
-     * environment, we cannot locate the Pipfile which was used to create the environment. The best we can do is guess
-     * the project it belongs to and see if everything else adds up. If it does, voila, we have a pipenv environment.
-     */
-    if (resource && isGlobalPipenvEnvironmentRelatedToProject(interpreterPath, resource.fsPath)) {
+    if (await isGlobalPipenvEnvironment(interpreterPath)) {
         return true;
     }
     return false;
@@ -253,7 +253,7 @@ async function isPipenvEnvironment(interpreterPath: string, resource: Resource)
  *
  * Last category is globally installed python, or system python.
  */
-export async function identifyEnvironment(interpreterPath: string, resource?: Resource): Promise<EnvironmentType> {
+export async function identifyEnvironment(interpreterPath: string): Promise<EnvironmentType> {
     if (await isCondaEnvironment(interpreterPath)) {
         return EnvironmentType.Conda;
     }
@@ -262,7 +262,7 @@ export async function identifyEnvironment(interpreterPath: string, resource?: Re
         return EnvironmentType.WindowsStore;
     }
 
-    if (await isPipenvEnvironment(interpreterPath, resource)) {
+    if (await isPipenvEnvironment(interpreterPath)) {
         return EnvironmentType.Pipenv;
     }