Skip to content

Commit c41bfb3

Browse files
jakebaileyjakebailey
authored
Add "Default" LS setting, which picks between Jedi/Pylance (#16155)
1 parent c46c583 commit c41bfb3

File tree

11 files changed

+193
-155
lines changed

11 files changed

+193
-155
lines changed

‎news/1 Enhancements/16157.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Add a "Default" language server option, which dynamically chooses which language server to use.

‎package.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1250,13 +1250,14 @@
12501250
"python.languageServer": {
12511251
"type": "string",
12521252
"enum": [
1253+
"Default",
12531254
"Jedi",
12541255
"JediLSP",
12551256
"Pylance",
12561257
"Microsoft",
12571258
"None"
12581259
],
1259-
"default": "Jedi",
1260+
"default": "Default",
12601261
"description": "Defines type of the language server.",
12611262
"scope": "window"
12621263
},

‎src/client/activation/activationService.ts‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,13 @@ export class LanguageServerExtensionActivationService
240240
}
241241
}
242242

243+
if (serverType === LanguageServerType.Node && interpreter && interpreter.version) {
244+
if (interpreter.version.major < 3) {
245+
sendTelemetryEvent(EventName.JEDI_FALLBACK);
246+
serverType = LanguageServerType.Jedi;
247+
}
248+
}
249+
243250
this.sendTelemetryForChosenLanguageServer(serverType).ignoreErrors();
244251

245252
await this.logStartup(serverType);
@@ -310,7 +317,7 @@ export class LanguageServerExtensionActivationService
310317
const configurationService = this.serviceContainer.get<IConfigurationService>(IConfigurationService);
311318
const serverType = configurationService.getSettings(this.resource).languageServer;
312319
if (serverType === LanguageServerType.Node) {
313-
return 'shared-ls';
320+
return LanguageServerType.Node;
314321
}
315322

