use new env variables service in jedi service

DonJayamanne · DonJayamanne · commit e6c947288cb6 · 2017-12-20T13:06:59.000-08:00
diff --git a/src/client/common/decorators.ts b/src/client/common/decorators.ts
@@ -0,0 +1,47 @@
+import * as _ from 'lodash';
+
+type AsyncVoidAction = (...params: {}[]) => Promise<void>;
+type VoidAction = (...params: {}[]) => void;
+
+/**
+ * Debounces a function execution. Function must return either a void or a promise that resolves to a void.
+ * @export
+ * @param {number} [wait] Wait time.
+ * @returns void
+ */
+export function debounce(wait?: number) {
+    // tslint:disable-next-line:no-any no-function-expression
+    return function (_target: any, _propertyName: string, descriptor: TypedPropertyDescriptor<VoidAction> | TypedPropertyDescriptor<AsyncVoidAction>) {
+        const originalMethod = descriptor.value!;
+        // tslint:disable-next-line:no-invalid-this no-any
+        (descriptor as any).value = _.debounce(function () { return originalMethod.apply(this, arguments); }, wait);
+    };
+}
+
+/**
+ * Swallows exceptions thrown by a function. Function must return either a void or a promise that resolves to a void.
+ * @export
+ * @param {string} [scopeName] Scope for the error message to be logged along with the error.
+ * @returns void
+ */
+export function swallowExceptions(scopeName: string) {
+    // tslint:disable-next-line:no-any no-function-expression
+    return function (_target: any, propertyName: string, descriptor: TypedPropertyDescriptor<VoidAction> | TypedPropertyDescriptor<AsyncVoidAction>) {
+        const originalMethod = descriptor.value!;
+        const errorMessage = `Python Extension (Error in ${scopeName}, method:${propertyName}):`;
+        // tslint:disable-next-line:no-any no-function-expression
+        descriptor.value = function (...args: any[]) {
+            try {
+                // tslint:disable-next-line:no-invalid-this no-use-before-declare no-unsafe-any
+                const result = originalMethod.apply(this, args);
+
+                // If method being wrapped returns a promise then wait and swallow errors.
+                if (result && typeof result.then === 'function' && typeof result.catch === 'function') {
+                    return (result as Promise<void>).catch(error => console.error(errorMessage, error));
+                }
+            } catch (error) {
+                console.error(errorMessage, error);
+            }
+        };
+    };
+}
diff --git a/src/client/common/variables/environmentVariablesProvider.ts b/src/client/common/variables/environmentVariablesProvider.ts
@@ -2,7 +2,7 @@
 // Licensed under the MIT License.
 
 import { inject, injectable } from 'inversify';
-import { Disposable, FileSystemWatcher, Uri, workspace } from 'vscode';
+import { Disposable, Event, EventEmitter, FileSystemWatcher, Uri, workspace } from 'vscode';
 import { PythonSettings } from '../configSettings';
 import { NON_WINDOWS_PATH_VARIABLE_NAME, WINDOWS_PATH_VARIABLE_NAME } from '../platform/constants';
 import { ICurrentProcess, IDisposableRegistry, IsWindows } from '../types';
@@ -13,22 +13,29 @@ export class EnvironmentVariablesProvider implements IEnvironmentVariablesProvid
     private cache = new Map<string, EnvironmentVariables>();
     private fileWatchers = new Map<string, FileSystemWatcher>();
     private disposables: Disposable[] = [];
