Simplify interpreter filter (#12792)

src/client/interpreter/locators/types.ts

@@ -4,7 +4,6 @@
 'use strict';
 
 import { Uri } from 'vscode';
-import { PythonInterpreter } from '../../pythonEnvironments/info';
 
 export const IPythonInPathCommandProvider = Symbol('IPythonInPathCommandProvider');
 export interface IPythonInPathCommandProvider {
@@ -63,7 +62,3 @@ export interface IWindowsStoreInterpreter {
      */
     isHiddenInterpreter(pythonPath: string): boolean;
 }
-
-export interface IInterpreterFilter {
-    isHiddenInterpreter(interpreter: PythonInterpreter): boolean;
-}

src/client/pythonEnvironments/discovery/locators/index.ts

@@ -17,10 +17,9 @@ import {
     WINDOWS_REGISTRY_SERVICE,
     WORKSPACE_VIRTUAL_ENV_SERVICE
 } from '../../../interpreter/contracts';
-import { IInterpreterFilter } from '../../../interpreter/locators/types';
 import { IServiceContainer } from '../../../ioc/types';
 import { PythonInterpreter } from '../../info';
-import { InterpreterFilter } from './services/interpreterFilter';
+import { isHiddenInterpreter } from './services/interpreterFilter';
 import { GetInterpreterLocatorOptions } from './types';
 
 // tslint:disable-next-line:no-require-imports no-var-requires
@@ -38,10 +37,7 @@ export class PythonInterpreterLocatorService implements IInterpreterLocatorServi
     private readonly interpreterLocatorHelper: IInterpreterLocatorHelper;
     private readonly _hasInterpreters: Deferred<boolean>;
 
-    constructor(
-        @inject(IServiceContainer) private serviceContainer: IServiceContainer,
-        @inject(InterpreterFilter) private readonly interpreterFilter: IInterpreterFilter
-    ) {
+    constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer) {
         this._hasInterpreters = createDeferred<boolean>();
         serviceContainer.get<Disposable[]>(IDisposableRegistry).push(this);
         this.platform = serviceContainer.get<IPlatformService>(IPlatformService);
@@ -96,7 +92,7 @@ export class PythonInterpreterLocatorService implements IInterpreterLocatorServi
         const items = flatten(listOfInterpreters)
             .filter((item) => !!item)
             .map((item) => item!)
-            .filter((item) => !this.interpreterFilter.isHiddenInterpreter(item));
+            .filter((item) => !isHiddenInterpreter(item));
         this._hasInterpreters.resolve(items.length > 0);
         return this.interpreterLocatorHelper.mergeInterpreters(items);
     }

src/client/pythonEnvironments/discovery/locators/services/interpreterFilter.ts

@@ -3,15 +3,10 @@
 
 'use strict';
 
-import { inject, injectable } from 'inversify';
-import { IInterpreterFilter, IWindowsStoreInterpreter } from '../../../../interpreter/locators/types';
 import { PythonInterpreter } from '../../../info';
-import { WindowsStoreInterpreter } from './windowsStoreInterpreter';
+import { isRestrictedWindowsStoreInterpreterPath } from './windowsStoreInterpreter';
 
-@injectable()
-export class InterpreterFilter implements IInterpreterFilter {
-    constructor(@inject(WindowsStoreInterpreter) private readonly windowsStoreInterpreter: IWindowsStoreInterpreter) {}
-    public isHiddenInterpreter(interpreter: PythonInterpreter): boolean {
-        return this.windowsStoreInterpreter.isHiddenInterpreter(interpreter.path);
-    }
+export function isHiddenInterpreter(interpreter: PythonInterpreter): boolean {
+    // Any set of rules to hide interpreters should go here
+    return isRestrictedWindowsStoreInterpreterPath(interpreter.path);
 }

src/client/pythonEnvironments/discovery/locators/services/windowsStoreInterpreter.ts

@@ -12,20 +12,39 @@ import { IPersistentStateFactory } from '../../../../common/types';
 import { IInterpreterHashProvider, IWindowsStoreInterpreter } from '../../../../interpreter/locators/types';
 import { IServiceContainer } from '../../../../ioc/types';
 
+/**
+ * When using Windows Store interpreter the path that should be used is under
+ * %USERPROFILE%\AppData\Local\Microsoft\WindowsApps\python*.exe. The python.exe path
+ * under ProgramFiles\WindowsApps should not be used at all. Execute permissions on
+ * that instance of the store interpreter are restricted to system. Paths under
+ * %USERPROFILE%\AppData\Local\Microsoft\WindowsApps\PythonSoftwareFoundation* are also ok
+ * to use. But currently this results in duplicate entries.
+ *
+ * @param {string} pythonPath
+ * @returns {boolean}
+ */
+export function isRestrictedWindowsStoreInterpreterPath(pythonPath: string): boolean {
+    const pythonPathToCompare = pythonPath.toUpperCase().replace(/\//g, '\\');
+    return (
+        pythonPathToCompare.includes('\\Program Files\\WindowsApps\\'.toUpperCase()) ||
+        pythonPathToCompare.includes('\\Microsoft\\WindowsApps\\PythonSoftwareFoundation'.toUpperCase())
+    );
+}
+
 /**
  * The default location of Windows apps are `%ProgramFiles%\WindowsApps`.
  * (https://www.samlogic.net/articles/windows-8-windowsapps-folder.htm)
  * When users access Python interpreter it is installed in `<users local folder>\Microsoft\WindowsApps`
  * Based on our testing this is where Python interpreters installed from Windows Store is always installed.
- * (unforutnately couldn't find any documentation on this).
+ * (unfortunately couldn't find any documentation on this).
  * What we've identified is the fact that:
  * - The Python interpreter in Microsoft\WIndowsApps\python.exe is a symbolic link to files located in:
  *  - Program Files\WindowsApps\ & Microsoft\WindowsApps\PythonSoftwareFoundation
  * - I.e. they all point to the same place.
- * However when the user lauches the executabale, its located in `Microsoft\WindowsAapps\python.exe`
+ * However when the user launches the executable, its located in `Microsoft\WindowsApps\python.exe`
  * Hence for all intensive purposes that's the main executable, that's what the user uses.
  * As a result:
- * - We'll only display what the user has access to, that being `Microsoft\WindowsAapps\python.exe`
+ * - We'll only display what the user has access to, that being `Microsoft\WindowsApps\python.exe`
  * - Others are hidden.
  *
  * Details can be found here (original issue https://github.com/microsoft/vscode-python/issues/5926).
@@ -66,11 +85,7 @@ export class WindowsStoreInterpreter implements IWindowsStoreInterpreter, IInter
      * @memberof IInterpreterHelper
      */
     public isHiddenInterpreter(pythonPath: string): boolean {
-        const pythonPathToCompare = pythonPath.toUpperCase().replace(/\//g, '\\');
-        return (
-            pythonPathToCompare.includes('\\Program Files\\WindowsApps\\'.toUpperCase()) ||
-            pythonPathToCompare.includes('\\Microsoft\\WindowsApps\\PythonSoftwareFoundation'.toUpperCase())
-        );
+        return isRestrictedWindowsStoreInterpreterPath(pythonPath);
     }
     /**
      * Gets the hash of the Python interpreter (installed from the windows store).

src/client/pythonEnvironments/legacyIOC.ts

@@ -35,7 +35,6 @@ import {
 } from './discovery/locators/services/globalVirtualEnvService';
 import { InterpreterHashProvider } from './discovery/locators/services/hashProvider';
 import { InterpeterHashProviderFactory } from './discovery/locators/services/hashProviderFactory';
-import { InterpreterFilter } from './discovery/locators/services/interpreterFilter';
 import { InterpreterWatcherBuilder } from './discovery/locators/services/interpreterWatcherBuilder';
 import { KnownPathsService, KnownSearchPathsForInterpreters } from './discovery/locators/services/KnownPathsService';
 import { PipEnvService } from './discovery/locators/services/pipEnvService';
@@ -114,7 +113,6 @@ export function registerForIOC(serviceManager: IServiceManager) {
         InterpeterHashProviderFactory,
         InterpeterHashProviderFactory
     );
-    serviceManager.addSingleton<InterpreterFilter>(InterpreterFilter, InterpreterFilter);
     serviceManager.addSingleton<IVirtualEnvironmentsSearchPathProvider>(
         IVirtualEnvironmentsSearchPathProvider,
         GlobalVirtualEnvironmentsSearchPathProvider,

src/test/pythonEnvironments/discovery/locators/index.unit.test.ts

@@ -25,7 +25,6 @@ import {
     WINDOWS_REGISTRY_SERVICE,
     WORKSPACE_VIRTUAL_ENV_SERVICE
 } from '../../../../client/interpreter/contracts';
-import { IInterpreterFilter } from '../../../../client/interpreter/locators/types';
 import { IServiceContainer } from '../../../../client/ioc/types';
 import { PythonInterpreterLocatorService } from '../../../../client/pythonEnvironments/discovery/locators';
 import { InterpreterType, PythonInterpreter } from '../../../../client/pythonEnvironments/info';
@@ -35,19 +34,17 @@ suite('Interpreters - Locators Index', () => {
     let platformSvc: TypeMoq.IMock<IPlatformService>;
     let helper: TypeMoq.IMock<IInterpreterLocatorHelper>;
     let locator: IInterpreterLocatorService;
-    let filter: TypeMoq.IMock<IInterpreterFilter>;
     setup(() => {
         serviceContainer = TypeMoq.Mock.ofType<IServiceContainer>();
         platformSvc = TypeMoq.Mock.ofType<IPlatformService>();
         helper = TypeMoq.Mock.ofType<IInterpreterLocatorHelper>();
-        filter = TypeMoq.Mock.ofType<IInterpreterFilter>();
         serviceContainer.setup((c) => c.get(TypeMoq.It.isValue(IDisposableRegistry))).returns(() => []);
         serviceContainer.setup((c) => c.get(TypeMoq.It.isValue(IPlatformService))).returns(() => platformSvc.object);
         serviceContainer
             .setup((c) => c.get(TypeMoq.It.isValue(IInterpreterLocatorHelper)))
             .returns(() => helper.object);
 
-        locator = new PythonInterpreterLocatorService(serviceContainer.object, filter.object);
+        locator = new PythonInterpreterLocatorService(serviceContainer.object);
     });
     [undefined, Uri.file('Something')].forEach((resource) => {
         getNamesAndValues<OSType>(OSType).forEach((osType) => {

src/test/pythonEnvironments/discovery/locators/interpreterFilter.unit.test.ts

@@ -0,0 +1,58 @@
+import { expect } from 'chai';
+import { isHiddenInterpreter } from '../../../../client/pythonEnvironments/discovery/locators/services/interpreterFilter';
+import { InterpreterType, PythonInterpreter } from '../../../../client/pythonEnvironments/info';
+
+// tslint:disable:no-unused-expression
+
+suite('Interpreters - Filter', () => {
+    const doNotHideThesePaths = [
+        'python',
+        'python.exe',
+        'python2',
+        'python2.exe',
+        'python38',
+        'python3.8.exe',
+        'C:\\Users\\SomeUser\\AppData\\Local\\Microsoft\\WindowsApps\\python.exe',
+        '%USERPROFILE%\\AppData\\Local\\Microsoft\\WindowsApps\\python.exe',
+        '%LOCALAPPDATA%\\Microsoft\\WindowsApps\\python.exe'
+    ];
+    const hideThesePaths = [
+        '%USERPROFILE%\\AppData\\Local\\Microsoft\\WindowsApps\\PythonSoftwareFoundation.Python.3.8_qbz5n2kfra8p0\\python.exe',
+        'C:\\Users\\SomeUser\\AppData\\Local\\Microsoft\\WindowsApps\\PythonSoftwareFoundation.Python.3.8_qbz5n2kfra8p0\\python.exe',
+        '%USERPROFILE%\\AppData\\Local\\Microsoft\\WindowsApps\\PythonSoftwareFoundation\\python.exe',
+        'C:\\Users\\SomeUser\\AppData\\Local\\Microsoft\\WindowsApps\\PythonSoftwareFoundation\\python.exe',
+        '%LOCALAPPDATA%\\Microsoft\\WindowsApps\\PythonSoftwareFoundation.Python.3.8_qbz5n2kfra8p0\\python.exe',
+        '%LOCALAPPDATA%\\Microsoft\\WindowsApps\\PythonSoftwareFoundation\\python.exe',
+        'C:\\Program Files\\WindowsApps\\python.exe',
+        'C:\\Program Files\\WindowsApps\\PythonSoftwareFoundation.Python.3.8_qbz5n2kfra8p0\\python.exe',
+        'C:\\Program Files\\WindowsApps\\PythonSoftwareFoundation\\python.exe'
+    ];
+
+    function getInterpreterFromPath(interpreterPath: string): PythonInterpreter {
+        return {
+            path: interpreterPath,
+            sysVersion: '',
+            sysPrefix: '',
+            architecture: 1,
+            companyDisplayName: '',
+            displayName: 'python',
+            type: InterpreterType.WindowsStore,
+            envName: '',
+            envPath: '',
+            cachedEntry: false
+        };
+    }
+
+    doNotHideThesePaths.forEach((interpreterPath) => {
+        test(`Interpreter path should NOT be hidden - ${interpreterPath}`, () => {
+            const interpreter: PythonInterpreter = getInterpreterFromPath(interpreterPath);
+            expect(isHiddenInterpreter(interpreter), `${interpreterPath} should NOT be treated as hidden.`).to.be.false;
+        });
+    });
+    hideThesePaths.forEach((interpreterPath) => {
+        test(`Interpreter path should be hidden - ${interpreterPath}`, () => {
+            const interpreter: PythonInterpreter = getInterpreterFromPath(interpreterPath);
+            expect(isHiddenInterpreter(interpreter), `${interpreterPath} should be treated as hidden.`).to.be.true;
+        });
+    });
+});