diff --git a/news/1 Enhancements/16157.md b/news/1 Enhancements/16157.md new file mode 100644 index 000000000000..71e63a92aa0a --- /dev/null +++ b/news/1 Enhancements/16157.md @@ -0,0 +1 @@ +Add a "Default" language server option, which dynamically chooses which language server to use. diff --git a/package.json b/package.json index cf6cc359a9bf..3bd558dfdbd5 100644 --- a/package.json +++ b/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" }, diff --git a/src/client/activation/activationService.ts b/src/client/activation/activationService.ts index 4fc6273992cc..7f8b86300fb6 100644 --- a/src/client/activation/activationService.ts +++ b/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); const serverType = configurationService.getSettings(this.resource).languageServer; if (serverType === LanguageServerType.Node) { - return 'shared-ls'; + return LanguageServerType.Node; } const resourcePortion = this.workspaceService.getWorkspaceFolderIdentifier( diff --git a/src/client/activation/common/defaultlanguageServer.ts b/src/client/activation/common/defaultlanguageServer.ts new file mode 100644 index 000000000000..1ef129ba2235 --- /dev/null +++ b/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 { + const lsType = await getDefaultLanguageServer(experimentService, extensions); + serviceManager.addSingletonInstance( + IDefaultLanguageServer, + new DefaultLanguageServer(lsType), + ); +} + +async function getDefaultLanguageServer( + experimentService: IExperimentService, + extensions: IExtensions, +): Promise { + if (extensions.getExtension(PYLANCE_EXTENSION_ID)) { + return LanguageServerType.Node; + } + + return (await experimentService.inExperiment(JediLSP.experiment)) + ? LanguageServerType.JediLSP + : LanguageServerType.Jedi; +} diff --git a/src/client/common/configSettings.ts b/src/client/common/configSettings.ts index 193817bc74b3..c937147c72cb 100644 --- a/src/client/common/configSettings.ts +++ b/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('useIsolation', true))!; - const defaultServer = this.defaultJedi - ? this.defaultJedi.defaultLSType - : pythonSettings.get('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('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('jediPath'))!; diff --git a/src/client/common/configuration/service.ts b/src/client/common/configuration/service.ts index 81a1070ddbd2..4a0e0919bc32 100644 --- a/src/client/common/configuration/service.ts +++ b/src/client/common/configuration/service.ts @@ -34,7 +34,7 @@ export class ConfigurationService implements IConfigurationService { const interpreterSecurityService = this.serviceContainer.get( IInterpreterSecurityService, ); - const defaultJedi = this.serviceContainer.tryGet(IDefaultLanguageServer); + const defaultLS = this.serviceContainer.tryGet(IDefaultLanguageServer); return PythonSettings.getInstance( resource, InterpreterAutoSelectionService, @@ -42,7 +42,7 @@ export class ConfigurationService implements IConfigurationService { experiments, interpreterPathService, interpreterSecurityService, - defaultJedi, + defaultLS, ); } diff --git a/src/client/common/experiments/helpers.ts b/src/client/common/experiments/helpers.ts index ac4b162f0f82..a85a468f494f 100644 --- a/src/client/common/experiments/helpers.ts +++ b/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 { 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 { - 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, - new DefaultLanguageServer(lsType), - ); - return Promise.resolve(); -} diff --git a/src/client/common/types.ts b/src/client/common/types.ts index 27e4d00ab54b..3d9badf7eb8c 100644 --- a/src/client/common/types.ts +++ b/src/client/common/types.ts @@ -589,8 +589,10 @@ export interface IInterpreterPathService { copyOldInterpreterStorageValuesToNew(resource: Uri | undefined): Promise; } +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; } diff --git a/src/client/extensionActivation.ts b/src/client/extensionActivation.ts index 0292da87c1bb..ed7c0fe4d1b4 100644 --- a/src/client/extensionActivation.ts +++ b/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 { await experimentService.activate(); const workspaceService = serviceContainer.get(IWorkspaceService); - await setDefaultLanguageServerByExperiment(experimentService, workspaceService, serviceManager); + const extensions = serviceContainer.get(IExtensions); + await setDefaultLanguageServer(experimentService, extensions, serviceManager); const configuration = serviceManager.get(IConfigurationService); // We should start logging using the log level as soon as possible, so set it as soon as we can access the level. diff --git a/src/test/activation/defaultLanguageServer.unit.test.ts b/src/test/activation/defaultLanguageServer.unit.test.ts new file mode 100644 index 000000000000..4dd8b4eaf88e --- /dev/null +++ b/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; + 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, 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, 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, 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, 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, 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, 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, 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, anything())).once(); + expect(defaultServerType).to.equal(LanguageServerType.Node); + }); +}); diff --git a/src/test/common/experiments/helpers.unit.test.ts b/src/test/common/experiments/helpers.unit.test.ts index a9bc306e8ece..0b9f8a19bc75 100644 --- a/src/test/common/experiments/helpers.unit.test.ts +++ b/src/test/common/experiments/helpers.unit.test.ts @@ -4,20 +4,11 @@ 'use strict'; import { expect } from 'chai'; -import { anything, instance, mock, when, verify } from 'ts-mockito'; -import { LanguageServerType } from '../../../client/activation/types'; -import { IWorkspaceService } from '../../../client/common/application/types'; -import { WorkspaceService } from '../../../client/common/application/workspace'; -import { DiscoveryVariants, JediLSP } from '../../../client/common/experiments/groups'; -import { - inDiscoveryExperiment, - setDefaultLanguageServerByExperiment, -} from '../../../client/common/experiments/helpers'; +import { anything, instance, mock, when } from 'ts-mockito'; +import { DiscoveryVariants } from '../../../client/common/experiments/groups'; +import { inDiscoveryExperiment } from '../../../client/common/experiments/helpers'; import { ExperimentService } from '../../../client/common/experiments/service'; -import { IDefaultLanguageServer, IExperimentService } from '../../../client/common/types'; -import { ServiceManager } from '../../../client/ioc/serviceManager'; -import { IServiceManager } from '../../../client/ioc/types'; -import { MockWorkspaceConfiguration } from '../../startPage/mockWorkspaceConfig'; +import { IExperimentService } from '../../../client/common/types'; suite('Experiments - inDiscoveryExperiment()', () => { let experimentService: IExperimentService; @@ -43,83 +34,3 @@ suite('Experiments - inDiscoveryExperiment()', () => { expect(result).to.equal(false); }); }); - -suite('Experiments - setDefaultLanguageServerByExperiment()', () => { - let experimentService: IExperimentService; - let workspaceService: IWorkspaceService; - let serviceManager: IServiceManager; - setup(() => { - experimentService = mock(ExperimentService); - workspaceService = mock(WorkspaceService); - serviceManager = mock(ServiceManager); - }); - - test('languageServer set by user', async () => { - when(workspaceService.getConfiguration('python')).thenReturn( - new MockWorkspaceConfiguration({ - languageServer: { globalValue: LanguageServerType.Node }, - }), - ); - await setDefaultLanguageServerByExperiment( - instance(experimentService), - instance(workspaceService), - instance(serviceManager), - ); - - verify(workspaceService.getConfiguration('python')).once(); - verify(experimentService.inExperiment(JediLSP.experiment)).never(); - verify(serviceManager.addSingletonInstance(IDefaultLanguageServer, anything())).never(); - }); - - test('languageServer NOT set by user and NOT in experiment', async () => { - let defaultServerType; - when(workspaceService.getConfiguration('python')).thenReturn( - new MockWorkspaceConfiguration({ - languageServer: { defaultValue: LanguageServerType.Jedi }, - }), - ); - when(experimentService.inExperiment(JediLSP.experiment)).thenResolve(false); - when(serviceManager.addSingletonInstance(IDefaultLanguageServer, anything())).thenCall( - (_symbol, value: IDefaultLanguageServer) => { - defaultServerType = value.defaultLSType; - }, - ); - - await setDefaultLanguageServerByExperiment( - instance(experimentService), - instance(workspaceService), - instance(serviceManager), - ); - - verify(workspaceService.getConfiguration('python')).once(); - verify(experimentService.inExperiment(JediLSP.experiment)).once(); - verify(serviceManager.addSingletonInstance(IDefaultLanguageServer, anything())).once(); - expect(defaultServerType).to.equal(LanguageServerType.Jedi); - }); - - test('languageServer NOT set by user and in experiment', async () => { - let defaultServerType; - when(workspaceService.getConfiguration('python')).thenReturn( - new MockWorkspaceConfiguration({ - languageServer: { defaultValue: LanguageServerType.Jedi }, - }), - ); - when(experimentService.inExperiment(JediLSP.experiment)).thenResolve(true); - when(serviceManager.addSingletonInstance(IDefaultLanguageServer, anything())).thenCall( - (_symbol, value: IDefaultLanguageServer) => { - defaultServerType = value.defaultLSType; - }, - ); - - await setDefaultLanguageServerByExperiment( - instance(experimentService), - instance(workspaceService), - instance(serviceManager), - ); - - verify(workspaceService.getConfiguration('python')).once(); - verify(experimentService.inExperiment(JediLSP.experiment)).once(); - verify(serviceManager.addSingletonInstance(IDefaultLanguageServer, anything())).once(); - expect(defaultServerType).to.equal(LanguageServerType.JediLSP); - }); -});