-
+    private changeEventEmitter: EventEmitter<Uri | undefined>;
     constructor( @inject(IEnvironmentVariablesService) private envVarsService: IEnvironmentVariablesService,
         @inject(IDisposableRegistry) disposableRegistry: Disposable[], @inject(IsWindows) private isWidows: boolean,
         @inject(ICurrentProcess) private process: ICurrentProcess) {
         disposableRegistry.push(this);
+        this.changeEventEmitter = new EventEmitter();
+    }
+
+    public get onDidEnvironmentVariablesChange(): Event<Uri | undefined> {
+        return this.changeEventEmitter.event;
     }
 
     public dispose() {
+        this.changeEventEmitter.dispose();
         this.fileWatchers.forEach(watcher => {
             watcher.dispose();
         });
     }
     public async getEnvironmentVariables(resource?: Uri): Promise<EnvironmentVariables> {
         const settings = PythonSettings.getInstance(resource);
         if (!this.cache.has(settings.envFile)) {
-            this.createFileWatcher(settings.envFile);
+            const workspaceFolderUri = this.getWorkspaceFolderUri(resource);
+            this.createFileWatcher(settings.envFile, workspaceFolderUri);
             let mergedVars = await this.envVarsService.parseFile(settings.envFile);
             if (!mergedVars) {
                 mergedVars = {};
@@ -41,14 +48,25 @@ export class EnvironmentVariablesProvider implements IEnvironmentVariablesProvid
         }
         return this.cache.get(settings.envFile)!;
     }
-    private createFileWatcher(envFile: string) {
+    private getWorkspaceFolderUri(resource?: Uri): Uri | undefined {
+        if (!resource) {
+            return;
+        }
+        const workspaceFolder = workspace.getWorkspaceFolder(resource!);
+        return workspaceFolder ? workspaceFolder.uri : undefined;
+    }
+    private createFileWatcher(envFile: string, workspaceFolderUri?: Uri) {
         if (this.fileWatchers.has(envFile)) {
             return;
         }
         const envFileWatcher = workspace.createFileSystemWatcher(envFile);
         this.fileWatchers.set(envFile, envFileWatcher);
-        this.disposables.push(envFileWatcher.onDidChange(() => this.cache.delete(envFile)));
-        this.disposables.push(envFileWatcher.onDidCreate(() => this.cache.delete(envFile)));
-        this.disposables.push(envFileWatcher.onDidDelete(() => this.cache.delete(envFile)));
+        this.disposables.push(envFileWatcher.onDidChange(() => this.onEnvironmentFileChanged(envFile, workspaceFolderUri)));
+        this.disposables.push(envFileWatcher.onDidCreate(() => this.onEnvironmentFileChanged(envFile, workspaceFolderUri)));
+        this.disposables.push(envFileWatcher.onDidDelete(() => this.onEnvironmentFileChanged(envFile, workspaceFolderUri)));
+    }
+    private onEnvironmentFileChanged(envFile, workspaceFolderUri?: Uri) {
+        this.cache.delete(envFile);
+        this.changeEventEmitter.fire(workspaceFolderUri);
     }
 }
diff --git a/src/client/common/variables/types.ts b/src/client/common/variables/types.ts
@@ -1,7 +1,7 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 
-import { Uri } from 'vscode';
+import { Event, Uri } from 'vscode';
 
 export type EnvironmentVariables = Object & {
     [key: string]: string;
@@ -38,5 +38,6 @@ export interface ISystemVariables {
 export const IEnvironmentVariablesProvider = Symbol('IEnvironmentVariablesProvider');
 
 export interface IEnvironmentVariablesProvider {
+    onDidEnvironmentVariablesChange: Event<Uri | undefined>;
     getEnvironmentVariables(resource?: Uri): Promise<EnvironmentVariables | undefined>;
 }
diff --git a/src/client/providers/jediProxy.ts b/src/client/providers/jediProxy.ts
@@ -7,6 +7,7 @@ import * as path from 'path';
 import * as vscode from 'vscode';
 import { Uri } from 'vscode';
 import { PythonSettings } from '../common/configSettings';
+import { debounce, swallowExceptions } from '../common/decorators';
 import '../common/extensions';
 import { createDeferred, Deferred } from '../common/helpers';
 import { IPythonExecutionFactory } from '../common/process/types';
@@ -129,25 +130,24 @@ export class JediProxy implements vscode.Disposable {
     private proc: child_process.ChildProcess | null;
     private pythonSettings: PythonSettings;
     private cmdId: number = 0;
-    private pythonProcessCWD = '';
     private lastKnownPythonInterpreter: string;
     private previousData = '';
     private commands = new Map<number, IExecutionCommand<ICommandResult>>();
     private commandQueue: number[] = [];
     private spawnRetryAttempts = 0;
-    private lastKnownPythonPath: string;
-    private additionalAutoCopletePaths: string[] = [];
+    private additionalAutoCompletePaths: string[] = [];
     private workspacePath: string;
+    private languageServerStarted: Deferred<void>;
     private initialized: Deferred<void>;
-
-    public constructor(extensionRootDir: string, workspacePath: string, private serviceContainer: IServiceContainer) {
+    private environmentVariablesProvider: IEnvironmentVariablesProvider;
+    public constructor(private extensionRootDir: string, workspacePath: string, private serviceContainer: IServiceContainer) {
         this.workspacePath = workspacePath;
         this.pythonSettings = PythonSettings.getInstance(vscode.Uri.file(workspacePath));
         this.lastKnownPythonInterpreter = this.pythonSettings.pythonPath;
-        this.pythonSettings.on('change', this.onPythonSettingsChanged.bind(this));
-        vscode.workspace.onDidChangeConfiguration(this.onConfigChanged.bind(this));
-        this.onConfigChanged();
-        this.initialize(extensionRootDir);
+        this.pythonSettings.on('change', () => this.pythonSettingsChangeHandler());
+        this.initialized = createDeferred<void>();
+        // tslint:disable-next-line:no-empty
+        this.startLanguageServer().catch(() => { }).then(() => this.initialized.resolve());
     }
 
     private static getProperty<T>(o: object, name: string): T {
@@ -166,15 +166,13 @@ export class JediProxy implements vscode.Disposable {
 
     public async sendCommand<T extends ICommandResult>(cmd: ICommand<T>): Promise<T> {
         await this.initialized.promise;
+        await this.languageServerStarted.promise;
         if (!this.proc) {
             return Promise.reject(new Error('Python proc not initialized'));
         }
         const executionCmd = <IExecutionCommand<T>>cmd;
         const payload = this.createPayload(executionCmd);
         executionCmd.deferred = createDeferred<T>();
-        // if (typeof executionCmd.telemetryEvent === 'string') {
-        //     executionCmd.delays = new telemetryHelper.Delays();
-        // }
         try {
             this.proc.stdin.write(`${JSON.stringify(payload)}\n`);
             this.commands.set(executionCmd.id, executionCmd);
@@ -193,25 +191,43 @@ export class JediProxy implements vscode.Disposable {
     }
 
     // keep track of the directory so we can re-spawn the process.
-    private initialize(dir: string) {
-        this.pythonProcessCWD = dir;
-        this.spawnProcess(path.join(dir, 'pythonFiles'))
+    private initialize() {
+        this.spawnProcess(path.join(this.extensionRootDir, 'pythonFiles'))
             .catch(ex => {
-                if (this.initialized) {
-                    this.initialized.reject(ex);
+                if (this.languageServerStarted) {
+                    this.languageServerStarted.reject(ex);
                 }
                 this.handleError('spawnProcess', ex);
             });
     }
-
-    // Check if settings changes.
-    private onPythonSettingsChanged() {
+    @swallowExceptions('JediProxy')
+    private async pythonSettingsChangeHandler() {
         if (this.lastKnownPythonInterpreter === this.pythonSettings.pythonPath) {
             return;
         }
+        this.lastKnownPythonInterpreter = this.pythonSettings.pythonPath;
+        this.additionalAutoCompletePaths = await this.buildAutoCompletePaths();
+        this.restartLanguageServer();
+    }
+    @debounce(1500)
+    @swallowExceptions('JediProxy')
+    private async environmentVariablesChangeHandler() {
+        const newAutoComletePaths = await this.buildAutoCompletePaths();
+        if (this.additionalAutoCompletePaths.join(',') !== newAutoComletePaths.join(',')) {
+            this.additionalAutoCompletePaths = newAutoComletePaths;
+            this.restartLanguageServer();
+        }
+    }
+    @swallowExceptions('JediProxy')
+    private async startLanguageServer() {
+        const newAutoComletePaths = await this.buildAutoCompletePaths();
+        this.additionalAutoCompletePaths = newAutoComletePaths;
+        this.restartLanguageServer();
+    }
+    private restartLanguageServer() {
         this.killProcess();
         this.clearPendingRequests();
-        this.initialize(this.pythonProcessCWD);
+        this.initialize();
     }
 
     private clearPendingRequests() {
@@ -240,7 +256,10 @@ export class JediProxy implements vscode.Disposable {
 
     // tslint:disable-next-line:max-func-body-length
     private async spawnProcess(cwd: string) {
-        this.initialized = createDeferred<void>();
+        if (this.languageServerStarted && !this.languageServerStarted.completed) {
+            this.languageServerStarted.reject();
+        }
+        this.languageServerStarted = createDeferred<void>();
         const pythonProcess = await this.serviceContainer.get<IPythonExecutionFactory>(IPythonExecutionFactory).create(Uri.file(this.workspacePath));
         const args = ['completion.py'];
         if (typeof this.pythonSettings.jediPath !== 'string' || this.pythonSettings.jediPath.length === 0) {
@@ -263,7 +282,7 @@ export class JediProxy implements vscode.Disposable {
         }
         const result = pythonProcess.execObservable(args, { cwd });
         this.proc = result.proc;
-        this.initialized.resolve();
+        this.languageServerStarted.resolve();
         this.proc.on('end', (end) => {
             logger.error('spawnProcess.end', `End - ${end}`);
         });
@@ -274,8 +293,8 @@ export class JediProxy implements vscode.Disposable {
                 error.message.indexOf('This socket has been ended by the other party') >= 0) {
                 this.spawnProcess(cwd)
                     .catch(ex => {
-                        if (this.initialized) {
-                            this.initialized.reject(ex);
+                        if (this.languageServerStarted) {
+                            this.languageServerStarted.reject(ex);
                         }
                         this.handleError('spawnProcess', ex);
                     });
@@ -533,14 +552,7 @@ export class JediProxy implements vscode.Disposable {
             return '';
         }
     }
-
-    private async onConfigChanged() {
-        // We're only interested in changes to the python path.
-        if (this.lastKnownPythonPath === this.pythonSettings.pythonPath) {
-            return;
-        }
-
-        this.lastKnownPythonPath = this.pythonSettings.pythonPath;
+    private async buildAutoCompletePaths(): Promise<string[]> {
         const filePathPromises = [
             // Sysprefix.
             this.getPathFromPythonCommand(['-c', 'import sys;print(sys.prefix)']).catch(() => ''),
@@ -561,19 +573,27 @@ export class JediProxy implements vscode.Disposable {
         ];
 
         try {
-            const environmentVariablesProvider = this.serviceContainer.get<IEnvironmentVariablesProvider>(IEnvironmentVariablesProvider);
-            const pythonPaths = await environmentVariablesProvider.getEnvironmentVariables(Uri.file(this.workspacePath))
+            const pythonPaths = await this.getEnvironmentVariablesProvider().getEnvironmentVariables(Uri.file(this.workspacePath))
                 .then(customEnvironmentVars => customEnvironmentVars ? JediProxy.getProperty<string>(customEnvironmentVars, 'PYTHONPATH') : '')
                 .then(pythonPath => (typeof pythonPath === 'string' && pythonPath.trim().length > 0) ? pythonPath.trim() : '')
                 .then(pythonPath => pythonPath.split(path.delimiter).filter(item => item.trim().length > 0));
-
+            const resolvedPaths = pythonPaths
+                .filter(pythonPath => !path.isAbsolute(pythonPath))
+                .map(pythonPath => path.resolve(this.workspacePath, pythonPath));
             const filePaths = await Promise.all(filePathPromises);
-            this.additionalAutoCopletePaths = filePaths.concat(pythonPaths).filter(p => p.length > 0);
+            return filePaths.concat(...pythonPaths, ...resolvedPaths).filter(p => p.length > 0);
         } catch (ex) {
             console.error('Python Extension: jediProxy.filePaths', ex);
+            return [];
         }
     }
-
+    private getEnvironmentVariablesProvider() {
+        if (!this.environmentVariablesProvider) {
+            this.environmentVariablesProvider = this.serviceContainer.get<IEnvironmentVariablesProvider>(IEnvironmentVariablesProvider);
+            this.environmentVariablesProvider.onDidEnvironmentVariablesChange(this.environmentVariablesChangeHandler.bind(this));
+        }
+        return this.environmentVariablesProvider;
+    }
     private getConfig() {
         // Add support for paths relative to workspace.
         const extraPaths = this.pythonSettings.autoComplete.extraPaths.map(extraPath => {
@@ -591,7 +611,7 @@ export class JediProxy implements vscode.Disposable {
             extraPaths.unshift(this.workspacePath);
         }
 
-        const distinctExtraPaths = extraPaths.concat(this.additionalAutoCopletePaths)
+        const distinctExtraPaths = extraPaths.concat(this.additionalAutoCompletePaths)
             .filter(value => value.length > 0)
             .filter((value, index, self) => self.indexOf(value) === index);
 
diff --git a/src/client/workspaceSymbols/generator.ts b/src/client/workspaceSymbols/generator.ts
@@ -25,21 +25,19 @@ export class Generator implements vscode.Disposable {
     public dispose() {
         this.disposables.forEach(d => d.dispose());
     }
-
+    public async generateWorkspaceTags(): Promise<void> {
+        if (!this.pythonSettings.workspaceSymbols.enabled) {
+            return;
+        }
+        return await this.generateTags({ directory: this.workspaceFolder.fsPath });
+    }
     private buildCmdArgs(): string[] {
         const optionsFile = this.optionsFile.indexOf(' ') > 0 ? `"${this.optionsFile}"` : this.optionsFile;
         const exclusions = this.pythonSettings.workspaceSymbols.exclusionPatterns;
         const excludes = exclusions.length === 0 ? [] : exclusions.map(pattern => `--exclude=${pattern}`);
 
         return [`--options=${optionsFile}`, '--languages=Python'].concat(excludes);
     }
-
-    public async generateWorkspaceTags(): Promise<void> {
-        if (!this.pythonSettings.workspaceSymbols.enabled) {
-            return;
-        }
-        return await this.generateTags({ directory: this.workspaceFolder.fsPath });
-    }
     @captureTelemetry(WORKSPACE_SYMBOLS_BUILD)
     private generateTags(source: { directory?: string, file?: string }): Promise<void> {
         const tagFile = path.normalize(this.pythonSettings.workspaceSymbols.tagFilePath);
@@ -60,10 +58,10 @@ export class Generator implements vscode.Disposable {
         }
         outputFile = outputFile.indexOf(' ') > 0 ? `"${outputFile}"` : outputFile;
         args.push(`-o ${outputFile}`, '.');
-        this.output.appendLine('-'.repeat(10) + 'Generating Tags' + '-'.repeat(10));
+        this.output.appendLine(`${'-'.repeat(10)}Generating Tags${'-'.repeat(10)}`);
         this.output.appendLine(`${cmd} ${args.join(' ')}`);
         const promise = new Promise<void>((resolve, reject) => {
-            let options: child_process.SpawnOptions = {
+            const options: child_process.SpawnOptions = {
                 cwd: source.directory
             };
 
@@ -76,6 +74,7 @@ export class Generator implements vscode.Disposable {
                 reject(error);
             });
             proc.stderr.on('data', (data: string) => {
+                hasErrors = true;
                 errorMsg += data;
                 this.output.append(data);
             });
@@ -85,8 +84,7 @@ export class Generator implements vscode.Disposable {
             proc.on('exit', () => {
                 if (hasErrors) {
                     reject(errorMsg);
-                }
-                else {
+                } else {
                     resolve();
                 }
             });
diff --git a/src/test/common/variables/envVarsProvider.multiroot.test.ts b/src/test/common/variables/envVarsProvider.multiroot.test.ts
