diff --git a/src/client/application/diagnostics/checks/jediPython27NotSupported.ts b/src/client/application/diagnostics/checks/jediPython27NotSupported.ts index 79a0c08936aa..cc53c49689bc 100644 --- a/src/client/application/diagnostics/checks/jediPython27NotSupported.ts +++ b/src/client/application/diagnostics/checks/jediPython27NotSupported.ts @@ -48,6 +48,8 @@ export class JediPython27NotSupportedDiagnosticService extends BaseDiagnosticsSe const interpreter = await this.interpreterService.getActiveInterpreter(resource); const { languageServer } = this.configurationService.getSettings(resource); + await this.updateLanguageServerSetting(resource); + // We don't need to check for JediLSP here, because we retrieve the setting from the configuration service, // Which already switched the JediLSP option to Jedi. if (interpreter && (interpreter.version?.major ?? 0) < 3 && languageServer === LanguageServerType.Jedi) { @@ -66,8 +68,6 @@ export class JediPython27NotSupportedDiagnosticService extends BaseDiagnosticsSe return; } - this.updateLanguageServerSetting(diagnostic.resource); - const commandFactory = this.serviceContainer.get(IDiagnosticsCommandFactory); const options = [ { @@ -82,7 +82,7 @@ export class JediPython27NotSupportedDiagnosticService extends BaseDiagnosticsSe await this.messageService.handle(diagnostic, { commandPrompts: options }); } - private updateLanguageServerSetting(resource: Resource): void { + private async updateLanguageServerSetting(resource: Resource): Promise { // Update settings.json value to Jedi if it's JediLSP. const settings = this.workspaceService .getConfiguration('python', resource) @@ -98,6 +98,11 @@ export class JediPython27NotSupportedDiagnosticService extends BaseDiagnosticsSe return; } - this.configurationService.updateSetting('languageServer', LanguageServerType.Jedi, resource, configTarget); + await this.configurationService.updateSetting( + 'languageServer', + LanguageServerType.Jedi, + resource, + configTarget, + ); } } diff --git a/src/test/application/diagnostics/checks/jediPython27NotSupported.unit.test.ts b/src/test/application/diagnostics/checks/jediPython27NotSupported.unit.test.ts index bd892d0bdb99..21c60d7860fe 100644 --- a/src/test/application/diagnostics/checks/jediPython27NotSupported.unit.test.ts +++ b/src/test/application/diagnostics/checks/jediPython27NotSupported.unit.test.ts @@ -25,7 +25,7 @@ import { import { IWorkspaceService } from '../../../../client/common/application/types'; import { WorkspaceService } from '../../../../client/common/application/workspace'; import { ConfigurationService } from '../../../../client/common/configuration/service'; -import { IConfigurationService } from '../../../../client/common/types'; +import { IConfigurationService, IPythonSettings } from '../../../../client/common/types'; import { Python27Support } from '../../../../client/common/utils/localize'; import { IInterpreterService } from '../../../../client/interpreter/contracts'; import { IServiceContainer } from '../../../../client/ioc/types'; @@ -34,6 +34,25 @@ suite('Application Diagnostics - Jedi with Python 2.7 deprecated', () => { suite('Diagnostics', () => { const resource = Uri.file('test.py'); + function createConfigurationAndWorkspaceServices( + languageServer: LanguageServerType, + ): { configurationService: IConfigurationService; workspaceService: IWorkspaceService } { + const configurationService = ({ + getSettings: () => ({ languageServer }), + updateSetting: () => Promise.resolve(), + } as unknown) as IConfigurationService; + + const workspaceService = ({ + getConfiguration: () => ({ + inspect: () => ({ + workspaceValue: languageServer, + }), + }), + } as unknown) as IWorkspaceService; + + return { configurationService, workspaceService }; + } + test('Should return an empty diagnostics array if the active interpreter version is Python 3', async () => { const interpreterService = { getActiveInterpreter: () => @@ -46,16 +65,16 @@ suite('Application Diagnostics - Jedi with Python 2.7 deprecated', () => { }), } as IInterpreterService; - const configurationService = { - getSettings: () => ({ languageServer: LanguageServerType.Jedi }), - } as IConfigurationService; + const { configurationService, workspaceService } = createConfigurationAndWorkspaceServices( + LanguageServerType.Jedi, + ); const service = new JediPython27NotSupportedDiagnosticService( ({ get: () => ({}), } as unknown) as IServiceContainer, interpreterService, - {} as IWorkspaceService, + workspaceService, configurationService, {} as IDiagnosticHandlerService, [], @@ -71,16 +90,16 @@ suite('Application Diagnostics - Jedi with Python 2.7 deprecated', () => { getActiveInterpreter: () => Promise.resolve(undefined), } as IInterpreterService; - const configurationService = { - getSettings: () => ({ languageServer: LanguageServerType.Jedi }), - } as IConfigurationService; + const { configurationService, workspaceService } = createConfigurationAndWorkspaceServices( + LanguageServerType.Jedi, + ); const service = new JediPython27NotSupportedDiagnosticService( ({ get: () => ({}), } as unknown) as IServiceContainer, interpreterService, - {} as IWorkspaceService, + workspaceService, configurationService, {} as IDiagnosticHandlerService, [], @@ -103,16 +122,16 @@ suite('Application Diagnostics - Jedi with Python 2.7 deprecated', () => { }), } as IInterpreterService; - const configurationService = { - getSettings: () => ({ languageServer: LanguageServerType.Jedi }), - } as IConfigurationService; + const { configurationService, workspaceService } = createConfigurationAndWorkspaceServices( + LanguageServerType.Jedi, + ); const service = new JediPython27NotSupportedDiagnosticService( ({ get: () => ({}), } as unknown) as IServiceContainer, interpreterService, - {} as IWorkspaceService, + workspaceService, configurationService, {} as IDiagnosticHandlerService, [], @@ -137,16 +156,16 @@ suite('Application Diagnostics - Jedi with Python 2.7 deprecated', () => { }), } as IInterpreterService; - const configurationService = { - getSettings: () => ({ languageServer: LanguageServerType.Jedi }), - } as IConfigurationService; + const { configurationService, workspaceService } = createConfigurationAndWorkspaceServices( + LanguageServerType.Jedi, + ); const service = new JediPython27NotSupportedDiagnosticService( ({ get: () => ({}), } as unknown) as IServiceContainer, interpreterService, - {} as IWorkspaceService, + workspaceService, configurationService, {} as IDiagnosticHandlerService, [], @@ -171,16 +190,16 @@ suite('Application Diagnostics - Jedi with Python 2.7 deprecated', () => { }), } as IInterpreterService; - const configurationService = { - getSettings: () => ({ languageServer: LanguageServerType.Node }), - } as IConfigurationService; + const { configurationService, workspaceService } = createConfigurationAndWorkspaceServices( + LanguageServerType.Node, + ); const service = new JediPython27NotSupportedDiagnosticService( ({ get: () => ({}), } as unknown) as IServiceContainer, interpreterService, - {} as IWorkspaceService, + workspaceService, configurationService, {} as IDiagnosticHandlerService, [], @@ -203,16 +222,16 @@ suite('Application Diagnostics - Jedi with Python 2.7 deprecated', () => { }), } as IInterpreterService; - const configurationService = { - getSettings: () => ({ languageServer: LanguageServerType.None }), - } as IConfigurationService; + const { configurationService, workspaceService } = createConfigurationAndWorkspaceServices( + LanguageServerType.None, + ); const service = new JediPython27NotSupportedDiagnosticService( ({ get: () => ({}), } as unknown) as IServiceContainer, interpreterService, - {} as IWorkspaceService, + workspaceService, configurationService, {} as IDiagnosticHandlerService, [], @@ -224,23 +243,15 @@ suite('Application Diagnostics - Jedi with Python 2.7 deprecated', () => { }); }); - suite('Handler', () => { - class TestJediPython27NotSupportedDiagnosticService extends JediPython27NotSupportedDiagnosticService { - // eslint-disable-next-line class-methods-use-this - public static clear() { - while (BaseDiagnosticsService.handledDiagnosticCodeKeys.length > 0) { - BaseDiagnosticsService.handledDiagnosticCodeKeys.shift(); - } - } - } - - let services: { - [key: string]: IWorkspaceService | IDiagnosticFilterService | IDiagnosticsCommandFactory; - }; - let serviceContainer: IServiceContainer; + suite('Setting update', () => { + const resource = Uri.file('test.py'); + let workspaceService: IWorkspaceService; let getConfigurationStub: sinon.SinonStub; let updateSettingStub: sinon.SinonStub; - let handleMessageStub: sinon.SinonStub; + let serviceContainer: IServiceContainer; + let services: { + [key: string]: IWorkspaceService; + }; const interpreterService = { getActiveInterpreter: () => @@ -254,192 +265,225 @@ suite('Application Diagnostics - Jedi with Python 2.7 deprecated', () => { } as IInterpreterService; setup(() => { + serviceContainer = ({ + get: (serviceIdentifier: symbol) => services[serviceIdentifier.toString()] as IWorkspaceService, + tryGet: () => ({}), + } as unknown) as IServiceContainer; + + workspaceService = new WorkspaceService(); services = { - 'Symbol(IDiagnosticsCommandFactory)': { - createCommand: () => ({} as IDiagnosticCommand), - }, + 'Symbol(IWorkspaceService)': workspaceService, }; - serviceContainer = { - get: (serviceIdentifier: symbol) => - services[serviceIdentifier.toString()] as - | IWorkspaceService - | IDiagnosticFilterService - | IDiagnosticsCommandFactory, - } as IServiceContainer; getConfigurationStub = sinon.stub(WorkspaceService.prototype, 'getConfiguration'); updateSettingStub = sinon.stub(ConfigurationService.prototype, 'updateSetting'); - handleMessageStub = sinon.stub(DiagnosticCommandPromptHandlerService.prototype, 'handle'); + + const getSettingsStub = sinon.stub(ConfigurationService.prototype, 'getSettings'); + getSettingsStub.returns(({ + getSettings: () => ({ languageServer: LanguageServerType.Jedi }), + } as unknown) as IPythonSettings); }); teardown(() => { sinon.restore(); - TestJediPython27NotSupportedDiagnosticService.clear(); }); - test('Handling an empty diagnostics array does not update the setting and does not display a prompt', async () => { - const service = new TestJediPython27NotSupportedDiagnosticService( - serviceContainer, + test('Running the diagnostic should update the workspace setting if set', async () => { + getConfigurationStub.returns({ + inspect: () => ({ + workspaceValue: LanguageServerType.JediLSP, + }), + }); + const configurationService = new ConfigurationService(serviceContainer); + + const service = new JediPython27NotSupportedDiagnosticService( + ({ + get: () => ({}), + } as unknown) as IServiceContainer, interpreterService, - {} as IWorkspaceService, - {} as IConfigurationService, + workspaceService, + configurationService, {} as IDiagnosticHandlerService, [], ); - await service.handle([]); + await service.diagnose(resource); - sinon.assert.notCalled(handleMessageStub); - sinon.assert.notCalled(getConfigurationStub); - sinon.assert.notCalled(updateSettingStub); + sinon.assert.calledOnce(getConfigurationStub); + sinon.assert.calledWith( + updateSettingStub, + 'languageServer', + LanguageServerType.Jedi, + resource, + ConfigurationTarget.Workspace, + ); }); - test('Handling a diagnostic that should be ignored does not update the setting and does not display a prompt', async () => { - const diagnosticHandlerService = new DiagnosticCommandPromptHandlerService(serviceContainer); - - services['Symbol(IDiagnosticFilterService)'] = ({ - shouldIgnoreDiagnostic: async () => Promise.resolve(true), - } as unknown) as IDiagnosticFilterService; + test('Running the diagnostic should update the global setting if set', async () => { + getConfigurationStub.returns({ + inspect: () => ({ + globalValue: LanguageServerType.JediLSP, + }), + }); + const configurationService = new ConfigurationService(serviceContainer); - const service = new TestJediPython27NotSupportedDiagnosticService( - serviceContainer, + const service = new JediPython27NotSupportedDiagnosticService( + ({ + get: () => ({}), + } as unknown) as IServiceContainer, interpreterService, - {} as IWorkspaceService, - {} as IConfigurationService, - diagnosticHandlerService, + workspaceService, + configurationService, + {} as IDiagnosticHandlerService, [], ); - await service.handle([new JediPython27NotSupportedDiagnostic('ignored', undefined)]); + await service.diagnose(resource); - sinon.assert.notCalled(handleMessageStub); - sinon.assert.notCalled(getConfigurationStub); - sinon.assert.notCalled(updateSettingStub); + sinon.assert.calledOnce(getConfigurationStub); + sinon.assert.calledWith( + updateSettingStub, + 'languageServer', + LanguageServerType.Jedi, + resource, + ConfigurationTarget.Global, + ); }); - test('Handling a diagnostic should show a prompt', async () => { + test('Running the diagnostic should not update the setting if not set in workspace or global scopes', async () => { getConfigurationStub.returns({ inspect: () => ({ - workspaceValue: LanguageServerType.JediLSP, + workspaceFolderValue: LanguageServerType.JediLSP, }), }); - const workspaceService = new WorkspaceService(); - services['Symbol(IWorkspaceService)'] = workspaceService; - - const diagnosticHandlerService = new DiagnosticCommandPromptHandlerService(serviceContainer); const configurationService = new ConfigurationService(serviceContainer); - services['Symbol(IDiagnosticFilterService)'] = ({ - shouldIgnoreDiagnostic: () => Promise.resolve(false), - } as unknown) as IDiagnosticFilterService; - - const service = new TestJediPython27NotSupportedDiagnosticService( - serviceContainer, + const service = new JediPython27NotSupportedDiagnosticService( + ({ + get: () => ({}), + } as unknown) as IServiceContainer, interpreterService, workspaceService, configurationService, - diagnosticHandlerService, + {} as IDiagnosticHandlerService, [], ); - const diagnostic = new JediPython27NotSupportedDiagnostic('diagnostic', undefined); + await service.diagnose(resource); - await service.handle([diagnostic]); - - sinon.assert.calledOnce(handleMessageStub); sinon.assert.calledOnce(getConfigurationStub); - sinon.assert.calledOnce(updateSettingStub); + sinon.assert.notCalled(updateSettingStub); }); - test('Handling a diagnostic should update the workspace setting if set', async () => { + test('Running the diagnostic should not update the setting if not set to Jedi LSP', async () => { getConfigurationStub.returns({ inspect: () => ({ - workspaceValue: LanguageServerType.JediLSP, + workspaceValue: LanguageServerType.Node, }), }); - const workspaceService = new WorkspaceService(); - services['Symbol(IWorkspaceService)'] = workspaceService; - - const diagnosticHandlerService = new DiagnosticCommandPromptHandlerService(serviceContainer); const configurationService = new ConfigurationService(serviceContainer); - services['Symbol(IDiagnosticFilterService)'] = ({ - shouldIgnoreDiagnostic: () => Promise.resolve(false), - } as unknown) as IDiagnosticFilterService; - - const service = new TestJediPython27NotSupportedDiagnosticService( - serviceContainer, + const service = new JediPython27NotSupportedDiagnosticService( + ({ + get: () => ({}), + } as unknown) as IServiceContainer, interpreterService, workspaceService, configurationService, - diagnosticHandlerService, + {} as IDiagnosticHandlerService, [], ); - const diagnostic = new JediPython27NotSupportedDiagnostic('diagnostic', undefined); - - await service.handle([diagnostic]); + await service.diagnose(resource); - sinon.assert.calledOnce(handleMessageStub); sinon.assert.calledOnce(getConfigurationStub); - sinon.assert.calledWith( - updateSettingStub, - 'languageServer', - LanguageServerType.Jedi, - undefined, - ConfigurationTarget.Workspace, - ); + sinon.assert.notCalled(updateSettingStub); }); + }); - test('Handling a diagnostic should update the global setting if set', async () => { - getConfigurationStub.returns({ - inspect: () => ({ - globalValue: LanguageServerType.JediLSP, + suite('Handler', () => { + class TestJediPython27NotSupportedDiagnosticService extends JediPython27NotSupportedDiagnosticService { + // eslint-disable-next-line class-methods-use-this + public static clear() { + while (BaseDiagnosticsService.handledDiagnosticCodeKeys.length > 0) { + BaseDiagnosticsService.handledDiagnosticCodeKeys.shift(); + } + } + } + + let services: { + [key: string]: IWorkspaceService | IDiagnosticFilterService | IDiagnosticsCommandFactory; + }; + let serviceContainer: IServiceContainer; + let handleMessageStub: sinon.SinonStub; + + const interpreterService = { + getActiveInterpreter: () => + Promise.resolve({ + version: { + major: 2, + minor: 7, + patch: 10, + }, }), - }); - const workspaceService = new WorkspaceService(); - services['Symbol(IWorkspaceService)'] = workspaceService; + } as IInterpreterService; + setup(() => { + services = { + 'Symbol(IDiagnosticsCommandFactory)': { + createCommand: () => ({} as IDiagnosticCommand), + }, + }; + serviceContainer = { + get: (serviceIdentifier: symbol) => + services[serviceIdentifier.toString()] as IDiagnosticFilterService | IDiagnosticsCommandFactory, + } as IServiceContainer; + + handleMessageStub = sinon.stub(DiagnosticCommandPromptHandlerService.prototype, 'handle'); + }); + + teardown(() => { + sinon.restore(); + TestJediPython27NotSupportedDiagnosticService.clear(); + }); + + test('Handling an empty diagnostics array does not display a prompt', async () => { + const service = new TestJediPython27NotSupportedDiagnosticService( + serviceContainer, + interpreterService, + {} as IWorkspaceService, + {} as IConfigurationService, + {} as IDiagnosticHandlerService, + [], + ); + + await service.handle([]); + + sinon.assert.notCalled(handleMessageStub); + }); + + test('Handling a diagnostic that should be ignored does not display a prompt', async () => { const diagnosticHandlerService = new DiagnosticCommandPromptHandlerService(serviceContainer); - const configurationService = new ConfigurationService(serviceContainer); services['Symbol(IDiagnosticFilterService)'] = ({ - shouldIgnoreDiagnostic: () => Promise.resolve(false), + shouldIgnoreDiagnostic: async () => Promise.resolve(true), } as unknown) as IDiagnosticFilterService; const service = new TestJediPython27NotSupportedDiagnosticService( serviceContainer, interpreterService, - workspaceService, - configurationService, + {} as IWorkspaceService, + {} as IConfigurationService, diagnosticHandlerService, [], ); - const diagnostic = new JediPython27NotSupportedDiagnostic('diagnostic', undefined); - - await service.handle([diagnostic]); + await service.handle([new JediPython27NotSupportedDiagnostic('ignored', undefined)]); - sinon.assert.calledOnce(handleMessageStub); - sinon.assert.calledOnce(getConfigurationStub); - sinon.assert.calledWith( - updateSettingStub, - 'languageServer', - LanguageServerType.Jedi, - undefined, - ConfigurationTarget.Global, - ); + sinon.assert.notCalled(handleMessageStub); }); - test('Handling a diagnostic should not update the setting if not set in workspace or global scopes', async () => { - getConfigurationStub.returns({ - inspect: () => ({ - workspaceFolderValue: LanguageServerType.JediLSP, - }), - }); - const workspaceService = new WorkspaceService(); - services['Symbol(IWorkspaceService)'] = workspaceService; - + test('Handling a diagnostic should show a prompt', async () => { const diagnosticHandlerService = new DiagnosticCommandPromptHandlerService(serviceContainer); const configurationService = new ConfigurationService(serviceContainer); @@ -450,7 +494,7 @@ suite('Application Diagnostics - Jedi with Python 2.7 deprecated', () => { const service = new TestJediPython27NotSupportedDiagnosticService( serviceContainer, interpreterService, - workspaceService, + {} as IWorkspaceService, configurationService, diagnosticHandlerService, [], @@ -461,8 +505,6 @@ suite('Application Diagnostics - Jedi with Python 2.7 deprecated', () => { await service.handle([diagnostic]); sinon.assert.calledOnce(handleMessageStub); - sinon.assert.calledOnce(getConfigurationStub); - sinon.assert.notCalled(updateSettingStub); }); }); });