Propagate files exclusion to the language server (#1977)

Mikhail Arkhipov · web-flow · commit bd1b74614ce6 · 2018-06-18T14:54:50.000-07:00
* LS symbol providers

* Provide exclude files option to LS

* Document Jedi flag

* Casing

* Add more exclusions

* File exclusion test

* PR feedback

* Comment

* Keep container around

* Test
diff --git a/CONTRIBUTING - PYTHON_ANALYSIS.md b/CONTRIBUTING - PYTHON_ANALYSIS.md
@@ -33,6 +33,7 @@ Visual Studio 2017:
 4. Delete contents of the *analysis* folder in the Python Extension folder
 5. Copy *.dll, *.pdb, *.json fron *Python/BuildOutput/VsCode/raw* to *analysis*
 6. In VS Code set setting *python.downloadCodeAnalysis* to *false*
+7. In VS Code set setting *python.jediEnabled* to *false*
 
 ### Debugging code in Python Extension to VS Code
 Folow regular TypeScript debugging steps
diff --git a/src/client/activation/analysis.ts b/src/client/activation/analysis.ts
@@ -3,15 +3,14 @@
 
 import { inject, injectable } from 'inversify';
 import * as path from 'path';
-import { ExtensionContext, OutputChannel } from 'vscode';
+import { OutputChannel, Uri } from 'vscode';
 import { Disposable, LanguageClient, LanguageClientOptions, ServerOptions } from 'vscode-languageclient';
-import { IApplicationShell, ICommandManager } from '../common/application/types';
+import { IApplicationShell, ICommandManager, IWorkspaceService } from '../common/application/types';
 import { isTestExecution, STANDARD_OUTPUT_CHANNEL } from '../common/constants';
 import { createDeferred, Deferred } from '../common/helpers';
 import { IFileSystem, IPlatformService } from '../common/platform/types';
 import { StopWatch } from '../common/stopWatch';
 import { IConfigurationService, IExtensionContext, IOutputChannel } from '../common/types';
-import { IEnvironmentVariablesProvider } from '../common/variables/types';
 import { IInterpreterService } from '../interpreter/contracts';
 import { IServiceContainer } from '../ioc/types';
 import {
@@ -21,7 +20,7 @@ import {
 } from '../telemetry/constants';
 import { getTelemetryReporter } from '../telemetry/telemetry';
 import { AnalysisEngineDownloader } from './downloader';
-import { InterpreterDataService } from './interpreterDataService';
+import { InterpreterData, InterpreterDataService } from './interpreterDataService';
 import { PlatformData } from './platformData';
 import { IExtensionActivator } from './types';
 
@@ -42,10 +41,13 @@ export class AnalysisExtensionActivator implements IExtensionActivator {
     private readonly interpreterService: IInterpreterService;
     private readonly startupCompleted: Deferred<void>;
     private readonly disposables: Disposable[] = [];
+    private readonly context: IExtensionContext;
+    private readonly workspace: IWorkspaceService;
+    private readonly root: Uri | undefined;
 
     private languageClient: LanguageClient | undefined;
-    private readonly context: ExtensionContext;
     private interpreterHash: string = '';
+    private loadExtensionArgs: {} | undefined;
 
     constructor(@inject(IServiceContainer) private readonly services: IServiceContainer) {
         this.context = this.services.get<IExtensionContext>(IExtensionContext);
@@ -55,32 +57,41 @@ export class AnalysisExtensionActivator implements IExtensionActivator {
         this.fs = this.services.get<IFileSystem>(IFileSystem);
         this.platformData = new PlatformData(services.get<IPlatformService>(IPlatformService), this.fs);
         this.interpreterService = this.services.get<IInterpreterService>(IInterpreterService);
+        this.workspace = this.services.get<IWorkspaceService>(IWorkspaceService);
+
+        // Currently only a single root. Multi-root support is future.
+        this.root = this.workspace && this.workspace.hasWorkspaceFolders
+            ? this.workspace.workspaceFolders![0]!.uri : undefined;
 
         this.startupCompleted = createDeferred<void>();
         const commandManager = this.services.get<ICommandManager>(ICommandManager);
+
         this.disposables.push(commandManager.registerCommand(loadExtensionCommand,
             async (args) => {
                 if (this.languageClient) {
                     await this.startupCompleted.promise;
                     this.languageClient.sendRequest('python/loadExtension', args);
+                } else {
+                    this.loadExtensionArgs = args;
                 }
             }
         ));
     }
 
     public async activate(): Promise<boolean> {
         this.sw.reset();
-        const clientOptions = await this.getAnalysisOptions(this.context);
+        const clientOptions = await this.getAnalysisOptions();
         if (!clientOptions) {
             return false;
         }
         this.disposables.push(this.interpreterService.onDidChangeInterpreter(() => this.restartLanguageServer()));
-        return this.startLanguageServer(this.context, clientOptions);
+        return this.startLanguageServer(clientOptions);
     }
 
     public async deactivate(): Promise<void> {
         if (this.languageClient) {
-            await this.languageClient.stop();
+            // Do not await on this
+            this.languageClient.stop();
         }
         for (const d of this.disposables) {
             d.dispose();
@@ -100,7 +111,7 @@ export class AnalysisExtensionActivator implements IExtensionActivator {
         }
     }
 
-    private async startLanguageServer(context: ExtensionContext, clientOptions: LanguageClientOptions): Promise<boolean> {
+    private async startLanguageServer(clientOptions: LanguageClientOptions): Promise<boolean> {
         // Determine if we are running MSIL/Universal via dotnet or self-contained app.
 
         const reporter = getTelemetryReporter();
@@ -109,22 +120,22 @@ export class AnalysisExtensionActivator implements IExtensionActivator {
         const settings = this.configuration.getSettings();
         if (!settings.downloadCodeAnalysis) {
             // Depends on .NET Runtime or SDK. Typically development-only case.
-            this.languageClient = this.createSimpleLanguageClient(context, clientOptions);
-            await this.startLanguageClient(context);
+            this.languageClient = this.createSimpleLanguageClient(clientOptions);
+            await this.startLanguageClient();
             return true;
         }
 
-        const mscorlib = path.join(context.extensionPath, analysisEngineFolder, 'mscorlib.dll');
+        const mscorlib = path.join(this.context.extensionPath, analysisEngineFolder, 'mscorlib.dll');
         if (!await this.fs.fileExists(mscorlib)) {
             const downloader = new AnalysisEngineDownloader(this.services, analysisEngineFolder);
-            await downloader.downloadAnalysisEngine(context);
+            await downloader.downloadAnalysisEngine(this.context);
             reporter.sendTelemetryEvent(PYTHON_ANALYSIS_ENGINE_DOWNLOADED);
         }
 
-        const serverModule = path.join(context.extensionPath, analysisEngineFolder, this.platformData.getEngineExecutableName());
-        this.languageClient = this.createSelfContainedLanguageClient(context, serverModule, clientOptions);
+        const serverModule = path.join(this.context.extensionPath, analysisEngineFolder, this.platformData.getEngineExecutableName());
+        this.languageClient = this.createSelfContainedLanguageClient(serverModule, clientOptions);
         try {
-            await this.startLanguageClient(context);
+            await this.startLanguageClient();
             return true;
         } catch (ex) {
             this.appShell.showErrorMessage(`Language server failed to start. Error ${ex}`);
@@ -133,30 +144,34 @@ export class AnalysisExtensionActivator implements IExtensionActivator {
         }
     }
 
-    private async startLanguageClient(context: ExtensionContext): Promise<void> {
+    private async startLanguageClient(): Promise<void> {
         this.languageClient!.onReady()
             .then(() => {
                 this.startupCompleted.resolve();
+                if (this.loadExtensionArgs) {
+                    this.languageClient!.sendRequest('python/loadExtension', this.loadExtensionArgs);
+                    this.loadExtensionArgs = undefined;
+                }
             })
             .catch(error => this.startupCompleted.reject(error));
 
-        context.subscriptions.push(this.languageClient!.start());
+        this.context.subscriptions.push(this.languageClient!.start());
         if (isTestExecution()) {
             await this.startupCompleted.promise;
         }
     }
 
-    private createSimpleLanguageClient(context: ExtensionContext, clientOptions: LanguageClientOptions): LanguageClient {
+    private createSimpleLanguageClient(clientOptions: LanguageClientOptions): LanguageClient {
         const commandOptions = { stdio: 'pipe' };
-        const serverModule = path.join(context.extensionPath, analysisEngineFolder, this.platformData.getEngineDllName());
+        const serverModule = path.join(this.context.extensionPath, analysisEngineFolder, this.platformData.getEngineDllName());
         const serverOptions: ServerOptions = {
             run: { command: dotNetCommand, args: [serverModule], options: commandOptions },
             debug: { command: dotNetCommand, args: [serverModule, '--debug'], options: commandOptions }
         };
         return new LanguageClient(PYTHON, languageClientName, serverOptions, clientOptions);
     }
 
-    private createSelfContainedLanguageClient(context: ExtensionContext, serverModule: string, clientOptions: LanguageClientOptions): LanguageClient {
+    private createSelfContainedLanguageClient(serverModule: string, clientOptions: LanguageClientOptions): LanguageClient {
         const options = { stdio: 'pipe' };
         const serverOptions: ServerOptions = {
             run: { command: serverModule, rgs: [], options: options },
@@ -165,19 +180,22 @@ export class AnalysisExtensionActivator implements IExtensionActivator {
         return new LanguageClient(PYTHON, languageClientName, serverOptions, clientOptions);
     }
 
-    private async getAnalysisOptions(context: ExtensionContext): Promise<LanguageClientOptions | undefined> {
+    private async getAnalysisOptions(): Promise<LanguageClientOptions | undefined> {
         // tslint:disable-next-line:no-any
         const properties = new Map<string, any>();
+        let interpreterData: InterpreterData | undefined;
+        let pythonPath = '';
 
-        // Microsoft Python code analysis engine needs full path to the interpreter
-        const interpreterDataService = new InterpreterDataService(context, this.services);
-        const interpreterData = await interpreterDataService.getInterpreterData();
-        if (!interpreterData) {
-            const appShell = this.services.get<IApplicationShell>(IApplicationShell);
-            appShell.showWarningMessage('Unable to determine path to Python interpreter. IntelliSense will be limited.');
+        try {
+            const interpreterDataService = new InterpreterDataService(this.context, this.services);
+            interpreterData = await interpreterDataService.getInterpreterData();
+        } catch (ex) {
+            this.appShell.showErrorMessage('Unable to determine path to the Python interpreter. IntelliSense will be limited.');
         }
 
+        this.interpreterHash = interpreterData ? interpreterData.hash : '';
         if (interpreterData) {
+            pythonPath = path.dirname(interpreterData.path);
             // tslint:disable-next-line:no-string-literal
             properties['InterpreterPath'] = interpreterData.path;
             // tslint:disable-next-line:no-string-literal
@@ -196,25 +214,17 @@ export class AnalysisExtensionActivator implements IExtensionActivator {
         }
 
         // tslint:disable-next-line:no-string-literal
-        properties['DatabasePath'] = path.join(context.extensionPath, analysisEngineFolder);
-
-        const envProvider = this.services.get<IEnvironmentVariablesProvider>(IEnvironmentVariablesProvider);
-        let pythonPath = (await envProvider.getEnvironmentVariables()).PYTHONPATH;
-        this.interpreterHash = interpreterData ? interpreterData.hash : '';
+        properties['DatabasePath'] = path.join(this.context.extensionPath, analysisEngineFolder);
 
         // Make sure paths do not contain multiple slashes so file URIs
         // in VS Code (Node.js) and in the language server (.NET) match.
         // Note: for the language server paths separator is always ;
         searchPaths = searchPaths.split(path.delimiter).map(p => path.normalize(p)).join(';');
-        pythonPath = pythonPath ? path.normalize(pythonPath) : '';
-
         // tslint:disable-next-line:no-string-literal
         properties['SearchPaths'] = `${searchPaths};${pythonPath}`;
-        const selector: string[] = [PYTHON];
 
-        // const searchExcludes = workspace.getConfiguration('search').get('exclude', null);
-        // const filesExcludes = workspace.getConfiguration('files').get('exclude', null);
-        // const watcherExcludes = workspace.getConfiguration('files').get('watcherExclude', null);
+        const selector = [{ language: PYTHON, scheme: 'file' }];
+        const excludeFiles = this.getExcludedFiles();
 
         // Options to control the language client
         return {
@@ -236,8 +246,38 @@ export class AnalysisExtensionActivator implements IExtensionActivator {
                     maxDocumentationTextLength: 0
                 },
                 asyncStartup: true,
+                excludeFiles: excludeFiles,
                 testEnvironment: isTestExecution()
             }
         };
     }
+
+    private getExcludedFiles(): string[] {
+        const list: string[] = ['**/Lib/**', '**/site-packages/**'];
+        this.getVsCodeExcludeSection('search.exclude', list);
+        this.getVsCodeExcludeSection('files.exclude', list);
+        this.getVsCodeExcludeSection('files.watcherExclude', list);
+        this.getPythonExcludeSection('linting.ignorePatterns', list);
+        this.getPythonExcludeSection('workspaceSymbols.exclusionPattern', list);
+        return list;
+    }
+
+    private getVsCodeExcludeSection(setting: string, list: string[]): void {
+        const states = this.workspace.getConfiguration(setting, this.root);
+        if (states) {
+            Object.keys(states)
+                .filter(k => (k.indexOf('*') >= 0 || k.indexOf('/') >= 0) && states[k])
+                .forEach(p => list.push(p));
+        }
+    }
+
+    private getPythonExcludeSection(setting: string, list: string[]): void {
+        const pythonSettings = this.configuration.getSettings(this.root);
+        const paths = pythonSettings && pythonSettings.linting ? pythonSettings.linting.ignorePatterns : undefined;
+        if (paths && Array.isArray(paths)) {
+            paths
+                .filter(p => p && p.length > 0)
+                .forEach(p => list.push(p));
+        }
+    }
 }
diff --git a/src/client/activation/downloader.ts b/src/client/activation/downloader.ts
@@ -5,11 +5,11 @@ import * as fileSystem from 'fs';
 import * as path from 'path';
 import * as request from 'request';
 import * as requestProgress from 'request-progress';
-import { ExtensionContext, OutputChannel, ProgressLocation, window } from 'vscode';
+import { OutputChannel, ProgressLocation, window } from 'vscode';
 import { STANDARD_OUTPUT_CHANNEL } from '../common/constants';
 import { createDeferred, createTemporaryFile } from '../common/helpers';
 import { IFileSystem, IPlatformService } from '../common/platform/types';
-import { IOutputChannel } from '../common/types';
+import { IExtensionContext, IOutputChannel } from '../common/types';
 import { IServiceContainer } from '../ioc/types';
 import { HashVerifier } from './hashVerifier';
 import { PlatformData } from './platformData';
@@ -35,7 +35,7 @@ export class AnalysisEngineDownloader {
         this.platformData = new PlatformData(this.platform, this.fs);
     }
 
-    public async downloadAnalysisEngine(context: ExtensionContext): Promise<void> {
+    public async downloadAnalysisEngine(context: IExtensionContext): Promise<void> {
         const platformString = await this.platformData.getPlatformName();
         const enginePackageFileName = `${downloadBaseFileName}-${platformString}.${downloadVersion}${downloadFileExtension}`;
 
diff --git a/src/test/activation/excludeFiles.ptvs.test.ts b/src/test/activation/excludeFiles.ptvs.test.ts
@@ -0,0 +1,95 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+import * as assert from 'assert';
+import { Container } from 'inversify';
+import * as path from 'path';
+import { commands, ConfigurationTarget, languages, Position, TextDocument, window, workspace } from 'vscode';
+import { ConfigurationService } from '../../client/common/configuration/service';
+import '../../client/common/extensions';
+import { IConfigurationService } from '../../client/common/types';
+import { activated } from '../../client/extension';
+import { ServiceContainer } from '../../client/ioc/container';
+import { ServiceManager } from '../../client/ioc/serviceManager';
+import { IServiceContainer, IServiceManager } from '../../client/ioc/types';
+import { IsAnalysisEngineTest } from '../constants';
+import { closeActiveWindows } from '../initialize';
+
+const wksPath = path.join(__dirname, '..', '..', '..', 'src', 'test', 'pythonFiles', 'exclusions');
+const fileOne = path.join(wksPath, 'one.py');
+
+// tslint:disable-next-line:max-func-body-length
+suite('Exclude files (Analysis Engine)', () => {
+    let textDocument: TextDocument;
+    let serviceManager: IServiceManager;
+    let serviceContainer: IServiceContainer;
+    let configService: IConfigurationService;
+
+    suiteSetup(async function () {
+        if (!IsAnalysisEngineTest()) {
+            // tslint:disable-next-line:no-invalid-this
+            this.skip();
+        }
+    });
+    setup(async () => {
+        const cont = new Container();
+        serviceContainer = new ServiceContainer(cont);
+        serviceManager = new ServiceManager(cont);
+
+        serviceManager.addSingleton<IConfigurationService>(IConfigurationService, ConfigurationService);
+        configService = serviceManager.get<IConfigurationService>(IConfigurationService);
+     });
+    suiteTeardown(closeActiveWindows);
+    teardown(closeActiveWindows);
+
+    async function openFile(file: string): Promise<void> {
+        textDocument = await workspace.openTextDocument(file);
+        await activated;
+        await window.showTextDocument(textDocument);
+        // Make sure LS completes file loading and analysis.
+        // In test mode it awaits for the completion before trying
+        // to fetch data for completion, hover.etc.
+        await commands.executeCommand('vscode.executeCompletionItemProvider', textDocument.uri, new Position(0, 0));
+    }
+
+    async function setSetting(name: string, value: {} | undefined): Promise<void> {
+        await configService.updateSettingAsync(name, value, undefined, ConfigurationTarget.Global);
+    }
+
+    test('Default exclusions', async () => {
+        await openFile(fileOne);
+        const diag = languages.getDiagnostics();
+
+        const main = diag.filter(d => d[0].fsPath.indexOf('one.py') >= 0);
+        assert.equal(main.length > 0, true);
+
+        const subdir = diag.filter(d => d[0].fsPath.indexOf('three.py') >= 0);
+        assert.equal(subdir.length > 0, true);
+
+        const node_modules = diag.filter(d => d[0].fsPath.indexOf('node.py') >= 0);
+        assert.equal(node_modules.length, 0);
+
+        const lib = diag.filter(d => d[0].fsPath.indexOf('fileLib.py') >= 0);
+        assert.equal(lib.length, 0);
+
+        const sitePackages = diag.filter(d => d[0].fsPath.indexOf('sitePackages.py') >= 0);
+        assert.equal(sitePackages.length, 0);
+    });
+    test('Exclude subfolder', async () => {
+        await setSetting('linting.ignorePatterns', ['**/dir1/**']);
+
+        await openFile(fileOne);
+        const diag = languages.getDiagnostics();
+
+        const main = diag.filter(d => d[0].fsPath.indexOf('one.py') >= 0);
+        assert.equal(main.length > 0, true);
+
+        const subdir1 = diag.filter(d => d[0].fsPath.indexOf('dir1file.py') >= 0);
+        assert.equal(subdir1.length, 0);
+
+        const subdir2 = diag.filter(d => d[0].fsPath.indexOf('dir2file.py') >= 0);
+        assert.equal(subdir2.length, 0);
+
+        await setSetting('linting.ignorePatterns', undefined);
+    });
+});
diff --git a/src/test/pythonFiles/exclusions/Lib/fileLib.py b/src/test/pythonFiles/exclusions/Lib/fileLib.py
@@ -0,0 +1 @@
+ a
diff --git a/src/test/pythonFiles/exclusions/Lib/site-packages/sitePackages.py b/src/test/pythonFiles/exclusions/Lib/site-packages/sitePackages.py
@@ -0,0 +1 @@
+ b
diff --git a/src/test/pythonFiles/exclusions/dir1/dir1file.py b/src/test/pythonFiles/exclusions/dir1/dir1file.py
@@ -0,0 +1 @@
+ for 
diff --git a/src/test/pythonFiles/exclusions/dir1/dir2/dir2file.py b/src/test/pythonFiles/exclusions/dir1/dir2/dir2file.py
diff --git a/src/test/pythonFiles/exclusions/one.py b/src/test/pythonFiles/exclusions/one.py
