Add "Default" LS setting, which picks between Jedi/Pylance

news/1 Enhancements/16157.md

@@ -0,0 +1 @@
+Add a "Default" language server option, which dynamically chooses which language server to use.

package.json

@@ -1250,13 +1250,14 @@
                 "python.languageServer": {
                     "type": "string",
                     "enum": [
+                        "Default",
                         "Jedi",
                         "JediLSP",
                         "Pylance",
                         "Microsoft",
                         "None"
                     ],
-                    "default": "Jedi",
+                    "default": "Default",
                     "description": "Defines type of the language server.",
                     "scope": "window"
                 },

src/client/activation/activationService.ts

@@ -240,6 +240,13 @@ export class LanguageServerExtensionActivationService
             }
         }
 
+        if (serverType === LanguageServerType.Node && interpreter && interpreter.version) {
+            if (interpreter.version.major < 3) {
+                sendTelemetryEvent(EventName.JEDI_FALLBACK);
+                serverType = LanguageServerType.Jedi;
+            }
+        }
+
         this.sendTelemetryForChosenLanguageServer(serverType).ignoreErrors();
 
         await this.logStartup(serverType);
@@ -310,7 +317,7 @@ export class LanguageServerExtensionActivationService
         const configurationService = this.serviceContainer.get<IConfigurationService>(IConfigurationService);
         const serverType = configurationService.getSettings(this.resource).languageServer;
         if (serverType === LanguageServerType.Node) {
-            return 'shared-ls';
+            return LanguageServerType.Node;
         }
 
         const resourcePortion = this.workspaceService.getWorkspaceFolderIdentifier(

src/client/activation/common/defaultlanguageServer.ts

@@ -0,0 +1,44 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+import { injectable } from 'inversify';
+import { PYLANCE_EXTENSION_ID } from '../../common/constants';
+import { JediLSP } from '../../common/experiments/groups';
+import { IDefaultLanguageServer, IExperimentService, IExtensions, DefaultLSType } from '../../common/types';
+import { IServiceManager } from '../../ioc/types';
+import { ILSExtensionApi } from '../node/languageServerFolderService';
+import { LanguageServerType } from '../types';
+
+@injectable()
+class DefaultLanguageServer implements IDefaultLanguageServer {
+    public readonly defaultLSType: DefaultLSType;
+
+    constructor(defaultServer: DefaultLSType) {
+        this.defaultLSType = defaultServer;
+    }
+}
+
+export async function setDefaultLanguageServer(
+    experimentService: IExperimentService,
+    extensions: IExtensions,
+    serviceManager: IServiceManager,
+): Promise<void> {
+    const lsType = await getDefaultLanguageServer(experimentService, extensions);
+    serviceManager.addSingletonInstance<IDefaultLanguageServer>(
+        IDefaultLanguageServer,
+        new DefaultLanguageServer(lsType),
+    );
+}
+
+async function getDefaultLanguageServer(
+    experimentService: IExperimentService,
+    extensions: IExtensions,
+): Promise<DefaultLSType> {
+    if (extensions.getExtension<ILSExtensionApi>(PYLANCE_EXTENSION_ID)) {
+        return LanguageServerType.Node;
+    }
+
+    return (await experimentService.inExperiment(JediLSP.experiment))
+        ? LanguageServerType.JediLSP
+        : LanguageServerType.Jedi;
+}

src/client/common/configSettings.ts

@@ -167,7 +167,7 @@ export class PythonSettings implements IPythonSettings {
         private readonly experimentsManager?: IExperimentsManager,
         private readonly interpreterPathService?: IInterpreterPathService,
         private readonly interpreterSecurityService?: IInterpreterSecurityService,
-        private readonly defaultJedi?: IDefaultLanguageServer,
+        private readonly defaultLS?: IDefaultLanguageServer,
     ) {
         this.workspace = workspace || new WorkspaceService();
         this.workspaceRoot = workspaceFolder;
@@ -181,7 +181,7 @@ export class PythonSettings implements IPythonSettings {
         experimentsManager?: IExperimentsManager,
         interpreterPathService?: IInterpreterPathService,
         interpreterSecurityService?: IInterpreterSecurityService,
-        defaultJedi?: IDefaultLanguageServer,
+        defaultLS?: IDefaultLanguageServer,
     ): PythonSettings {
         workspace = workspace || new WorkspaceService();
         const workspaceFolderUri = PythonSettings.getSettingsUriAndTarget(resource, workspace).uri;
@@ -195,7 +195,7 @@ export class PythonSettings implements IPythonSettings {
                 experimentsManager,
                 interpreterPathService,
                 interpreterSecurityService,
-                defaultJedi,
+                defaultLS,
             );
             PythonSettings.pythonSettings.set(workspaceFolderKey, settings);
             // Pass null to avoid VSC from complaining about not passing in a value.
@@ -284,14 +284,23 @@ export class PythonSettings implements IPythonSettings {
 
         this.useIsolation = systemVariables.resolveAny(pythonSettings.get<boolean>('useIsolation', true))!;
 
-        const defaultServer = this.defaultJedi
-            ? this.defaultJedi.defaultLSType
-            : pythonSettings.get<LanguageServerType>('languageServer');
-        let ls = defaultServer ?? LanguageServerType.Jedi;
-        ls = systemVariables.resolveAny(ls);
-        if (!Object.values(LanguageServerType).includes(ls)) {
-            ls = LanguageServerType.Jedi;
+        // Get as a string and verify; don't just accept.
+        let userLS = pythonSettings.get<string>('languageServer');
+        userLS = systemVariables.resolveAny(userLS);
+
+        let ls: LanguageServerType;
+
+        // Validate the user's input; if invalid, set it to the default.
+        if (
+            !userLS ||
+            userLS === 'Default' ||
+            !Object.values(LanguageServerType).includes(userLS as LanguageServerType)
+        ) {
+            ls = this.defaultLS?.defaultLSType ?? LanguageServerType.Jedi;
+        } else {
+            ls = userLS as LanguageServerType;
         }
+
         this.languageServer = ls;
 
         this.jediPath = systemVariables.resolveAny(pythonSettings.get<string>('jediPath'))!;

src/client/common/configuration/service.ts

@@ -34,15 +34,15 @@ export class ConfigurationService implements IConfigurationService {
         const interpreterSecurityService = this.serviceContainer.get<IInterpreterSecurityService>(
             IInterpreterSecurityService,
         );
-        const defaultJedi = this.serviceContainer.tryGet<IDefaultLanguageServer>(IDefaultLanguageServer);
+        const defaultLS = this.serviceContainer.tryGet<IDefaultLanguageServer>(IDefaultLanguageServer);
         return PythonSettings.getInstance(
             resource,
             InterpreterAutoSelectionService,
             this.workspaceService,
             experiments,
             interpreterPathService,
             interpreterSecurityService,
-            defaultJedi,
+            defaultLS,
         );
     }
 

src/client/common/experiments/helpers.ts

@@ -3,12 +3,8 @@
 
 'use strict';
 
-import { injectable } from 'inversify';
-import { LanguageServerType } from '../../activation/types';
-import { IServiceManager } from '../../ioc/types';
-import { IWorkspaceService } from '../application/types';
-import { IDefaultLanguageServer, IExperimentService } from '../types';
-import { DiscoveryVariants, JediLSP } from './groups';
+import { IExperimentService } from '../types';
+import { DiscoveryVariants } from './groups';
 
 export async function inDiscoveryExperiment(experimentService: IExperimentService): Promise<boolean> {
     const results = await Promise.all([
@@ -17,41 +13,3 @@ export async function inDiscoveryExperiment(experimentService: IExperimentServic
     ]);
     return results.includes(true);
 }
-
-@injectable()
-class DefaultLanguageServer implements IDefaultLanguageServer {
-    public readonly defaultLSType: LanguageServerType.Jedi | LanguageServerType.JediLSP;
-
-    constructor(defaultServer: LanguageServerType.Jedi | LanguageServerType.JediLSP) {
-        this.defaultLSType = defaultServer;
-    }
-}
-
-export async function setDefaultLanguageServerByExperiment(
-    experimentService: IExperimentService,
-    workspaceService: IWorkspaceService,
-    serviceManager: IServiceManager,
-): Promise<void> {
-    const settings = workspaceService.getConfiguration('python');
-    const lsSetting = settings.inspect('languageServer');
-    if (lsSetting) {
-        if (
-            lsSetting.globalValue ||
-            lsSetting.globalLanguageValue ||
-            lsSetting.workspaceFolderValue ||
-            lsSetting.workspaceFolderLanguageValue ||
-            lsSetting.workspaceValue ||
-            lsSetting.workspaceLanguageValue
-        ) {
-            return Promise.resolve();
-        }
-    }
-    const lsType = (await experimentService.inExperiment(JediLSP.experiment))
-        ? LanguageServerType.JediLSP
-        : LanguageServerType.Jedi;
-    serviceManager.addSingletonInstance<IDefaultLanguageServer>(
-        IDefaultLanguageServer,
-        new DefaultLanguageServer(lsType),
-    );
-    return Promise.resolve();
-}

src/client/common/types.ts

@@ -589,8 +589,10 @@ export interface IInterpreterPathService {
     copyOldInterpreterStorageValuesToNew(resource: Uri | undefined): Promise<void>;
 }
 
+export type DefaultLSType = LanguageServerType.Jedi | LanguageServerType.JediLSP | LanguageServerType.Node;
+
 /**
- * Interface used to retrieve the default language server to use when in experiment
+ * Interface used to retrieve the default language server.
  *
  * Note: This is added to get around a problem that the config service is not `async`.
  * Adding experiment check there would mean touching the entire extension. For simplicity
@@ -599,5 +601,5 @@ export interface IInterpreterPathService {
 export const IDefaultLanguageServer = Symbol('IDefaultLanguageServer');
 
 export interface IDefaultLanguageServer {
-    readonly defaultLSType: LanguageServerType;
+    readonly defaultLSType: DefaultLSType;
 }

src/client/extensionActivation.ts

@@ -26,6 +26,7 @@ import {
     IDisposableRegistry,
     IExperimentService,
     IExperimentsManager,
+    IExtensions,
     IOutputChannel,
 } from './common/types';
 import { noop } from './common/utils/misc';
@@ -61,7 +62,7 @@ import * as pythonEnvironments from './pythonEnvironments';
 
 import { ActivationResult, ExtensionState } from './components';
 import { Components } from './extensionInit';
-import { setDefaultLanguageServerByExperiment } from './common/experiments/helpers';
+import { setDefaultLanguageServer } from './activation/common/defaultlanguageServer';
 
 export async function activateComponents(
     // `ext` is passed to any extra activation funcs.
@@ -131,7 +132,8 @@ async function activateLegacy(ext: ExtensionState): Promise<ActivationResult> {
     await experimentService.activate();
 
     const workspaceService = serviceContainer.get<IWorkspaceService>(IWorkspaceService);
-    await setDefaultLanguageServerByExperiment(experimentService, workspaceService, serviceManager);
+    const extensions = serviceContainer.get<IExtensions>(IExtensions);
+    await setDefaultLanguageServer(experimentService, extensions, serviceManager);
 
     const configuration = serviceManager.get<IConfigurationService>(IConfigurationService);
     // We should start logging using the log level as soon as possible, so set it as soon as we can access the level.

src/test/activation/defaultLanguageServer.unit.test.ts

@@ -0,0 +1,103 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+'use strict';
+
+import { expect } from 'chai';
+import { anything, instance, mock, when, verify } from 'ts-mockito';
+import { Extension } from 'vscode';
+import { setDefaultLanguageServer } from '../../client/activation/common/defaultlanguageServer';
+import { LanguageServerType } from '../../client/activation/types';
+import { PYLANCE_EXTENSION_ID } from '../../client/common/constants';
+import { JediLSP } from '../../client/common/experiments/groups';
+import { ExperimentService } from '../../client/common/experiments/service';
+import { IDefaultLanguageServer, IExperimentService, IExtensions } from '../../client/common/types';
+import { ServiceManager } from '../../client/ioc/serviceManager';
+import { IServiceManager } from '../../client/ioc/types';
+
+suite('Activation - setDefaultLanguageServer()', () => {
+    let experimentService: IExperimentService;
+    let extensions: IExtensions;
+    let extension: Extension<unknown>;
+    let serviceManager: IServiceManager;
+    setup(() => {
+        experimentService = mock(ExperimentService);
+        extensions = mock();
+        extension = mock();
+        serviceManager = mock(ServiceManager);
+    });
+
+    test('Pylance not installed and NOT in experiment', async () => {
+        let defaultServerType;
+
+        when(extensions.getExtension(PYLANCE_EXTENSION_ID)).thenReturn(undefined);
+        when(experimentService.inExperiment(JediLSP.experiment)).thenResolve(false);
+        when(serviceManager.addSingletonInstance<IDefaultLanguageServer>(IDefaultLanguageServer, anything())).thenCall(
+            (_symbol, value: IDefaultLanguageServer) => {
+                defaultServerType = value.defaultLSType;
+            },
+        );
+
+        await setDefaultLanguageServer(instance(experimentService), instance(extensions), instance(serviceManager));
+
+        verify(extensions.getExtension(PYLANCE_EXTENSION_ID)).once();
+        verify(experimentService.inExperiment(JediLSP.experiment)).once();
+        verify(serviceManager.addSingletonInstance<IDefaultLanguageServer>(IDefaultLanguageServer, anything())).once();
+        expect(defaultServerType).to.equal(LanguageServerType.Jedi);
+    });
+
+    test('Pylance not installed and in experiment', async () => {
+        let defaultServerType;
+        when(extensions.getExtension(PYLANCE_EXTENSION_ID)).thenReturn(undefined);
+        when(experimentService.inExperiment(JediLSP.experiment)).thenResolve(true);
+        when(serviceManager.addSingletonInstance<IDefaultLanguageServer>(IDefaultLanguageServer, anything())).thenCall(
+            (_symbol, value: IDefaultLanguageServer) => {
+                defaultServerType = value.defaultLSType;
+            },
+        );
+
+        await setDefaultLanguageServer(instance(experimentService), instance(extensions), instance(serviceManager));
+
+        verify(extensions.getExtension(PYLANCE_EXTENSION_ID)).once();
+        verify(experimentService.inExperiment(JediLSP.experiment)).once();
+        verify(serviceManager.addSingletonInstance<IDefaultLanguageServer>(IDefaultLanguageServer, anything())).once();
+        expect(defaultServerType).to.equal(LanguageServerType.JediLSP);
+    });
+
+    test('Pylance installed and NOT in experiment', async () => {
+        let defaultServerType;
+
+        when(extensions.getExtension(PYLANCE_EXTENSION_ID)).thenReturn(instance(extension));
+        when(experimentService.inExperiment(JediLSP.experiment)).thenResolve(false);
+        when(serviceManager.addSingletonInstance<IDefaultLanguageServer>(IDefaultLanguageServer, anything())).thenCall(
+            (_symbol, value: IDefaultLanguageServer) => {
+                defaultServerType = value.defaultLSType;
+            },
+        );
+
+        await setDefaultLanguageServer(instance(experimentService), instance(extensions), instance(serviceManager));
+
+        verify(extensions.getExtension(PYLANCE_EXTENSION_ID)).once();
+        verify(experimentService.inExperiment(JediLSP.experiment)).never();
+        verify(serviceManager.addSingletonInstance<IDefaultLanguageServer>(IDefaultLanguageServer, anything())).once();
+        expect(defaultServerType).to.equal(LanguageServerType.Node);
+    });
+
+    test('Pylance installed and in experiment', async () => {
+        let defaultServerType;
+        when(extensions.getExtension(PYLANCE_EXTENSION_ID)).thenReturn(instance(extension));
+        when(experimentService.inExperiment(JediLSP.experiment)).thenResolve(true);
+        when(serviceManager.addSingletonInstance<IDefaultLanguageServer>(IDefaultLanguageServer, anything())).thenCall(
+            (_symbol, value: IDefaultLanguageServer) => {
+                defaultServerType = value.defaultLSType;
+            },
+        );
+
+        await setDefaultLanguageServer(instance(experimentService), instance(extensions), instance(serviceManager));
+
+        verify(extensions.getExtension(PYLANCE_EXTENSION_ID)).once();
+        verify(experimentService.inExperiment(JediLSP.experiment)).never();
+        verify(serviceManager.addSingletonInstance<IDefaultLanguageServer>(IDefaultLanguageServer, anything())).once();
+        expect(defaultServerType).to.equal(LanguageServerType.Node);
+    });
+});