Added support for multi root workspaces with the new language server …

…server (#4244) * Added functionality * News entry * Corrected functionality * Register command only once * Activate jedi only once * Make sure activation is filtered to resource * Corrected activationManager * Corrected configSettings bug * Added functional tests * Added unit tests * Handle multiple folders being removed simultaneously * code reviews
microsoft · Feb 12, 2019 · cf686d2 · cf686d2
1 parent 37d6818
commit cf686d2
Show file tree

Hide file tree

Showing 19 changed files with 373 additions and 151 deletions.
diff --git a/news/2 Fixes/3008.md b/news/2 Fixes/3008.md
@@ -0,0 +1 @@
+Add support for multi root workspaces with the new language server server
diff --git a/src/client/activation/activationManager.ts b/src/client/activation/activationManager.ts
@@ -77,6 +77,15 @@ export class ExtensionActivationManager implements IExtensionActivationManager {
         }
     }
     protected onWorkspaceFoldersChanged() {
+        //If an activated workspace folder was removed, delete its key
+        const workspaceKeys = this.workspaceService.workspaceFolders!.map(workspaceFolder => this.getWorkspaceKey(workspaceFolder.uri));
+        const activatedWkspcKeys = Array.from(this.activatedWorkspaces.keys());
+        const activatedWkspcFoldersRemoved = activatedWkspcKeys.filter(item => workspaceKeys.indexOf(item) < 0);
+        if (activatedWkspcFoldersRemoved.length > 0) {
+            for (const folder of activatedWkspcFoldersRemoved) {
+                this.activatedWorkspaces.delete(folder);
+            }
+        }
         this.addRemoveDocOpenedHandlers();
     }
     protected hasMultipleWorkspaces() {

diff --git a/src/client/activation/activationService.ts b/src/client/activation/activationService.ts
@@ -17,12 +17,14 @@ import { EventName } from '../telemetry/constants';
 import { IExtensionActivationService, ILanguageServerActivator, LanguageServerActivator } from './types';
 
 const jediEnabledSetting: keyof IPythonSettings = 'jediEnabled';
+const workspacePathNameForGlobalWorkspaces = '';
 type ActivatorInfo = { jedi: boolean; activator: ILanguageServerActivator };
 
 @injectable()
 export class LanguageServerExtensionActivationService implements IExtensionActivationService, Disposable {
+    private lsActivatedWorkspaces = new Map<string, ILanguageServerActivator>();
     private currentActivator?: ActivatorInfo;
-    private activatedOnce: boolean = false;
+    private jediActivatedOnce: boolean = false;
     private readonly workspaceService: IWorkspaceService;
     private readonly output: OutputChannel;
     private readonly appShell: IApplicationShell;
@@ -40,45 +42,54 @@ export class LanguageServerExtensionActivationService implements IExtensionActiv
         const disposables = serviceContainer.get<IDisposableRegistry>(IDisposableRegistry);
         disposables.push(this);
         disposables.push(this.workspaceService.onDidChangeConfiguration(this.onDidChangeConfiguration.bind(this)));
+        disposables.push(this.workspaceService.onDidChangeWorkspaceFolders(this.onWorkspaceFoldersChanged, this));
     }
 
     public async activate(resource: Resource): Promise<void> {
-        if (this.currentActivator || this.activatedOnce) {
-            return;
-        }
-        this.resource = resource;
-        this.activatedOnce = true;
-
         let jedi = this.useJedi();
         if (!jedi) {
+            if (this.lsActivatedWorkspaces.has(this.getWorkspacePathKey(resource))) {
+                return;
+            }
             const diagnostic = await this.lsNotSupportedDiagnosticService.diagnose(undefined);
             this.lsNotSupportedDiagnosticService.handle(diagnostic).ignoreErrors();
             if (diagnostic.length) {
                 sendTelemetryEvent(EventName.PYTHON_LANGUAGE_SERVER_PLATFORM_NOT_SUPPORTED);
                 jedi = true;
             }
+        } else {
+            if (this.jediActivatedOnce) {
+                return;
+            }
+            this.jediActivatedOnce = true;
         }
 
+        this.resource = resource;
         await this.logStartup(jedi);
-
         let activatorName = jedi ? LanguageServerActivator.Jedi : LanguageServerActivator.DotNet;
         let activator = this.serviceContainer.get<ILanguageServerActivator>(ILanguageServerActivator, activatorName);
         this.currentActivator = { jedi, activator };
 
         try {
-            await activator.activate();
-            return;
+            await activator.activate(resource);
+            if (!jedi) {
+                this.lsActivatedWorkspaces.set(this.getWorkspacePathKey(resource), activator);
+            }
         } catch (ex) {
             if (jedi) {
                 return;
             }
             //Language server fails, reverting to jedi
+            if (this.jediActivatedOnce) {
+                return;
+            }
+            this.jediActivatedOnce = true;
             jedi = true;
             await this.logStartup(jedi);
             activatorName = LanguageServerActivator.Jedi;
             activator = this.serviceContainer.get<ILanguageServerActivator>(ILanguageServerActivator, activatorName);
             this.currentActivator = { jedi, activator };
-            await activator.activate();
+            await activator.activate(resource);
         }
     }
 
@@ -88,6 +99,19 @@ export class LanguageServerExtensionActivationService implements IExtensionActiv
         }
     }
 
