Fixes to the LS no reload work (#18984)

kimadeline · web-flow · commit 4e20c2bc2ac2 · 2022-04-22T14:16:21.000-07:00
* Delete refCountedLanguageServer.ts

* Stop LS is workspace folder removed

* Only one LS for Pylance and None

* Dispose LS in multiroot scenario only with Jedi
diff --git a/.eslintignore b/.eslintignore
@@ -174,7 +174,6 @@ src/client/activation/commands.ts
 src/client/activation/progress.ts
 src/client/activation/extensionSurvey.ts
 src/client/activation/common/analysisOptions.ts
-src/client/activation/refCountedLanguageServer.ts
 src/client/activation/languageClientMiddleware.ts
 
 src/client/formatters/serviceRegistry.ts
diff --git a/package.nls.json b/package.nls.json
@@ -175,9 +175,9 @@
     "LanguageService.extractionCompletedOutputMessage": "Language server download complete",
     "LanguageService.extractionDoneOutputMessage": "done",
     "LanguageService.reloadVSCodeIfSeachPathHasChanged": "Search paths have changed for this Python interpreter. Please reload the extension to ensure that the IntelliSense works correctly",
-    "LanguageService.startingPylance": "Starting Pylance language server for {0}.",
+    "LanguageService.startingPylance": "Starting Pylance language server.",
     "LanguageService.startingJedi": "Starting Jedi language server for {0}.",
-    "LanguageService.startingNone": "Editor support is inactive since language server is set to None for {0}.",
+    "LanguageService.startingNone": "Editor support is inactive since language server is set to None.",
     "LanguageService.reloadAfterLanguageServerChange": "Please reload the window switching between language servers.",
     "AttachProcess.unsupportedOS": "Operating system '{0}' not supported.",
     "AttachProcess.attachTitle": "Attach to process",
diff --git a/src/client/activation/refCountedLanguageServer.ts b/src/client/activation/refCountedLanguageServer.ts
diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts
@@ -213,14 +213,11 @@ export namespace LanguageService {
         text: localize('LanguageService.statusItem.text', 'Partial Mode'),
         detail: localize('LanguageService.statusItem.detail', 'Limited IntelliSense provided by Pylance'),
     };
-    export const startingPylance = localize(
-        'LanguageService.startingPylance',
-        'Starting Pylance language server for {0}.',
-    );
+    export const startingPylance = localize('LanguageService.startingPylance', 'Starting Pylance language server.');
     export const startingJedi = localize('LanguageService.startingJedi', 'Starting Jedi language server for {0}.');
     export const startingNone = localize(
         'LanguageService.startingNone',
-        'Editor support is inactive since language server is set to None for {0}.',
+        'Editor support is inactive since language server is set to None.',
     );
     export const untrustedWorkspaceMessage = localize(
         'LanguageService.untrustedWorkspaceMessage',
diff --git a/src/client/languageServer/watcher.ts b/src/client/languageServer/watcher.ts
@@ -3,7 +3,7 @@
 
 import * as path from 'path';
 import { inject, injectable } from 'inversify';
-import { ConfigurationChangeEvent, Uri } from 'vscode';
+import { ConfigurationChangeEvent, Uri, WorkspaceFoldersChangeEvent } from 'vscode';
 import { LanguageServerChangeHandler } from '../activation/common/languageServerChangeHandler';
 import {
     IExtensionActivationService,
@@ -50,7 +50,9 @@ export class LanguageServerWatcher
 
     private workspaceInterpreters: Map<string, PythonEnvironment | undefined>;
 
-    // In a multiroot workspace scenario we will have one language server per folder.
+    // In a multiroot workspace scenario we may have multiple language servers running:
+    // When using Jedi, there will be one language server per workspace folder.
+    // When using Pylance, there will only be one language server for the project.
     private workspaceLanguageServers: Map<string, ILanguageServerExtensionManager | undefined>;
 
     private languageServerChangeHandler: LanguageServerChangeHandler;
@@ -77,6 +79,10 @@ export class LanguageServerWatcher
 
         disposables.push(this.workspaceService.onDidChangeConfiguration(this.onDidChangeConfiguration.bind(this)));
 
+        disposables.push(
+            this.workspaceService.onDidChangeWorkspaceFolders(this.onDidChangeWorkspaceFolders.bind(this)),
+        );
+
         if (this.workspaceService.isTrusted) {
             disposables.push(this.interpreterPathService.onDidChange(this.onDidChangeInterpreter.bind(this)));
         }
@@ -113,7 +119,7 @@ export class LanguageServerWatcher
         languageServerType: LanguageServerType,
         resource?: Resource,
     ): Promise<ILanguageServerExtensionManager> {
-        const lsResource = this.getWorkspaceKey(resource);
+        const lsResource = this.getWorkspaceUri(resource);
         const currentInterpreter = this.workspaceInterpreters.get(lsResource.fsPath);
         const interpreter = await this.interpreterService?.getActiveInterpreter(resource);
 
@@ -142,6 +148,15 @@ export class LanguageServerWatcher
             serverType = LanguageServerType.None;
         }
 
+        // If the language server type is Pylance or None,
+        // We only need to instantiate the language server once, even in multiroot workspace scenarios,
+        // so we only need one language server extension manager.
+        const key = this.getWorkspaceKey(resource, serverType);
+        const languageServer = this.workspaceLanguageServers.get(key);
+        if ((serverType === LanguageServerType.Node || serverType === LanguageServerType.None) && languageServer) {
+            return languageServer;
+        }
+
         // Instantiate the language server extension manager.
         const languageServerExtensionManager = this.createLanguageServer(serverType);
 
@@ -156,16 +171,16 @@ export class LanguageServerWatcher
             await languageServerExtensionManager.languageServerNotAvailable();
         }
 
-        this.workspaceLanguageServers.set(lsResource.fsPath, languageServerExtensionManager);
+        this.workspaceLanguageServers.set(key, languageServerExtensionManager);
 
         return languageServerExtensionManager;
     }
 
     // ILanguageServerCache
 
     public async get(resource?: Resource): Promise<ILanguageServer> {
-        const lsResource = this.getWorkspaceKey(resource);
-        let languageServerExtensionManager = this.workspaceLanguageServers.get(lsResource.fsPath);
+        const key = this.getWorkspaceKey(resource, this.languageServerType);
+        let languageServerExtensionManager = this.workspaceLanguageServers.get(key);
 
         if (!languageServerExtensionManager) {
             languageServerExtensionManager = await this.startAndGetLanguageServer(this.languageServerType, resource);
@@ -177,13 +192,13 @@ export class LanguageServerWatcher
     // Private methods
 
     private stopLanguageServer(resource?: Resource): void {
-        const lsResource = this.getWorkspaceKey(resource);
-        const languageServerExtensionManager = this.workspaceLanguageServers.get(lsResource.fsPath);
+        const key = this.getWorkspaceKey(resource, this.languageServerType);
+        const languageServerExtensionManager = this.workspaceLanguageServers.get(key);
 
         if (languageServerExtensionManager) {
             languageServerExtensionManager.stopLanguageServer();
             languageServerExtensionManager.dispose();
-            this.workspaceLanguageServers.delete(lsResource.fsPath);
+            this.workspaceLanguageServers.delete(key);
         }
     }
 
@@ -228,11 +243,11 @@ export class LanguageServerWatcher
     }
 
     private async refreshLanguageServer(resource?: Resource): Promise<void> {
-        const lsResource = this.getWorkspaceKey(resource);
+        const lsResource = this.getWorkspaceUri(resource);
         const languageServerType = this.configurationService.getSettings(lsResource).languageServer;
 
         if (languageServerType !== this.languageServerType) {
-            this.stopLanguageServer(lsResource);
+            this.stopLanguageServer(resource);
             await this.startLanguageServer(languageServerType, lsResource);
         }
     }
@@ -267,8 +282,19 @@ export class LanguageServerWatcher
         }
     }
 