316323
const resourcePortion = this.workspaceService.getWorkspaceFolderIdentifier(

‎src/client/activation/common/defaultlanguageServer.ts‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
import { injectable } from 'inversify';
5+
import { PYLANCE_EXTENSION_ID } from '../../common/constants';
6+
import { JediLSP } from '../../common/experiments/groups';
7+
import { IDefaultLanguageServer, IExperimentService, IExtensions, DefaultLSType } from '../../common/types';
8+
import { IServiceManager } from '../../ioc/types';
9+
import { ILSExtensionApi } from '../node/languageServerFolderService';
10+
import { LanguageServerType } from '../types';
11+
12+
@injectable()
13+
class DefaultLanguageServer implements IDefaultLanguageServer {
14+
public readonly defaultLSType: DefaultLSType;
15+
16+
constructor(defaultServer: DefaultLSType) {
17+
this.defaultLSType = defaultServer;
18+
}
19+
}
20+
21+
export async function setDefaultLanguageServer(
22+
experimentService: IExperimentService,
23+
extensions: IExtensions,
24+
serviceManager: IServiceManager,
25+
): Promise<void> {
26+
const lsType = await getDefaultLanguageServer(experimentService, extensions);
27+
serviceManager.addSingletonInstance<IDefaultLanguageServer>(
28+
IDefaultLanguageServer,
29+
new DefaultLanguageServer(lsType),
30+
);
31+
}
32+
33+
async function getDefaultLanguageServer(
34+
experimentService: IExperimentService,
35+
extensions: IExtensions,
36+
): Promise<DefaultLSType> {
37+
if (extensions.getExtension<ILSExtensionApi>(PYLANCE_EXTENSION_ID)) {
38+
return LanguageServerType.Node;
39+
}
40+
41+
return (await experimentService.inExperiment(JediLSP.experiment))
42+
? LanguageServerType.JediLSP
43+
: LanguageServerType.Jedi;
44+
}

‎src/client/common/configSettings.ts‎

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ export class PythonSettings implements IPythonSettings {
167167
private readonly experimentsManager?: IExperimentsManager,
168168
private readonly interpreterPathService?: IInterpreterPathService,
169169
private readonly interpreterSecurityService?: IInterpreterSecurityService,
170-
private readonly defaultJedi?: IDefaultLanguageServer,
170+
private readonly defaultLS?: IDefaultLanguageServer,
171171
) {
172172
this.workspace = workspace || new WorkspaceService();
173173
this.workspaceRoot = workspaceFolder;
@@ -181,7 +181,7 @@ export class PythonSettings implements IPythonSettings {
181181
experimentsManager?: IExperimentsManager,
182182
interpreterPathService?: IInterpreterPathService,
183183
interpreterSecurityService?: IInterpreterSecurityService,
184-
defaultJedi?: IDefaultLanguageServer,
184+
defaultLS?: IDefaultLanguageServer,
185185
): PythonSettings {
186186
workspace = workspace || new WorkspaceService();
187187
const workspaceFolderUri = PythonSettings.getSettingsUriAndTarget(resource, workspace).uri;
@@ -195,7 +195,7 @@ export class PythonSettings implements IPythonSettings {
195195
experimentsManager,
196196
interpreterPathService,
197197
interpreterSecurityService,
198-
defaultJedi,
198+
defaultLS,
199199
);
200200
PythonSettings.pythonSettings.set(workspaceFolderKey, settings);
201201
// Pass null to avoid VSC from complaining about not passing in a value.
@@ -284,14 +284,23 @@ export class PythonSettings implements IPythonSettings {
284284

285285
this.useIsolation = systemVariables.resolveAny(pythonSettings.get<boolean>('useIsolation', true))!;
286286

287-
const defaultServer = this.defaultJedi
288-
? this.defaultJedi.defaultLSType
289-
: pythonSettings.get<LanguageServerType>('languageServer');
290-
let ls = defaultServer ?? LanguageServerType.Jedi;
291-
ls = systemVariables.resolveAny(ls);
292-
if (!Object.values(LanguageServerType).includes(ls)) {
293-
ls = LanguageServerType.Jedi;
287+
// Get as a string and verify; don't just accept.
288+
let userLS = pythonSettings.get<string>('languageServer');
289+
userLS = systemVariables.resolveAny(userLS);
290+
291+
let ls: LanguageServerType;
292+
293+
// Validate the user's input; if invalid, set it to the default.
294+
if (
295+
!userLS ||
296+
userLS === 'Default' ||
297+
!Object.values(LanguageServerType).includes(userLS as LanguageServerType)
298+
) {
299+
ls = this.defaultLS?.defaultLSType ?? LanguageServerType.Jedi;
300+
} else {
301+
ls = userLS as LanguageServerType;
294302
}
303+
295304
this.languageServer = ls;
296305

297306
this.jediPath = systemVariables.resolveAny(pythonSettings.get<string>('jediPath'))!;

‎src/client/common/configuration/service.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,15 +34,15 @@ export class ConfigurationService implements IConfigurationService {
3434
const interpreterSecurityService = this.serviceContainer.get<IInterpreterSecurityService>(
3535
IInterpreterSecurityService,
3636
);
37-
const defaultJedi = this.serviceContainer.tryGet<IDefaultLanguageServer>(IDefaultLanguageServer);
37+
const defaultLS = this.serviceContainer.tryGet<IDefaultLanguageServer>(IDefaultLanguageServer);
3838
return PythonSettings.getInstance(
3939
resource,
4040
InterpreterAutoSelectionService,
4141
this.workspaceService,
4242
experiments,
4343
interpreterPathService,
4444
interpreterSecurityService,
45-
defaultJedi,
45+
defaultLS,
4646
);
4747
}
4848

‎src/client/common/experiments/helpers.ts‎

Lines changed: 2 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,8 @@
33

44
'use strict';
55

6-
import { injectable } from 'inversify';
7-
import { LanguageServerType } from '../../activation/types';
8-
import { IServiceManager } from '../../ioc/types';
9-
import { IWorkspaceService } from '../application/types';
10-
import { IDefaultLanguageServer, IExperimentService } from '../types';
11-
import { DiscoveryVariants, JediLSP } from './groups';
6+
import { IExperimentService } from '../types';
7+
import { DiscoveryVariants } from './groups';
128

139
export async function inDiscoveryExperiment(experimentService: IExperimentService): Promise<boolean> {
1410
const results = await Promise.all([
@@ -17,41 +13,3 @@ export async function inDiscoveryExperiment(experimentService: IExperimentServic
1713
]);
1814
return results.includes(true);
1915
}
20-
21-
@injectable()
22-
class DefaultLanguageServer implements IDefaultLanguageServer {
23-
public readonly defaultLSType: LanguageServerType.Jedi | LanguageServerType.JediLSP;
24-
25-
constructor(defaultServer: LanguageServerType.Jedi | LanguageServerType.JediLSP) {
26-
this.defaultLSType = defaultServer;
27-
}
28-
}
29-
30-
export async function setDefaultLanguageServerByExperiment(
31-
experimentService: IExperimentService,
32-
workspaceService: IWorkspaceService,
33-
serviceManager: IServiceManager,
34-
): Promise<void> {
35-
const settings = workspaceService.getConfiguration('python');
36-
const lsSetting = settings.inspect('languageServer');
37-
if (lsSetting) {
38-
if (
39-
lsSetting.globalValue ||
40-
lsSetting.globalLanguageValue ||
41-
lsSetting.workspaceFolderValue ||
42-
lsSetting.workspaceFolderLanguageValue ||
43-
lsSetting.workspaceValue ||
44-
lsSetting.workspaceLanguageValue
45-
) {
46-
return Promise.resolve();
47-
}
48-
}
49-
const lsType = (await experimentService.inExperiment(JediLSP.experiment))
50-
? LanguageServerType.JediLSP
51-
: LanguageServerType.Jedi;
52-
serviceManager.addSingletonInstance<IDefaultLanguageServer>(
53-
IDefaultLanguageServer,
54-
new DefaultLanguageServer(lsType),
55-
);
56-
return Promise.resolve();
57-
}

‎src/client/common/types.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -589,8 +589,10 @@ export interface IInterpreterPathService {
589589
copyOldInterpreterStorageValuesToNew(resource: Uri | undefined): Promise<void>;
590590
}
591591

592+
export type DefaultLSType = LanguageServerType.Jedi | LanguageServerType.JediLSP | LanguageServerType.Node;
593+
592594
/**
593-
* Interface used to retrieve the default language server to use when in experiment
595+
* Interface used to retrieve the default language server.
594596
*
595597
* Note: This is added to get around a problem that the config service is not `async`.
596598
* Adding experiment check there would mean touching the entire extension. For simplicity
@@ -599,5 +601,5 @@ export interface IInterpreterPathService {
599601
export const IDefaultLanguageServer = Symbol('IDefaultLanguageServer');
600602

601603
export interface IDefaultLanguageServer {
602-
readonly defaultLSType: LanguageServerType;
604+
readonly defaultLSType: DefaultLSType;
603605
}

‎src/client/extensionActivation.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import {
2626
IDisposableRegistry,
2727
IExperimentService,
2828
IExperimentsManager,
29+
IExtensions,
2930
IOutputChannel,
3031
} from './common/types';
3132
import { noop } from './common/utils/misc';
@@ -61,7 +62,7 @@ import * as pythonEnvironments from './pythonEnvironments';
6162

6263
import { ActivationResult, ExtensionState } from './components';
6364
import { Components } from './extensionInit';
64-
import { setDefaultLanguageServerByExperiment } from './common/experiments/helpers';
65+
import { setDefaultLanguageServer } from './activation/common/defaultlanguageServer';
6566

6667
export async function activateComponents(
6768
// `ext` is passed to any extra activation funcs.
@@ -131,7 +132,8 @@ async function activateLegacy(ext: ExtensionState): Promise<ActivationResult> {
131132
await experimentService.activate();
132133

133134
const workspaceService = serviceContainer.get<IWorkspaceService>(IWorkspaceService);
134-
await setDefaultLanguageServerByExperiment(experimentService, workspaceService, serviceManager);
135+
const extensions = serviceContainer.get<IExtensions>(IExtensions);
136+
await setDefaultLanguageServer(experimentService, extensions, serviceManager);
135137

136138
const configuration = serviceManager.get<IConfigurationService>(IConfigurationService);
137139
// 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‎

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
'use strict';
5+
6+
import { expect } from 'chai';
7+
import { anything, instance, mock, when, verify } from 'ts-mockito';
8+
import { Extension } from 'vscode';
9+
import { setDefaultLanguageServer } from '../../client/activation/common/defaultlanguageServer';
10+
import { LanguageServerType } from '../../client/activation/types';
11+
import { PYLANCE_EXTENSION_ID } from '../../client/common/constants';
12+
import { JediLSP } from '../../client/common/experiments/groups';
13+
import { ExperimentService } from '../../client/common/experiments/service';
14+
import { IDefaultLanguageServer, IExperimentService, IExtensions } from '../../client/common/types';
15+
import { ServiceManager } from '../../client/ioc/serviceManager';
16+
import { IServiceManager } from '../../client/ioc/types';
17+
18+
suite('Activation - setDefaultLanguageServer()', () => {
19+
let experimentService: IExperimentService;
20+
let extensions: IExtensions;
21+
let extension: Extension<unknown>;
22+
let serviceManager: IServiceManager;
23+
setup(() => {
24+
experimentService = mock(ExperimentService);
25+
extensions = mock();
26+
extension = mock();
27+
serviceManager = mock(ServiceManager);
28+
});
29+
30+
test('Pylance not installed and NOT in experiment', async () => {
31+
let defaultServerType;
32+
33+
when(extensions.getExtension(PYLANCE_EXTENSION_ID)).thenReturn(undefined);
34+
when(experimentService.inExperiment(JediLSP.experiment)).thenResolve(false);
35+
when(serviceManager.addSingletonInstance<IDefaultLanguageServer>(IDefaultLanguageServer, anything())).thenCall(
36+
(_symbol, value: IDefaultLanguageServer) => {
37+
defaultServerType = value.defaultLSType;
38+
},
39+
);
40+
41+
await setDefaultLanguageServer(instance(experimentService), instance(extensions), instance(serviceManager));
42+
43+
verify(extensions.getExtension(PYLANCE_EXTENSION_ID)).once();
44+
verify(experimentService.inExperiment(JediLSP.experiment)).once();
45+
verify(serviceManager.addSingletonInstance<IDefaultLanguageServer>(IDefaultLanguageServer, anything())).once();
46+
expect(defaultServerType).to.equal(LanguageServerType.Jedi);
47+
});
48+
49+
test('Pylance not installed and in experiment', async () => {
50+
let defaultServerType;
51+
when(extensions.getExtension(PYLANCE_EXTENSION_ID)).thenReturn(undefined);
52+
when(experimentService.inExperiment(JediLSP.experiment)).thenResolve(true);
53+
when(serviceManager.addSingletonInstance<IDefaultLanguageServer>(IDefaultLanguageServer, anything())).thenCall(
54+
(_symbol, value: IDefaultLanguageServer) => {
55+
defaultServerType = value.defaultLSType;
56+
},
57+
);
58+
59+
await setDefaultLanguageServer(instance(experimentService), instance(extensions), instance(serviceManager));
60+
61+
verify(extensions.getExtension(PYLANCE_EXTENSION_ID)).once();
62+
verify(experimentService.inExperiment(JediLSP.experiment)).once();
63+
verify(serviceManager.addSingletonInstance<IDefaultLanguageServer>(IDefaultLanguageServer, anything())).once();
64+
expect(defaultServerType).to.equal(LanguageServerType.JediLSP);
65+
});
66+
67+
test('Pylance installed and NOT in experiment', async () => {
68+
let defaultServerType;
69+
70+
when(extensions.getExtension(PYLANCE_EXTENSION_ID)).thenReturn(instance(extension));
71+
when(experimentService.inExperiment(JediLSP.experiment)).thenResolve(false);
72+
when(serviceManager.addSingletonInstance<IDefaultLanguageServer>(IDefaultLanguageServer, anything())).thenCall(
73+
(_symbol, value: IDefaultLanguageServer) => {
74+
defaultServerType = value.defaultLSType;
75+
},
76+
);
77+
78+
await setDefaultLanguageServer(instance(experimentService), instance(extensions), instance(serviceManager));
79+
80+
verify(extensions.getExtension(PYLANCE_EXTENSION_ID)).once();
81+
verify(experimentService.inExperiment(JediLSP.experiment)).never();
82+
verify(serviceManager.addSingletonInstance<IDefaultLanguageServer>(IDefaultLanguageServer, anything())).once();
83+
expect(defaultServerType).to.equal(LanguageServerType.Node);
84+
});
85+
86+
test('Pylance installed and in experiment', async () => {
87+
let defaultServerType;
88+
when(extensions.getExtension(PYLANCE_EXTENSION_ID)).thenReturn(instance(extension));
89+
when(experimentService.inExperiment(JediLSP.experiment)).thenResolve(true);
90+
when(serviceManager.addSingletonInstance<IDefaultLanguageServer>(IDefaultLanguageServer, anything())).thenCall(
91+
(_symbol, value: IDefaultLanguageServer) => {
92+
defaultServerType = value.defaultLSType;
93+
},
94+
);
95+
96+
await setDefaultLanguageServer(instance(experimentService), instance(extensions), instance(serviceManager));
97+
98+
verify(extensions.getExtension(PYLANCE_EXTENSION_ID)).once();
99+
verify(experimentService.inExperiment(JediLSP.experiment)).never();
100+
verify(serviceManager.addSingletonInstance<IDefaultLanguageServer>(IDefaultLanguageServer, anything())).once();
101+
expect(defaultServerType).to.equal(LanguageServerType.Node);
102+
});
103+
});

0 commit comments

Comments
 (0)