+    protected onWorkspaceFoldersChanged() {
+        //If an activated workspace folder was removed, dispose its activator
+        const workspaceKeys = this.workspaceService.workspaceFolders!.map(workspaceFolder => this.getWorkspacePathKey(workspaceFolder.uri));
+        const activatedWkspcKeys = Array.from(this.lsActivatedWorkspaces.keys());
+        const activatedWkspcFoldersRemoved = activatedWkspcKeys.filter(item => workspaceKeys.indexOf(item) < 0);
+        if (activatedWkspcFoldersRemoved.length > 0) {
+            for (const folder of activatedWkspcFoldersRemoved) {
+                this.lsActivatedWorkspaces.get(folder).dispose();
+                this.lsActivatedWorkspaces.delete(folder);
+            }
+        }
+    }
+
     private async logStartup(isJedi: boolean): Promise<void> {
         const outputLine = isJedi
             ? 'Starting Jedi Python language engine.'
@@ -119,4 +143,7 @@ export class LanguageServerExtensionActivationService implements IExtensionActiv
         const configurationService = this.serviceContainer.get<IConfigurationService>(IConfigurationService);
         return configurationService.getSettings(this.resource).jediEnabled;
     }
+    private getWorkspacePathKey(resource: Resource): string {
+        return this.workspaceService.getWorkspaceFolderIdentifier(resource, workspacePathNameForGlobalWorkspaces);
+    }
 }
diff --git a/src/client/activation/jedi.ts b/src/client/activation/jedi.ts
@@ -4,7 +4,7 @@
 import { inject, injectable } from 'inversify';
 import { DocumentFilter, languages } from 'vscode';
 import { PYTHON } from '../common/constants';
-import { IConfigurationService, IExtensionContext, ILogger } from '../common/types';
+import { IConfigurationService, IExtensionContext, ILogger, Resource } from '../common/types';
 import { IShebangCodeLensProvider } from '../interpreter/contracts';
 import { IServiceContainer, IServiceManager } from '../ioc/types';
 import { JediFactory } from '../languageServices/jediProxyFactory';
@@ -33,7 +33,10 @@ export class JediExtensionActivator implements ILanguageServerActivator {
         this.documentSelector = PYTHON;
     }
 
-    public async activate(): Promise<void> {
+    public async activate(resource: Resource): Promise<void> {
+        if (this.jediFactory) {
+            throw new Error('Jedi already started');
+        }
         const context = this.context;
 
         const jediFactory = (this.jediFactory = new JediFactory(context.asAbsolutePath('.'), this.serviceManager));

diff --git a/src/client/activation/languageServer/activator.ts b/src/client/activation/languageServer/activator.ts
@@ -34,14 +34,16 @@ export class LanguageServerExtensionActivator implements ILanguageServerActivato
         @inject(ILanguageServerFolderService)
         private readonly languageServerFolderService: ILanguageServerFolderService,
         @inject(IConfigurationService) private readonly configurationService: IConfigurationService
-    ) {}
+    ) { }
     @traceDecorators.error('Failed to activate language server')