-    // Get the workspace key for the given resource, in order to query this.workspaceInterpreters and this.workspaceLanguageServers.
-    private getWorkspaceKey(resource?: Resource): Uri {
+    // Watch for workspace folder changes.
+    private async onDidChangeWorkspaceFolders(event: WorkspaceFoldersChangeEvent): Promise<void> {
+        // Since Jedi is the only language server type where we instantiate multiple language servers,
+        // Make sure to dispose of them only in that scenario.
+        if (event.removed.length && this.languageServerType === LanguageServerType.Jedi) {
+            event.removed.forEach((workspace) => {
+                this.stopLanguageServer(workspace.uri);
+            });
+        }
+    }
+
+    // Get the workspace Uri for the given resource, in order to query this.workspaceInterpreters and this.workspaceLanguageServers.
+    private getWorkspaceUri(resource?: Resource): Uri {
         let uri;
 
         if (resource) {
@@ -279,6 +305,19 @@ export class LanguageServerWatcher
 
         return uri ?? Uri.parse('default');
     }
+
+    // Get the key used to identify which language server extension manager is associated to which workspace.
+    // When using Pylance or having no LS enabled, we return a static key since there should only be one LS extension manager for these LS types.
+    private getWorkspaceKey(resource: Resource | undefined, languageServerType: LanguageServerType): string {
+        switch (languageServerType) {
+            case LanguageServerType.Node:
+                return 'Pylance';
+            case LanguageServerType.None:
+                return 'None';
+            default:
+                return this.getWorkspaceUri(resource).fsPath;
+        }
+    }
 }
 
 function logStartup(languageServerType: LanguageServerType, resource: Uri): void {
@@ -290,10 +329,10 @@ function logStartup(languageServerType: LanguageServerType, resource: Uri): void
             outputLine = LanguageService.startingJedi().format(basename);
             break;
         case LanguageServerType.Node:
-            outputLine = LanguageService.startingPylance().format(basename);
+            outputLine = LanguageService.startingPylance();
             break;
         case LanguageServerType.None:
-            outputLine = LanguageService.startingNone().format(basename);
+            outputLine = LanguageService.startingNone();
             break;
         default:
             throw new Error(`Unknown language server type: ${languageServerType}`);
diff --git a/src/test/languageServer/watcher.unit.test.ts b/src/test/languageServer/watcher.unit.test.ts