-    public async activate(): Promise<void> {
-        const mainWorkspaceUri = this.workspace.hasWorkspaceFolders
-            ? this.workspace.workspaceFolders![0].uri
-            : undefined;
-        await this.ensureLanguageServerIsAvailable(mainWorkspaceUri);
-        await this.manager.start(mainWorkspaceUri);
+    public async activate(resource: Resource): Promise<void> {
+        if (!resource) {
+            resource = this.workspace.hasWorkspaceFolders
+                ? this.workspace.workspaceFolders![0].uri
+                : undefined;
+        }
+        await this.ensureLanguageServerIsAvailable(resource);
+        await this.manager.start(resource);
     }
     public dispose(): void {
         this.manager.dispose();

diff --git a/src/client/activation/languageServer/analysisOptions.ts b/src/client/activation/languageServer/analysisOptions.ts
@@ -8,7 +8,7 @@ import * as path from 'path';
 import { CancellationToken, CompletionContext, ConfigurationChangeEvent, Disposable, Event, EventEmitter, OutputChannel, Position, TextDocument } from 'vscode';
 import { LanguageClientOptions, ProvideCompletionItemsSignature } from 'vscode-languageclient';
 import { IWorkspaceService } from '../../common/application/types';
-import { isTestExecution, PYTHON, PYTHON_LANGUAGE, STANDARD_OUTPUT_CHANNEL } from '../../common/constants';
+import { isTestExecution, PYTHON_LANGUAGE, STANDARD_OUTPUT_CHANNEL } from '../../common/constants';
 import { traceDecorators, traceError } from '../../common/logger';
 import { BANNER_NAME_PROPOSE_LS, IConfigurationService, IExtensionContext, IOutputChannel, IPathUtils, IPythonExtensionBanner, Resource } from '../../common/types';
 import { debounce } from '../../common/utils/decorators';
@@ -79,7 +79,7 @@ export class LanguageServerAnalysisOptions implements ILanguageServerAnalysisOpt
         properties['DatabasePath'] = path.join(this.context.extensionPath, this.languageServerFolder);
 
         let searchPaths = interpreterData ? interpreterData.searchPaths.split(path.delimiter) : [];
-        const settings = this.configuration.getSettings();
+        const settings = this.configuration.getSettings(this.resource);
         if (settings.autoComplete) {
             const extraPaths = settings.autoComplete.extraPaths;
             if (extraPaths && extraPaths.length > 0) {
@@ -99,11 +99,20 @@ export class LanguageServerAnalysisOptions implements ILanguageServerAnalysisOpt
 
         this.excludedFiles = this.getExcludedFiles();
         this.typeshedPaths = this.getTypeshedPaths();
-
+        const workspaceFolder = this.workspace.getWorkspaceFolder(this.resource);
+        const documentSelector = [
+            { scheme: 'file', language: PYTHON_LANGUAGE },
+            { scheme: 'untitled', language: PYTHON_LANGUAGE }
+        ];
+        if (workspaceFolder){
+            // tslint:disable-next-line:no-any
+            (documentSelector[0] as any).pattern = `${workspaceFolder.uri.fsPath}/**/*`;
+        }
         // Options to control the language client
         return {
             // Register the server for Python documents
-            documentSelector: PYTHON,
+            documentSelector,
+            workspaceFolder,
             synchronize: {
                 configurationSection: PYTHON_LANGUAGE
             },

diff --git a/src/client/activation/languageServer/languageServerExtension.ts b/src/client/activation/languageServer/languageServerExtension.ts
@@ -0,0 +1,38 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+'use strict';
+
+import { inject, injectable } from 'inversify';
+import { Event, EventEmitter } from 'vscode';
+import { ICommandManager } from '../../common/application/types';
+import '../../common/extensions';
+import { IDisposable } from '../../common/types';
+import { ILanguageServerExtension } from '../types';
+
+const loadExtensionCommand = 'python._loadLanguageServerExtension';
+
+@injectable()
+export class LanguageServerExtension implements ILanguageServerExtension {
+    public loadExtensionArgs?: {};
+    protected readonly _invoked = new EventEmitter<void>();
+    private disposable?: IDisposable;
+    constructor(@inject(ICommandManager) private readonly commandManager: ICommandManager) { }
+    public dispose() {
+        if (this.disposable) {
+            this.disposable.dispose();
+        }
+    }
+    public register(): Promise<void> {
+        if (this.disposable) {
+            return;
+        }
+        this.disposable = this.commandManager.registerCommand(loadExtensionCommand, args => {
+            this.loadExtensionArgs = args;
+            this._invoked.fire();
+        });
+    }
+    public get invoked(): Event<void> {
+        return this._invoked.event;
+    }
+}
diff --git a/src/client/activation/languageServer/manager.ts b/src/client/activation/languageServer/manager.ts
@@ -4,29 +4,25 @@
 'use strict';
 
 import { inject, injectable } from 'inversify';
-import { ICommandManager } from '../../common/application/types';
 import '../../common/extensions';
 import { traceDecorators } from '../../common/logger';
 import { IDisposable, Resource } from '../../common/types';
 import { debounce } from '../../common/utils/decorators';
 import { IServiceContainer } from '../../ioc/types';
 import { captureTelemetry } from '../../telemetry';
 import { EventName } from '../../telemetry/constants';
-import { ILanguageServer, ILanguageServerAnalysisOptions, ILanguageServerManager } from '../types';
-
-const loadExtensionCommand = 'python._loadLanguageServerExtension';
+import { ILanguageServer, ILanguageServerAnalysisOptions, ILanguageServerExtension, ILanguageServerManager } from '../types';
 
 @injectable()
 export class LanguageServerManager implements ILanguageServerManager {
-    protected static loadExtensionArgs?: {};
     private languageServer?: ILanguageServer;
     private resource!: Resource;
     private disposables: IDisposable[] = [];
     constructor(
         @inject(IServiceContainer) private readonly serviceContainer: IServiceContainer,
-        @inject(ICommandManager) private readonly commandManager: ICommandManager,
-        @inject(ILanguageServerAnalysisOptions) private readonly analysisOptions: ILanguageServerAnalysisOptions
-    ) {}
+        @inject(ILanguageServerAnalysisOptions) private readonly analysisOptions: ILanguageServerAnalysisOptions,
+        @inject(ILanguageServerExtension) private readonly lsExtension: ILanguageServerExtension
+    ) { }
     public dispose() {
         if (this.languageServer) {
             this.languageServer.dispose();
@@ -46,15 +42,11 @@ export class LanguageServerManager implements ILanguageServerManager {
         await this.startLanguageServer();
     }
     protected registerCommandHandler() {
-        const disposable = this.commandManager.registerCommand(loadExtensionCommand, args => {
-            LanguageServerManager.loadExtensionArgs = args;
-            this.loadExtensionIfNecessary();
-        });
-        this.disposables.push(disposable);
+        this.lsExtension.invoked(this.loadExtensionIfNecessary, this, this.disposables);
     }
     protected loadExtensionIfNecessary() {
-        if (this.languageServer && LanguageServerManager.loadExtensionArgs) {
-            this.languageServer.loadExtension(LanguageServerManager.loadExtensionArgs);
+        if (this.languageServer && this.lsExtension.loadExtensionArgs) {
+            this.languageServer.loadExtension(this.lsExtension.loadExtensionArgs);
         }
     }
     @debounce(1000)

diff --git a/src/client/activation/serviceRegistry.ts b/src/client/activation/serviceRegistry.ts
@@ -21,16 +21,18 @@ import { InterpreterDataService } from './languageServer/interpreterDataService'
 import { BaseLanguageClientFactory, DownloadedLanguageClientFactory, SimpleLanguageClientFactory } from './languageServer/languageClientFactory';
 import { LanguageServer } from './languageServer/languageServer';
 import { LanguageServerCompatibilityService } from './languageServer/languageServerCompatibilityService';
+import { LanguageServerExtension } from './languageServer/languageServerExtension';
 import { LanguageServerFolderService } from './languageServer/languageServerFolderService';
 import { BetaLanguageServerPackageRepository, DailyLanguageServerPackageRepository, LanguageServerDownloadChannel, StableLanguageServerPackageRepository } from './languageServer/languageServerPackageRepository';
 import { LanguageServerPackageService } from './languageServer/languageServerPackageService';
 import { LanguageServerManager } from './languageServer/manager';
 import { PlatformData } from './languageServer/platformData';
-import { IDownloadChannelRule, IExtensionActivationManager, IExtensionActivationService, IInterpreterDataService, ILanguageClientFactory, ILanguageServer, ILanguageServerActivator, ILanguageServerAnalysisOptions, ILanguageServerCompatibilityService as ILanagueServerCompatibilityService, ILanguageServerDownloader, ILanguageServerFolderService, ILanguageServerManager, ILanguageServerPackageService, IPlatformData, LanguageClientFactory, LanguageServerActivator } from './types';
+import { IDownloadChannelRule, IExtensionActivationManager, IExtensionActivationService, IInterpreterDataService, ILanguageClientFactory, ILanguageServer, ILanguageServerActivator, ILanguageServerAnalysisOptions, ILanguageServerCompatibilityService as ILanagueServerCompatibilityService, ILanguageServerDownloader, ILanguageServerExtension, ILanguageServerFolderService, ILanguageServerManager, ILanguageServerPackageService, IPlatformData, LanguageClientFactory, LanguageServerActivator } from './types';
 
 export function registerTypes(serviceManager: IServiceManager) {
-    serviceManager.addSingleton<IExtensionActivationManager>(IExtensionActivationManager, ExtensionActivationManager);
     serviceManager.addSingleton<IExtensionActivationService>(IExtensionActivationService, LanguageServerExtensionActivationService);
+    serviceManager.addSingleton<ILanguageServerExtension>(ILanguageServerExtension, LanguageServerExtension);
+    serviceManager.add<IExtensionActivationManager>(IExtensionActivationManager, ExtensionActivationManager);
     serviceManager.add<ILanguageServerActivator>(ILanguageServerActivator, JediExtensionActivator, LanguageServerActivator.Jedi);
     serviceManager.add<ILanguageServerActivator>(ILanguageServerActivator, LanguageServerExtensionActivator, LanguageServerActivator.DotNet);
     serviceManager.addSingleton<IPythonExtensionBanner>(IPythonExtensionBanner, LanguageServerSurveyBanner, BANNER_NAME_LS_SURVEY);

diff --git a/src/client/activation/types.ts b/src/client/activation/types.ts
@@ -28,7 +28,7 @@ export enum LanguageServerActivator {
 
 export const ILanguageServerActivator = Symbol('ILanguageServerActivator');
 export interface ILanguageServerActivator extends IDisposable {
-    activate(): Promise<void>;
+    activate(resource: Resource): Promise<void>;
 }
 
 export const IHttpClient = Symbol('IHttpClient');
@@ -88,6 +88,12 @@ export const ILanguageServerManager = Symbol('ILanguageServerManager');
 export interface ILanguageServerManager extends IDisposable {
     start(resource: Resource): Promise<void>;
 }
+export const ILanguageServerExtension = Symbol('ILanguageServerExtension');
+export interface ILanguageServerExtension extends IDisposable {
+    readonly invoked: Event<void>;
+    loadExtensionArgs?: {};
+    register(): void;
+}
 export const ILanguageServer = Symbol('ILanguageServer');
 export interface ILanguageServer extends IDisposable {
     start(resource: Resource, options: LanguageClientOptions): Promise<void>;

diff --git a/src/client/common/configSettings.ts b/src/client/common/configSettings.ts
@@ -373,6 +373,17 @@ export class PythonSettings implements IPythonSettings {
     protected getPythonExecutable(pythonPath: string) {
         return getPythonExecutable(pythonPath);
     }
+    protected onWorkspaceFoldersChanged() {
+        //If an activated workspace folder was removed, delete its key
+        const workspaceKeys = this.workspace.workspaceFolders!.map(workspaceFolder => workspaceFolder.uri.fsPath);
+        const activatedWkspcKeys = Array.from(PythonSettings.pythonSettings.keys());
+        const activatedWkspcFoldersRemoved = activatedWkspcKeys.filter(item => workspaceKeys.indexOf(item) < 0);
+        if (activatedWkspcFoldersRemoved.length > 0) {
+            for (const folder of activatedWkspcFoldersRemoved) {
+                PythonSettings.pythonSettings.delete(folder);
+            }
+        }
+    }
     protected initialize(): void {
         const onDidChange = () => {
             const currentConfig = this.workspace.getConfiguration('python', this.workspaceRoot);
@@ -382,6 +393,7 @@ export class PythonSettings implements IPythonSettings {
             // Let's defer the change notification.
             this.debounceChangeNotification();
         };
+        this.disposables.push(this.workspace.onDidChangeWorkspaceFolders(this.onWorkspaceFoldersChanged, this));
         this.disposables.push(this.interpreterAutoSelectionService.onDidChangeAutoSelectedInterpreter(onDidChange.bind(this)));
         this.disposables.push(this.workspace.onDidChangeConfiguration((event: ConfigurationChangeEvent) => {
             if (event.affectsConfiguration('python')) {