From 04e0b3441647a894d73ba5bb36e45550b79de46f Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Tue, 5 May 2020 21:15:32 -0700 Subject: [PATCH 01/34] Fix path --- src/client/activation/node/languageClientFactory.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/activation/node/languageClientFactory.ts b/src/client/activation/node/languageClientFactory.ts index 8490c4910041..d607afdd5d31 100644 --- a/src/client/activation/node/languageClientFactory.ts +++ b/src/client/activation/node/languageClientFactory.ts @@ -20,7 +20,7 @@ export class NodeLanguageClientFactory implements ILanguageClientFactory { constructor( @inject(IFileSystem) private readonly fs: IFileSystem, @inject(ILanguageServerFolderService) private readonly languageServerFolderService: ILanguageServerFolderService - ) {} + ) { } public async createLanguageClient( resource: Resource, From e8640a29807ad5a64f108cb85f2585376e7ccde1 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Mon, 11 May 2020 21:11:11 -0700 Subject: [PATCH 02/34] Partial --- .github/ISSUE_TEMPLATE/1_ds_bug_report.md | 2 +- .github/ISSUE_TEMPLATE/2_bug_report.md | 1 - .github/release_plan.md | 2 +- .gitignore | 2 +- .vscode/settings.json | 2 +- CHANGELOG.md | 4 - news/.vscode/settings.json | 2 +- package.json | 14 +- src/client/activation/activationService.ts | 19 +- .../common/languageServerFolderService.ts | 20 +- .../languageServerFolderService.ts | 14 +- .../node/languageServerFolderService.ts | 5 +- src/client/activation/types.ts | 31 ++- src/client/common/configSettings.ts | 205 +++++++++--------- .../common/featureDeprecationManager.ts | 4 +- src/client/common/types.ts | 7 +- .../proposeLanguageServerBanner.ts | 7 +- src/client/linters/linterAvailability.ts | 9 +- src/client/linters/linterInfo.ts | 3 +- src/client/telemetry/index.ts | 6 +- .../testing/common/updateTestSettings.ts | 7 +- src/test/.vscode/settings.json | 6 +- .../activation/activationService.unit.test.ts | 152 ++----------- .../checks/updateTestSettings.unit.test.ts | 10 - src/test/common.ts | 4 +- .../configSettings.unit.test.ts | 8 +- .../datascience/dataScienceIocContainer.ts | 3 +- .../datascience/intellisense.unit.test.ts | 3 +- src/test/linters/common.ts | 3 +- .../linters/linter.availability.unit.test.ts | 29 +-- src/test/linters/linterinfo.unit.test.ts | 12 +- src/test/performance/settings.json | 2 +- src/test/performanceTest.ts | 3 +- src/test/smoke/common.ts | 5 +- src/test/smokeTest.ts | 3 +- 35 files changed, 243 insertions(+), 366 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/1_ds_bug_report.md b/.github/ISSUE_TEMPLATE/1_ds_bug_report.md index 37d7fc69cdec..20649615777e 100644 --- a/.github/ISSUE_TEMPLATE/1_ds_bug_report.md +++ b/.github/ISSUE_TEMPLATE/1_ds_bug_report.md @@ -33,7 +33,7 @@ _Please provide as much info as you readily know_ - **Jupyter server running:** Local | Remote | N/A - **Extension version:** 20YY.MM.#####-xxx - **VS Code version:** #.## -- **Setting python.jediEnabled:** true | false +- **Setting python.languageServer:** Jedi | Microsoft (V1) | Microsoft (V2) | None - **Python and/or Anaconda version:** #.#.# - **OS:** Windows | Mac | Linux (distro): - **Virtual environment:** conda | venv | virtualenv | N/A | ... diff --git a/.github/ISSUE_TEMPLATE/2_bug_report.md b/.github/ISSUE_TEMPLATE/2_bug_report.md index adbb8516ae32..339ed0cb6f90 100644 --- a/.github/ISSUE_TEMPLATE/2_bug_report.md +++ b/.github/ISSUE_TEMPLATE/2_bug_report.md @@ -15,7 +15,6 @@ labels: classify, type-bug - Type of virtual environment used (N/A | venv | virtualenv | conda | ...): XXX - Relevant/affected Python packages and their versions: XXX - Relevant/affected Python-related VS Code extensions and their versions: XXX -- Jedi or Language Server? (i.e. what is `"python.jediEnabled"` set to; more info #3977): XXX - Value of the `python.languageServer` setting: XXX ## Expected behaviour diff --git a/.github/release_plan.md b/.github/release_plan.md index e263cbf00c08..19f02da25d46 100644 --- a/.github/release_plan.md +++ b/.github/release_plan.md @@ -12,7 +12,7 @@ - [ ] Change the version in [`package.json`](https://github.com/Microsoft/vscode-python/blob/master/package.json) from a `-dev` suffix to `-rc` (🤖) - [ ] Run `npm install` to make sure [`package-lock.json`](https://github.com/Microsoft/vscode-python/blob/master/package.json) is up-to-date (🤖) - [ ] Update `requirements.txt` to point to latest release version of [ptvsd](https://github.com/microsoft/ptvsd). - - [ ] Update `languageServerVersion` in `package.json` to point to the latest version (???) of [the Language Server](https://github.com/Microsoft/python-language-server). + - [ ] Update `languageServerVersionV1` in `package.json` to point to the latest version of the [.NET Language Server](https://github.com/Microsoft/ - [ ] Update `languageServerVersionV2` in `package.json` to point to the latest version of the [Node Language Server](https://github.com/Microsoft/pyrx). - [ ] Update [`CHANGELOG.md`](https://github.com/Microsoft/vscode-python/blob/master/CHANGELOG.md) (🤖) - [ ] Run [`news`](https://github.com/Microsoft/vscode-python/tree/master/news) (typically `python news --final --update CHANGELOG.md | code-insiders -`) - [ ] Copy over the "Thanks" section from the previous release diff --git a/.gitignore b/.gitignore index f3befcb98957..17480c9bbcbc 100644 --- a/.gitignore +++ b/.gitignore @@ -25,7 +25,7 @@ precommit.hook pythonFiles/experimental/ptvsd/** pythonFiles/lib/** debug_coverage*/** -languageServer/** +languageServer*/** languageServer.*/** bin/** obj/** diff --git a/.vscode/settings.json b/.vscode/settings.json index 64e62ac725c8..1e52447e9d55 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -45,7 +45,7 @@ "source.fixAll.eslint": true, "source.fixAll.tslint": true }, - "python.jediEnabled": false, + "python.languageServer": "Microsoft (V2)", "python.analysis.logLevel": "Trace", "python.analysis.downloadChannel": "beta", "python.linting.pylintEnabled": false, diff --git a/CHANGELOG.md b/CHANGELOG.md index f5c68097199a..ce9015435ed5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -885,10 +885,6 @@ part of! (thanks to [ChenKB91](https://github.com/ChenKB91/)) ([#10072](https://github.com/Microsoft/vscode-python/issues/10072)) -### Note - -1. Please only set the `python.languageServer` setting if you want to turn IntelliSense off. To switch between language servers, please keep using the `python.jediEnabled` setting for now. - ### Thanks Thanks to the following projects which we fully rely on to provide some of diff --git a/news/.vscode/settings.json b/news/.vscode/settings.json index e70c9f336c57..b36d242f0c4f 100644 --- a/news/.vscode/settings.json +++ b/news/.vscode/settings.json @@ -1,5 +1,5 @@ { - "python.jediEnabled": false, + "python.languageServer": "Node", "python.formatting.provider": "black", "editor.formatOnSave": true, "python.testing.pytestArgs": ["."], diff --git a/package.json b/package.json index 36a615ef489c..8b73dd075972 100644 --- a/package.json +++ b/package.json @@ -6,7 +6,8 @@ "featureFlags": { "usingNewInterpreterStorage": true }, - "languageServerVersion": "0.5.30", + "minLanguageServerVersionV1": "0.5.30", + "minLanguageServerVersionV2": "0.0.100", "publisher": "ms-python", "enableProposedApi": false, "author": { @@ -2056,12 +2057,6 @@ "description": "Whether to install Python modules globally when not using an environment.", "scope": "resource" }, - "python.jediEnabled": { - "type": "boolean", - "default": true, - "description": "Enables Jedi as IntelliSense engine instead of Microsoft Python Analysis Engine.", - "scope": "resource" - }, "python.jediMemoryLimit": { "type": "number", "default": 0, @@ -2078,10 +2073,11 @@ "type": "string", "enum": [ "Jedi", - "Microsoft", + "Microsoft (V1)", + "Microsoft (V2)", "None" ], - "default": "Microsoft", + "default": "Microsoft (V1)", "description": "Defines type of the language server.", "scope": "resource" }, diff --git a/src/client/activation/activationService.ts b/src/client/activation/activationService.ts index 3cc2d7f18b80..4773f93c396c 100644 --- a/src/client/activation/activationService.ts +++ b/src/client/activation/activationService.ts @@ -35,7 +35,6 @@ import { LanguageServerType } from './types'; -const jediEnabledSetting: keyof IPythonSettings = 'jediEnabled'; const languageServerSetting: keyof IPythonSettings = 'languageServer'; const workspacePathNameForGlobalWorkspaces = ''; @@ -130,19 +129,16 @@ export class LanguageServerExtensionActivationService } } @swallowExceptions('Send telemetry for Language Server current selection') - public async sendTelemetryForChosenLanguageServer(jediEnabled: boolean): Promise { + public async sendTelemetryForChosenLanguageServer(languageServer: LanguageServerType): Promise { const state = this.stateFactory.createGlobalPersistentState('SWITCH_LS', undefined); - if (typeof state.value !== 'boolean') { - await state.updateValue(jediEnabled); - } if (state.value !== jediEnabled) { await state.updateValue(jediEnabled); sendTelemetryEvent(EventName.PYTHON_LANGUAGE_SERVER_CURRENT_SELECTION, undefined, { - switchTo: jediEnabled + switchTo: languageServer }); } else { sendTelemetryEvent(EventName.PYTHON_LANGUAGE_SERVER_CURRENT_SELECTION, undefined, { - lsStartup: jediEnabled + lsStartup: languageServer }); } } @@ -178,9 +174,7 @@ export class LanguageServerExtensionActivationService this.abExperiments.sendTelemetryIfInExperiment(LSControl); } const configurationService = this.serviceContainer.get(IConfigurationService); - let enabled = configurationService.getSettings(this.resource).jediEnabled; - const languageServerType = configurationService.getSettings(this.resource).languageServer; - enabled = enabled || languageServerType === LanguageServerType.Jedi; + let enabled = configurationService.getSettings(this.resource).languageServer === LanguageServerType.Jedi; this.sendTelemetryForChosenLanguageServer(enabled).ignoreErrors(); return enabled; } @@ -295,10 +289,7 @@ export class LanguageServerExtensionActivationService const workspacesUris: (Uri | undefined)[] = this.workspaceService.hasWorkspaceFolders ? this.workspaceService.workspaceFolders!.map((workspace) => workspace.uri) : [undefined]; - if ( - workspacesUris.findIndex((uri) => event.affectsConfiguration(`python.${jediEnabledSetting}`, uri)) === -1 && - workspacesUris.findIndex((uri) => event.affectsConfiguration(`python.${languageServerSetting}`, uri)) === -1 - ) { + if (workspacesUris.findIndex((uri) => event.affectsConfiguration(`python.${languageServerSetting}`, uri)) === -1) { return; } const jedi = this.useJedi(); diff --git a/src/client/activation/common/languageServerFolderService.ts b/src/client/activation/common/languageServerFolderService.ts index 3aa99f8e20e4..92a5eb6c9e7e 100644 --- a/src/client/activation/common/languageServerFolderService.ts +++ b/src/client/activation/common/languageServerFolderService.ts @@ -16,15 +16,17 @@ import { FolderVersionPair, IDownloadChannelRule, ILanguageServerFolderService, - ILanguageServerPackageService + ILanguageServerPackageService, + DotNetLanguageServerFolder } from '../types'; +import { IApplicationEnvironment } from '../../common/application/types'; @injectable() export abstract class LanguageServerFolderService implements ILanguageServerFolderService { constructor( @inject(IServiceContainer) protected readonly serviceContainer: IServiceContainer, @unmanaged() protected readonly languageServerFolder: string - ) {} + ) { } @traceDecorators.verbose('Get language server folder name') public async getLanguageServerFolderName(resource: Resource): Promise { @@ -51,7 +53,7 @@ export abstract class LanguageServerFolderService implements ILanguageServerFold @traceDecorators.verbose('Get latest version of Language Server') public getLatestLanguageServerVersion(resource: Resource): Promise { - const minVersion = this.getMinimalLanguageServerVersion(); + const minVersion = this.getMinimalLanguageServerVersion(DotNetLanguageServerFolder); const lsPackageService = this.serviceContainer.get( ILanguageServerPackageService ); @@ -100,7 +102,17 @@ export abstract class LanguageServerFolderService implements ILanguageServerFold : semver.parse(suffix, true) || new semver.SemVer('0.0.0'); } - protected abstract getMinimalLanguageServerVersion(): string; + protected getMinimalLanguageServerVersion(key: string): string { + let minVersion = '0.0.0'; + try { + const appEnv = this.serviceContainer.get(IApplicationEnvironment); + if (appEnv) { + minVersion = appEnv.packageJson[key] as string; + } + // tslint:disable-next-line: no-empty + } catch { } + return minVersion; + } private getDownloadChannel() { const lsPackageService = this.serviceContainer.get( diff --git a/src/client/activation/languageServer/languageServerFolderService.ts b/src/client/activation/languageServer/languageServerFolderService.ts index bc89bb95ecea..a2c32a8df02f 100644 --- a/src/client/activation/languageServer/languageServerFolderService.ts +++ b/src/client/activation/languageServer/languageServerFolderService.ts @@ -4,25 +4,17 @@ 'use strict'; import { inject, injectable } from 'inversify'; -import { IApplicationEnvironment } from '../../common/application/types'; import { IServiceContainer } from '../../ioc/types'; import { LanguageServerFolderService } from '../common/languageServerFolderService'; +import { DotNetLanguageServerFolder, DotNetLanguageServerMinVersionKey } from '../types'; @injectable() export class DotNetLanguageServerFolderService extends LanguageServerFolderService { constructor(@inject(IServiceContainer) serviceContainer: IServiceContainer) { - super(serviceContainer, 'languageServer'); + super(serviceContainer, DotNetLanguageServerFolder); } protected getMinimalLanguageServerVersion(): string { - let minVersion = '0.0.0'; - try { - const appEnv = this.serviceContainer.get(IApplicationEnvironment); - if (appEnv) { - minVersion = appEnv.packageJson.languageServerVersion as string; - } - // tslint:disable-next-line: no-empty - } catch {} - return minVersion; + return super.getMinimalLanguageServerVersion(DotNetLanguageServerMinVersionKey); } } diff --git a/src/client/activation/node/languageServerFolderService.ts b/src/client/activation/node/languageServerFolderService.ts index dc81535c7bd3..6c73b3757d3b 100644 --- a/src/client/activation/node/languageServerFolderService.ts +++ b/src/client/activation/node/languageServerFolderService.ts @@ -6,14 +6,15 @@ import { inject, injectable } from 'inversify'; import { IServiceContainer } from '../../ioc/types'; import { LanguageServerFolderService } from '../common/languageServerFolderService'; +import { NodeLanguageServerMinVersionKey, NodeLanguageServerFolder } from '../types'; @injectable() export class NodeLanguageServerFolderService extends LanguageServerFolderService { constructor(@inject(IServiceContainer) serviceContainer: IServiceContainer) { - super(serviceContainer, 'nodeLanguageServer'); + super(serviceContainer, NodeLanguageServerFolder); } protected getMinimalLanguageServerVersion(): string { - return '0.0.0'; + return super.getMinimalLanguageServerVersion(NodeLanguageServerMinVersionKey); } } diff --git a/src/client/activation/types.ts b/src/client/activation/types.ts index 4fce2850ffd3..5aa740a9e0a6 100644 --- a/src/client/activation/types.ts +++ b/src/client/activation/types.ts @@ -65,11 +65,18 @@ export interface IExtensionActivationService { export enum LanguageServerType { Jedi = 'Jedi', - Microsoft = 'Microsoft', - Node = 'Node', + Microsoft = 'Microsoft (V1)', + Node = 'Microsoft (V2)', None = 'None' } +export const DotNetLanguageServerFolder = 'languageServerV1'; +export const NodeLanguageServerFolder = 'languageServerV2'; + +// Must match minLanguageServerVersionV* keys in package.json +export const DotNetLanguageServerMinVersionKey = 'languageServerVersionV1'; +export const NodeLanguageServerMinVersionKey = 'languageServerVersionV2'; + // tslint:disable-next-line: interface-name export interface DocumentHandler { handleOpen(document: TextDocument): void; @@ -83,16 +90,16 @@ export interface LanguageServerCommandHandler { export interface ILanguageServer extends RenameProvider, - DefinitionProvider, - HoverProvider, - ReferenceProvider, - CompletionItemProvider, - CodeLensProvider, - DocumentSymbolProvider, - SignatureHelpProvider, - Partial, - Partial, - IDisposable {} + DefinitionProvider, + HoverProvider, + ReferenceProvider, + CompletionItemProvider, + CodeLensProvider, + DocumentSymbolProvider, + SignatureHelpProvider, + Partial, + Partial, + IDisposable { } export const ILanguageServerActivator = Symbol('ILanguageServerActivator'); export interface ILanguageServerActivator extends ILanguageServer { diff --git a/src/client/common/configSettings.ts b/src/client/common/configSettings.ts index 89aa79e5f7eb..23edeff65fc7 100644 --- a/src/client/common/configSettings.ts +++ b/src/client/common/configSettings.ts @@ -86,7 +86,6 @@ export class PythonSettings implements IPythonSettings { } private static pythonSettings: Map = new Map(); public downloadLanguageServer = true; - public jediEnabled = true; public jediPath = ''; public jediMemoryLimit = 1024; public envFile = ''; @@ -223,11 +222,17 @@ export class PythonSettings implements IPythonSettings { this.downloadLanguageServer = systemVariables.resolveAny( pythonSettings.get('downloadLanguageServer', true) )!; - this.jediEnabled = systemVariables.resolveAny(pythonSettings.get('jediEnabled', true))!; this.autoUpdateLanguageServer = systemVariables.resolveAny( pythonSettings.get('autoUpdateLanguageServer', true) )!; - if (this.jediEnabled) { + + let ls = pythonSettings.get('languageServer'); + if (!ls) { + ls = LanguageServerType.Jedi; + } + this.languageServer = systemVariables.resolveAny(ls)!; + + if (this.languageServer === LanguageServerType.Jedi) { // tslint:disable-next-line:no-backbone-get-set-outside-model no-non-null-assertion this.jediPath = systemVariables.resolveAny(pythonSettings.get('jediPath'))!; if (typeof this.jediPath === 'string' && this.jediPath.length > 0) { @@ -238,12 +243,6 @@ export class PythonSettings implements IPythonSettings { this.jediMemoryLimit = pythonSettings.get('jediMemoryLimit')!; } - let ls = pythonSettings.get('languageServer'); - if (!ls) { - ls = LanguageServerType.Jedi; - } - this.languageServer = systemVariables.resolveAny(ls)!; - const envFileSetting = pythonSettings.get('envFile'); this.envFile = systemVariables.resolveAny(envFileSetting)!; sendSettingTelemetry(this.workspace, envFileSetting); @@ -285,59 +284,59 @@ export class PythonSettings implements IPythonSettings { this.linting = this.linting ? this.linting : { - enabled: false, - ignorePatterns: [], - flake8Args: [], - flake8Enabled: false, - flake8Path: 'flake', - lintOnSave: false, - maxNumberOfProblems: 100, - mypyArgs: [], - mypyEnabled: false, - mypyPath: 'mypy', - banditArgs: [], - banditEnabled: false, - banditPath: 'bandit', - pycodestyleArgs: [], - pycodestyleEnabled: false, - pycodestylePath: 'pycodestyle', - pylamaArgs: [], - pylamaEnabled: false, - pylamaPath: 'pylama', - prospectorArgs: [], - prospectorEnabled: false, - prospectorPath: 'prospector', - pydocstyleArgs: [], - pydocstyleEnabled: false, - pydocstylePath: 'pydocstyle', - pylintArgs: [], - pylintEnabled: false, - pylintPath: 'pylint', - pylintCategorySeverity: { - convention: DiagnosticSeverity.Hint, - error: DiagnosticSeverity.Error, - fatal: DiagnosticSeverity.Error, - refactor: DiagnosticSeverity.Hint, - warning: DiagnosticSeverity.Warning - }, - pycodestyleCategorySeverity: { - E: DiagnosticSeverity.Error, - W: DiagnosticSeverity.Warning - }, - flake8CategorySeverity: { - E: DiagnosticSeverity.Error, - W: DiagnosticSeverity.Warning, - // Per http://flake8.pycqa.org/en/latest/glossary.html#term-error-code - // 'F' does not mean 'fatal as in PyLint but rather 'pyflakes' such as - // unused imports, variables, etc. - F: DiagnosticSeverity.Warning - }, - mypyCategorySeverity: { - error: DiagnosticSeverity.Error, - note: DiagnosticSeverity.Hint - }, - pylintUseMinimalCheckers: false - }; + enabled: false, + ignorePatterns: [], + flake8Args: [], + flake8Enabled: false, + flake8Path: 'flake', + lintOnSave: false, + maxNumberOfProblems: 100, + mypyArgs: [], + mypyEnabled: false, + mypyPath: 'mypy', + banditArgs: [], + banditEnabled: false, + banditPath: 'bandit', + pycodestyleArgs: [], + pycodestyleEnabled: false, + pycodestylePath: 'pycodestyle', + pylamaArgs: [], + pylamaEnabled: false, + pylamaPath: 'pylama', + prospectorArgs: [], + prospectorEnabled: false, + prospectorPath: 'prospector', + pydocstyleArgs: [], + pydocstyleEnabled: false, + pydocstylePath: 'pydocstyle', + pylintArgs: [], + pylintEnabled: false, + pylintPath: 'pylint', + pylintCategorySeverity: { + convention: DiagnosticSeverity.Hint, + error: DiagnosticSeverity.Error, + fatal: DiagnosticSeverity.Error, + refactor: DiagnosticSeverity.Hint, + warning: DiagnosticSeverity.Warning + }, + pycodestyleCategorySeverity: { + E: DiagnosticSeverity.Error, + W: DiagnosticSeverity.Warning + }, + flake8CategorySeverity: { + E: DiagnosticSeverity.Error, + W: DiagnosticSeverity.Warning, + // Per http://flake8.pycqa.org/en/latest/glossary.html#term-error-code + // 'F' does not mean 'fatal as in PyLint but rather 'pyflakes' such as + // unused imports, variables, etc. + F: DiagnosticSeverity.Warning + }, + mypyCategorySeverity: { + error: DiagnosticSeverity.Error, + note: DiagnosticSeverity.Hint + }, + pylintUseMinimalCheckers: false + }; this.linting.pylintPath = getAbsolutePath(systemVariables.resolveAny(this.linting.pylintPath), workspaceRoot); this.linting.flake8Path = getAbsolutePath(systemVariables.resolveAny(this.linting.flake8Path), workspaceRoot); this.linting.pycodestylePath = getAbsolutePath( @@ -367,14 +366,14 @@ export class PythonSettings implements IPythonSettings { this.formatting = this.formatting ? this.formatting : { - autopep8Args: [], - autopep8Path: 'autopep8', - provider: 'autopep8', - blackArgs: [], - blackPath: 'black', - yapfArgs: [], - yapfPath: 'yapf' - }; + autopep8Args: [], + autopep8Path: 'autopep8', + provider: 'autopep8', + blackArgs: [], + blackPath: 'black', + yapfArgs: [], + yapfPath: 'yapf' + }; this.formatting.autopep8Path = getAbsolutePath( systemVariables.resolveAny(this.formatting.autopep8Path), workspaceRoot @@ -398,11 +397,11 @@ export class PythonSettings implements IPythonSettings { this.autoComplete = this.autoComplete ? this.autoComplete : { - extraPaths: [], - addBrackets: false, - showAdvancedMembers: false, - typeshedPaths: [] - }; + extraPaths: [], + addBrackets: false, + showAdvancedMembers: false, + typeshedPaths: [] + }; // tslint:disable-next-line:no-backbone-get-set-outside-model no-non-null-assertion const workspaceSymbolsSettings = systemVariables.resolveAny( @@ -420,13 +419,13 @@ export class PythonSettings implements IPythonSettings { this.workspaceSymbols = this.workspaceSymbols ? this.workspaceSymbols : { - ctagsPath: 'ctags', - enabled: true, - exclusionPatterns: [], - rebuildOnFileSave: true, - rebuildOnStart: true, - tagFilePath: workspaceRoot ? path.join(workspaceRoot, 'tags') : '' - }; + ctagsPath: 'ctags', + enabled: true, + exclusionPatterns: [], + rebuildOnFileSave: true, + rebuildOnStart: true, + tagFilePath: workspaceRoot ? path.join(workspaceRoot, 'tags') : '' + }; this.workspaceSymbols.tagFilePath = getAbsolutePath( systemVariables.resolveAny(this.workspaceSymbols.tagFilePath), workspaceRoot @@ -461,18 +460,18 @@ export class PythonSettings implements IPythonSettings { this.testing = this.testing ? this.testing : { - promptToConfigure: true, - debugPort: 3000, - nosetestArgs: [], - nosetestPath: 'nosetest', - nosetestsEnabled: false, - pytestArgs: [], - pytestEnabled: false, - pytestPath: 'pytest', - unittestArgs: [], - unittestEnabled: false, - autoTestDiscoverOnSaveEnabled: true - }; + promptToConfigure: true, + debugPort: 3000, + nosetestArgs: [], + nosetestPath: 'nosetest', + nosetestsEnabled: false, + pytestArgs: [], + pytestEnabled: false, + pytestPath: 'pytest', + unittestArgs: [], + unittestEnabled: false, + autoTestDiscoverOnSaveEnabled: true + }; this.testing.pytestPath = getAbsolutePath(systemVariables.resolveAny(this.testing.pytestPath), workspaceRoot); this.testing.nosetestPath = getAbsolutePath( systemVariables.resolveAny(this.testing.nosetestPath), @@ -503,11 +502,11 @@ export class PythonSettings implements IPythonSettings { this.terminal = this.terminal ? this.terminal : { - executeInFileDir: true, - launchArgs: [], - activateEnvironment: true, - activateEnvInCurrentTerminal: false - }; + executeInFileDir: true, + launchArgs: [], + activateEnvironment: true, + activateEnvInCurrentTerminal: false + }; const experiments = systemVariables.resolveAny(pythonSettings.get('experiments'))!; if (this.experiments) { @@ -518,10 +517,10 @@ export class PythonSettings implements IPythonSettings { this.experiments = this.experiments ? this.experiments : { - enabled: true, - optInto: [], - optOutFrom: [] - }; + enabled: true, + optInto: [], + optOutFrom: [] + }; const dataScienceSettings = systemVariables.resolveAny( pythonSettings.get('dataScience') diff --git a/src/client/common/featureDeprecationManager.ts b/src/client/common/featureDeprecationManager.ts index 51f923698eb6..1ab971034c12 100644 --- a/src/client/common/featureDeprecationManager.ts +++ b/src/client/common/featureDeprecationManager.ts @@ -30,7 +30,7 @@ const deprecatedFeatures: DeprecatedFeatureInfo[] = [ { doNotDisplayPromptStateKey: 'SHOW_DEPRECATED_FEATURE_PROMPT_FOR_AUTO_COMPLETE_PRELOAD_MODULES', message: - "The setting 'python.autoComplete.preloadModules' is deprecated, please consider using the new Language Server ('python.jediEnabled = false').", + "The setting 'python.autoComplete.preloadModules' is deprecated, please consider using Microsoft Language Server ('python.languageServer' setting).", moreInfoUrl: 'https://github.com/Microsoft/vscode-python/issues/1704', setting: { setting: 'autoComplete.preloadModules' } } @@ -44,7 +44,7 @@ export class FeatureDeprecationManager implements IFeatureDeprecationManager { @inject(ICommandManager) private cmdMgr: ICommandManager, @inject(IWorkspaceService) private workspace: IWorkspaceService, @inject(IApplicationShell) private appShell: IApplicationShell - ) {} + ) { } public dispose() { this.disposables.forEach((disposable) => disposable.dispose()); diff --git a/src/client/common/types.ts b/src/client/common/types.ts index 06e827798a18..d1bcf8f050ce 100644 --- a/src/client/common/types.ts +++ b/src/client/common/types.ts @@ -23,9 +23,9 @@ import { ExtensionChannels } from './insidersBuild/types'; import { InterpreterUri } from './installer/types'; import { EnvironmentVariables } from './variables/types'; export const IOutputChannel = Symbol('IOutputChannel'); -export interface IOutputChannel extends OutputChannel {} +export interface IOutputChannel extends OutputChannel { } export const IDocumentSymbolProvider = Symbol('IDocumentSymbolProvider'); -export interface IDocumentSymbolProvider extends DocumentSymbolProvider {} +export interface IDocumentSymbolProvider extends DocumentSymbolProvider { } export const IsWindows = Symbol('IS_WINDOWS'); export const IDisposableRegistry = Symbol('IDisposableRegistry'); export type IDisposableRegistry = { push(disposable: Disposable): void }; @@ -165,7 +165,6 @@ export interface IPythonSettings { readonly poetryPath: string; readonly insidersChannel: ExtensionChannels; readonly downloadLanguageServer: boolean; - readonly jediEnabled: boolean; readonly jediPath: string; readonly jediMemoryLimit: number; readonly devOptions: string[]; @@ -471,7 +470,7 @@ export interface IHttpClient { } export const IExtensionContext = Symbol('ExtensionContext'); -export interface IExtensionContext extends ExtensionContext {} +export interface IExtensionContext extends ExtensionContext { } export const IExtensions = Symbol('IExtensions'); export interface IExtensions { diff --git a/src/client/languageServices/proposeLanguageServerBanner.ts b/src/client/languageServices/proposeLanguageServerBanner.ts index c259f007282b..b962b4d39753 100644 --- a/src/client/languageServices/proposeLanguageServerBanner.ts +++ b/src/client/languageServices/proposeLanguageServerBanner.ts @@ -9,6 +9,7 @@ import { IApplicationShell } from '../common/application/types'; import '../common/extensions'; import { IConfigurationService, IPersistentStateFactory, IPythonExtensionBanner } from '../common/types'; import { getRandomBetween } from '../common/utils/random'; +import { LanguageServerType } from '../activation/types'; // persistent state names, exported to make use of in testing export enum ProposeLSStateKeys { @@ -82,7 +83,7 @@ export class ProposeLanguageServerBanner implements IPythonExtensionBanner { const response = await this.appShell.showInformationMessage(this.bannerMessage, ...this.bannerLabels); switch (response) { case this.bannerLabels[ProposeLSLabelIndex.Yes]: { - await this.enableNewLanguageServer(); + await this.enableLanguageServer(); await this.disable(); break; } @@ -111,7 +112,7 @@ export class ProposeLanguageServerBanner implements IPythonExtensionBanner { .updateValue(false); } - public async enableNewLanguageServer(): Promise { - await this.configuration.updateSetting('jediEnabled', false, undefined, ConfigurationTarget.Global); + public async enableLanguageServer(): Promise { + await this.configuration.updateSetting('languageServer', LanguageServerType.Node, undefined, ConfigurationTarget.Global); } } diff --git a/src/client/linters/linterAvailability.ts b/src/client/linters/linterAvailability.ts index 228ec7935403..eeea0302bd17 100644 --- a/src/client/linters/linterAvailability.ts +++ b/src/client/linters/linterAvailability.ts @@ -14,6 +14,7 @@ import { Common, Linters } from '../common/utils/localize'; import { sendTelemetryEvent } from '../telemetry'; import { EventName } from '../telemetry/constants'; import { IAvailableLinterActivator, ILinterInfo } from './types'; +import { LanguageServerType } from '../activation/types'; const doNotDisplayPromptStateKey = 'MESSAGE_KEY_FOR_CONFIGURE_AVAILABLE_LINTER_PROMPT'; @injectable() @@ -24,7 +25,7 @@ export class AvailableLinterActivator implements IAvailableLinterActivator { @inject(IWorkspaceService) private workspaceService: IWorkspaceService, @inject(IConfigurationService) private configService: IConfigurationService, @inject(IPersistentStateFactory) private persistentStateFactory: IPersistentStateFactory - ) {} + ) { } /** * Check if it is possible to enable an otherwise-unconfigured linter in @@ -133,11 +134,11 @@ export class AvailableLinterActivator implements IAvailableLinterActivator { * * This is a feature of the vscode-python extension that will become enabled once the * Python Language Server becomes the default, replacing Jedi as the default. Testing - * the global default setting for `"python.jediEnabled": false` enables it. + * the global default setting for `"python.languageServer": !Jedi` enables it. * - * @returns true if the global default for python.jediEnabled is false. + * @returns true if the global default for python.languageServer is not Jedi. */ public get isFeatureEnabled(): boolean { - return !this.configService.getSettings().jediEnabled; + return this.configService.getSettings().languageServer !== LanguageServerType.Jedi; } } diff --git a/src/client/linters/linterInfo.ts b/src/client/linters/linterInfo.ts index c9632599b1b6..a5b2a29e36fd 100644 --- a/src/client/linters/linterInfo.ts +++ b/src/client/linters/linterInfo.ts @@ -6,6 +6,7 @@ import { Uri } from 'vscode'; import { IWorkspaceService } from '../common/application/types'; import { ExecutionInfo, IConfigurationService, Product } from '../common/types'; import { ILinterInfo, LinterId } from './types'; +import { LanguageServerType } from '../activation/types'; // tslint:disable:no-any @@ -86,7 +87,7 @@ export class PylintLinterInfo extends LinterInfo { } public isEnabled(resource?: Uri): boolean { const enabled = super.isEnabled(resource); - if (!enabled || this.configService.getSettings(resource).jediEnabled) { + if (!enabled || this.configService.getSettings(resource).languageServer === LanguageServerType.Jedi) { return enabled; } // If we're using new LS, then by default Pylint is disabled (unless the user provides a value). diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index 4583f26e9046..c96076b3431d 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -116,8 +116,8 @@ export function sendTelemetryEvent

{ this.updateTestSettings(resource).ignoreErrors(); } @@ -63,11 +63,6 @@ export class UpdateTestSettingService implements IExtensionActivationService { fileContents = fileContents.replace(setting_pytest_args, '.pytestArgs"'); fileContents = fileContents.replace(setting_pytest_path, '.pytestPath"'); - const setting_microsoftLanguageServer = new RegExp('\\.languageServer": "microsoft"', 'g'); - const setting_JediLanguageServer = new RegExp('\\.languageServer": "jedi"', 'g'); - fileContents = fileContents.replace(setting_microsoftLanguageServer, '.jediEnabled": false'); - fileContents = fileContents.replace(setting_JediLanguageServer, '.jediEnabled": true'); - const setting_pep8_args = new RegExp('\\.(? { - [true, false].forEach((jediIsEnabled) => { - suite(`Test activation - ${jediIsEnabled ? 'Jedi is enabled' : 'Jedi is disabled'}`, () => { + [LanguageServerType.Jedi, LanguageServerType.Node].forEach((languageServerType) => { + suite(`Test activation - ${languageServerType === LanguageServerType.Jedi ? 'Jedi is enabled' : 'Jedi is disabled'}`, () => { let serviceContainer: TypeMoq.IMock; let pythonSettings: TypeMoq.IMock; let appShell: TypeMoq.IMock; @@ -94,12 +94,10 @@ suite('Language Server Activation - ActivationService', () => { .returns(() => state.object); state.setup((s) => s.value).returns(() => undefined); state.setup((s) => s.updateValue(TypeMoq.It.isAny())).returns(() => Promise.resolve()); - const setting = { workspaceFolderValue: jediIsEnabled }; workspaceConfig = TypeMoq.Mock.ofType(); workspaceService .setup((ws) => ws.getConfiguration('python', TypeMoq.It.isAny())) .returns(() => workspaceConfig.object); - workspaceConfig.setup((c) => c.inspect('jediEnabled')).returns(() => setting as any); const output = TypeMoq.Mock.ofType(); serviceContainer .setup((c) => c.get(TypeMoq.It.isValue(IOutputChannel), TypeMoq.It.isAny())) @@ -148,12 +146,12 @@ suite('Language Server Activation - ActivationService', () => { .verifiable(TypeMoq.Times.once()); activator.setup((a) => a.activate()).verifiable(TypeMoq.Times.once()); - if (activatorName !== LanguageServerType.None && lsSupported && !jediIsEnabled) { - activatorName = LanguageServerType.Microsoft; + if (activatorName !== LanguageServerType.None && lsSupported && activatorName !== LanguageServerType.Jedi) { + activatorName = LanguageServerType.Node; } let diagnostics: IDiagnostic[]; - if (!lsSupported && !jediIsEnabled) { + if (!lsSupported && activatorName !== LanguageServerType.Jedi) { diagnostics = [TypeMoq.It.isAny()]; } else { diagnostics = []; @@ -186,7 +184,6 @@ suite('Language Server Activation - ActivationService', () => { async function testReloadMessage(settingName: string): Promise { let callbackHandler!: (e: ConfigurationChangeEvent) => Promise; - let jediIsEnabledValueInSetting = jediIsEnabled; workspaceService .setup((w) => w.onDidChangeConfiguration(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny()) @@ -195,8 +192,7 @@ suite('Language Server Activation - ActivationService', () => { .returns(() => TypeMoq.Mock.ofType().object) .verifiable(TypeMoq.Times.once()); - pythonSettings.setup((p) => p.jediEnabled).returns(() => jediIsEnabledValueInSetting); - pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Microsoft); + pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Node); const activator = TypeMoq.Mock.ofType(); const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, @@ -223,7 +219,8 @@ suite('Language Server Activation - ActivationService', () => { .verifiable(TypeMoq.Times.once()); // Toggle the value in the setting and invoke the callback. - jediIsEnabledValueInSetting = !jediIsEnabledValueInSetting; + languageServerType = languageServerType === LanguageServerType.Jedi + ? LanguageServerType.Node : LanguageServerType.Jedi; await callbackHandler(event.object); event.verifyAll(); @@ -232,8 +229,7 @@ suite('Language Server Activation - ActivationService', () => { } test('LS is supported', async () => { - pythonSettings.setup((p) => p.jediEnabled).returns(() => jediIsEnabled); - pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Microsoft); + pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Node); const activator = TypeMoq.Mock.ofType(); const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, @@ -244,8 +240,7 @@ suite('Language Server Activation - ActivationService', () => { await testActivation(activationService, activator, true); }); test('LS is not supported', async () => { - pythonSettings.setup((p) => p.jediEnabled).returns(() => jediIsEnabled); - pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Microsoft); + pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Node); const activator = TypeMoq.Mock.ofType(); const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, @@ -257,8 +252,7 @@ suite('Language Server Activation - ActivationService', () => { }); test('Activator must be activated', async () => { - pythonSettings.setup((p) => p.jediEnabled).returns(() => jediIsEnabled); - pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Microsoft); + pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Node); const activator = TypeMoq.Mock.ofType(); const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, @@ -269,8 +263,7 @@ suite('Language Server Activation - ActivationService', () => { await testActivation(activationService, activator); }); test('Activator must be deactivated', async () => { - pythonSettings.setup((p) => p.jediEnabled).returns(() => jediIsEnabled); - pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Microsoft); + pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Node); const activator = TypeMoq.Mock.ofType(); const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, @@ -286,7 +279,6 @@ suite('Language Server Activation - ActivationService', () => { activator.verifyAll(); }); test('No language service', async () => { - pythonSettings.setup((p) => p.jediEnabled).returns(() => false); pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.None); const activator = TypeMoq.Mock.ofType(); const activationService = new LanguageServerExtensionActivationService( @@ -296,98 +288,9 @@ suite('Language Server Activation - ActivationService', () => { ); await testActivation(activationService, activator, false, LanguageServerType.None); }); - test('Prompt user to reload VS Code and reload, when jediEnabled setting is toggled', async () => { - await testReloadMessage('jediEnabled'); - }); test('Prompt user to reload VS Code and reload, when languageServer setting is toggled', async () => { await testReloadMessage('languageServer'); }); - test('Prompt user to reload VS Code and do not reload, when setting is toggled', async () => { - let callbackHandler!: (e: ConfigurationChangeEvent) => Promise; - let jediIsEnabledValueInSetting = jediIsEnabled; - workspaceService - .setup((w) => - w.onDidChangeConfiguration(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny()) - ) - .callback((cb) => (callbackHandler = cb)) - .returns(() => TypeMoq.Mock.ofType().object) - .verifiable(TypeMoq.Times.once()); - - pythonSettings.setup((p) => p.jediEnabled).returns(() => jediIsEnabledValueInSetting); - pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Microsoft); - const activator = TypeMoq.Mock.ofType(); - const activationService = new LanguageServerExtensionActivationService( - serviceContainer.object, - stateFactory.object, - experiments.object - ); - - workspaceService.verifyAll(); - await testActivation(activationService, activator); - - const event = TypeMoq.Mock.ofType(); - event - .setup((e) => e.affectsConfiguration(TypeMoq.It.isValue('python.jediEnabled'), TypeMoq.It.isAny())) - .returns(() => true) - .verifiable(TypeMoq.Times.atLeastOnce()); - appShell - .setup((a) => a.showInformationMessage(TypeMoq.It.isAny(), TypeMoq.It.isValue('Reload'))) - .returns(() => Promise.resolve(undefined)) - .verifiable(TypeMoq.Times.once()); - cmdManager - .setup((c) => c.executeCommand(TypeMoq.It.isValue('workbench.action.reloadWindow'))) - .verifiable(TypeMoq.Times.never()); - - // Toggle the value in the setting and invoke the callback. - jediIsEnabledValueInSetting = !jediIsEnabledValueInSetting; - await callbackHandler(event.object); - - event.verifyAll(); - appShell.verifyAll(); - cmdManager.verifyAll(); - }); - test('Do not prompt user to reload VS Code when setting is not toggled', async () => { - let callbackHandler!: (e: ConfigurationChangeEvent) => Promise; - workspaceService - .setup((w) => - w.onDidChangeConfiguration(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny()) - ) - .callback((cb) => (callbackHandler = cb)) - .returns(() => TypeMoq.Mock.ofType().object) - .verifiable(TypeMoq.Times.once()); - - pythonSettings.setup((p) => p.jediEnabled).returns(() => jediIsEnabled); - pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Microsoft); - const activator = TypeMoq.Mock.ofType(); - const activationService = new LanguageServerExtensionActivationService( - serviceContainer.object, - stateFactory.object, - experiments.object - ); - - workspaceService.verifyAll(); - await testActivation(activationService, activator); - - const event = TypeMoq.Mock.ofType(); - event - .setup((e) => e.affectsConfiguration(TypeMoq.It.isValue('python.jediEnabled'), TypeMoq.It.isAny())) - .returns(() => true) - .verifiable(TypeMoq.Times.atLeastOnce()); - appShell - .setup((a) => a.showInformationMessage(TypeMoq.It.isAny(), TypeMoq.It.isValue('Reload'))) - .returns(() => Promise.resolve(undefined)) - .verifiable(TypeMoq.Times.never()); - cmdManager - .setup((c) => c.executeCommand(TypeMoq.It.isValue('workbench.action.reloadWindow'))) - .verifiable(TypeMoq.Times.never()); - - // Invoke the config changed callback. - await callbackHandler(event.object); - - event.verifyAll(); - appShell.verifyAll(); - cmdManager.verifyAll(); - }); test('Do not prompt user to reload VS Code when setting is not changed', async () => { let callbackHandler!: (e: ConfigurationChangeEvent) => Promise; workspaceService @@ -398,8 +301,7 @@ suite('Language Server Activation - ActivationService', () => { .returns(() => TypeMoq.Mock.ofType().object) .verifiable(TypeMoq.Times.once()); - pythonSettings.setup((p) => p.jediEnabled).returns(() => jediIsEnabled); - pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Microsoft); + pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Node); const activator = TypeMoq.Mock.ofType(); const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, @@ -412,7 +314,7 @@ suite('Language Server Activation - ActivationService', () => { const event = TypeMoq.Mock.ofType(); event - .setup((e) => e.affectsConfiguration(TypeMoq.It.isValue('python.jediEnabled'), TypeMoq.It.isAny())) + .setup((e) => e.affectsConfiguration(TypeMoq.It.isValue('python.languageServer'), TypeMoq.It.isAny())) .returns(() => false) .verifiable(TypeMoq.Times.atLeastOnce()); appShell @@ -466,12 +368,7 @@ suite('Language Server Activation - ActivationService', () => { serviceContainer .setup((c) => c.get(TypeMoq.It.isValue(ILanguageServerActivator), TypeMoq.It.isAny())) .returns(() => activator.object); - let diagnostics: IDiagnostic[]; - if (!jediIsEnabled) { - diagnostics = [TypeMoq.It.isAny()]; - } else { - diagnostics = []; - } + let diagnostics: IDiagnostic[] = [TypeMoq.It.isAny()]; lsNotSupportedDiagnosticService .setup((l) => l.diagnose(undefined)) .returns(() => Promise.resolve(diagnostics)); @@ -479,8 +376,7 @@ suite('Language Server Activation - ActivationService', () => { .setup((l) => l.handle(TypeMoq.It.isValue(diagnostics))) .returns(() => Promise.resolve()); - pythonSettings.setup((p) => p.jediEnabled).returns(() => jediIsEnabled); - pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Microsoft); + pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Node); const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, stateFactory.object, @@ -543,12 +439,7 @@ suite('Language Server Activation - ActivationService', () => { serviceContainer .setup((c) => c.get(TypeMoq.It.isValue(ILanguageServerActivator), TypeMoq.It.isAny())) .returns(() => activator.object); - let diagnostics: IDiagnostic[]; - if (!jediIsEnabled) { - diagnostics = [TypeMoq.It.isAny()]; - } else { - diagnostics = []; - } + let diagnostics: IDiagnostic[] = [TypeMoq.It.isAny()]; lsNotSupportedDiagnosticService .setup((l) => l.diagnose(undefined)) .returns(() => Promise.resolve(diagnostics)); @@ -556,8 +447,7 @@ suite('Language Server Activation - ActivationService', () => { .setup((l) => l.handle(TypeMoq.It.isValue(diagnostics))) .returns(() => Promise.resolve()); - pythonSettings.setup((p) => p.jediEnabled).returns(() => jediIsEnabled); - pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Microsoft); + pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Node); const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, stateFactory.object, @@ -578,8 +468,7 @@ suite('Language Server Activation - ActivationService', () => { }); if (!jediIsEnabled) { test('Revert to jedi when LS activation fails', async () => { - pythonSettings.setup((p) => p.jediEnabled).returns(() => jediIsEnabled); - pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Microsoft); + pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Node); const activatorDotNet = TypeMoq.Mock.ofType(); const activatorJedi = TypeMoq.Mock.ofType(); const activationService = new LanguageServerExtensionActivationService( @@ -673,8 +562,7 @@ suite('Language Server Activation - ActivationService', () => { experiments.verifyAll(); } test('Activator is disposed if activated workspace is removed', async () => { - pythonSettings.setup((p) => p.jediEnabled).returns(() => jediIsEnabled); - pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Microsoft); + pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Node); let workspaceFoldersChangedHandler!: Function; workspaceService .setup((w) => w.onDidChangeWorkspaceFolders(TypeMoq.It.isAny(), TypeMoq.It.isAny())) diff --git a/src/test/application/diagnostics/checks/updateTestSettings.unit.test.ts b/src/test/application/diagnostics/checks/updateTestSettings.unit.test.ts index 8699426016bc..bc062f2524be 100644 --- a/src/test/application/diagnostics/checks/updateTestSettings.unit.test.ts +++ b/src/test/application/diagnostics/checks/updateTestSettings.unit.test.ts @@ -195,16 +195,6 @@ suite('Application Diagnostics - Check Test Settings', () => { '{"python.pythonPath":"1234", "python.unittest.unitTestArgs":[], "python.unitTest.pytestArgs":[], "python.testing.pytestArgs":[], "python.testing.pytestPath":[]}', expectedContents: '{"python.pythonPath":"1234", "python.unittest.unitTestArgs":[], "python.testing.pytestArgs":[], "python.testing.pytestArgs":[], "python.testing.pytestPath":[]}' - }, - { - testTitle: 'Should replace python.jediEnabled.', - expectedContents: '{"python.jediEnabled": false}', - contents: '{"python.languageServer": "microsoft"}' - }, - { - testTitle: 'Should replace python.jediEnabled.', - expectedContents: '{"python.jediEnabled": true}', - contents: '{"python.languageServer": "jedi"}' } ].forEach((item) => { test(item.testTitle, async () => { diff --git a/src/test/common.ts b/src/test/common.ts index b6a6fb43ad56..dc39f4ae2523 100644 --- a/src/test/common.ts +++ b/src/test/common.ts @@ -42,6 +42,7 @@ export enum OSType { export type PythonSettingKeys = | 'workspaceSymbols.enabled' | 'pythonPath' + | 'languageServer' | 'linting.lintOnSave' | 'linting.enabled' | 'linting.pylintEnabled' @@ -61,7 +62,6 @@ export type PythonSettingKeys = | 'testing.pytestEnabled' | 'testing.unittestEnabled' | 'envFile' - | 'jediEnabled' | 'linting.ignorePatterns' | 'terminal.activateEnvironment'; @@ -571,7 +571,7 @@ export class FakeClock { * @param {number} [advacenTimeMs=10_000] Default `timeout` value. Defaults to 10s. Assuming we do not have anything bigger. * @memberof FakeClock */ - constructor(private readonly advacenTimeMs: number = 10_000) {} + constructor(private readonly advacenTimeMs: number = 10_000) { } public install() { // tslint:disable-next-line:no-require-imports const lolex = require('lolex'); diff --git a/src/test/common/configSettings/configSettings.unit.test.ts b/src/test/common/configSettings/configSettings.unit.test.ts index 87b913b8b2be..fbbefcc939f0 100644 --- a/src/test/common/configSettings/configSettings.unit.test.ts +++ b/src/test/common/configSettings/configSettings.unit.test.ts @@ -73,7 +73,7 @@ suite('Python Settings', async () => { // tslint:disable-next-line:no-any .returns(() => (sourceSettings as any)[name]); } - if (sourceSettings.jediEnabled) { + if (sourceSettings.languageServer === LanguageServerType.Jedi) { config.setup((c) => c.get('jediPath')).returns(() => sourceSettings.jediPath); } for (const name of ['venvFolders']) { @@ -84,7 +84,7 @@ suite('Python Settings', async () => { } // boolean settings - for (const name of ['downloadLanguageServer', 'jediEnabled', 'autoUpdateLanguageServer']) { + for (const name of ['downloadLanguageServer', 'autoUpdateLanguageServer']) { config .setup((c) => c.get(name, true)) // tslint:disable-next-line:no-any @@ -98,7 +98,7 @@ suite('Python Settings', async () => { } // number settings - if (sourceSettings.jediEnabled) { + if (sourceSettings.languageServer === LanguageServerType.Jedi) { config.setup((c) => c.get('jediMemoryLimit')).returns(() => sourceSettings.jediMemoryLimit); } @@ -153,7 +153,7 @@ suite('Python Settings', async () => { }); suite('Boolean settings', async () => { - ['downloadLanguageServer', 'jediEnabled', 'autoUpdateLanguageServer', 'globalModuleInstallation'].forEach( + ['downloadLanguageServer', 'autoUpdateLanguageServer', 'globalModuleInstallation'].forEach( async (settingName) => { testIfValueIsUpdated(settingName, true); } diff --git a/src/test/datascience/dataScienceIocContainer.ts b/src/test/datascience/dataScienceIocContainer.ts index af1458afdab5..842df1604e01 100644 --- a/src/test/datascience/dataScienceIocContainer.ts +++ b/src/test/datascience/dataScienceIocContainer.ts @@ -1501,7 +1501,6 @@ export class DataScienceIocContainer extends UnitTestIocContainer { disableJupyterAutoStart: true, widgetScriptSources: ['jsdelivr.com', 'unpkg.com'] }; - pythonSettings.jediEnabled = false; pythonSettings.downloadLanguageServer = false; const folders = ['Envs', '.virtualenvs']; pythonSettings.venvFolders = folders; @@ -1628,7 +1627,7 @@ export class DataScienceIocContainer extends UnitTestIocContainer { this.postMessageToWebPanel(msg); }, // tslint:disable-next-line:no-any no-empty - setState: (_msg: any) => {}, + setState: (_msg: any) => { }, // tslint:disable-next-line:no-any no-empty getState: () => { return {}; diff --git a/src/test/datascience/intellisense.unit.test.ts b/src/test/datascience/intellisense.unit.test.ts index 44855b188e0b..335ac0c563e6 100644 --- a/src/test/datascience/intellisense.unit.test.ts +++ b/src/test/datascience/intellisense.unit.test.ts @@ -26,6 +26,7 @@ import { createEmptyCell, generateTestCells } from '../../datascience-ui/interac import { generateReverseChange, IMonacoTextModel } from '../../datascience-ui/react-common/monacoHelpers'; import { MockAutoSelectionService } from '../mocks/autoSelector'; import { MockLanguageServerCache } from './mockLanguageServerCache'; +import { LanguageServerType } from '../../client/activation/types'; // tslint:disable:no-any unified-signatures const TestCellContents = `myvar = """ # Lorem Ipsum @@ -68,7 +69,7 @@ suite('DataScience Intellisense Unit Tests', () => { notebookProvider = TypeMoq.Mock.ofType(); const variableProvider = mock(JupyterVariables); - pythonSettings.jediEnabled = false; + pythonSettings.languageServer = LanguageServerType.Node; configService.setup((c) => c.getSettings(TypeMoq.It.isAny())).returns(() => pythonSettings); workspaceService.setup((w) => w.rootPath).returns(() => '/foo/bar'); fileSystem diff --git a/src/test/linters/common.ts b/src/test/linters/common.ts index e010c8aab750..ad6ccb56ef99 100644 --- a/src/test/linters/common.ts +++ b/src/test/linters/common.ts @@ -24,6 +24,7 @@ import { IServiceContainer } from '../../client/ioc/types'; import { LINTERID_BY_PRODUCT } from '../../client/linters/constants'; import { LinterManager } from '../../client/linters/linterManager'; import { ILinter, ILinterManager, ILintMessage, LinterId } from '../../client/linters/types'; +import { LanguageServerType } from '../../client/activation/types'; export function newMockDocument(filename: string): TypeMoq.IMock { const uri = Uri.file(filename); @@ -318,7 +319,7 @@ export class BaseTestFixture { }) .returns(() => Promise.resolve(undefined)); - this.pythonSettings.setup((s) => s.jediEnabled).returns(() => true); + this.pythonSettings.setup((s) => s.languageServer).returns(() => LanguageServerType.Jedi); } private initData(): void { diff --git a/src/test/linters/linter.availability.unit.test.ts b/src/test/linters/linter.availability.unit.test.ts index 55583d229534..fb39b496b3e4 100644 --- a/src/test/linters/linter.availability.unit.test.ts +++ b/src/test/linters/linter.availability.unit.test.ts @@ -26,12 +26,13 @@ import { Common, Linters } from '../../client/common/utils/localize'; import { AvailableLinterActivator } from '../../client/linters/linterAvailability'; import { LinterInfo } from '../../client/linters/linterInfo'; import { IAvailableLinterActivator, ILinterInfo } from '../../client/linters/types'; +import { LanguageServerType } from '../../client/activation/types'; // tslint:disable:max-func-body-length no-any suite('Linter Availability Provider tests', () => { - test('Availability feature is disabled when global default for jediEnabled=true.', async () => { + test('Availability feature is disabled when global default for languageServer === Jedi.', async () => { // set expectations - const jediEnabledValue = true; + const languageServerValue = LanguageServerType.Jedi; const expectedResult = false; // arrange @@ -42,7 +43,7 @@ suite('Linter Availability Provider tests', () => { configServiceMock, factoryMock ] = getDependenciesForAvailabilityTests(); - setupConfigurationServiceForJediSettingsTest(jediEnabledValue, configServiceMock); + setupConfigurationServiceForJediSettingsTest(languageServerValue, configServiceMock); // call const availabilityProvider = new AvailableLinterActivator( @@ -56,14 +57,14 @@ suite('Linter Availability Provider tests', () => { // check expectaions expect(availabilityProvider.isFeatureEnabled).is.equal( expectedResult, - 'Avaialability feature should be disabled when python.jediEnabled is true' + 'Avaialability feature should be disabled when python.languageServer is Jedi' ); workspaceServiceMock.verifyAll(); }); - test('Availability feature is enabled when global default for jediEnabled=false.', async () => { + test('Availability feature is enabled when global default for languageServer is Node.', async () => { // set expectations - const jediEnabledValue = false; + const languageServerValue = LanguageServerType.Node; const expectedResult = true; // arrange @@ -74,7 +75,7 @@ suite('Linter Availability Provider tests', () => { configServiceMock, factoryMock ] = getDependenciesForAvailabilityTests(); - setupConfigurationServiceForJediSettingsTest(jediEnabledValue, configServiceMock); + setupConfigurationServiceForJediSettingsTest(languageServerValue, configServiceMock); const availabilityProvider = new AvailableLinterActivator( appShellMock.object, @@ -86,7 +87,7 @@ suite('Linter Availability Provider tests', () => { expect(availabilityProvider.isFeatureEnabled).is.equal( expectedResult, - 'Avaialability feature should be enabled when python.jediEnabled defaults to false' + 'Avaialability feature should be enabled when python.languageServer defaults to non-Jedi' ); workspaceServiceMock.verifyAll(); }); @@ -385,7 +386,7 @@ suite('Linter Availability Provider tests', () => { // Options to test the implementation of the IAvailableLinterActivator. // All options default to values that would otherwise allow the prompt to appear. class AvailablityTestOverallOptions { - public jediEnabledValue: boolean = false; + public languageServerValue = LanguageServerType.Node; public pylintUserEnabled?: boolean; public pylintWorkspaceEnabled?: boolean; public pylintWorkspaceFolderEnabled?: boolean; @@ -433,7 +434,7 @@ suite('Linter Availability Provider tests', () => { .returns(async () => options.linterIsInstalled) .verifiable(TypeMoq.Times.atLeastOnce()); - setupConfigurationServiceForJediSettingsTest(options.jediEnabledValue, configServiceMock); + setupConfigurationServiceForJediSettingsTest(options.languageServerValue, configServiceMock); setupWorkspaceMockForLinterConfiguredTests( options.pylintUserEnabled, options.pylintWorkspaceEnabled, @@ -460,7 +461,7 @@ suite('Linter Availability Provider tests', () => { test('Overall implementation does not change configuration when feature disabled', async () => { // set expectations const testOpts = new AvailablityTestOverallOptions(); - testOpts.jediEnabledValue = true; + testOpts.languageServerValue = LanguageServerType.Jedi; const expectedResult = false; // arrange @@ -469,7 +470,7 @@ suite('Linter Availability Provider tests', () => { // perform test expect(expectedResult).to.equal( result, - 'promptIfLinterAvailable should not change any configuration when python.jediEnabled is true.' + 'promptIfLinterAvailable should not change any configuration when python.languageServer is Jedi.' ); }); @@ -803,14 +804,14 @@ function setupWorkspaceMockForLinterConfiguredTests( } function setupConfigurationServiceForJediSettingsTest( - jediEnabledValue: boolean, + languageServerValue: LanguageServerType, configServiceMock: TypeMoq.IMock ): [TypeMoq.IMock, TypeMoq.IMock] { if (!configServiceMock) { configServiceMock = TypeMoq.Mock.ofType(); } const pythonSettings = TypeMoq.Mock.ofType(); - pythonSettings.setup((ps) => ps.jediEnabled).returns(() => jediEnabledValue); + pythonSettings.setup((ps) => ps.languageServer).returns(() => languageServerValue); configServiceMock.setup((cs) => cs.getSettings()).returns(() => pythonSettings.object); return [configServiceMock, pythonSettings]; diff --git a/src/test/linters/linterinfo.unit.test.ts b/src/test/linters/linterinfo.unit.test.ts index e989361336f8..b9de1addc9fe 100644 --- a/src/test/linters/linterinfo.unit.test.ts +++ b/src/test/linters/linterinfo.unit.test.ts @@ -10,6 +10,7 @@ import { anything, instance, mock, when } from 'ts-mockito'; import { WorkspaceService } from '../../client/common/application/workspace'; import { ConfigurationService } from '../../client/common/configuration/service'; import { PylintLinterInfo } from '../../client/linters/linterInfo'; +import { LanguageServerType } from '../../client/activation/types'; suite('Linter Info - Pylint', () => { test('Test disabled when Pylint is explicitly disabled', async () => { @@ -28,7 +29,7 @@ suite('Linter Info - Pylint', () => { when(config.getSettings(anything())).thenReturn({ linting: { pylintEnabled: false }, - jediEnabled: true + languageServer: LanguageServerType.Jedi } as any); expect(linterInfo.isEnabled()).to.be.false; @@ -38,7 +39,10 @@ suite('Linter Info - Pylint', () => { const workspaceService = mock(WorkspaceService); const linterInfo = new PylintLinterInfo(instance(config), instance(workspaceService), []); - when(config.getSettings(anything())).thenReturn({ linting: { pylintEnabled: true }, jediEnabled: true } as any); + when(config.getSettings(anything())).thenReturn({ + linting: { pylintEnabled: true }, + languageServer: LanguageServerType.Jedi + } as any); expect(linterInfo.isEnabled()).to.be.true; }); @@ -53,7 +57,7 @@ suite('Linter Info - Pylint', () => { }; when(config.getSettings(anything())).thenReturn({ linting: { pylintEnabled: true }, - jediEnabled: false + languageServer: LanguageServerType.Node } as any); when(workspaceService.getConfiguration('python', anything())).thenReturn(pythonConfig as any); @@ -86,7 +90,7 @@ suite('Linter Info - Pylint', () => { }; when(config.getSettings(anything())).thenReturn({ linting: { pylintEnabled: true }, - jediEnabled: false + languageServer: LanguageServerType.Node } as any); when(workspaceService.getConfiguration('python', anything())).thenReturn(pythonConfig as any); diff --git a/src/test/performance/settings.json b/src/test/performance/settings.json index 809ebf6ab2f4..ffc9d2a990cd 100644 --- a/src/test/performance/settings.json +++ b/src/test/performance/settings.json @@ -1 +1 @@ -{ "python.jediEnabled": true } +{ "python.languageServer": "Jedi" } diff --git a/src/test/performanceTest.ts b/src/test/performanceTest.ts index d489c76dd011..594ebfee9f35 100644 --- a/src/test/performanceTest.ts +++ b/src/test/performanceTest.ts @@ -24,6 +24,7 @@ import * as path from 'path'; import * as request from 'request'; import { EXTENSION_ROOT_DIR, PVSC_EXTENSION_ID } from '../client/common/constants'; import { unzip } from './common'; +import { LanguageServerType } from '../client/activation/types'; const NamedRegexp = require('named-js-regexp'); const del = require('del'); @@ -72,7 +73,7 @@ class TestRunner { await this.runPerfTest(devLogFiles, releaseLogFiles, languageServerLogFiles); } private async enableLanguageServer(enable: boolean) { - const settings = `{ "python.jediEnabled": ${!enable} }`; + const settings = `{ "python.languageServer": ${enable ? LanguageServerType.Node : LanguageServerType.Jedi} }`; await fs.writeFile(path.join(EXTENSION_ROOT_DIR, 'src', 'test', 'performance', 'settings.json'), settings); } diff --git a/src/test/smoke/common.ts b/src/test/smoke/common.ts index da02ff92131d..e6e0902fe50b 100644 --- a/src/test/smoke/common.ts +++ b/src/test/smoke/common.ts @@ -12,6 +12,7 @@ import * as path from 'path'; import * as vscode from 'vscode'; import { SMOKE_TEST_EXTENSIONS_DIR } from '../constants'; import { noop, sleep } from '../core'; +import { LanguageServerType } from '../../client/activation/types'; export async function updateSetting(setting: string, value: any) { const resource = vscode.workspace.workspaceFolders![0].uri; @@ -33,13 +34,13 @@ async function getLanaguageServerFolders(): Promise { export function isJediEnabled() { const resource = vscode.workspace.workspaceFolders![0].uri; const settings = vscode.workspace.getConfiguration('python', resource); - return settings.get('jediEnabled') === true; + return settings.get('languageServer') === LanguageServerType.Jedi; } export async function enableJedi(enable: boolean | undefined) { if (isJediEnabled() === enable) { return; } - await updateSetting('jediEnabled', enable); + await updateSetting('languageServer', LanguageServerType.Jedi); } export async function openFileAndWaitForLS(file: string): Promise { const textDocument = await vscode.workspace.openTextDocument(file); diff --git a/src/test/smokeTest.ts b/src/test/smokeTest.ts index 815648c22706..93eff3cec5b6 100644 --- a/src/test/smokeTest.ts +++ b/src/test/smokeTest.ts @@ -14,6 +14,7 @@ import * as glob from 'glob'; import * as path from 'path'; import { unzip } from './common'; import { EXTENSION_ROOT_DIR_FOR_TESTS, SMOKE_TEST_EXTENSIONS_DIR } from './constants'; +import { LanguageServerType } from '../client/activation/types'; class TestRunner { public async start() { @@ -31,7 +32,7 @@ class TestRunner { await this.launchTest(env); } private async enableLanguageServer(enable: boolean) { - const settings = `{ "python.jediEnabled": ${!enable} }`; + const settings = `{ "python.languageServer": ${enable ? LanguageServerType.Node : LanguageServerType.Jedi} }`; await fs.ensureDir( path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'src', 'testMultiRootWkspc', 'smokeTests', '.vscode') ); From c0211a74a2de73b2fbbf74ae0e9ef25b73fba090 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Wed, 13 May 2020 10:14:19 -0700 Subject: [PATCH 03/34] Fix remaining settings --- .github/ISSUE_TEMPLATE/1_ds_bug_report.md | 2 +- package.json | 6 +-- src/client/activation/activationService.ts | 30 ++++++-------- src/client/activation/types.ts | 4 +- src/client/telemetry/index.ts | 3 +- src/test/.vscode/settings.json | 2 +- .../activation/activationService.unit.test.ts | 41 +++++++++---------- 7 files changed, 41 insertions(+), 47 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/1_ds_bug_report.md b/.github/ISSUE_TEMPLATE/1_ds_bug_report.md index 20649615777e..c5cca0ffcf32 100644 --- a/.github/ISSUE_TEMPLATE/1_ds_bug_report.md +++ b/.github/ISSUE_TEMPLATE/1_ds_bug_report.md @@ -33,7 +33,7 @@ _Please provide as much info as you readily know_ - **Jupyter server running:** Local | Remote | N/A - **Extension version:** 20YY.MM.#####-xxx - **VS Code version:** #.## -- **Setting python.languageServer:** Jedi | Microsoft (V1) | Microsoft (V2) | None +- **Setting python.languageServer:** Jedi | Microsoft (v1) | Microsoft (v2) | None - **Python and/or Anaconda version:** #.#.# - **OS:** Windows | Mac | Linux (distro): - **Virtual environment:** conda | venv | virtualenv | N/A | ... diff --git a/package.json b/package.json index 8b73dd075972..f10f5b71ac04 100644 --- a/package.json +++ b/package.json @@ -2073,11 +2073,11 @@ "type": "string", "enum": [ "Jedi", - "Microsoft (V1)", - "Microsoft (V2)", + "Microsoft (v1)", + "Microsoft (v2)", "None" ], - "default": "Microsoft (V1)", + "default": "Microsoft (v1)", "description": "Defines type of the language server.", "scope": "resource" }, diff --git a/src/client/activation/activationService.ts b/src/client/activation/activationService.ts index 4773f93c396c..31744f18bf0c 100644 --- a/src/client/activation/activationService.ts +++ b/src/client/activation/activationService.ts @@ -130,28 +130,22 @@ export class LanguageServerExtensionActivationService } @swallowExceptions('Send telemetry for Language Server current selection') public async sendTelemetryForChosenLanguageServer(languageServer: LanguageServerType): Promise { - const state = this.stateFactory.createGlobalPersistentState('SWITCH_LS', undefined); - if (state.value !== jediEnabled) { - await state.updateValue(jediEnabled); - sendTelemetryEvent(EventName.PYTHON_LANGUAGE_SERVER_CURRENT_SELECTION, undefined, { - switchTo: languageServer - }); - } else { - sendTelemetryEvent(EventName.PYTHON_LANGUAGE_SERVER_CURRENT_SELECTION, undefined, { - lsStartup: languageServer - }); - } + const state = this.stateFactory.createGlobalPersistentState('SWITCH_LS', undefined); + await state.updateValue(languageServer); + sendTelemetryEvent(EventName.PYTHON_LANGUAGE_SERVER_CURRENT_SELECTION, undefined, { + switchTo: languageServer + }); } /** - * Checks if user has not manually set `jediEnabled` setting + * Checks if user has not manually set `languageServer` setting * @param resource - * @returns `true` if user has NOT manually added the setting and is using default configuration, `false` if user has `jediEnabled` setting added + * @returns `true` if user has NOT manually added the setting and is using default configuration, `false` if user has `languageServer` setting added */ public isJediUsingDefaultConfiguration(resource: Resource): boolean { - const settings = this.workspaceService.getConfiguration('python', resource).inspect('jediEnabled'); + const settings = this.workspaceService.getConfiguration('python', resource).inspect('languageServer'); if (!settings) { - traceError('WorkspaceConfiguration.inspect returns `undefined` for setting `python.jediEnabled`'); + traceError('WorkspaceConfiguration.inspect returns `undefined` for setting `python.languageServer`'); return false; } return ( @@ -174,9 +168,9 @@ export class LanguageServerExtensionActivationService this.abExperiments.sendTelemetryIfInExperiment(LSControl); } const configurationService = this.serviceContainer.get(IConfigurationService); - let enabled = configurationService.getSettings(this.resource).languageServer === LanguageServerType.Jedi; - this.sendTelemetryForChosenLanguageServer(enabled).ignoreErrors(); - return enabled; + const lstType = configurationService.getSettings(this.resource).languageServer; + this.sendTelemetryForChosenLanguageServer(lstType).ignoreErrors(); + return lstType === LanguageServerType.Jedi; } protected async onWorkspaceFoldersChanged() { diff --git a/src/client/activation/types.ts b/src/client/activation/types.ts index 5aa740a9e0a6..bc5f22a75545 100644 --- a/src/client/activation/types.ts +++ b/src/client/activation/types.ts @@ -65,8 +65,8 @@ export interface IExtensionActivationService { export enum LanguageServerType { Jedi = 'Jedi', - Microsoft = 'Microsoft (V1)', - Node = 'Microsoft (V2)', + Microsoft = 'Microsoft (v1)', + Node = 'Microsoft (v2)', None = 'None' } diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index c96076b3431d..74f77462e586 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -27,6 +27,7 @@ import { LinterId } from '../linters/types'; import { TestProvider } from '../testing/common/types'; import { EventName, PlatformErrors } from './constants'; import { LinterTrigger, TestTool } from './types'; +import { LanguageServerType } from '../activation/types'; /** * Checks whether telemetry is supported. @@ -1135,7 +1136,7 @@ export interface IEventNamePropertyMapping { /** * Used to track switch between LS and Jedi. Carries the final state after the switch. */ - switchTo?: boolean; + switchTo?: LanguageServerType; }; /** * Telemetry event sent with details after attempting to download LS diff --git a/src/test/.vscode/settings.json b/src/test/.vscode/settings.json index 6afcf7675527..38ffd37c8799 100644 --- a/src/test/.vscode/settings.json +++ b/src/test/.vscode/settings.json @@ -17,7 +17,7 @@ "python.formatting.provider": "yapf", "python.linting.pylintUseMinimalCheckers": false, "python.pythonPath": "python", - // Do not set this to "Microsoft (V1)" or V2 even when LS is the default, else + // Do not set this to "Microsoft (v1)" or v2 even when LS is the default, else // it will result in LS being downloaded on CI and slow down tests significantly. // We have other tests on CI for testing downloading of CI with this setting enabled. "python.languageServer": "Jedi" diff --git a/src/test/activation/activationService.unit.test.ts b/src/test/activation/activationService.unit.test.ts index 25806304658e..0b98eb7237a9 100644 --- a/src/test/activation/activationService.unit.test.ts +++ b/src/test/activation/activationService.unit.test.ts @@ -466,7 +466,7 @@ suite('Language Server Activation - ActivationService', () => { expect(connectCount).to.be.equal(4, 'Reconnect is not happening'); server.dispose(); }); - if (!jediIsEnabled) { + if (languageServerType !== LanguageServerType.Jedi) { test('Revert to jedi when LS activation fails', async () => { pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Node); const activatorDotNet = TypeMoq.Mock.ofType(); @@ -607,7 +607,6 @@ suite('Language Server Activation - ActivationService', () => { }); } else { test('Jedi is only started once', async () => { - pythonSettings.setup((p) => p.jediEnabled).returns(() => jediIsEnabled); pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Microsoft); const activator1 = TypeMoq.Mock.ofType(); const activationService = new LanguageServerExtensionActivationService( @@ -775,7 +774,7 @@ suite('Language Server Activation - ActivationService', () => { stateFactory.object, experiments.object ); - await activationService.sendTelemetryForChosenLanguageServer(true); + await activationService.sendTelemetryForChosenLanguageServer(LanguageServerType.Jedi); state.verifyAll(); }); @@ -795,7 +794,7 @@ suite('Language Server Activation - ActivationService', () => { stateFactory.object, experiments.object ); - await activationService.sendTelemetryForChosenLanguageServer(false); + await activationService.sendTelemetryForChosenLanguageServer(LanguageServerType.Node); state.verifyAll(); }); @@ -815,7 +814,7 @@ suite('Language Server Activation - ActivationService', () => { stateFactory.object, experiments.object ); - await activationService.sendTelemetryForChosenLanguageServer(true); + await activationService.sendTelemetryForChosenLanguageServer(LanguageServerType.Jedi); state.verifyAll(); }); @@ -835,7 +834,7 @@ suite('Language Server Activation - ActivationService', () => { stateFactory.object, experiments.object ); - await activationService.sendTelemetryForChosenLanguageServer(true); + await activationService.sendTelemetryForChosenLanguageServer(LanguageServerType.Jedi); state.verifyAll(); }); @@ -927,7 +926,7 @@ suite('Language Server Activation - ActivationService', () => { .returns(() => lsNotSupportedDiagnosticService.object); }); - test('If default value of jedi is being used, and LSEnabled experiment is enabled, then return false', async () => { + test('If default value (Jedi) of language server is being used, and LSEnabled experiment is enabled, then return false', async () => { const settings = {}; experiments .setup((ex) => ex.inExperiment(LSEnabled)) @@ -938,7 +937,7 @@ suite('Language Server Activation - ActivationService', () => { .returns(() => undefined) .verifiable(TypeMoq.Times.never()); workspaceConfig - .setup((c) => c.inspect('jediEnabled')) + .setup((c) => c.inspect('languageServer')) .returns(() => settings as any) .verifiable(TypeMoq.Times.once()); @@ -955,7 +954,7 @@ suite('Language Server Activation - ActivationService', () => { experiments.verifyAll(); }); - test('If default value of jedi is being used, and LSEnabled experiment is disabled, then send telemetry if user is in Experiment LSControl and return python settings value (which will always be true as default value is true)', async () => { + test('If default value of languageServer (Jedi) is being used, and LSEnabled experiment is disabled, then send telemetry if user is in Experiment LSControl and return python settings value (which will always be true as default value is true)', async () => { const settings = {}; experiments .setup((ex) => ex.inExperiment(LSEnabled)) @@ -966,12 +965,12 @@ suite('Language Server Activation - ActivationService', () => { .returns(() => undefined) .verifiable(TypeMoq.Times.once()); workspaceConfig - .setup((c) => c.inspect('jediEnabled')) + .setup((c) => c.inspect('languageServer')) .returns(() => settings as any) .verifiable(TypeMoq.Times.once()); pythonSettings - .setup((p) => p.jediEnabled) - .returns(() => true) + .setup((p) => p.languageServer) + .returns(() => LanguageServerType.Jedi) .verifiable(TypeMoq.Times.once()); const activationService = new LanguageServerExtensionActivationService( @@ -989,17 +988,17 @@ suite('Language Server Activation - ActivationService', () => { }); suite( - 'If default value of jedi is not being used, then no experiments are used, and python settings value is returned', + 'If default value of languageServer (Jedi) is not being used, then no experiments are used, and python settings value is returned', async () => { [ { - testName: 'Returns false when python settings value is false', - pythonSettingsValue: false, + testName: 'Returns false when python settings value is Microsoft (v2)', + pythonSettingsValue: LanguageServerType.Node, expectedResult: false }, { - testName: 'Returns true when python settings value is true', - pythonSettingsValue: true, + testName: 'Returns true when python settings value is Jedi', + pythonSettingsValue: LanguageServerType.Jedi, expectedResult: true } ].forEach((testParams) => { @@ -1014,11 +1013,11 @@ suite('Language Server Activation - ActivationService', () => { .returns(() => undefined) .verifiable(TypeMoq.Times.never()); workspaceConfig - .setup((c) => c.inspect('jediEnabled')) + .setup((c) => c.inspect('languageServer')) .returns(() => settings as any) .verifiable(TypeMoq.Times.once()); pythonSettings - .setup((p) => p.jediEnabled) + .setup((p) => p.languageServer) .returns(() => testParams.pythonSettingsValue) .verifiable(TypeMoq.Times.once()); @@ -1151,7 +1150,7 @@ suite('Language Server Activation - ActivationService', () => { test(testName, async () => { workspaceConfig.reset(); workspaceConfig - .setup((c) => c.inspect('jediEnabled')) + .setup((c) => c.inspect('languageServer')) .returns(() => settings as any) .verifiable(TypeMoq.Times.once()); @@ -1172,7 +1171,7 @@ suite('Language Server Activation - ActivationService', () => { test('Returns false for settings = undefined', async () => { workspaceConfig.reset(); workspaceConfig - .setup((c) => c.inspect('jediEnabled')) + .setup((c) => c.inspect('languageServer')) .returns(() => undefined as any) .verifiable(TypeMoq.Times.once()); From 8a6e122f79191081322a37d303c01f909b9bc7c1 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Thu, 14 May 2020 15:04:46 -0700 Subject: [PATCH 04/34] Keep names for now --- package.json | 3 +-- src/client/activation/types.ts | 24 ++++++++++++------------ 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/package.json b/package.json index f10f5b71ac04..d4b5c25cbe48 100644 --- a/package.json +++ b/package.json @@ -2073,8 +2073,7 @@ "type": "string", "enum": [ "Jedi", - "Microsoft (v1)", - "Microsoft (v2)", + "Microsoft", "None" ], "default": "Microsoft (v1)", diff --git a/src/client/activation/types.ts b/src/client/activation/types.ts index bc5f22a75545..9b345965a2f2 100644 --- a/src/client/activation/types.ts +++ b/src/client/activation/types.ts @@ -65,8 +65,8 @@ export interface IExtensionActivationService { export enum LanguageServerType { Jedi = 'Jedi', - Microsoft = 'Microsoft (v1)', - Node = 'Microsoft (v2)', + Microsoft = 'Microsoft', + Node = 'Node', None = 'None' } @@ -90,16 +90,16 @@ export interface LanguageServerCommandHandler { export interface ILanguageServer extends RenameProvider, - DefinitionProvider, - HoverProvider, - ReferenceProvider, - CompletionItemProvider, - CodeLensProvider, - DocumentSymbolProvider, - SignatureHelpProvider, - Partial, - Partial, - IDisposable { } + DefinitionProvider, + HoverProvider, + ReferenceProvider, + CompletionItemProvider, + CodeLensProvider, + DocumentSymbolProvider, + SignatureHelpProvider, + Partial, + Partial, + IDisposable {} export const ILanguageServerActivator = Symbol('ILanguageServerActivator'); export interface ILanguageServerActivator extends ILanguageServer { From 5de66a01c870f5e92ab2b78ec9f05a8ff1a78648 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Thu, 14 May 2020 15:15:58 -0700 Subject: [PATCH 05/34] Fix default --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index d4b5c25cbe48..5b020e46156d 100644 --- a/package.json +++ b/package.json @@ -2076,7 +2076,7 @@ "Microsoft", "None" ], - "default": "Microsoft (v1)", + "default": "Microsoft", "description": "Defines type of the language server.", "scope": "resource" }, From 04b14b53d5f79d314cd2bdc26387c9e64b37e3af Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Fri, 15 May 2020 08:17:38 -0700 Subject: [PATCH 06/34] Restore names --- .github/ISSUE_TEMPLATE/1_ds_bug_report.md | 2 +- .github/release_plan.md | 3 +- .vscode/settings.json | 2 +- package.json | 3 +- .../node/languageServerFolderService.ts | 4 +- src/client/activation/types.ts | 9 +- .../activation/activationService.unit.test.ts | 1161 +++++++++-------- 7 files changed, 598 insertions(+), 586 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/1_ds_bug_report.md b/.github/ISSUE_TEMPLATE/1_ds_bug_report.md index c5cca0ffcf32..8887effe1d52 100644 --- a/.github/ISSUE_TEMPLATE/1_ds_bug_report.md +++ b/.github/ISSUE_TEMPLATE/1_ds_bug_report.md @@ -33,7 +33,7 @@ _Please provide as much info as you readily know_ - **Jupyter server running:** Local | Remote | N/A - **Extension version:** 20YY.MM.#####-xxx - **VS Code version:** #.## -- **Setting python.languageServer:** Jedi | Microsoft (v1) | Microsoft (v2) | None +- **Setting python.languageServer:** Jedi | Microsoft | None - **Python and/or Anaconda version:** #.#.# - **OS:** Windows | Mac | Linux (distro): - **Virtual environment:** conda | venv | virtualenv | N/A | ... diff --git a/.github/release_plan.md b/.github/release_plan.md index 19f02da25d46..421ad949a4f2 100644 --- a/.github/release_plan.md +++ b/.github/release_plan.md @@ -12,7 +12,8 @@ - [ ] Change the version in [`package.json`](https://github.com/Microsoft/vscode-python/blob/master/package.json) from a `-dev` suffix to `-rc` (🤖) - [ ] Run `npm install` to make sure [`package-lock.json`](https://github.com/Microsoft/vscode-python/blob/master/package.json) is up-to-date (🤖) - [ ] Update `requirements.txt` to point to latest release version of [ptvsd](https://github.com/microsoft/ptvsd). - - [ ] Update `languageServerVersionV1` in `package.json` to point to the latest version of the [.NET Language Server](https://github.com/Microsoft/ - [ ] Update `languageServerVersionV2` in `package.json` to point to the latest version of the [Node Language Server](https://github.com/Microsoft/pyrx). + - [ ] Update `languageServerVersion` in `package.json` to point to the latest version of the [Language Server](https://github.com/Microsoft/ +github.com/Microsoft/pyrx). - [ ] Update [`CHANGELOG.md`](https://github.com/Microsoft/vscode-python/blob/master/CHANGELOG.md) (🤖) - [ ] Run [`news`](https://github.com/Microsoft/vscode-python/tree/master/news) (typically `python news --final --update CHANGELOG.md | code-insiders -`) - [ ] Copy over the "Thanks" section from the previous release diff --git a/.vscode/settings.json b/.vscode/settings.json index 1e52447e9d55..a07e6550fd3e 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -45,7 +45,7 @@ "source.fixAll.eslint": true, "source.fixAll.tslint": true }, - "python.languageServer": "Microsoft (V2)", + "python.languageServer": "Microsoft", "python.analysis.logLevel": "Trace", "python.analysis.downloadChannel": "beta", "python.linting.pylintEnabled": false, diff --git a/package.json b/package.json index 5b020e46156d..f6b0aea98d71 100644 --- a/package.json +++ b/package.json @@ -6,8 +6,7 @@ "featureFlags": { "usingNewInterpreterStorage": true }, - "minLanguageServerVersionV1": "0.5.30", - "minLanguageServerVersionV2": "0.0.100", + "languageServerVersion": "0.5.30", "publisher": "ms-python", "enableProposedApi": false, "author": { diff --git a/src/client/activation/node/languageServerFolderService.ts b/src/client/activation/node/languageServerFolderService.ts index 6c73b3757d3b..205b9bf65ef7 100644 --- a/src/client/activation/node/languageServerFolderService.ts +++ b/src/client/activation/node/languageServerFolderService.ts @@ -6,7 +6,7 @@ import { inject, injectable } from 'inversify'; import { IServiceContainer } from '../../ioc/types'; import { LanguageServerFolderService } from '../common/languageServerFolderService'; -import { NodeLanguageServerMinVersionKey, NodeLanguageServerFolder } from '../types'; +import { NodeLanguageServerFolder } from '../types'; @injectable() export class NodeLanguageServerFolderService extends LanguageServerFolderService { @@ -15,6 +15,6 @@ export class NodeLanguageServerFolderService extends LanguageServerFolderService } protected getMinimalLanguageServerVersion(): string { - return super.getMinimalLanguageServerVersion(NodeLanguageServerMinVersionKey); + return '0.0.1'; // super.getMinimalLanguageServerVersion(NodeLanguageServerMinVersionKey); } } diff --git a/src/client/activation/types.ts b/src/client/activation/types.ts index 9b345965a2f2..3a73f75c9a70 100644 --- a/src/client/activation/types.ts +++ b/src/client/activation/types.ts @@ -70,12 +70,11 @@ export enum LanguageServerType { None = 'None' } -export const DotNetLanguageServerFolder = 'languageServerV1'; -export const NodeLanguageServerFolder = 'languageServerV2'; +export const DotNetLanguageServerFolder = 'languageServer'; +export const NodeLanguageServerFolder = 'nodeLanguageServer'; -// Must match minLanguageServerVersionV* keys in package.json -export const DotNetLanguageServerMinVersionKey = 'languageServerVersionV1'; -export const NodeLanguageServerMinVersionKey = 'languageServerVersionV2'; +// Must match languageServerVersion* keys in package.json +export const DotNetLanguageServerMinVersionKey = 'languageServerVersion'; // tslint:disable-next-line: interface-name export interface DocumentHandler { diff --git a/src/test/activation/activationService.unit.test.ts b/src/test/activation/activationService.unit.test.ts index 0b98eb7237a9..5d4457de4d2f 100644 --- a/src/test/activation/activationService.unit.test.ts +++ b/src/test/activation/activationService.unit.test.ts @@ -38,635 +38,648 @@ import { IServiceContainer } from '../../client/ioc/types'; suite('Language Server Activation - ActivationService', () => { [LanguageServerType.Jedi, LanguageServerType.Node].forEach((languageServerType) => { - suite(`Test activation - ${languageServerType === LanguageServerType.Jedi ? 'Jedi is enabled' : 'Jedi is disabled'}`, () => { - let serviceContainer: TypeMoq.IMock; - let pythonSettings: TypeMoq.IMock; - let appShell: TypeMoq.IMock; - let cmdManager: TypeMoq.IMock; - let workspaceService: TypeMoq.IMock; - let platformService: TypeMoq.IMock; - let lsNotSupportedDiagnosticService: TypeMoq.IMock; - let stateFactory: TypeMoq.IMock; - let state: TypeMoq.IMock>; - let experiments: TypeMoq.IMock; - let workspaceConfig: TypeMoq.IMock; - let interpreterService: TypeMoq.IMock; - let interpreterChangedHandler!: Function; - setup(() => { - serviceContainer = TypeMoq.Mock.ofType(); - appShell = TypeMoq.Mock.ofType(); - workspaceService = TypeMoq.Mock.ofType(); - cmdManager = TypeMoq.Mock.ofType(); - platformService = TypeMoq.Mock.ofType(); - stateFactory = TypeMoq.Mock.ofType(); - state = TypeMoq.Mock.ofType>(); - const configService = TypeMoq.Mock.ofType(); - pythonSettings = TypeMoq.Mock.ofType(); - experiments = TypeMoq.Mock.ofType(); - const langFolderServiceMock = TypeMoq.Mock.ofType(); - const folderVer: FolderVersionPair = { - path: '', - version: new SemVer('1.2.3') - }; - lsNotSupportedDiagnosticService = TypeMoq.Mock.ofType(); - workspaceService.setup((w) => w.hasWorkspaceFolders).returns(() => false); - workspaceService.setup((w) => w.workspaceFolders).returns(() => []); - configService.setup((c) => c.getSettings(TypeMoq.It.isAny())).returns(() => pythonSettings.object); - interpreterService = TypeMoq.Mock.ofType(); - const disposable = TypeMoq.Mock.ofType(); - interpreterService - .setup((i) => i.onDidChangeInterpreter(TypeMoq.It.isAny())) - .returns((cb) => { - interpreterChangedHandler = cb; - return disposable.object; - }); - langFolderServiceMock - .setup((l) => l.getCurrentLanguageServerDirectory()) - .returns(() => Promise.resolve(folderVer)); - stateFactory - .setup((f) => - f.createGlobalPersistentState( - TypeMoq.It.isValue('SWITCH_LS'), - TypeMoq.It.isAny(), - TypeMoq.It.isAny() - ) - ) - .returns(() => state.object); - state.setup((s) => s.value).returns(() => undefined); - state.setup((s) => s.updateValue(TypeMoq.It.isAny())).returns(() => Promise.resolve()); - workspaceConfig = TypeMoq.Mock.ofType(); - workspaceService - .setup((ws) => ws.getConfiguration('python', TypeMoq.It.isAny())) - .returns(() => workspaceConfig.object); - const output = TypeMoq.Mock.ofType(); - serviceContainer - .setup((c) => c.get(TypeMoq.It.isValue(IOutputChannel), TypeMoq.It.isAny())) - .returns(() => output.object); - serviceContainer - .setup((c) => c.get(TypeMoq.It.isValue(IWorkspaceService))) - .returns(() => workspaceService.object); - serviceContainer - .setup((c) => c.get(TypeMoq.It.isValue(IApplicationShell))) - .returns(() => appShell.object); - serviceContainer.setup((c) => c.get(TypeMoq.It.isValue(IDisposableRegistry))).returns(() => []); - serviceContainer - .setup((c) => c.get(TypeMoq.It.isValue(IConfigurationService))) - .returns(() => configService.object); - serviceContainer - .setup((c) => c.get(TypeMoq.It.isValue(ICommandManager))) - .returns(() => cmdManager.object); - serviceContainer - .setup((c) => c.get(TypeMoq.It.isValue(IPlatformService))) - .returns(() => platformService.object); - serviceContainer - .setup((c) => c.get(TypeMoq.It.isValue(IInterpreterService))) - .returns(() => interpreterService.object); - serviceContainer - .setup((c) => c.get(TypeMoq.It.isValue(ILanguageServerFolderService))) - .returns(() => langFolderServiceMock.object); - serviceContainer - .setup((s) => - s.get( - TypeMoq.It.isValue(IDiagnosticsService), - TypeMoq.It.isValue(LSNotSupportedDiagnosticServiceId) - ) - ) - .returns(() => lsNotSupportedDiagnosticService.object); - }); - - async function testActivation( - activationService: IExtensionActivationService, - activator: TypeMoq.IMock, - lsSupported: boolean = true, - activatorName: LanguageServerType = LanguageServerType.Jedi - ) { - activator - .setup((a) => a.start(undefined, undefined)) - .returns(() => Promise.resolve()) - .verifiable(TypeMoq.Times.once()); - activator.setup((a) => a.activate()).verifiable(TypeMoq.Times.once()); - - if (activatorName !== LanguageServerType.None && lsSupported && activatorName !== LanguageServerType.Jedi) { - activatorName = LanguageServerType.Node; - } - - let diagnostics: IDiagnostic[]; - if (!lsSupported && activatorName !== LanguageServerType.Jedi) { - diagnostics = [TypeMoq.It.isAny()]; - } else { - diagnostics = []; - } - - lsNotSupportedDiagnosticService - .setup((l) => l.diagnose(undefined)) - .returns(() => Promise.resolve(diagnostics)); - lsNotSupportedDiagnosticService - .setup((l) => l.handle(TypeMoq.It.isValue(diagnostics))) - .returns(() => Promise.resolve()); - serviceContainer - .setup((c) => - c.get(TypeMoq.It.isValue(ILanguageServerActivator), TypeMoq.It.isValue(activatorName)) - ) - .returns(() => activator.object) - .verifiable(TypeMoq.Times.once()); - - experiments - .setup((ex) => ex.inExperiment(TypeMoq.It.isAny())) - .returns(() => false) - .verifiable(TypeMoq.Times.never()); - - await activationService.activate(undefined); - - activator.verifyAll(); - serviceContainer.verifyAll(); - experiments.verifyAll(); - } - - async function testReloadMessage(settingName: string): Promise { - let callbackHandler!: (e: ConfigurationChangeEvent) => Promise; - workspaceService - .setup((w) => - w.onDidChangeConfiguration(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny()) - ) - .callback((cb) => (callbackHandler = cb)) - .returns(() => TypeMoq.Mock.ofType().object) - .verifiable(TypeMoq.Times.once()); - - pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Node); - const activator = TypeMoq.Mock.ofType(); - const activationService = new LanguageServerExtensionActivationService( - serviceContainer.object, - stateFactory.object, - experiments.object - ); - - workspaceService.verifyAll(); - await testActivation(activationService, activator); - - const event = TypeMoq.Mock.ofType(); - event - .setup((e) => - e.affectsConfiguration(TypeMoq.It.isValue(`python.${settingName}`), TypeMoq.It.isAny()) - ) - .returns(() => true) - .verifiable(TypeMoq.Times.atLeastOnce()); - appShell - .setup((a) => a.showInformationMessage(TypeMoq.It.isAny(), TypeMoq.It.isValue('Reload'))) - .returns(() => Promise.resolve('Reload')) - .verifiable(TypeMoq.Times.once()); - cmdManager - .setup((c) => c.executeCommand(TypeMoq.It.isValue('workbench.action.reloadWindow'))) - .verifiable(TypeMoq.Times.once()); - - // Toggle the value in the setting and invoke the callback. - languageServerType = languageServerType === LanguageServerType.Jedi - ? LanguageServerType.Node : LanguageServerType.Jedi; - await callbackHandler(event.object); - - event.verifyAll(); - appShell.verifyAll(); - cmdManager.verifyAll(); - } - - test('LS is supported', async () => { - pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Node); - const activator = TypeMoq.Mock.ofType(); - const activationService = new LanguageServerExtensionActivationService( - serviceContainer.object, - stateFactory.object, - experiments.object - ); - - await testActivation(activationService, activator, true); - }); - test('LS is not supported', async () => { - pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Node); - const activator = TypeMoq.Mock.ofType(); - const activationService = new LanguageServerExtensionActivationService( - serviceContainer.object, - stateFactory.object, - experiments.object - ); - - await testActivation(activationService, activator, false); - }); - - test('Activator must be activated', async () => { - pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Node); - const activator = TypeMoq.Mock.ofType(); - const activationService = new LanguageServerExtensionActivationService( - serviceContainer.object, - stateFactory.object, - experiments.object - ); - - await testActivation(activationService, activator); - }); - test('Activator must be deactivated', async () => { - pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Node); - const activator = TypeMoq.Mock.ofType(); - const activationService = new LanguageServerExtensionActivationService( - serviceContainer.object, - stateFactory.object, - experiments.object - ); - - await testActivation(activationService, activator); - - activator.setup((a) => a.dispose()).verifiable(TypeMoq.Times.once()); - - activationService.dispose(); - activator.verifyAll(); - }); - test('No language service', async () => { - pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.None); - const activator = TypeMoq.Mock.ofType(); - const activationService = new LanguageServerExtensionActivationService( - serviceContainer.object, - stateFactory.object, - experiments.object - ); - await testActivation(activationService, activator, false, LanguageServerType.None); - }); - test('Prompt user to reload VS Code and reload, when languageServer setting is toggled', async () => { - await testReloadMessage('languageServer'); - }); - test('Do not prompt user to reload VS Code when setting is not changed', async () => { - let callbackHandler!: (e: ConfigurationChangeEvent) => Promise; - workspaceService - .setup((w) => - w.onDidChangeConfiguration(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny()) - ) - .callback((cb) => (callbackHandler = cb)) - .returns(() => TypeMoq.Mock.ofType().object) - .verifiable(TypeMoq.Times.once()); - - pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Node); - const activator = TypeMoq.Mock.ofType(); - const activationService = new LanguageServerExtensionActivationService( - serviceContainer.object, - stateFactory.object, - experiments.object - ); - - workspaceService.verifyAll(); - await testActivation(activationService, activator); - - const event = TypeMoq.Mock.ofType(); - event - .setup((e) => e.affectsConfiguration(TypeMoq.It.isValue('python.languageServer'), TypeMoq.It.isAny())) - .returns(() => false) - .verifiable(TypeMoq.Times.atLeastOnce()); - appShell - .setup((a) => a.showInformationMessage(TypeMoq.It.isAny(), TypeMoq.It.isValue('Reload'))) - .returns(() => Promise.resolve(undefined)) - .verifiable(TypeMoq.Times.never()); - cmdManager - .setup((c) => c.executeCommand(TypeMoq.It.isValue('workbench.action.reloadWindow'))) - .verifiable(TypeMoq.Times.never()); - - // Invoke the config changed callback. - await callbackHandler(event.object); - - event.verifyAll(); - appShell.verifyAll(); - cmdManager.verifyAll(); - }); - test('More than one LS is created for multiple interpreters', async () => { - const interpreter1: PythonInterpreter = { - path: '/foo/bar/python', - sysPrefix: '1', - envName: '1', - sysVersion: '3.1.1.1', - architecture: Architecture.x64, - type: InterpreterType.Unknown - }; - const interpreter2: PythonInterpreter = { - path: '/foo/baz/python', - sysPrefix: '1', - envName: '2', - sysVersion: '3.1.1.1', - architecture: Architecture.x64, - type: InterpreterType.Unknown - }; - const folder1 = { name: 'one', uri: Uri.parse('one'), index: 1 }; - const activator = TypeMoq.Mock.ofType(); - activator - .setup((a) => a.start(TypeMoq.It.isValue(folder1.uri), TypeMoq.It.isValue(interpreter1))) - .returns(() => Promise.resolve()) - .verifiable(TypeMoq.Times.once()); - activator - .setup((a) => a.start(TypeMoq.It.isValue(folder1.uri), TypeMoq.It.isValue(interpreter2))) - .returns(() => Promise.resolve()) - .verifiable(TypeMoq.Times.once()); - activator.setup((a) => a.deactivate()).verifiable(TypeMoq.Times.never()); - activator.setup((a) => a.activate()).verifiable(TypeMoq.Times.never()); - activator - .setup((a) => a.dispose()) - .returns(noop) - .verifiable(TypeMoq.Times.exactly(2)); - serviceContainer - .setup((c) => c.get(TypeMoq.It.isValue(ILanguageServerActivator), TypeMoq.It.isAny())) - .returns(() => activator.object); - let diagnostics: IDiagnostic[] = [TypeMoq.It.isAny()]; - lsNotSupportedDiagnosticService - .setup((l) => l.diagnose(undefined)) - .returns(() => Promise.resolve(diagnostics)); - lsNotSupportedDiagnosticService - .setup((l) => l.handle(TypeMoq.It.isValue(diagnostics))) - .returns(() => Promise.resolve()); - - pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Node); - const activationService = new LanguageServerExtensionActivationService( - serviceContainer.object, - stateFactory.object, - experiments.object - ); - const ls1 = await activationService.get(folder1.uri, interpreter1); - const ls2 = await activationService.get(folder1.uri, interpreter2); - expect(ls1).not.to.be.equal(ls2, 'Interpreter does not create new LS'); - const ls3 = await activationService.get(undefined, interpreter1); - expect(ls1).to.be.equal(ls3, 'Interpreter does return same LS'); - ls3.dispose(); - ls1.dispose(); - ls2.dispose(); - activator.verifyAll(); - }); - test('Changing interpreter will activate a new LS', async () => { - const interpreter1: PythonInterpreter = { - path: '/foo/bar/python', - sysPrefix: '1', - envName: '1', - sysVersion: '3.1.1.1', - architecture: Architecture.x64, - type: InterpreterType.Unknown - }; - const interpreter2: PythonInterpreter = { - path: '/foo/baz/python', - sysPrefix: '1', - envName: '2', - sysVersion: '3.1.1.1', - architecture: Architecture.x64, - type: InterpreterType.Unknown - }; - let getActiveCount = 0; - interpreterService - .setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny())) - .returns(() => { - if (getActiveCount % 2 === 0) { - getActiveCount += 1; - return Promise.resolve(interpreter1); - } - getActiveCount += 1; - return Promise.resolve(interpreter2); - }); - const folder1 = { name: 'one', uri: Uri.parse('one'), index: 1 }; - const activator = TypeMoq.Mock.ofType(); - activator - .setup((a) => a.start(TypeMoq.It.isValue(folder1.uri), TypeMoq.It.isValue(interpreter1))) - .returns(() => Promise.resolve()) - .verifiable(TypeMoq.Times.once()); - activator - .setup((a) => a.start(TypeMoq.It.isValue(folder1.uri), TypeMoq.It.isValue(interpreter2))) - .returns(() => Promise.resolve()) - .verifiable(TypeMoq.Times.once()); - let connectCount = 0; - activator - .setup((a) => a.activate()) - .returns(() => { - connectCount = connectCount + 1; - }); - serviceContainer - .setup((c) => c.get(TypeMoq.It.isValue(ILanguageServerActivator), TypeMoq.It.isAny())) - .returns(() => activator.object); - let diagnostics: IDiagnostic[] = [TypeMoq.It.isAny()]; - lsNotSupportedDiagnosticService - .setup((l) => l.diagnose(undefined)) - .returns(() => Promise.resolve(diagnostics)); - lsNotSupportedDiagnosticService - .setup((l) => l.handle(TypeMoq.It.isValue(diagnostics))) - .returns(() => Promise.resolve()); - - pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Node); - const activationService = new LanguageServerExtensionActivationService( - serviceContainer.object, - stateFactory.object, - experiments.object - ); - await activationService.activate(folder1.uri); - await interpreterChangedHandler(); - activator.verifyAll(); - - // Hold onto the second item and switch two more times. Verify that - // reconnect happens - const server = await activationService.get(folder1.uri); - await interpreterChangedHandler(); - expect(connectCount).to.be.equal(3, 'Reconnect is not happening'); - await interpreterChangedHandler(); - expect(connectCount).to.be.equal(4, 'Reconnect is not happening'); - server.dispose(); - }); - if (languageServerType !== LanguageServerType.Jedi) { - test('Revert to jedi when LS activation fails', async () => { - pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Node); - const activatorDotNet = TypeMoq.Mock.ofType(); - const activatorJedi = TypeMoq.Mock.ofType(); - const activationService = new LanguageServerExtensionActivationService( - serviceContainer.object, - stateFactory.object, - experiments.object - ); - const diagnostics: IDiagnostic[] = []; - lsNotSupportedDiagnosticService - .setup((l) => l.diagnose(undefined)) - .returns(() => Promise.resolve(diagnostics)); - lsNotSupportedDiagnosticService - .setup((l) => l.handle(TypeMoq.It.isValue(diagnostics))) - .returns(() => Promise.resolve()); - serviceContainer - .setup((c) => - c.get( - TypeMoq.It.isValue(ILanguageServerActivator), - TypeMoq.It.isValue(LanguageServerType.Microsoft) + suite( + `Test activation - ${ + languageServerType === LanguageServerType.Jedi ? 'Jedi is enabled' : 'Jedi is disabled' + }`, + () => { + let serviceContainer: TypeMoq.IMock; + let pythonSettings: TypeMoq.IMock; + let appShell: TypeMoq.IMock; + let cmdManager: TypeMoq.IMock; + let workspaceService: TypeMoq.IMock; + let platformService: TypeMoq.IMock; + let lsNotSupportedDiagnosticService: TypeMoq.IMock; + let stateFactory: TypeMoq.IMock; + let state: TypeMoq.IMock>; + let experiments: TypeMoq.IMock; + let workspaceConfig: TypeMoq.IMock; + let interpreterService: TypeMoq.IMock; + let interpreterChangedHandler!: Function; + setup(() => { + serviceContainer = TypeMoq.Mock.ofType(); + appShell = TypeMoq.Mock.ofType(); + workspaceService = TypeMoq.Mock.ofType(); + cmdManager = TypeMoq.Mock.ofType(); + platformService = TypeMoq.Mock.ofType(); + stateFactory = TypeMoq.Mock.ofType(); + state = TypeMoq.Mock.ofType>(); + const configService = TypeMoq.Mock.ofType(); + pythonSettings = TypeMoq.Mock.ofType(); + experiments = TypeMoq.Mock.ofType(); + const langFolderServiceMock = TypeMoq.Mock.ofType(); + const folderVer: FolderVersionPair = { + path: '', + version: new SemVer('1.2.3') + }; + lsNotSupportedDiagnosticService = TypeMoq.Mock.ofType(); + workspaceService.setup((w) => w.hasWorkspaceFolders).returns(() => false); + workspaceService.setup((w) => w.workspaceFolders).returns(() => []); + configService.setup((c) => c.getSettings(TypeMoq.It.isAny())).returns(() => pythonSettings.object); + interpreterService = TypeMoq.Mock.ofType(); + const disposable = TypeMoq.Mock.ofType(); + interpreterService + .setup((i) => i.onDidChangeInterpreter(TypeMoq.It.isAny())) + .returns((cb) => { + interpreterChangedHandler = cb; + return disposable.object; + }); + langFolderServiceMock + .setup((l) => l.getCurrentLanguageServerDirectory()) + .returns(() => Promise.resolve(folderVer)); + stateFactory + .setup((f) => + f.createGlobalPersistentState( + TypeMoq.It.isValue('SWITCH_LS'), + TypeMoq.It.isAny(), + TypeMoq.It.isAny() ) ) - .returns(() => activatorDotNet.object) - .verifiable(TypeMoq.Times.once()); - activatorDotNet - .setup((a) => a.start(undefined, undefined)) - .returns(() => Promise.reject(new Error(''))) - .verifiable(TypeMoq.Times.once()); + .returns(() => state.object); + state.setup((s) => s.value).returns(() => undefined); + state.setup((s) => s.updateValue(TypeMoq.It.isAny())).returns(() => Promise.resolve()); + workspaceConfig = TypeMoq.Mock.ofType(); + workspaceService + .setup((ws) => ws.getConfiguration('python', TypeMoq.It.isAny())) + .returns(() => workspaceConfig.object); + const output = TypeMoq.Mock.ofType(); serviceContainer - .setup((c) => - c.get( - TypeMoq.It.isValue(ILanguageServerActivator), - TypeMoq.It.isValue(LanguageServerType.Jedi) + .setup((c) => c.get(TypeMoq.It.isValue(IOutputChannel), TypeMoq.It.isAny())) + .returns(() => output.object); + serviceContainer + .setup((c) => c.get(TypeMoq.It.isValue(IWorkspaceService))) + .returns(() => workspaceService.object); + serviceContainer + .setup((c) => c.get(TypeMoq.It.isValue(IApplicationShell))) + .returns(() => appShell.object); + serviceContainer.setup((c) => c.get(TypeMoq.It.isValue(IDisposableRegistry))).returns(() => []); + serviceContainer + .setup((c) => c.get(TypeMoq.It.isValue(IConfigurationService))) + .returns(() => configService.object); + serviceContainer + .setup((c) => c.get(TypeMoq.It.isValue(ICommandManager))) + .returns(() => cmdManager.object); + serviceContainer + .setup((c) => c.get(TypeMoq.It.isValue(IPlatformService))) + .returns(() => platformService.object); + serviceContainer + .setup((c) => c.get(TypeMoq.It.isValue(IInterpreterService))) + .returns(() => interpreterService.object); + serviceContainer + .setup((c) => c.get(TypeMoq.It.isValue(ILanguageServerFolderService))) + .returns(() => langFolderServiceMock.object); + serviceContainer + .setup((s) => + s.get( + TypeMoq.It.isValue(IDiagnosticsService), + TypeMoq.It.isValue(LSNotSupportedDiagnosticServiceId) ) ) - .returns(() => activatorJedi.object) - .verifiable(TypeMoq.Times.once()); - activatorJedi - .setup((a) => a.start(undefined, undefined)) - .returns(() => Promise.resolve()) - .verifiable(TypeMoq.Times.once()); - activatorJedi - .setup((a) => a.activate()) - .returns(() => Promise.resolve()) - .verifiable(TypeMoq.Times.once()); - - await activationService.activate(undefined); - - activatorDotNet.verifyAll(); - activatorJedi.verifyAll(); - serviceContainer.verifyAll(); + .returns(() => lsNotSupportedDiagnosticService.object); }); - async function testActivationOfResource( + + async function testActivation( activationService: IExtensionActivationService, activator: TypeMoq.IMock, - resource: Resource + lsSupported: boolean = true, + activatorName: LanguageServerType = LanguageServerType.Jedi ) { activator - .setup((a) => a.start(TypeMoq.It.isValue(resource), undefined)) + .setup((a) => a.start(undefined, undefined)) .returns(() => Promise.resolve()) .verifiable(TypeMoq.Times.once()); activator.setup((a) => a.activate()).verifiable(TypeMoq.Times.once()); + + if ( + activatorName !== LanguageServerType.None && + lsSupported && + activatorName !== LanguageServerType.Jedi + ) { + activatorName = LanguageServerType.Node; + } + + let diagnostics: IDiagnostic[]; + if (!lsSupported && activatorName !== LanguageServerType.Jedi) { + diagnostics = [TypeMoq.It.isAny()]; + } else { + diagnostics = []; + } + lsNotSupportedDiagnosticService .setup((l) => l.diagnose(undefined)) - .returns(() => Promise.resolve([])); + .returns(() => Promise.resolve(diagnostics)); lsNotSupportedDiagnosticService - .setup((l) => l.handle(TypeMoq.It.isValue([]))) + .setup((l) => l.handle(TypeMoq.It.isValue(diagnostics))) .returns(() => Promise.resolve()); serviceContainer .setup((c) => - c.get( - TypeMoq.It.isValue(ILanguageServerActivator), - TypeMoq.It.isValue(LanguageServerType.Microsoft) - ) + c.get(TypeMoq.It.isValue(ILanguageServerActivator), TypeMoq.It.isValue(activatorName)) ) .returns(() => activator.object) - .verifiable(TypeMoq.Times.atLeastOnce()); - workspaceService - .setup((w) => w.getWorkspaceFolderIdentifier(resource, '')) - .returns(() => resource!.fsPath) - .verifiable(TypeMoq.Times.atLeastOnce()); + .verifiable(TypeMoq.Times.once()); + experiments .setup((ex) => ex.inExperiment(TypeMoq.It.isAny())) .returns(() => false) .verifiable(TypeMoq.Times.never()); - await activationService.activate(resource); + await activationService.activate(undefined); activator.verifyAll(); serviceContainer.verifyAll(); - workspaceService.verifyAll(); experiments.verifyAll(); } - test('Activator is disposed if activated workspace is removed', async () => { - pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Node); - let workspaceFoldersChangedHandler!: Function; + + async function testReloadMessage(settingName: string): Promise { + let callbackHandler!: (e: ConfigurationChangeEvent) => Promise; workspaceService - .setup((w) => w.onDidChangeWorkspaceFolders(TypeMoq.It.isAny(), TypeMoq.It.isAny())) - .callback((cb) => (workspaceFoldersChangedHandler = cb)) - .returns(() => TypeMoq.Mock.ofType().object) + .setup((w) => + w.onDidChangeConfiguration(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny()) + ) + .callback((cb) => (callbackHandler = cb)) + .returns(() => TypeMoq.Mock.ofType().object) .verifiable(TypeMoq.Times.once()); + + pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Node); + const activator = TypeMoq.Mock.ofType(); const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, stateFactory.object, experiments.object ); + workspaceService.verifyAll(); - expect(workspaceFoldersChangedHandler).not.to.be.equal(undefined, 'Handler not set'); - const folder1 = { name: 'one', uri: Uri.parse('one'), index: 1 }; - const folder2 = { name: 'two', uri: Uri.parse('two'), index: 2 }; - const folder3 = { name: 'three', uri: Uri.parse('three'), index: 3 }; - - const activator1 = TypeMoq.Mock.ofType(); - await testActivationOfResource(activationService, activator1, folder1.uri); - const activator2 = TypeMoq.Mock.ofType(); - await testActivationOfResource(activationService, activator2, folder2.uri); - const activator3 = TypeMoq.Mock.ofType(); - await testActivationOfResource(activationService, activator3, folder3.uri); - - //Now remove folder3 - workspaceService.reset(); - workspaceService.setup((w) => w.workspaceFolders).returns(() => [folder1, folder2]); - workspaceService - .setup((w) => w.getWorkspaceFolderIdentifier(folder1.uri, '')) - .returns(() => folder1.uri.fsPath) - .verifiable(TypeMoq.Times.atLeastOnce()); - workspaceService - .setup((w) => w.getWorkspaceFolderIdentifier(folder2.uri, '')) - .returns(() => folder2.uri.fsPath) + await testActivation(activationService, activator); + + const event = TypeMoq.Mock.ofType(); + event + .setup((e) => + e.affectsConfiguration(TypeMoq.It.isValue(`python.${settingName}`), TypeMoq.It.isAny()) + ) + .returns(() => true) .verifiable(TypeMoq.Times.atLeastOnce()); - activator1.setup((d) => d.dispose()).verifiable(TypeMoq.Times.never()); - activator2.setup((d) => d.dispose()).verifiable(TypeMoq.Times.never()); - activator3.setup((d) => d.dispose()).verifiable(TypeMoq.Times.once()); - await workspaceFoldersChangedHandler.call(activationService); - workspaceService.verifyAll(); - activator3.verifyAll(); + appShell + .setup((a) => a.showInformationMessage(TypeMoq.It.isAny(), TypeMoq.It.isValue('Reload'))) + .returns(() => Promise.resolve('Reload')) + .verifiable(TypeMoq.Times.once()); + cmdManager + .setup((c) => c.executeCommand(TypeMoq.It.isValue('workbench.action.reloadWindow'))) + .verifiable(TypeMoq.Times.once()); + + // Toggle the value in the setting and invoke the callback. + languageServerType = + languageServerType === LanguageServerType.Jedi + ? LanguageServerType.Node + : LanguageServerType.Jedi; + await callbackHandler(event.object); + + event.verifyAll(); + appShell.verifyAll(); + cmdManager.verifyAll(); + } + + test('LS is supported', async () => { + pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Node); + const activator = TypeMoq.Mock.ofType(); + const activationService = new LanguageServerExtensionActivationService( + serviceContainer.object, + stateFactory.object, + experiments.object + ); + + await testActivation(activationService, activator, true); }); - } else { - test('Jedi is only started once', async () => { - pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Microsoft); - const activator1 = TypeMoq.Mock.ofType(); + test('LS is not supported', async () => { + pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Node); + const activator = TypeMoq.Mock.ofType(); const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, stateFactory.object, experiments.object ); - const folder1 = { name: 'one', uri: Uri.parse('one'), index: 1 }; - const folder2 = { name: 'two', uri: Uri.parse('two'), index: 2 }; - serviceContainer - .setup((c) => - c.get( - TypeMoq.It.isValue(ILanguageServerActivator), - TypeMoq.It.isValue(LanguageServerType.Jedi) - ) + + await testActivation(activationService, activator, false); + }); + + test('Activator must be activated', async () => { + pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Node); + const activator = TypeMoq.Mock.ofType(); + const activationService = new LanguageServerExtensionActivationService( + serviceContainer.object, + stateFactory.object, + experiments.object + ); + + await testActivation(activationService, activator); + }); + test('Activator must be deactivated', async () => { + pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Node); + const activator = TypeMoq.Mock.ofType(); + const activationService = new LanguageServerExtensionActivationService( + serviceContainer.object, + stateFactory.object, + experiments.object + ); + + await testActivation(activationService, activator); + + activator.setup((a) => a.dispose()).verifiable(TypeMoq.Times.once()); + + activationService.dispose(); + activator.verifyAll(); + }); + test('No language service', async () => { + pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.None); + const activator = TypeMoq.Mock.ofType(); + const activationService = new LanguageServerExtensionActivationService( + serviceContainer.object, + stateFactory.object, + experiments.object + ); + await testActivation(activationService, activator, false, LanguageServerType.None); + }); + test('Prompt user to reload VS Code and reload, when languageServer setting is toggled', async () => { + await testReloadMessage('languageServer'); + }); + test('Do not prompt user to reload VS Code when setting is not changed', async () => { + let callbackHandler!: (e: ConfigurationChangeEvent) => Promise; + workspaceService + .setup((w) => + w.onDidChangeConfiguration(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny()) ) - .returns(() => activator1.object) + .callback((cb) => (callbackHandler = cb)) + .returns(() => TypeMoq.Mock.ofType().object) .verifiable(TypeMoq.Times.once()); - activator1 - .setup((a) => a.start(folder1.uri, undefined)) - .returns(() => Promise.resolve()) - .verifiable(TypeMoq.Times.once()); - experiments - .setup((ex) => ex.inExperiment(TypeMoq.It.isAny())) + + pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Node); + const activator = TypeMoq.Mock.ofType(); + const activationService = new LanguageServerExtensionActivationService( + serviceContainer.object, + stateFactory.object, + experiments.object + ); + + workspaceService.verifyAll(); + await testActivation(activationService, activator); + + const event = TypeMoq.Mock.ofType(); + event + .setup((e) => + e.affectsConfiguration(TypeMoq.It.isValue('python.languageServer'), TypeMoq.It.isAny()) + ) .returns(() => false) + .verifiable(TypeMoq.Times.atLeastOnce()); + appShell + .setup((a) => a.showInformationMessage(TypeMoq.It.isAny(), TypeMoq.It.isValue('Reload'))) + .returns(() => Promise.resolve(undefined)) .verifiable(TypeMoq.Times.never()); - await activationService.activate(folder1.uri); - activator1.verifyAll(); - activator1.verify((a) => a.activate(), TypeMoq.Times.once()); - serviceContainer.verifyAll(); - experiments.verifyAll(); + cmdManager + .setup((c) => c.executeCommand(TypeMoq.It.isValue('workbench.action.reloadWindow'))) + .verifiable(TypeMoq.Times.never()); + + // Invoke the config changed callback. + await callbackHandler(event.object); - const activator2 = TypeMoq.Mock.ofType(); + event.verifyAll(); + appShell.verifyAll(); + cmdManager.verifyAll(); + }); + test('More than one LS is created for multiple interpreters', async () => { + const interpreter1: PythonInterpreter = { + path: '/foo/bar/python', + sysPrefix: '1', + envName: '1', + sysVersion: '3.1.1.1', + architecture: Architecture.x64, + type: InterpreterType.Unknown + }; + const interpreter2: PythonInterpreter = { + path: '/foo/baz/python', + sysPrefix: '1', + envName: '2', + sysVersion: '3.1.1.1', + architecture: Architecture.x64, + type: InterpreterType.Unknown + }; + const folder1 = { name: 'one', uri: Uri.parse('one'), index: 1 }; + const activator = TypeMoq.Mock.ofType(); + activator + .setup((a) => a.start(TypeMoq.It.isValue(folder1.uri), TypeMoq.It.isValue(interpreter1))) + .returns(() => Promise.resolve()) + .verifiable(TypeMoq.Times.once()); + activator + .setup((a) => a.start(TypeMoq.It.isValue(folder1.uri), TypeMoq.It.isValue(interpreter2))) + .returns(() => Promise.resolve()) + .verifiable(TypeMoq.Times.once()); + activator.setup((a) => a.deactivate()).verifiable(TypeMoq.Times.never()); + activator.setup((a) => a.activate()).verifiable(TypeMoq.Times.never()); + activator + .setup((a) => a.dispose()) + .returns(noop) + .verifiable(TypeMoq.Times.exactly(2)); serviceContainer - .setup((c) => - c.get( - TypeMoq.It.isValue(ILanguageServerActivator), - TypeMoq.It.isValue(LanguageServerType.Jedi) - ) - ) - .returns(() => activator2.object) + .setup((c) => c.get(TypeMoq.It.isValue(ILanguageServerActivator), TypeMoq.It.isAny())) + .returns(() => activator.object); + let diagnostics: IDiagnostic[] = [TypeMoq.It.isAny()]; + lsNotSupportedDiagnosticService + .setup((l) => l.diagnose(undefined)) + .returns(() => Promise.resolve(diagnostics)); + lsNotSupportedDiagnosticService + .setup((l) => l.handle(TypeMoq.It.isValue(diagnostics))) + .returns(() => Promise.resolve()); + + pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Node); + const activationService = new LanguageServerExtensionActivationService( + serviceContainer.object, + stateFactory.object, + experiments.object + ); + const ls1 = await activationService.get(folder1.uri, interpreter1); + const ls2 = await activationService.get(folder1.uri, interpreter2); + expect(ls1).not.to.be.equal(ls2, 'Interpreter does not create new LS'); + const ls3 = await activationService.get(undefined, interpreter1); + expect(ls1).to.be.equal(ls3, 'Interpreter does return same LS'); + ls3.dispose(); + ls1.dispose(); + ls2.dispose(); + activator.verifyAll(); + }); + test('Changing interpreter will activate a new LS', async () => { + const interpreter1: PythonInterpreter = { + path: '/foo/bar/python', + sysPrefix: '1', + envName: '1', + sysVersion: '3.1.1.1', + architecture: Architecture.x64, + type: InterpreterType.Unknown + }; + const interpreter2: PythonInterpreter = { + path: '/foo/baz/python', + sysPrefix: '1', + envName: '2', + sysVersion: '3.1.1.1', + architecture: Architecture.x64, + type: InterpreterType.Unknown + }; + let getActiveCount = 0; + interpreterService + .setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny())) + .returns(() => { + if (getActiveCount % 2 === 0) { + getActiveCount += 1; + return Promise.resolve(interpreter1); + } + getActiveCount += 1; + return Promise.resolve(interpreter2); + }); + const folder1 = { name: 'one', uri: Uri.parse('one'), index: 1 }; + const activator = TypeMoq.Mock.ofType(); + activator + .setup((a) => a.start(TypeMoq.It.isValue(folder1.uri), TypeMoq.It.isValue(interpreter1))) + .returns(() => Promise.resolve()) .verifiable(TypeMoq.Times.once()); - activator2 - .setup((a) => a.start(folder2.uri, undefined)) + activator + .setup((a) => a.start(TypeMoq.It.isValue(folder1.uri), TypeMoq.It.isValue(interpreter2))) .returns(() => Promise.resolve()) - .verifiable(TypeMoq.Times.never()); - activator2.setup((a) => a.activate()).verifiable(TypeMoq.Times.never()); - experiments - .setup((ex) => ex.inExperiment(TypeMoq.It.isAny())) - .returns(() => false) - .verifiable(TypeMoq.Times.never()); - await activationService.activate(folder2.uri); - serviceContainer.verifyAll(); - activator1.verifyAll(); - activator1.verify((a) => a.activate(), TypeMoq.Times.exactly(2)); - activator2.verifyAll(); - experiments.verifyAll(); + .verifiable(TypeMoq.Times.once()); + let connectCount = 0; + activator + .setup((a) => a.activate()) + .returns(() => { + connectCount = connectCount + 1; + }); + serviceContainer + .setup((c) => c.get(TypeMoq.It.isValue(ILanguageServerActivator), TypeMoq.It.isAny())) + .returns(() => activator.object); + let diagnostics: IDiagnostic[] = [TypeMoq.It.isAny()]; + lsNotSupportedDiagnosticService + .setup((l) => l.diagnose(undefined)) + .returns(() => Promise.resolve(diagnostics)); + lsNotSupportedDiagnosticService + .setup((l) => l.handle(TypeMoq.It.isValue(diagnostics))) + .returns(() => Promise.resolve()); + + pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Node); + const activationService = new LanguageServerExtensionActivationService( + serviceContainer.object, + stateFactory.object, + experiments.object + ); + await activationService.activate(folder1.uri); + await interpreterChangedHandler(); + activator.verifyAll(); + + // Hold onto the second item and switch two more times. Verify that + // reconnect happens + const server = await activationService.get(folder1.uri); + await interpreterChangedHandler(); + expect(connectCount).to.be.equal(3, 'Reconnect is not happening'); + await interpreterChangedHandler(); + expect(connectCount).to.be.equal(4, 'Reconnect is not happening'); + server.dispose(); }); + if (languageServerType !== LanguageServerType.Jedi) { + test('Revert to jedi when LS activation fails', async () => { + pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Node); + const activatorDotNet = TypeMoq.Mock.ofType(); + const activatorJedi = TypeMoq.Mock.ofType(); + const activationService = new LanguageServerExtensionActivationService( + serviceContainer.object, + stateFactory.object, + experiments.object + ); + const diagnostics: IDiagnostic[] = []; + lsNotSupportedDiagnosticService + .setup((l) => l.diagnose(undefined)) + .returns(() => Promise.resolve(diagnostics)); + lsNotSupportedDiagnosticService + .setup((l) => l.handle(TypeMoq.It.isValue(diagnostics))) + .returns(() => Promise.resolve()); + serviceContainer + .setup((c) => + c.get( + TypeMoq.It.isValue(ILanguageServerActivator), + TypeMoq.It.isValue(LanguageServerType.Microsoft) + ) + ) + .returns(() => activatorDotNet.object) + .verifiable(TypeMoq.Times.once()); + activatorDotNet + .setup((a) => a.start(undefined, undefined)) + .returns(() => Promise.reject(new Error(''))) + .verifiable(TypeMoq.Times.once()); + serviceContainer + .setup((c) => + c.get( + TypeMoq.It.isValue(ILanguageServerActivator), + TypeMoq.It.isValue(LanguageServerType.Jedi) + ) + ) + .returns(() => activatorJedi.object) + .verifiable(TypeMoq.Times.once()); + activatorJedi + .setup((a) => a.start(undefined, undefined)) + .returns(() => Promise.resolve()) + .verifiable(TypeMoq.Times.once()); + activatorJedi + .setup((a) => a.activate()) + .returns(() => Promise.resolve()) + .verifiable(TypeMoq.Times.once()); + + await activationService.activate(undefined); + + activatorDotNet.verifyAll(); + activatorJedi.verifyAll(); + serviceContainer.verifyAll(); + }); + async function testActivationOfResource( + activationService: IExtensionActivationService, + activator: TypeMoq.IMock, + resource: Resource + ) { + activator + .setup((a) => a.start(TypeMoq.It.isValue(resource), undefined)) + .returns(() => Promise.resolve()) + .verifiable(TypeMoq.Times.once()); + activator.setup((a) => a.activate()).verifiable(TypeMoq.Times.once()); + lsNotSupportedDiagnosticService + .setup((l) => l.diagnose(undefined)) + .returns(() => Promise.resolve([])); + lsNotSupportedDiagnosticService + .setup((l) => l.handle(TypeMoq.It.isValue([]))) + .returns(() => Promise.resolve()); + serviceContainer + .setup((c) => + c.get( + TypeMoq.It.isValue(ILanguageServerActivator), + TypeMoq.It.isValue(LanguageServerType.Microsoft) + ) + ) + .returns(() => activator.object) + .verifiable(TypeMoq.Times.atLeastOnce()); + workspaceService + .setup((w) => w.getWorkspaceFolderIdentifier(resource, '')) + .returns(() => resource!.fsPath) + .verifiable(TypeMoq.Times.atLeastOnce()); + experiments + .setup((ex) => ex.inExperiment(TypeMoq.It.isAny())) + .returns(() => false) + .verifiable(TypeMoq.Times.never()); + + await activationService.activate(resource); + + activator.verifyAll(); + serviceContainer.verifyAll(); + workspaceService.verifyAll(); + experiments.verifyAll(); + } + test('Activator is disposed if activated workspace is removed', async () => { + pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Node); + let workspaceFoldersChangedHandler!: Function; + workspaceService + .setup((w) => w.onDidChangeWorkspaceFolders(TypeMoq.It.isAny(), TypeMoq.It.isAny())) + .callback((cb) => (workspaceFoldersChangedHandler = cb)) + .returns(() => TypeMoq.Mock.ofType().object) + .verifiable(TypeMoq.Times.once()); + const activationService = new LanguageServerExtensionActivationService( + serviceContainer.object, + stateFactory.object, + experiments.object + ); + workspaceService.verifyAll(); + expect(workspaceFoldersChangedHandler).not.to.be.equal(undefined, 'Handler not set'); + const folder1 = { name: 'one', uri: Uri.parse('one'), index: 1 }; + const folder2 = { name: 'two', uri: Uri.parse('two'), index: 2 }; + const folder3 = { name: 'three', uri: Uri.parse('three'), index: 3 }; + + const activator1 = TypeMoq.Mock.ofType(); + await testActivationOfResource(activationService, activator1, folder1.uri); + const activator2 = TypeMoq.Mock.ofType(); + await testActivationOfResource(activationService, activator2, folder2.uri); + const activator3 = TypeMoq.Mock.ofType(); + await testActivationOfResource(activationService, activator3, folder3.uri); + + //Now remove folder3 + workspaceService.reset(); + workspaceService.setup((w) => w.workspaceFolders).returns(() => [folder1, folder2]); + workspaceService + .setup((w) => w.getWorkspaceFolderIdentifier(folder1.uri, '')) + .returns(() => folder1.uri.fsPath) + .verifiable(TypeMoq.Times.atLeastOnce()); + workspaceService + .setup((w) => w.getWorkspaceFolderIdentifier(folder2.uri, '')) + .returns(() => folder2.uri.fsPath) + .verifiable(TypeMoq.Times.atLeastOnce()); + activator1.setup((d) => d.dispose()).verifiable(TypeMoq.Times.never()); + activator2.setup((d) => d.dispose()).verifiable(TypeMoq.Times.never()); + activator3.setup((d) => d.dispose()).verifiable(TypeMoq.Times.once()); + await workspaceFoldersChangedHandler.call(activationService); + workspaceService.verifyAll(); + activator3.verifyAll(); + }); + } else { + test('Jedi is only started once', async () => { + pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Microsoft); + const activator1 = TypeMoq.Mock.ofType(); + const activationService = new LanguageServerExtensionActivationService( + serviceContainer.object, + stateFactory.object, + experiments.object + ); + const folder1 = { name: 'one', uri: Uri.parse('one'), index: 1 }; + const folder2 = { name: 'two', uri: Uri.parse('two'), index: 2 }; + serviceContainer + .setup((c) => + c.get( + TypeMoq.It.isValue(ILanguageServerActivator), + TypeMoq.It.isValue(LanguageServerType.Jedi) + ) + ) + .returns(() => activator1.object) + .verifiable(TypeMoq.Times.once()); + activator1 + .setup((a) => a.start(folder1.uri, undefined)) + .returns(() => Promise.resolve()) + .verifiable(TypeMoq.Times.once()); + experiments + .setup((ex) => ex.inExperiment(TypeMoq.It.isAny())) + .returns(() => false) + .verifiable(TypeMoq.Times.never()); + await activationService.activate(folder1.uri); + activator1.verifyAll(); + activator1.verify((a) => a.activate(), TypeMoq.Times.once()); + serviceContainer.verifyAll(); + experiments.verifyAll(); + + const activator2 = TypeMoq.Mock.ofType(); + serviceContainer + .setup((c) => + c.get( + TypeMoq.It.isValue(ILanguageServerActivator), + TypeMoq.It.isValue(LanguageServerType.Jedi) + ) + ) + .returns(() => activator2.object) + .verifiable(TypeMoq.Times.once()); + activator2 + .setup((a) => a.start(folder2.uri, undefined)) + .returns(() => Promise.resolve()) + .verifiable(TypeMoq.Times.never()); + activator2.setup((a) => a.activate()).verifiable(TypeMoq.Times.never()); + experiments + .setup((ex) => ex.inExperiment(TypeMoq.It.isAny())) + .returns(() => false) + .verifiable(TypeMoq.Times.never()); + await activationService.activate(folder2.uri); + serviceContainer.verifyAll(); + activator1.verifyAll(); + activator1.verify((a) => a.activate(), TypeMoq.Times.exactly(2)); + activator2.verifyAll(); + experiments.verifyAll(); + }); + } } - }); + ); }); suite('Test sendTelemetryForChosenLanguageServer()', () => { @@ -992,7 +1005,7 @@ suite('Language Server Activation - ActivationService', () => { async () => { [ { - testName: 'Returns false when python settings value is Microsoft (v2)', + testName: 'Returns false when python settings value is Microsoft', pythonSettingsValue: LanguageServerType.Node, expectedResult: false }, From 7646a817fd699597b04b5ab9103f27b56d8babb6 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Fri, 15 May 2020 08:22:48 -0700 Subject: [PATCH 07/34] Update url --- .github/release_plan.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/release_plan.md b/.github/release_plan.md index 421ad949a4f2..77a89eb46095 100644 --- a/.github/release_plan.md +++ b/.github/release_plan.md @@ -12,8 +12,7 @@ - [ ] Change the version in [`package.json`](https://github.com/Microsoft/vscode-python/blob/master/package.json) from a `-dev` suffix to `-rc` (🤖) - [ ] Run `npm install` to make sure [`package-lock.json`](https://github.com/Microsoft/vscode-python/blob/master/package.json) is up-to-date (🤖) - [ ] Update `requirements.txt` to point to latest release version of [ptvsd](https://github.com/microsoft/ptvsd). - - [ ] Update `languageServerVersion` in `package.json` to point to the latest version of the [Language Server](https://github.com/Microsoft/ -github.com/Microsoft/pyrx). + - [ ] Update `languageServerVersion` in `package.json` to point to the latest version of the [Language Server](https://github.com/Microsoft/python-language-server). - [ ] Update [`CHANGELOG.md`](https://github.com/Microsoft/vscode-python/blob/master/CHANGELOG.md) (🤖) - [ ] Run [`news`](https://github.com/Microsoft/vscode-python/tree/master/news) (typically `python news --final --update CHANGELOG.md | code-insiders -`) - [ ] Copy over the "Thanks" section from the previous release From 058e032c7d0c43bacc0ebf6211fa2f8db610ec2a Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Fri, 15 May 2020 08:23:57 -0700 Subject: [PATCH 08/34] NAmes --- news/.vscode/settings.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/news/.vscode/settings.json b/news/.vscode/settings.json index b36d242f0c4f..2b759d72bd35 100644 --- a/news/.vscode/settings.json +++ b/news/.vscode/settings.json @@ -1,5 +1,5 @@ { - "python.languageServer": "Node", + "python.languageServer": "Microsoft", "python.formatting.provider": "black", "editor.formatOnSave": true, "python.testing.pytestArgs": ["."], From 4c1fb81bd3f02026b3948cd0397f5ea72e3a1e9e Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Fri, 15 May 2020 18:05:38 -0700 Subject: [PATCH 09/34] Formatting --- news/1 Enhancements/README.md | 2 + src/client/activation/activationService.ts | 13 +- .../common/languageServerFolderService.ts | 4 +- .../activation/node/languageClientFactory.ts | 2 +- src/client/common/configSettings.ts | 188 +++++++++--------- .../common/featureDeprecationManager.ts | 2 +- src/client/common/types.ts | 6 +- .../proposeLanguageServerBanner.ts | 9 +- src/client/linters/linterAvailability.ts | 4 +- src/client/linters/linterInfo.ts | 2 +- src/client/telemetry/index.ts | 6 +- .../testing/common/updateTestSettings.ts | 2 +- .../activation/activationService.unit.test.ts | 4 +- src/test/common.ts | 2 +- .../datascience/dataScienceIocContainer.ts | 2 +- .../datascience/intellisense.unit.test.ts | 2 +- .../linters/linter.availability.unit.test.ts | 2 +- src/test/linters/linterinfo.unit.test.ts | 2 +- src/test/performanceTest.ts | 2 +- src/test/smoke/common.ts | 2 +- src/test/smokeTest.ts | 2 +- 21 files changed, 137 insertions(+), 123 deletions(-) diff --git a/news/1 Enhancements/README.md b/news/1 Enhancements/README.md index ecc51777759d..0c3574379fa0 100644 --- a/news/1 Enhancements/README.md +++ b/news/1 Enhancements/README.md @@ -1 +1,3 @@ Changes that add new features. + +Removed `python.jediEnabled` setting. Use `"python.languageServer": "Jedi"` instead. diff --git a/src/client/activation/activationService.ts b/src/client/activation/activationService.ts index 31744f18bf0c..aac508624620 100644 --- a/src/client/activation/activationService.ts +++ b/src/client/activation/activationService.ts @@ -130,7 +130,10 @@ export class LanguageServerExtensionActivationService } @swallowExceptions('Send telemetry for Language Server current selection') public async sendTelemetryForChosenLanguageServer(languageServer: LanguageServerType): Promise { - const state = this.stateFactory.createGlobalPersistentState('SWITCH_LS', undefined); + const state = this.stateFactory.createGlobalPersistentState( + 'SWITCH_LS', + undefined + ); await state.updateValue(languageServer); sendTelemetryEvent(EventName.PYTHON_LANGUAGE_SERVER_CURRENT_SELECTION, undefined, { switchTo: languageServer @@ -143,7 +146,9 @@ export class LanguageServerExtensionActivationService * @returns `true` if user has NOT manually added the setting and is using default configuration, `false` if user has `languageServer` setting added */ public isJediUsingDefaultConfiguration(resource: Resource): boolean { - const settings = this.workspaceService.getConfiguration('python', resource).inspect('languageServer'); + const settings = this.workspaceService + .getConfiguration('python', resource) + .inspect('languageServer'); if (!settings) { traceError('WorkspaceConfiguration.inspect returns `undefined` for setting `python.languageServer`'); return false; @@ -283,7 +288,9 @@ export class LanguageServerExtensionActivationService const workspacesUris: (Uri | undefined)[] = this.workspaceService.hasWorkspaceFolders ? this.workspaceService.workspaceFolders!.map((workspace) => workspace.uri) : [undefined]; - if (workspacesUris.findIndex((uri) => event.affectsConfiguration(`python.${languageServerSetting}`, uri)) === -1) { + if ( + workspacesUris.findIndex((uri) => event.affectsConfiguration(`python.${languageServerSetting}`, uri)) === -1 + ) { return; } const jedi = this.useJedi(); diff --git a/src/client/activation/common/languageServerFolderService.ts b/src/client/activation/common/languageServerFolderService.ts index 92a5eb6c9e7e..bf7b48f25e58 100644 --- a/src/client/activation/common/languageServerFolderService.ts +++ b/src/client/activation/common/languageServerFolderService.ts @@ -26,7 +26,7 @@ export abstract class LanguageServerFolderService implements ILanguageServerFold constructor( @inject(IServiceContainer) protected readonly serviceContainer: IServiceContainer, @unmanaged() protected readonly languageServerFolder: string - ) { } + ) {} @traceDecorators.verbose('Get language server folder name') public async getLanguageServerFolderName(resource: Resource): Promise { @@ -110,7 +110,7 @@ export abstract class LanguageServerFolderService implements ILanguageServerFold minVersion = appEnv.packageJson[key] as string; } // tslint:disable-next-line: no-empty - } catch { } + } catch {} return minVersion; } diff --git a/src/client/activation/node/languageClientFactory.ts b/src/client/activation/node/languageClientFactory.ts index d607afdd5d31..8490c4910041 100644 --- a/src/client/activation/node/languageClientFactory.ts +++ b/src/client/activation/node/languageClientFactory.ts @@ -20,7 +20,7 @@ export class NodeLanguageClientFactory implements ILanguageClientFactory { constructor( @inject(IFileSystem) private readonly fs: IFileSystem, @inject(ILanguageServerFolderService) private readonly languageServerFolderService: ILanguageServerFolderService - ) { } + ) {} public async createLanguageClient( resource: Resource, diff --git a/src/client/common/configSettings.ts b/src/client/common/configSettings.ts index 23edeff65fc7..ab16070721ad 100644 --- a/src/client/common/configSettings.ts +++ b/src/client/common/configSettings.ts @@ -284,59 +284,59 @@ export class PythonSettings implements IPythonSettings { this.linting = this.linting ? this.linting : { - enabled: false, - ignorePatterns: [], - flake8Args: [], - flake8Enabled: false, - flake8Path: 'flake', - lintOnSave: false, - maxNumberOfProblems: 100, - mypyArgs: [], - mypyEnabled: false, - mypyPath: 'mypy', - banditArgs: [], - banditEnabled: false, - banditPath: 'bandit', - pycodestyleArgs: [], - pycodestyleEnabled: false, - pycodestylePath: 'pycodestyle', - pylamaArgs: [], - pylamaEnabled: false, - pylamaPath: 'pylama', - prospectorArgs: [], - prospectorEnabled: false, - prospectorPath: 'prospector', - pydocstyleArgs: [], - pydocstyleEnabled: false, - pydocstylePath: 'pydocstyle', - pylintArgs: [], - pylintEnabled: false, - pylintPath: 'pylint', - pylintCategorySeverity: { - convention: DiagnosticSeverity.Hint, - error: DiagnosticSeverity.Error, - fatal: DiagnosticSeverity.Error, - refactor: DiagnosticSeverity.Hint, - warning: DiagnosticSeverity.Warning - }, - pycodestyleCategorySeverity: { - E: DiagnosticSeverity.Error, - W: DiagnosticSeverity.Warning - }, - flake8CategorySeverity: { - E: DiagnosticSeverity.Error, - W: DiagnosticSeverity.Warning, - // Per http://flake8.pycqa.org/en/latest/glossary.html#term-error-code - // 'F' does not mean 'fatal as in PyLint but rather 'pyflakes' such as - // unused imports, variables, etc. - F: DiagnosticSeverity.Warning - }, - mypyCategorySeverity: { - error: DiagnosticSeverity.Error, - note: DiagnosticSeverity.Hint - }, - pylintUseMinimalCheckers: false - }; + enabled: false, + ignorePatterns: [], + flake8Args: [], + flake8Enabled: false, + flake8Path: 'flake', + lintOnSave: false, + maxNumberOfProblems: 100, + mypyArgs: [], + mypyEnabled: false, + mypyPath: 'mypy', + banditArgs: [], + banditEnabled: false, + banditPath: 'bandit', + pycodestyleArgs: [], + pycodestyleEnabled: false, + pycodestylePath: 'pycodestyle', + pylamaArgs: [], + pylamaEnabled: false, + pylamaPath: 'pylama', + prospectorArgs: [], + prospectorEnabled: false, + prospectorPath: 'prospector', + pydocstyleArgs: [], + pydocstyleEnabled: false, + pydocstylePath: 'pydocstyle', + pylintArgs: [], + pylintEnabled: false, + pylintPath: 'pylint', + pylintCategorySeverity: { + convention: DiagnosticSeverity.Hint, + error: DiagnosticSeverity.Error, + fatal: DiagnosticSeverity.Error, + refactor: DiagnosticSeverity.Hint, + warning: DiagnosticSeverity.Warning + }, + pycodestyleCategorySeverity: { + E: DiagnosticSeverity.Error, + W: DiagnosticSeverity.Warning + }, + flake8CategorySeverity: { + E: DiagnosticSeverity.Error, + W: DiagnosticSeverity.Warning, + // Per http://flake8.pycqa.org/en/latest/glossary.html#term-error-code + // 'F' does not mean 'fatal as in PyLint but rather 'pyflakes' such as + // unused imports, variables, etc. + F: DiagnosticSeverity.Warning + }, + mypyCategorySeverity: { + error: DiagnosticSeverity.Error, + note: DiagnosticSeverity.Hint + }, + pylintUseMinimalCheckers: false + }; this.linting.pylintPath = getAbsolutePath(systemVariables.resolveAny(this.linting.pylintPath), workspaceRoot); this.linting.flake8Path = getAbsolutePath(systemVariables.resolveAny(this.linting.flake8Path), workspaceRoot); this.linting.pycodestylePath = getAbsolutePath( @@ -366,14 +366,14 @@ export class PythonSettings implements IPythonSettings { this.formatting = this.formatting ? this.formatting : { - autopep8Args: [], - autopep8Path: 'autopep8', - provider: 'autopep8', - blackArgs: [], - blackPath: 'black', - yapfArgs: [], - yapfPath: 'yapf' - }; + autopep8Args: [], + autopep8Path: 'autopep8', + provider: 'autopep8', + blackArgs: [], + blackPath: 'black', + yapfArgs: [], + yapfPath: 'yapf' + }; this.formatting.autopep8Path = getAbsolutePath( systemVariables.resolveAny(this.formatting.autopep8Path), workspaceRoot @@ -397,11 +397,11 @@ export class PythonSettings implements IPythonSettings { this.autoComplete = this.autoComplete ? this.autoComplete : { - extraPaths: [], - addBrackets: false, - showAdvancedMembers: false, - typeshedPaths: [] - }; + extraPaths: [], + addBrackets: false, + showAdvancedMembers: false, + typeshedPaths: [] + }; // tslint:disable-next-line:no-backbone-get-set-outside-model no-non-null-assertion const workspaceSymbolsSettings = systemVariables.resolveAny( @@ -419,13 +419,13 @@ export class PythonSettings implements IPythonSettings { this.workspaceSymbols = this.workspaceSymbols ? this.workspaceSymbols : { - ctagsPath: 'ctags', - enabled: true, - exclusionPatterns: [], - rebuildOnFileSave: true, - rebuildOnStart: true, - tagFilePath: workspaceRoot ? path.join(workspaceRoot, 'tags') : '' - }; + ctagsPath: 'ctags', + enabled: true, + exclusionPatterns: [], + rebuildOnFileSave: true, + rebuildOnStart: true, + tagFilePath: workspaceRoot ? path.join(workspaceRoot, 'tags') : '' + }; this.workspaceSymbols.tagFilePath = getAbsolutePath( systemVariables.resolveAny(this.workspaceSymbols.tagFilePath), workspaceRoot @@ -460,18 +460,18 @@ export class PythonSettings implements IPythonSettings { this.testing = this.testing ? this.testing : { - promptToConfigure: true, - debugPort: 3000, - nosetestArgs: [], - nosetestPath: 'nosetest', - nosetestsEnabled: false, - pytestArgs: [], - pytestEnabled: false, - pytestPath: 'pytest', - unittestArgs: [], - unittestEnabled: false, - autoTestDiscoverOnSaveEnabled: true - }; + promptToConfigure: true, + debugPort: 3000, + nosetestArgs: [], + nosetestPath: 'nosetest', + nosetestsEnabled: false, + pytestArgs: [], + pytestEnabled: false, + pytestPath: 'pytest', + unittestArgs: [], + unittestEnabled: false, + autoTestDiscoverOnSaveEnabled: true + }; this.testing.pytestPath = getAbsolutePath(systemVariables.resolveAny(this.testing.pytestPath), workspaceRoot); this.testing.nosetestPath = getAbsolutePath( systemVariables.resolveAny(this.testing.nosetestPath), @@ -502,11 +502,11 @@ export class PythonSettings implements IPythonSettings { this.terminal = this.terminal ? this.terminal : { - executeInFileDir: true, - launchArgs: [], - activateEnvironment: true, - activateEnvInCurrentTerminal: false - }; + executeInFileDir: true, + launchArgs: [], + activateEnvironment: true, + activateEnvInCurrentTerminal: false + }; const experiments = systemVariables.resolveAny(pythonSettings.get('experiments'))!; if (this.experiments) { @@ -517,10 +517,10 @@ export class PythonSettings implements IPythonSettings { this.experiments = this.experiments ? this.experiments : { - enabled: true, - optInto: [], - optOutFrom: [] - }; + enabled: true, + optInto: [], + optOutFrom: [] + }; const dataScienceSettings = systemVariables.resolveAny( pythonSettings.get('dataScience') diff --git a/src/client/common/featureDeprecationManager.ts b/src/client/common/featureDeprecationManager.ts index 1ab971034c12..870743ce0723 100644 --- a/src/client/common/featureDeprecationManager.ts +++ b/src/client/common/featureDeprecationManager.ts @@ -44,7 +44,7 @@ export class FeatureDeprecationManager implements IFeatureDeprecationManager { @inject(ICommandManager) private cmdMgr: ICommandManager, @inject(IWorkspaceService) private workspace: IWorkspaceService, @inject(IApplicationShell) private appShell: IApplicationShell - ) { } + ) {} public dispose() { this.disposables.forEach((disposable) => disposable.dispose()); diff --git a/src/client/common/types.ts b/src/client/common/types.ts index d1bcf8f050ce..1e91cae7a6ac 100644 --- a/src/client/common/types.ts +++ b/src/client/common/types.ts @@ -23,9 +23,9 @@ import { ExtensionChannels } from './insidersBuild/types'; import { InterpreterUri } from './installer/types'; import { EnvironmentVariables } from './variables/types'; export const IOutputChannel = Symbol('IOutputChannel'); -export interface IOutputChannel extends OutputChannel { } +export interface IOutputChannel extends OutputChannel {} export const IDocumentSymbolProvider = Symbol('IDocumentSymbolProvider'); -export interface IDocumentSymbolProvider extends DocumentSymbolProvider { } +export interface IDocumentSymbolProvider extends DocumentSymbolProvider {} export const IsWindows = Symbol('IS_WINDOWS'); export const IDisposableRegistry = Symbol('IDisposableRegistry'); export type IDisposableRegistry = { push(disposable: Disposable): void }; @@ -470,7 +470,7 @@ export interface IHttpClient { } export const IExtensionContext = Symbol('ExtensionContext'); -export interface IExtensionContext extends ExtensionContext { } +export interface IExtensionContext extends ExtensionContext {} export const IExtensions = Symbol('IExtensions'); export interface IExtensions { diff --git a/src/client/languageServices/proposeLanguageServerBanner.ts b/src/client/languageServices/proposeLanguageServerBanner.ts index b962b4d39753..2c9e219859c1 100644 --- a/src/client/languageServices/proposeLanguageServerBanner.ts +++ b/src/client/languageServices/proposeLanguageServerBanner.ts @@ -5,11 +5,11 @@ import { inject, injectable } from 'inversify'; import { ConfigurationTarget } from 'vscode'; +import { LanguageServerType } from '../activation/types'; import { IApplicationShell } from '../common/application/types'; import '../common/extensions'; import { IConfigurationService, IPersistentStateFactory, IPythonExtensionBanner } from '../common/types'; import { getRandomBetween } from '../common/utils/random'; -import { LanguageServerType } from '../activation/types'; // persistent state names, exported to make use of in testing export enum ProposeLSStateKeys { @@ -113,6 +113,11 @@ export class ProposeLanguageServerBanner implements IPythonExtensionBanner { } public async enableLanguageServer(): Promise { - await this.configuration.updateSetting('languageServer', LanguageServerType.Node, undefined, ConfigurationTarget.Global); + await this.configuration.updateSetting( + 'languageServer', + LanguageServerType.Node, + undefined, + ConfigurationTarget.Global + ); } } diff --git a/src/client/linters/linterAvailability.ts b/src/client/linters/linterAvailability.ts index eeea0302bd17..f03643ecceb2 100644 --- a/src/client/linters/linterAvailability.ts +++ b/src/client/linters/linterAvailability.ts @@ -6,6 +6,7 @@ import { inject, injectable } from 'inversify'; import * as path from 'path'; import { Uri } from 'vscode'; +import { LanguageServerType } from '../activation/types'; import { IApplicationShell, IWorkspaceService } from '../common/application/types'; import '../common/extensions'; import { IFileSystem } from '../common/platform/types'; @@ -14,7 +15,6 @@ import { Common, Linters } from '../common/utils/localize'; import { sendTelemetryEvent } from '../telemetry'; import { EventName } from '../telemetry/constants'; import { IAvailableLinterActivator, ILinterInfo } from './types'; -import { LanguageServerType } from '../activation/types'; const doNotDisplayPromptStateKey = 'MESSAGE_KEY_FOR_CONFIGURE_AVAILABLE_LINTER_PROMPT'; @injectable() @@ -25,7 +25,7 @@ export class AvailableLinterActivator implements IAvailableLinterActivator { @inject(IWorkspaceService) private workspaceService: IWorkspaceService, @inject(IConfigurationService) private configService: IConfigurationService, @inject(IPersistentStateFactory) private persistentStateFactory: IPersistentStateFactory - ) { } + ) {} /** * Check if it is possible to enable an otherwise-unconfigured linter in diff --git a/src/client/linters/linterInfo.ts b/src/client/linters/linterInfo.ts index a5b2a29e36fd..fbb65fbadfd5 100644 --- a/src/client/linters/linterInfo.ts +++ b/src/client/linters/linterInfo.ts @@ -3,10 +3,10 @@ import * as path from 'path'; import { Uri } from 'vscode'; +import { LanguageServerType } from '../activation/types'; import { IWorkspaceService } from '../common/application/types'; import { ExecutionInfo, IConfigurationService, Product } from '../common/types'; import { ILinterInfo, LinterId } from './types'; -import { LanguageServerType } from '../activation/types'; // tslint:disable:no-any diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index 74f77462e586..c61b43bb4ed3 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -7,6 +7,7 @@ import { basename as pathBasename, sep as pathSep } from 'path'; import * as stackTrace from 'stack-trace'; import TelemetryReporter from 'vscode-extension-telemetry'; +import { LanguageServerType } from '../activation/types'; import { DiagnosticCodes } from '../application/diagnostics/constants'; import { IWorkspaceService } from '../common/application/types'; import { AppinsightsKey, EXTENSION_ROOT_DIR, isTestExecution, PVSC_EXTENSION_ID } from '../common/constants'; @@ -27,7 +28,6 @@ import { LinterId } from '../linters/types'; import { TestProvider } from '../testing/common/types'; import { EventName, PlatformErrors } from './constants'; import { LinterTrigger, TestTool } from './types'; -import { LanguageServerType } from '../activation/types'; /** * Checks whether telemetry is supported. @@ -117,8 +117,8 @@ export function sendTelemetryEvent

{ this.updateTestSettings(resource).ignoreErrors(); } diff --git a/src/test/activation/activationService.unit.test.ts b/src/test/activation/activationService.unit.test.ts index 5d4457de4d2f..a0d1e68d16f1 100644 --- a/src/test/activation/activationService.unit.test.ts +++ b/src/test/activation/activationService.unit.test.ts @@ -380,7 +380,7 @@ suite('Language Server Activation - ActivationService', () => { serviceContainer .setup((c) => c.get(TypeMoq.It.isValue(ILanguageServerActivator), TypeMoq.It.isAny())) .returns(() => activator.object); - let diagnostics: IDiagnostic[] = [TypeMoq.It.isAny()]; + const diagnostics: IDiagnostic[] = [TypeMoq.It.isAny()]; lsNotSupportedDiagnosticService .setup((l) => l.diagnose(undefined)) .returns(() => Promise.resolve(diagnostics)); @@ -451,7 +451,7 @@ suite('Language Server Activation - ActivationService', () => { serviceContainer .setup((c) => c.get(TypeMoq.It.isValue(ILanguageServerActivator), TypeMoq.It.isAny())) .returns(() => activator.object); - let diagnostics: IDiagnostic[] = [TypeMoq.It.isAny()]; + const diagnostics: IDiagnostic[] = [TypeMoq.It.isAny()]; lsNotSupportedDiagnosticService .setup((l) => l.diagnose(undefined)) .returns(() => Promise.resolve(diagnostics)); diff --git a/src/test/common.ts b/src/test/common.ts index dc39f4ae2523..22cd720f639f 100644 --- a/src/test/common.ts +++ b/src/test/common.ts @@ -571,7 +571,7 @@ export class FakeClock { * @param {number} [advacenTimeMs=10_000] Default `timeout` value. Defaults to 10s. Assuming we do not have anything bigger. * @memberof FakeClock */ - constructor(private readonly advacenTimeMs: number = 10_000) { } + constructor(private readonly advacenTimeMs: number = 10_000) {} public install() { // tslint:disable-next-line:no-require-imports const lolex = require('lolex'); diff --git a/src/test/datascience/dataScienceIocContainer.ts b/src/test/datascience/dataScienceIocContainer.ts index 842df1604e01..6417d637a1b2 100644 --- a/src/test/datascience/dataScienceIocContainer.ts +++ b/src/test/datascience/dataScienceIocContainer.ts @@ -1627,7 +1627,7 @@ export class DataScienceIocContainer extends UnitTestIocContainer { this.postMessageToWebPanel(msg); }, // tslint:disable-next-line:no-any no-empty - setState: (_msg: any) => { }, + setState: (_msg: any) => {}, // tslint:disable-next-line:no-any no-empty getState: () => { return {}; diff --git a/src/test/datascience/intellisense.unit.test.ts b/src/test/datascience/intellisense.unit.test.ts index 335ac0c563e6..016282699f5e 100644 --- a/src/test/datascience/intellisense.unit.test.ts +++ b/src/test/datascience/intellisense.unit.test.ts @@ -7,6 +7,7 @@ import * as uuid from 'uuid/v4'; import { instance, mock } from 'ts-mockito'; import { Uri } from 'vscode'; +import { LanguageServerType } from '../../client/activation/types'; import { IWorkspaceService } from '../../client/common/application/types'; import { PythonSettings } from '../../client/common/configSettings'; import { IFileSystem } from '../../client/common/platform/types'; @@ -26,7 +27,6 @@ import { createEmptyCell, generateTestCells } from '../../datascience-ui/interac import { generateReverseChange, IMonacoTextModel } from '../../datascience-ui/react-common/monacoHelpers'; import { MockAutoSelectionService } from '../mocks/autoSelector'; import { MockLanguageServerCache } from './mockLanguageServerCache'; -import { LanguageServerType } from '../../client/activation/types'; // tslint:disable:no-any unified-signatures const TestCellContents = `myvar = """ # Lorem Ipsum diff --git a/src/test/linters/linter.availability.unit.test.ts b/src/test/linters/linter.availability.unit.test.ts index fb39b496b3e4..bae239ea58e2 100644 --- a/src/test/linters/linter.availability.unit.test.ts +++ b/src/test/linters/linter.availability.unit.test.ts @@ -8,6 +8,7 @@ import * as path from 'path'; import { anything, instance, mock, verify, when } from 'ts-mockito'; import * as TypeMoq from 'typemoq'; import { Uri, WorkspaceConfiguration, WorkspaceFolder } from 'vscode'; +import { LanguageServerType } from '../../client/activation/types'; import { ApplicationShell } from '../../client/common/application/applicationShell'; import { IApplicationShell, IWorkspaceService } from '../../client/common/application/types'; import { WorkspaceService } from '../../client/common/application/workspace'; @@ -26,7 +27,6 @@ import { Common, Linters } from '../../client/common/utils/localize'; import { AvailableLinterActivator } from '../../client/linters/linterAvailability'; import { LinterInfo } from '../../client/linters/linterInfo'; import { IAvailableLinterActivator, ILinterInfo } from '../../client/linters/types'; -import { LanguageServerType } from '../../client/activation/types'; // tslint:disable:max-func-body-length no-any suite('Linter Availability Provider tests', () => { diff --git a/src/test/linters/linterinfo.unit.test.ts b/src/test/linters/linterinfo.unit.test.ts index b9de1addc9fe..86adcf8b14ca 100644 --- a/src/test/linters/linterinfo.unit.test.ts +++ b/src/test/linters/linterinfo.unit.test.ts @@ -7,10 +7,10 @@ import { expect } from 'chai'; import { anything, instance, mock, when } from 'ts-mockito'; +import { LanguageServerType } from '../../client/activation/types'; import { WorkspaceService } from '../../client/common/application/workspace'; import { ConfigurationService } from '../../client/common/configuration/service'; import { PylintLinterInfo } from '../../client/linters/linterInfo'; -import { LanguageServerType } from '../../client/activation/types'; suite('Linter Info - Pylint', () => { test('Test disabled when Pylint is explicitly disabled', async () => { diff --git a/src/test/performanceTest.ts b/src/test/performanceTest.ts index 594ebfee9f35..e5325d552643 100644 --- a/src/test/performanceTest.ts +++ b/src/test/performanceTest.ts @@ -22,9 +22,9 @@ import * as download from 'download'; import * as fs from 'fs-extra'; import * as path from 'path'; import * as request from 'request'; +import { LanguageServerType } from '../client/activation/types'; import { EXTENSION_ROOT_DIR, PVSC_EXTENSION_ID } from '../client/common/constants'; import { unzip } from './common'; -import { LanguageServerType } from '../client/activation/types'; const NamedRegexp = require('named-js-regexp'); const del = require('del'); diff --git a/src/test/smoke/common.ts b/src/test/smoke/common.ts index e6e0902fe50b..da3c3eafa13b 100644 --- a/src/test/smoke/common.ts +++ b/src/test/smoke/common.ts @@ -10,9 +10,9 @@ import * as fs from 'fs-extra'; import * as glob from 'glob'; import * as path from 'path'; import * as vscode from 'vscode'; +import { LanguageServerType } from '../../client/activation/types'; import { SMOKE_TEST_EXTENSIONS_DIR } from '../constants'; import { noop, sleep } from '../core'; -import { LanguageServerType } from '../../client/activation/types'; export async function updateSetting(setting: string, value: any) { const resource = vscode.workspace.workspaceFolders![0].uri; diff --git a/src/test/smokeTest.ts b/src/test/smokeTest.ts index 93eff3cec5b6..1d0c68b2e4c3 100644 --- a/src/test/smokeTest.ts +++ b/src/test/smokeTest.ts @@ -12,9 +12,9 @@ import { spawn } from 'child_process'; import * as fs from 'fs-extra'; import * as glob from 'glob'; import * as path from 'path'; +import { LanguageServerType } from '../client/activation/types'; import { unzip } from './common'; import { EXTENSION_ROOT_DIR_FOR_TESTS, SMOKE_TEST_EXTENSIONS_DIR } from './constants'; -import { LanguageServerType } from '../client/activation/types'; class TestRunner { public async start() { From 8120a4d2b38fce06f4a1e9a1dfbf8bf7e938d894 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Fri, 15 May 2020 18:18:53 -0700 Subject: [PATCH 10/34] Formatting --- src/client/activation/common/languageServerFolderService.ts | 6 +++--- src/test/linters/common.ts | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/client/activation/common/languageServerFolderService.ts b/src/client/activation/common/languageServerFolderService.ts index bf7b48f25e58..591653e903e0 100644 --- a/src/client/activation/common/languageServerFolderService.ts +++ b/src/client/activation/common/languageServerFolderService.ts @@ -6,6 +6,7 @@ import { inject, injectable, unmanaged } from 'inversify'; import * as path from 'path'; import * as semver from 'semver'; +import { IApplicationEnvironment } from '../../common/application/types'; import { EXTENSION_ROOT_DIR } from '../../common/constants'; import { traceDecorators } from '../../common/logger'; import { NugetPackage } from '../../common/nuget/types'; @@ -13,13 +14,12 @@ import { IFileSystem } from '../../common/platform/types'; import { IConfigurationService, Resource } from '../../common/types'; import { IServiceContainer } from '../../ioc/types'; import { + DotNetLanguageServerFolder, FolderVersionPair, IDownloadChannelRule, ILanguageServerFolderService, - ILanguageServerPackageService, - DotNetLanguageServerFolder + ILanguageServerPackageService } from '../types'; -import { IApplicationEnvironment } from '../../common/application/types'; @injectable() export abstract class LanguageServerFolderService implements ILanguageServerFolderService { diff --git a/src/test/linters/common.ts b/src/test/linters/common.ts index ad6ccb56ef99..4c5d03f066ef 100644 --- a/src/test/linters/common.ts +++ b/src/test/linters/common.ts @@ -5,6 +5,7 @@ import * as os from 'os'; import * as TypeMoq from 'typemoq'; import { DiagnosticSeverity, TextDocument, Uri, WorkspaceFolder } from 'vscode'; +import { LanguageServerType } from '../../client/activation/types'; import { IApplicationShell, IWorkspaceService } from '../../client/common/application/types'; import { Product } from '../../client/common/installer/productInstaller'; import { ProductNames } from '../../client/common/installer/productNames'; @@ -24,7 +25,6 @@ import { IServiceContainer } from '../../client/ioc/types'; import { LINTERID_BY_PRODUCT } from '../../client/linters/constants'; import { LinterManager } from '../../client/linters/linterManager'; import { ILinter, ILinterManager, ILintMessage, LinterId } from '../../client/linters/types'; -import { LanguageServerType } from '../../client/activation/types'; export function newMockDocument(filename: string): TypeMoq.IMock { const uri = Uri.file(filename); From 10ee2db8106a59bf2a7a8c988c10c40eb2d643da Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Sat, 16 May 2020 13:17:59 -0700 Subject: [PATCH 11/34] Test fixes --- src/client/activation/activationService.ts | 4 +-- src/client/common/configSettings.ts | 16 ++++----- .../activation/activationService.unit.test.ts | 34 ++++++++----------- .../configSettings.unit.test.ts | 11 ++---- 4 files changed, 27 insertions(+), 38 deletions(-) diff --git a/src/client/activation/activationService.ts b/src/client/activation/activationService.ts index aac508624620..c8689dace59b 100644 --- a/src/client/activation/activationService.ts +++ b/src/client/activation/activationService.ts @@ -141,9 +141,9 @@ export class LanguageServerExtensionActivationService } /** - * Checks if user has not manually set `languageServer` setting + * Checks if user does not have any `languageServer` setting set. * @param resource - * @returns `true` if user has NOT manually added the setting and is using default configuration, `false` if user has `languageServer` setting added + * @returns `true` if user is using default configuration, `false` if user has `languageServer` setting added. */ public isJediUsingDefaultConfiguration(resource: Resource): boolean { const settings = this.workspaceService diff --git a/src/client/common/configSettings.ts b/src/client/common/configSettings.ts index ab16070721ad..9b10d5f4ea0e 100644 --- a/src/client/common/configSettings.ts +++ b/src/client/common/configSettings.ts @@ -232,16 +232,14 @@ export class PythonSettings implements IPythonSettings { } this.languageServer = systemVariables.resolveAny(ls)!; - if (this.languageServer === LanguageServerType.Jedi) { - // tslint:disable-next-line:no-backbone-get-set-outside-model no-non-null-assertion - this.jediPath = systemVariables.resolveAny(pythonSettings.get('jediPath'))!; - if (typeof this.jediPath === 'string' && this.jediPath.length > 0) { - this.jediPath = getAbsolutePath(systemVariables.resolveAny(this.jediPath), workspaceRoot); - } else { - this.jediPath = ''; - } - this.jediMemoryLimit = pythonSettings.get('jediMemoryLimit')!; + // tslint:disable-next-line:no-backbone-get-set-outside-model no-non-null-assertion + this.jediPath = systemVariables.resolveAny(pythonSettings.get('jediPath'))!; + if (typeof this.jediPath === 'string' && this.jediPath.length > 0) { + this.jediPath = getAbsolutePath(systemVariables.resolveAny(this.jediPath), workspaceRoot); + } else { + this.jediPath = ''; } + this.jediMemoryLimit = pythonSettings.get('jediMemoryLimit')!; const envFileSetting = pythonSettings.get('envFile'); this.envFile = systemVariables.resolveAny(envFileSetting)!; diff --git a/src/test/activation/activationService.unit.test.ts b/src/test/activation/activationService.unit.test.ts index a0d1e68d16f1..17cc8320d51f 100644 --- a/src/test/activation/activationService.unit.test.ts +++ b/src/test/activation/activationService.unit.test.ts @@ -691,7 +691,7 @@ suite('Language Server Activation - ActivationService', () => { let platformService: TypeMoq.IMock; let lsNotSupportedDiagnosticService: TypeMoq.IMock; let stateFactory: TypeMoq.IMock; - let state: TypeMoq.IMock>; + let state: TypeMoq.IMock>; let experiments: TypeMoq.IMock; let workspaceConfig: TypeMoq.IMock; let interpreterService: TypeMoq.IMock; @@ -702,7 +702,7 @@ suite('Language Server Activation - ActivationService', () => { cmdManager = TypeMoq.Mock.ofType(); platformService = TypeMoq.Mock.ofType(); stateFactory = TypeMoq.Mock.ofType(); - state = TypeMoq.Mock.ofType>(); + state = TypeMoq.Mock.ofType>(); const configService = TypeMoq.Mock.ofType(); pythonSettings = TypeMoq.Mock.ofType(); experiments = TypeMoq.Mock.ofType(); @@ -773,11 +773,11 @@ suite('Language Server Activation - ActivationService', () => { state .setup((s) => s.value) .returns(() => undefined) - .verifiable(TypeMoq.Times.exactly(2)); + .verifiable(TypeMoq.Times.exactly(1)); state - .setup((s) => s.updateValue(TypeMoq.It.isValue(true))) + .setup((s) => s.updateValue(TypeMoq.It.isValue(LanguageServerType.Jedi))) .returns(() => { - state.setup((s) => s.value).returns(() => true); + state.setup((s) => s.value).returns(() => LanguageServerType.Jedi); return Promise.resolve(); }) .verifiable(TypeMoq.Times.once()); @@ -795,10 +795,10 @@ suite('Language Server Activation - ActivationService', () => { state.reset(); state .setup((s) => s.value) - .returns(() => true) - .verifiable(TypeMoq.Times.exactly(2)); + .returns(() => LanguageServerType.Jedi) + .verifiable(TypeMoq.Times.exactly(1)); state - .setup((s) => s.updateValue(TypeMoq.It.isValue(false))) + .setup((s) => s.updateValue(TypeMoq.It.isValue(LanguageServerType.Microsoft))) .returns(() => Promise.resolve()) .verifiable(TypeMoq.Times.once()); @@ -807,7 +807,7 @@ suite('Language Server Activation - ActivationService', () => { stateFactory.object, experiments.object ); - await activationService.sendTelemetryForChosenLanguageServer(LanguageServerType.Node); + await activationService.sendTelemetryForChosenLanguageServer(LanguageServerType.Microsoft); state.verifyAll(); }); @@ -815,10 +815,10 @@ suite('Language Server Activation - ActivationService', () => { state.reset(); state .setup((s) => s.value) - .returns(() => false) - .verifiable(TypeMoq.Times.exactly(2)); + .returns(() => LanguageServerType.Microsoft) + .verifiable(TypeMoq.Times.exactly(1)); state - .setup((s) => s.updateValue(TypeMoq.It.isValue(true))) + .setup((s) => s.updateValue(TypeMoq.It.isValue(LanguageServerType.Jedi))) .returns(() => Promise.resolve()) .verifiable(TypeMoq.Times.once()); @@ -835,8 +835,8 @@ suite('Language Server Activation - ActivationService', () => { state.reset(); state .setup((s) => s.value) - .returns(() => true) - .verifiable(TypeMoq.Times.exactly(2)); + .returns(() => LanguageServerType.Jedi) + .verifiable(TypeMoq.Times.exactly(1)); state .setup((s) => s.updateValue(TypeMoq.It.isAny())) .returns(() => Promise.resolve()) @@ -981,10 +981,6 @@ suite('Language Server Activation - ActivationService', () => { .setup((c) => c.inspect('languageServer')) .returns(() => settings as any) .verifiable(TypeMoq.Times.once()); - pythonSettings - .setup((p) => p.languageServer) - .returns(() => LanguageServerType.Jedi) - .verifiable(TypeMoq.Times.once()); const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, @@ -1006,7 +1002,7 @@ suite('Language Server Activation - ActivationService', () => { [ { testName: 'Returns false when python settings value is Microsoft', - pythonSettingsValue: LanguageServerType.Node, + pythonSettingsValue: LanguageServerType.Microsoft, expectedResult: false }, { diff --git a/src/test/common/configSettings/configSettings.unit.test.ts b/src/test/common/configSettings/configSettings.unit.test.ts index fbbefcc939f0..b022ced14144 100644 --- a/src/test/common/configSettings/configSettings.unit.test.ts +++ b/src/test/common/configSettings/configSettings.unit.test.ts @@ -66,16 +66,14 @@ suite('Python Settings', async () => { 'envFile', 'poetryPath', 'insidersChannel', - 'defaultInterpreterPath' + 'defaultInterpreterPath', + 'jediPath' ]) { config .setup((c) => c.get(name)) // tslint:disable-next-line:no-any .returns(() => (sourceSettings as any)[name]); } - if (sourceSettings.languageServer === LanguageServerType.Jedi) { - config.setup((c) => c.get('jediPath')).returns(() => sourceSettings.jediPath); - } for (const name of ['venvFolders']) { config .setup((c) => c.get(name)) @@ -98,10 +96,7 @@ suite('Python Settings', async () => { } // number settings - if (sourceSettings.languageServer === LanguageServerType.Jedi) { - config.setup((c) => c.get('jediMemoryLimit')).returns(() => sourceSettings.jediMemoryLimit); - } - + config.setup((c) => c.get('jediMemoryLimit')).returns(() => sourceSettings.jediMemoryLimit); // Language server type settings config.setup((c) => c.get('languageServer')).returns(() => sourceSettings.languageServer); From 109f9c5f1bb65aa99c77aecae8eca4861e3a323e Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Mon, 18 May 2020 10:56:51 -0700 Subject: [PATCH 12/34] Test fixes --- src/client/activation/activationService.ts | 23 +++++++--- src/client/telemetry/index.ts | 2 +- .../activation/activationService.unit.test.ts | 44 +++++++++---------- 3 files changed, 41 insertions(+), 28 deletions(-) diff --git a/src/client/activation/activationService.ts b/src/client/activation/activationService.ts index c8689dace59b..6ecc672b2388 100644 --- a/src/client/activation/activationService.ts +++ b/src/client/activation/activationService.ts @@ -130,14 +130,23 @@ export class LanguageServerExtensionActivationService } @swallowExceptions('Send telemetry for Language Server current selection') public async sendTelemetryForChosenLanguageServer(languageServer: LanguageServerType): Promise { - const state = this.stateFactory.createGlobalPersistentState( + const state = this.stateFactory.createGlobalPersistentState( 'SWITCH_LS', undefined ); - await state.updateValue(languageServer); - sendTelemetryEvent(EventName.PYTHON_LANGUAGE_SERVER_CURRENT_SELECTION, undefined, { - switchTo: languageServer - }); + if (typeof state.value !== 'string') { + await state.updateValue(languageServer); + } + if (state.value !== languageServer) { + await state.updateValue(languageServer); + sendTelemetryEvent(EventName.PYTHON_LANGUAGE_SERVER_CURRENT_SELECTION, undefined, { + switchTo: languageServer + }); + } else { + sendTelemetryEvent(EventName.PYTHON_LANGUAGE_SERVER_CURRENT_SELECTION, undefined, { + lsStartup: languageServer + }); + } } /** @@ -165,13 +174,17 @@ export class LanguageServerExtensionActivationService * @returns `true` if user is using jedi, `false` if user is using language server */ public useJedi(): boolean { + // Check if `languageServer` setting is missing (default configuration). if (this.isJediUsingDefaultConfiguration(this.resource)) { + // If user is assigned to an experiment (i.e. use LS), return false. if (this.abExperiments.inExperiment(LSEnabled)) { return false; } // Send telemetry if user is in control group this.abExperiments.sendTelemetryIfInExperiment(LSControl); + return true; // Do use Jedi as it is default. } + // Configuration is non-default, so `languageServer` should be present. const configurationService = this.serviceContainer.get(IConfigurationService); const lstType = configurationService.getSettings(this.resource).languageServer; this.sendTelemetryForChosenLanguageServer(lstType).ignoreErrors(); diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index c61b43bb4ed3..6b391487c255 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -1132,7 +1132,7 @@ export interface IEventNamePropertyMapping { /** * The startup value of the language server setting */ - lsStartup?: boolean; + lsStartup?: LanguageServerType; /** * Used to track switch between LS and Jedi. Carries the final state after the switch. */ diff --git a/src/test/activation/activationService.unit.test.ts b/src/test/activation/activationService.unit.test.ts index 17cc8320d51f..1b8a5aac2a92 100644 --- a/src/test/activation/activationService.unit.test.ts +++ b/src/test/activation/activationService.unit.test.ts @@ -344,7 +344,7 @@ suite('Language Server Activation - ActivationService', () => { appShell.verifyAll(); cmdManager.verifyAll(); }); - test('More than one LS is created for multiple interpreters', async () => { + test('More than one LS is created for multiple interpreters if LS is "Microsoft"', async () => { const interpreter1: PythonInterpreter = { path: '/foo/bar/python', sysPrefix: '1', @@ -380,15 +380,15 @@ suite('Language Server Activation - ActivationService', () => { serviceContainer .setup((c) => c.get(TypeMoq.It.isValue(ILanguageServerActivator), TypeMoq.It.isAny())) .returns(() => activator.object); - const diagnostics: IDiagnostic[] = [TypeMoq.It.isAny()]; + lsNotSupportedDiagnosticService .setup((l) => l.diagnose(undefined)) - .returns(() => Promise.resolve(diagnostics)); + .returns(() => Promise.resolve([])); lsNotSupportedDiagnosticService - .setup((l) => l.handle(TypeMoq.It.isValue(diagnostics))) + .setup((l) => l.handle(TypeMoq.It.isValue([]))) .returns(() => Promise.resolve()); - pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Node); + pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Microsoft); const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, stateFactory.object, @@ -404,7 +404,7 @@ suite('Language Server Activation - ActivationService', () => { ls2.dispose(); activator.verifyAll(); }); - test('Changing interpreter will activate a new LS', async () => { + test('Changing interpreter will activate a new LS if it is "Microsoft"', async () => { const interpreter1: PythonInterpreter = { path: '/foo/bar/python', sysPrefix: '1', @@ -459,7 +459,7 @@ suite('Language Server Activation - ActivationService', () => { .setup((l) => l.handle(TypeMoq.It.isValue(diagnostics))) .returns(() => Promise.resolve()); - pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Node); + pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Microsoft); const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, stateFactory.object, @@ -480,8 +480,8 @@ suite('Language Server Activation - ActivationService', () => { }); if (languageServerType !== LanguageServerType.Jedi) { test('Revert to jedi when LS activation fails', async () => { - pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Node); - const activatorDotNet = TypeMoq.Mock.ofType(); + pythonSettings.setup((p) => p.languageServer).returns(() => languageServerType); + const activatorLS = TypeMoq.Mock.ofType(); const activatorJedi = TypeMoq.Mock.ofType(); const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, @@ -499,12 +499,12 @@ suite('Language Server Activation - ActivationService', () => { .setup((c) => c.get( TypeMoq.It.isValue(ILanguageServerActivator), - TypeMoq.It.isValue(LanguageServerType.Microsoft) + TypeMoq.It.isValue(languageServerType) ) ) - .returns(() => activatorDotNet.object) + .returns(() => activatorLS.object) .verifiable(TypeMoq.Times.once()); - activatorDotNet + activatorLS .setup((a) => a.start(undefined, undefined)) .returns(() => Promise.reject(new Error(''))) .verifiable(TypeMoq.Times.once()); @@ -528,7 +528,7 @@ suite('Language Server Activation - ActivationService', () => { await activationService.activate(undefined); - activatorDotNet.verifyAll(); + activatorLS.verifyAll(); activatorJedi.verifyAll(); serviceContainer.verifyAll(); }); @@ -573,8 +573,8 @@ suite('Language Server Activation - ActivationService', () => { workspaceService.verifyAll(); experiments.verifyAll(); } - test('Activator is disposed if activated workspace is removed', async () => { - pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Node); + test('Activator is disposed if activated workspace is removed and LS is "Microsoft"', async () => { + pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Microsoft); let workspaceFoldersChangedHandler!: Function; workspaceService .setup((w) => w.onDidChangeWorkspaceFolders(TypeMoq.It.isAny(), TypeMoq.It.isAny())) @@ -619,7 +619,7 @@ suite('Language Server Activation - ActivationService', () => { }); } else { test('Jedi is only started once', async () => { - pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Microsoft); + pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Jedi); const activator1 = TypeMoq.Mock.ofType(); const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, @@ -798,7 +798,7 @@ suite('Language Server Activation - ActivationService', () => { .returns(() => LanguageServerType.Jedi) .verifiable(TypeMoq.Times.exactly(1)); state - .setup((s) => s.updateValue(TypeMoq.It.isValue(LanguageServerType.Microsoft))) + .setup((s) => s.updateValue(TypeMoq.It.isValue(LanguageServerType.Node))) .returns(() => Promise.resolve()) .verifiable(TypeMoq.Times.once()); @@ -807,7 +807,7 @@ suite('Language Server Activation - ActivationService', () => { stateFactory.object, experiments.object ); - await activationService.sendTelemetryForChosenLanguageServer(LanguageServerType.Microsoft); + await activationService.sendTelemetryForChosenLanguageServer(LanguageServerType.Node); state.verifyAll(); }); @@ -815,7 +815,7 @@ suite('Language Server Activation - ActivationService', () => { state.reset(); state .setup((s) => s.value) - .returns(() => LanguageServerType.Microsoft) + .returns(() => LanguageServerType.Node) .verifiable(TypeMoq.Times.exactly(1)); state .setup((s) => s.updateValue(TypeMoq.It.isValue(LanguageServerType.Jedi))) @@ -967,7 +967,7 @@ suite('Language Server Activation - ActivationService', () => { experiments.verifyAll(); }); - test('If default value of languageServer (Jedi) is being used, and LSEnabled experiment is disabled, then send telemetry if user is in Experiment LSControl and return python settings value (which will always be true as default value is true)', async () => { + test('If default value of languageServer (Jedi) is being used, and LSEnabled experiment is disabled, then send telemetry if user is in Experiment LSControl and useJedi() should be true)', async () => { const settings = {}; experiments .setup((ex) => ex.inExperiment(LSEnabled)) @@ -1037,8 +1037,8 @@ suite('Language Server Activation - ActivationService', () => { ); const result = activationService.useJedi(); expect(result).to.equal( - testParams.pythonSettingsValue, - `Return value should be ${testParams.pythonSettingsValue}` + testParams.expectedResult, + `Return value should be ${testParams.expectedResult}` ); pythonSettings.verifyAll(); From 268c3a64ae17611a2b187731704a8716f822ce5e Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Mon, 18 May 2020 12:50:41 -0700 Subject: [PATCH 13/34] Test fixes --- src/test/.vscode/settings.json | 6 +++--- src/test/activation/activationService.unit.test.ts | 12 ++++++------ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/test/.vscode/settings.json b/src/test/.vscode/settings.json index 38ffd37c8799..b8080fb6b6a2 100644 --- a/src/test/.vscode/settings.json +++ b/src/test/.vscode/settings.json @@ -17,8 +17,8 @@ "python.formatting.provider": "yapf", "python.linting.pylintUseMinimalCheckers": false, "python.pythonPath": "python", - // Do not set this to "Microsoft (v1)" or v2 even when LS is the default, else - // it will result in LS being downloaded on CI and slow down tests significantly. - // We have other tests on CI for testing downloading of CI with this setting enabled. + // Do not set this to "Microsoft", else it will result in LS being downloaded on CI + // and that slows down tests significantly. We have other tests on CI for testing + // downloading of LS with this setting enabled. "python.languageServer": "Jedi" } diff --git a/src/test/activation/activationService.unit.test.ts b/src/test/activation/activationService.unit.test.ts index 1b8a5aac2a92..a568e298fcfc 100644 --- a/src/test/activation/activationService.unit.test.ts +++ b/src/test/activation/activationService.unit.test.ts @@ -480,7 +480,7 @@ suite('Language Server Activation - ActivationService', () => { }); if (languageServerType !== LanguageServerType.Jedi) { test('Revert to jedi when LS activation fails', async () => { - pythonSettings.setup((p) => p.languageServer).returns(() => languageServerType); + pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Microsoft); const activatorLS = TypeMoq.Mock.ofType(); const activatorJedi = TypeMoq.Mock.ofType(); const activationService = new LanguageServerExtensionActivationService( @@ -499,7 +499,7 @@ suite('Language Server Activation - ActivationService', () => { .setup((c) => c.get( TypeMoq.It.isValue(ILanguageServerActivator), - TypeMoq.It.isValue(languageServerType) + TypeMoq.It.isValue(LanguageServerType.Microsoft) ) ) .returns(() => activatorLS.object) @@ -773,7 +773,7 @@ suite('Language Server Activation - ActivationService', () => { state .setup((s) => s.value) .returns(() => undefined) - .verifiable(TypeMoq.Times.exactly(1)); + .verifiable(TypeMoq.Times.exactly(2)); state .setup((s) => s.updateValue(TypeMoq.It.isValue(LanguageServerType.Jedi))) .returns(() => { @@ -796,7 +796,7 @@ suite('Language Server Activation - ActivationService', () => { state .setup((s) => s.value) .returns(() => LanguageServerType.Jedi) - .verifiable(TypeMoq.Times.exactly(1)); + .verifiable(TypeMoq.Times.exactly(2)); state .setup((s) => s.updateValue(TypeMoq.It.isValue(LanguageServerType.Node))) .returns(() => Promise.resolve()) @@ -816,7 +816,7 @@ suite('Language Server Activation - ActivationService', () => { state .setup((s) => s.value) .returns(() => LanguageServerType.Node) - .verifiable(TypeMoq.Times.exactly(1)); + .verifiable(TypeMoq.Times.exactly(2)); state .setup((s) => s.updateValue(TypeMoq.It.isValue(LanguageServerType.Jedi))) .returns(() => Promise.resolve()) @@ -836,7 +836,7 @@ suite('Language Server Activation - ActivationService', () => { state .setup((s) => s.value) .returns(() => LanguageServerType.Jedi) - .verifiable(TypeMoq.Times.exactly(1)); + .verifiable(TypeMoq.Times.exactly(2)); state .setup((s) => s.updateValue(TypeMoq.It.isAny())) .returns(() => Promise.resolve()) From 794c5f3f7beef7760c8a1b2d72b6ea22cac078f3 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Mon, 18 May 2020 17:14:34 -0700 Subject: [PATCH 14/34] Linting test fixes --- CHANGELOG.md | 4 ++++ src/test/linters/lint.multiroot.test.ts | 7 +++++++ src/test/linters/lint.provider.test.ts | 10 ++++++++++ src/test/linters/pylint.test.ts | 24 +++++++++++++++++++++++- 4 files changed, 44 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ce9015435ed5..f5c68097199a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -885,6 +885,10 @@ part of! (thanks to [ChenKB91](https://github.com/ChenKB91/)) ([#10072](https://github.com/Microsoft/vscode-python/issues/10072)) +### Note + +1. Please only set the `python.languageServer` setting if you want to turn IntelliSense off. To switch between language servers, please keep using the `python.jediEnabled` setting for now. + ### Thanks Thanks to the following projects which we fully rely on to provide some of diff --git a/src/test/linters/lint.multiroot.test.ts b/src/test/linters/lint.multiroot.test.ts index 302361099680..4caa373f8f58 100644 --- a/src/test/linters/lint.multiroot.test.ts +++ b/src/test/linters/lint.multiroot.test.ts @@ -1,6 +1,7 @@ import * as assert from 'assert'; import * as path from 'path'; import { CancellationTokenSource, ConfigurationTarget, OutputChannel, Uri, workspace } from 'vscode'; +import { LanguageServerType } from '../../client/activation/types'; import { PythonSettings } from '../../client/common/configSettings'; import { CTagsProductPathService, @@ -133,6 +134,12 @@ suite('Multiroot Linting', () => { async function runTest(product: Product, global: boolean, wks: boolean, setting: string): Promise { const config = ioc.serviceContainer.get(IConfigurationService); + await config.updateSetting( + 'languageServer', + LanguageServerType.Jedi, + Uri.file(multirootPath), + ConfigurationTarget.Global + ); await Promise.all([ config.updateSetting(setting, global, Uri.file(multirootPath), ConfigurationTarget.Global), config.updateSetting(setting, wks, Uri.file(multirootPath), ConfigurationTarget.Workspace) diff --git a/src/test/linters/lint.provider.test.ts b/src/test/linters/lint.provider.test.ts index ff4ae1c34953..d5f9d3f3220a 100644 --- a/src/test/linters/lint.provider.test.ts +++ b/src/test/linters/lint.provider.test.ts @@ -5,6 +5,7 @@ import { Container } from 'inversify'; import * as TypeMoq from 'typemoq'; import * as vscode from 'vscode'; +import { LanguageServerType } from '../../client/activation/types'; import { IApplicationShell, IDocumentManager, IWorkspaceService } from '../../client/common/application/types'; import { PersistentStateFactory } from '../../client/common/persistentState'; import { IFileSystem } from '../../client/common/platform/types'; @@ -50,6 +51,7 @@ suite('Linting - Provider', () => { let appShell: TypeMoq.IMock; let linterInstaller: TypeMoq.IMock; let workspaceService: TypeMoq.IMock; + let workspaceConfig: TypeMoq.IMock; suiteSetup(initialize); setup(async () => { @@ -80,6 +82,7 @@ suite('Linting - Provider', () => { settings = TypeMoq.Mock.ofType(); settings.setup((x) => x.linting).returns(() => lintSettings.object); + settings.setup((p) => p.languageServer).returns(() => LanguageServerType.Jedi); configService = TypeMoq.Mock.ofType(); configService.setup((x) => x.getSettings(TypeMoq.It.isAny())).returns(() => settings.object); @@ -87,7 +90,14 @@ suite('Linting - Provider', () => { appShell = TypeMoq.Mock.ofType(); linterInstaller = TypeMoq.Mock.ofType(); + workspaceService = TypeMoq.Mock.ofType(); + workspaceConfig = TypeMoq.Mock.ofType(); + workspaceService + .setup((w) => w.getConfiguration('python', TypeMoq.It.isAny())) + .returns(() => workspaceConfig.object); + workspaceService.setup((w) => w.getConfiguration('python')).returns(() => workspaceConfig.object); + serviceManager.addSingletonInstance(IApplicationShell, appShell.object); serviceManager.addSingletonInstance(IInstaller, linterInstaller.object); serviceManager.addSingletonInstance(IWorkspaceService, workspaceService.object); diff --git a/src/test/linters/pylint.test.ts b/src/test/linters/pylint.test.ts index c8017d136970..02b19a7ef9bd 100644 --- a/src/test/linters/pylint.test.ts +++ b/src/test/linters/pylint.test.ts @@ -6,7 +6,16 @@ import { Container } from 'inversify'; import * as os from 'os'; import * as path from 'path'; import * as TypeMoq from 'typemoq'; -import { CancellationTokenSource, DiagnosticSeverity, OutputChannel, TextDocument, Uri, WorkspaceFolder } from 'vscode'; +import { + CancellationTokenSource, + DiagnosticSeverity, + OutputChannel, + TextDocument, + Uri, + WorkspaceConfiguration, + WorkspaceFolder +} from 'vscode'; +import { LanguageServerType } from '../../client/activation/types'; import { IWorkspaceService } from '../../client/common/application/types'; import { IFileSystem, IPlatformService } from '../../client/common/platform/types'; import { IPythonToolExecutionService } from '../../client/common/process/types'; @@ -34,6 +43,8 @@ suite('Linting - Pylint', () => { let workspace: TypeMoq.IMock; let execService: TypeMoq.IMock; let config: TypeMoq.IMock; + let workspaceConfig: TypeMoq.IMock; + let pythonSettings: TypeMoq.IMock; let serviceContainer: ServiceContainer; setup(() => { @@ -67,7 +78,16 @@ suite('Linting - Pylint', () => { IInterpreterAutoSeletionProxyService, MockAutoSelectionService ); + + pythonSettings = TypeMoq.Mock.ofType(); + pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Jedi); + config = TypeMoq.Mock.ofType(); + config.setup((c) => c.getSettings()).returns(() => pythonSettings.object); + + workspaceConfig = TypeMoq.Mock.ofType(); + workspace.setup((w) => w.getConfiguration('python')).returns(() => workspaceConfig.object); + serviceManager.addSingletonInstance(IConfigurationService, config.object); const linterManager = new LinterManager(serviceContainer, workspace.object); serviceManager.addSingletonInstance(ILinterManager, linterManager); @@ -198,6 +218,7 @@ suite('Linting - Pylint', () => { const settings = TypeMoq.Mock.ofType(); settings.setup((x) => x.linting).returns(() => lintSettings); + settings.setup((x) => x.languageServer).returns(() => LanguageServerType.Jedi); config.setup((x) => x.getSettings(TypeMoq.It.isAny())).returns(() => settings.object); await pylinter.lint(document.object, new CancellationTokenSource().token); @@ -244,6 +265,7 @@ suite('Linting - Pylint', () => { const settings = TypeMoq.Mock.ofType(); settings.setup((x) => x.linting).returns(() => lintSettings); + settings.setup((x) => x.languageServer).returns(() => LanguageServerType.Jedi); config.setup((x) => x.getSettings(TypeMoq.It.isAny())).returns(() => settings.object); const messages = await pylinter.lint(document.object, new CancellationTokenSource().token); From 2c6d70a1273302ce908abbf2cc498a72c82cd9c8 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Mon, 18 May 2020 17:14:49 -0700 Subject: [PATCH 15/34] Linting tests --- src/client/linters/linterInfo.ts | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/src/client/linters/linterInfo.ts b/src/client/linters/linterInfo.ts index fbb65fbadfd5..80a899bbf8bd 100644 --- a/src/client/linters/linterInfo.ts +++ b/src/client/linters/linterInfo.ts @@ -87,21 +87,11 @@ export class PylintLinterInfo extends LinterInfo { } public isEnabled(resource?: Uri): boolean { const enabled = super.isEnabled(resource); - if (!enabled || this.configService.getSettings(resource).languageServer === LanguageServerType.Jedi) { - return enabled; - } - // If we're using new LS, then by default Pylint is disabled (unless the user provides a value). - const inspection = this.workspaceService - .getConfiguration('python', resource) - .inspect('linting.pylintEnabled'); - if ( - !inspection || - (inspection.globalValue === undefined && - inspection.workspaceFolderValue === undefined && - inspection.workspaceValue === undefined) - ) { + const usingJedi = this.configService.getSettings(resource).languageServer === LanguageServerType.Jedi; + if (!enabled || !usingJedi) { return false; } - return enabled; + // If we're using new LS, then by default Pylint is disabled (unless the user provides a value). + return this.configService.getSettings(resource).linting?.pylintEnabled; } } From a272c419641ecd5892f7a6752df7f1978d95c89f Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Mon, 18 May 2020 19:13:50 -0700 Subject: [PATCH 16/34] Unused var --- src/client/linters/linterInfo.ts | 7 +------ src/client/linters/linterManager.ts | 2 +- src/test/linters/linterinfo.unit.test.ts | 25 +++++------------------- 3 files changed, 7 insertions(+), 27 deletions(-) diff --git a/src/client/linters/linterInfo.ts b/src/client/linters/linterInfo.ts index 80a899bbf8bd..ab49985b4140 100644 --- a/src/client/linters/linterInfo.ts +++ b/src/client/linters/linterInfo.ts @@ -4,7 +4,6 @@ import * as path from 'path'; import { Uri } from 'vscode'; import { LanguageServerType } from '../activation/types'; -import { IWorkspaceService } from '../common/application/types'; import { ExecutionInfo, IConfigurationService, Product } from '../common/types'; import { ILinterInfo, LinterId } from './types'; @@ -78,11 +77,7 @@ export class LinterInfo implements ILinterInfo { } export class PylintLinterInfo extends LinterInfo { - constructor( - configService: IConfigurationService, - private readonly workspaceService: IWorkspaceService, - configFileNames: string[] = [] - ) { + constructor(configService: IConfigurationService, configFileNames: string[] = []) { super(Product.pylint, 'pylint', configService, configFileNames); } public isEnabled(resource?: Uri): boolean { diff --git a/src/client/linters/linterManager.ts b/src/client/linters/linterManager.ts index bfe9db05e692..4dd44aadc016 100644 --- a/src/client/linters/linterManager.ts +++ b/src/client/linters/linterManager.ts @@ -45,7 +45,7 @@ export class LinterManager implements ILinterManager { this.linters = [ new LinterInfo(Product.bandit, 'bandit', this.configService), new LinterInfo(Product.flake8, 'flake8', this.configService), - new PylintLinterInfo(this.configService, this.workspaceService, ['.pylintrc', 'pylintrc']), + new PylintLinterInfo(this.configService, ['.pylintrc', 'pylintrc']), new LinterInfo(Product.mypy, 'mypy', this.configService), new LinterInfo(Product.pycodestyle, 'pycodestyle', this.configService), new LinterInfo(Product.prospector, 'prospector', this.configService), diff --git a/src/test/linters/linterinfo.unit.test.ts b/src/test/linters/linterinfo.unit.test.ts index 86adcf8b14ca..b9b76158e120 100644 --- a/src/test/linters/linterinfo.unit.test.ts +++ b/src/test/linters/linterinfo.unit.test.ts @@ -8,15 +8,13 @@ import { expect } from 'chai'; import { anything, instance, mock, when } from 'ts-mockito'; import { LanguageServerType } from '../../client/activation/types'; -import { WorkspaceService } from '../../client/common/application/workspace'; import { ConfigurationService } from '../../client/common/configuration/service'; import { PylintLinterInfo } from '../../client/linters/linterInfo'; suite('Linter Info - Pylint', () => { test('Test disabled when Pylint is explicitly disabled', async () => { const config = mock(ConfigurationService); - const workspaceService = mock(WorkspaceService); - const linterInfo = new PylintLinterInfo(instance(config), instance(workspaceService), []); + const linterInfo = new PylintLinterInfo(instance(config), []); when(config.getSettings(anything())).thenReturn({ linting: { pylintEnabled: false } } as any); @@ -24,8 +22,7 @@ suite('Linter Info - Pylint', () => { }); test('Test disabled when Jedi is enabled and Pylint is explicitly disabled', async () => { const config = mock(ConfigurationService); - const workspaceService = mock(WorkspaceService); - const linterInfo = new PylintLinterInfo(instance(config), instance(workspaceService), []); + const linterInfo = new PylintLinterInfo(instance(config), []); when(config.getSettings(anything())).thenReturn({ linting: { pylintEnabled: false }, @@ -36,8 +33,7 @@ suite('Linter Info - Pylint', () => { }); test('Test enabled when Jedi is enabled and Pylint is explicitly enabled', async () => { const config = mock(ConfigurationService); - const workspaceService = mock(WorkspaceService); - const linterInfo = new PylintLinterInfo(instance(config), instance(workspaceService), []); + const linterInfo = new PylintLinterInfo(instance(config), []); when(config.getSettings(anything())).thenReturn({ linting: { pylintEnabled: true }, @@ -48,18 +44,12 @@ suite('Linter Info - Pylint', () => { }); test('Test disabled when using Language Server and Pylint is not configured', async () => { const config = mock(ConfigurationService); - const workspaceService = mock(WorkspaceService); - const linterInfo = new PylintLinterInfo(instance(config), instance(workspaceService), []); + const linterInfo = new PylintLinterInfo(instance(config), []); - const inspection = {}; - const pythonConfig = { - inspect: () => inspection - }; when(config.getSettings(anything())).thenReturn({ linting: { pylintEnabled: true }, languageServer: LanguageServerType.Node } as any); - when(workspaceService.getConfiguration('python', anything())).thenReturn(pythonConfig as any); expect(linterInfo.isEnabled()).to.be.false; }); @@ -82,17 +72,12 @@ suite('Linter Info - Pylint', () => { testsForisEnabled.forEach((testParams) => { test(testParams.testName, async () => { const config = mock(ConfigurationService); - const workspaceService = mock(WorkspaceService); - const linterInfo = new PylintLinterInfo(instance(config), instance(workspaceService), []); + const linterInfo = new PylintLinterInfo(instance(config), []); - const pythonConfig = { - inspect: () => testParams.inspection - }; when(config.getSettings(anything())).thenReturn({ linting: { pylintEnabled: true }, languageServer: LanguageServerType.Node } as any); - when(workspaceService.getConfiguration('python', anything())).thenReturn(pythonConfig as any); expect(linterInfo.isEnabled()).to.be.true; }); From 24754cb0a812f26fcb4a358a31c12f1468a756eb Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Tue, 19 May 2020 13:17:36 -0700 Subject: [PATCH 17/34] Restore inspection --- src/client/linters/linterInfo.ts | 31 ++++++++++-- src/client/linters/linterManager.ts | 2 +- src/test/linters/lint.manager.unit.test.ts | 16 +++++- src/test/linters/linterinfo.unit.test.ts | 57 +++++++--------------- 4 files changed, 59 insertions(+), 47 deletions(-) diff --git a/src/client/linters/linterInfo.ts b/src/client/linters/linterInfo.ts index ab49985b4140..21ead3b13c70 100644 --- a/src/client/linters/linterInfo.ts +++ b/src/client/linters/linterInfo.ts @@ -4,6 +4,7 @@ import * as path from 'path'; import { Uri } from 'vscode'; import { LanguageServerType } from '../activation/types'; +import { IWorkspaceService } from '../common/application/types'; import { ExecutionInfo, IConfigurationService, Product } from '../common/types'; import { ILinterInfo, LinterId } from './types'; @@ -77,16 +78,36 @@ export class LinterInfo implements ILinterInfo { } export class PylintLinterInfo extends LinterInfo { - constructor(configService: IConfigurationService, configFileNames: string[] = []) { + constructor( + configService: IConfigurationService, + private readonly workspaceService: IWorkspaceService, + configFileNames: string[] = [] + ) { super(Product.pylint, 'pylint', configService, configFileNames); } public isEnabled(resource?: Uri): boolean { - const enabled = super.isEnabled(resource); + // We want to be sure the setting is not default since default is `true` and hence + // missing setting yields `true`. When setting is missing and LS is non-Jedi, + // we want default to be `false`. So inspection here makes sure we are not getting + // `true` because there is no setting and LS is active. + const enabled = super.isEnabled(resource); // Is it enabled by settings? const usingJedi = this.configService.getSettings(resource).languageServer === LanguageServerType.Jedi; - if (!enabled || !usingJedi) { + if (usingJedi) { + // In Jedi case adhere to default behavior. Missing setting means `enabled`. + return enabled; + } + // If we're using LS, then by default Pylint is disabled unless user provided + // the value. We have to resort to direct inspection of settings here. + const configuration = this.workspaceService.getConfiguration('python', resource); + const inspection = configuration.inspect(this.enabledSettingName); + if ( + !inspection || + (inspection.globalValue === undefined && + inspection.workspaceFolderValue === undefined && + inspection.workspaceValue === undefined) + ) { return false; } - // If we're using new LS, then by default Pylint is disabled (unless the user provides a value). - return this.configService.getSettings(resource).linting?.pylintEnabled; + return enabled; } } diff --git a/src/client/linters/linterManager.ts b/src/client/linters/linterManager.ts index 4dd44aadc016..bfe9db05e692 100644 --- a/src/client/linters/linterManager.ts +++ b/src/client/linters/linterManager.ts @@ -45,7 +45,7 @@ export class LinterManager implements ILinterManager { this.linters = [ new LinterInfo(Product.bandit, 'bandit', this.configService), new LinterInfo(Product.flake8, 'flake8', this.configService), - new PylintLinterInfo(this.configService, ['.pylintrc', 'pylintrc']), + new PylintLinterInfo(this.configService, this.workspaceService, ['.pylintrc', 'pylintrc']), new LinterInfo(Product.mypy, 'mypy', this.configService), new LinterInfo(Product.pycodestyle, 'pycodestyle', this.configService), new LinterInfo(Product.prospector, 'prospector', this.configService), diff --git a/src/test/linters/lint.manager.unit.test.ts b/src/test/linters/lint.manager.unit.test.ts index fabe314e2b68..be23cef27d9b 100644 --- a/src/test/linters/lint.manager.unit.test.ts +++ b/src/test/linters/lint.manager.unit.test.ts @@ -11,6 +11,8 @@ import { IConfigurationService, IPythonSettings } from '../../client/common/type import { IServiceContainer } from '../../client/ioc/types'; import { LinterManager } from '../../client/linters/linterManager'; +const workspaceService = TypeMoq.Mock.ofType(); + // setup class instance class TestLinterManager extends LinterManager { public enableUnconfiguredLintersCallCount: number = 0; @@ -23,17 +25,27 @@ class TestLinterManager extends LinterManager { function getServiceContainerMockForLinterManagerTests(): TypeMoq.IMock { // setup test mocks const serviceContainerMock = TypeMoq.Mock.ofType(); - const configMock = TypeMoq.Mock.ofType(); + const pythonSettingsMock = TypeMoq.Mock.ofType(); + const configMock = TypeMoq.Mock.ofType(); configMock.setup((cm) => cm.getSettings(TypeMoq.It.isAny())).returns(() => pythonSettingsMock.object); serviceContainerMock.setup((c) => c.get(IConfigurationService)).returns(() => configMock.object); + const pythonConfig = { + // tslint:disable-next-line:no-empty + inspect: () => {} + }; + workspaceService + .setup((x) => x.getConfiguration('python', TypeMoq.It.isAny())) + // tslint:disable-next-line:no-any + .returns(() => pythonConfig as any); + serviceContainerMock.setup((c) => c.get(IWorkspaceService)).returns(() => workspaceService.object); + return serviceContainerMock; } // tslint:disable-next-line:max-func-body-length suite('Lint Manager Unit Tests', () => { - const workspaceService = TypeMoq.Mock.ofType(); test('Linter manager isLintingEnabled checks availability when silent = false.', async () => { // set expectations const expectedCallCount = 1; diff --git a/src/test/linters/linterinfo.unit.test.ts b/src/test/linters/linterinfo.unit.test.ts index b9b76158e120..3537e8625f52 100644 --- a/src/test/linters/linterinfo.unit.test.ts +++ b/src/test/linters/linterinfo.unit.test.ts @@ -8,21 +8,26 @@ import { expect } from 'chai'; import { anything, instance, mock, when } from 'ts-mockito'; import { LanguageServerType } from '../../client/activation/types'; +import { WorkspaceService } from '../../client/common/application/workspace'; import { ConfigurationService } from '../../client/common/configuration/service'; import { PylintLinterInfo } from '../../client/linters/linterInfo'; suite('Linter Info - Pylint', () => { + const workspace = mock(WorkspaceService); + const config = mock(ConfigurationService); + test('Test disabled when Pylint is explicitly disabled', async () => { - const config = mock(ConfigurationService); - const linterInfo = new PylintLinterInfo(instance(config), []); + const linterInfo = new PylintLinterInfo(instance(config), instance(workspace), []); - when(config.getSettings(anything())).thenReturn({ linting: { pylintEnabled: false } } as any); + when(config.getSettings(anything())).thenReturn({ + linting: { pylintEnabled: false }, + languageServer: LanguageServerType.Jedi + } as any); expect(linterInfo.isEnabled()).to.be.false; }); test('Test disabled when Jedi is enabled and Pylint is explicitly disabled', async () => { - const config = mock(ConfigurationService); - const linterInfo = new PylintLinterInfo(instance(config), []); + const linterInfo = new PylintLinterInfo(instance(config), instance(workspace), []); when(config.getSettings(anything())).thenReturn({ linting: { pylintEnabled: false }, @@ -32,8 +37,7 @@ suite('Linter Info - Pylint', () => { expect(linterInfo.isEnabled()).to.be.false; }); test('Test enabled when Jedi is enabled and Pylint is explicitly enabled', async () => { - const config = mock(ConfigurationService); - const linterInfo = new PylintLinterInfo(instance(config), []); + const linterInfo = new PylintLinterInfo(instance(config), instance(workspace), []); when(config.getSettings(anything())).thenReturn({ linting: { pylintEnabled: true }, @@ -43,44 +47,19 @@ suite('Linter Info - Pylint', () => { expect(linterInfo.isEnabled()).to.be.true; }); test('Test disabled when using Language Server and Pylint is not configured', async () => { - const config = mock(ConfigurationService); - const linterInfo = new PylintLinterInfo(instance(config), []); + const linterInfo = new PylintLinterInfo(instance(config), instance(workspace), []); when(config.getSettings(anything())).thenReturn({ linting: { pylintEnabled: true }, languageServer: LanguageServerType.Node } as any); - expect(linterInfo.isEnabled()).to.be.false; - }); - const testsForisEnabled = [ - { - testName: 'When workspaceFolder setting is provided', - inspection: { workspaceFolderValue: true } - }, - { - testName: 'When workspace setting is provided', - inspection: { workspaceValue: true } - }, - { - testName: 'When global setting is provided', - inspection: { globalValue: true } - } - ]; - - suite('Test is enabled when using Language Server and Pylint is configured', () => { - testsForisEnabled.forEach((testParams) => { - test(testParams.testName, async () => { - const config = mock(ConfigurationService); - const linterInfo = new PylintLinterInfo(instance(config), []); + const pythonConfig = { + // tslint:disable-next-line:no-empty + inspect: () => {} + }; + when(workspace.getConfiguration('python', anything())).thenReturn(pythonConfig as any); - when(config.getSettings(anything())).thenReturn({ - linting: { pylintEnabled: true }, - languageServer: LanguageServerType.Node - } as any); - - expect(linterInfo.isEnabled()).to.be.true; - }); - }); + expect(linterInfo.isEnabled()).to.be.false; }); }); From 76b47ba4f844f24ad3b76193645163cf02b61420 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Tue, 19 May 2020 15:48:11 -0700 Subject: [PATCH 18/34] Add news --- news/1 Enhancements/7010.md | 1 + news/1 Enhancements/README.md | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) create mode 100644 news/1 Enhancements/7010.md diff --git a/news/1 Enhancements/7010.md b/news/1 Enhancements/7010.md new file mode 100644 index 000000000000..978ac26bc97c --- /dev/null +++ b/news/1 Enhancements/7010.md @@ -0,0 +1 @@ +Removed `python.jediEnabled` setting. Use `"python.languageServer": "Jedi"` instead. diff --git a/news/1 Enhancements/README.md b/news/1 Enhancements/README.md index 0c3574379fa0..2a159b65f4f0 100644 --- a/news/1 Enhancements/README.md +++ b/news/1 Enhancements/README.md @@ -1,3 +1,2 @@ Changes that add new features. -Removed `python.jediEnabled` setting. Use `"python.languageServer": "Jedi"` instead. From d55a417ee47d9ed713a110bf4bf66e99ede418fa Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Tue, 19 May 2020 18:03:12 -0700 Subject: [PATCH 19/34] MErge issues --- src/client/telemetry/index.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index a44e97f4a30a..e7ef060fe54f 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -70,7 +70,7 @@ function getTelemetryReporter() { // tslint:disable-next-line:no-require-imports const reporter = require('vscode-extension-telemetry').default as typeof TelemetryReporter; - return (telemetryReporter = new reporter(extensionId, extensionVersion, AppinsightsKey, true)); + return (telemetryReporter = new reporter(extensionId, extensionVersion, AppinsightsKey)); } export function clearTelemetryReporter() { @@ -99,7 +99,7 @@ export function sendTelemetryEvent

Date: Tue, 19 May 2020 19:23:37 -0700 Subject: [PATCH 20/34] Merge issues --- src/client/telemetry/index.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index e7ef060fe54f..a44e97f4a30a 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -70,7 +70,7 @@ function getTelemetryReporter() { // tslint:disable-next-line:no-require-imports const reporter = require('vscode-extension-telemetry').default as typeof TelemetryReporter; - return (telemetryReporter = new reporter(extensionId, extensionVersion, AppinsightsKey)); + return (telemetryReporter = new reporter(extensionId, extensionVersion, AppinsightsKey, true)); } export function clearTelemetryReporter() { @@ -99,7 +99,7 @@ export function sendTelemetryEvent

Date: Tue, 19 May 2020 21:33:11 -0700 Subject: [PATCH 21/34] Wor around CI --- src/test/smokeTest.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/smokeTest.ts b/src/test/smokeTest.ts index 1d0c68b2e4c3..686deec4a618 100644 --- a/src/test/smokeTest.ts +++ b/src/test/smokeTest.ts @@ -12,7 +12,6 @@ import { spawn } from 'child_process'; import * as fs from 'fs-extra'; import * as glob from 'glob'; import * as path from 'path'; -import { LanguageServerType } from '../client/activation/types'; import { unzip } from './common'; import { EXTENSION_ROOT_DIR_FOR_TESTS, SMOKE_TEST_EXTENSIONS_DIR } from './constants'; @@ -32,7 +31,8 @@ class TestRunner { await this.launchTest(env); } private async enableLanguageServer(enable: boolean) { - const settings = `{ "python.languageServer": ${enable ? LanguageServerType.Node : LanguageServerType.Jedi} }`; + // Work around CI issue when tests unable to resolve `../client/activation/types` + const settings = `{ "python.languageServer": ${enable ? 'Node' : 'Jedi'} }`; await fs.ensureDir( path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'src', 'testMultiRootWkspc', 'smokeTests', '.vscode') ); From ef6286fc1721a39b0f0262c79981adfbb3600ef7 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Tue, 19 May 2020 23:11:30 -0700 Subject: [PATCH 22/34] Remove client reference --- src/test/smoke/common.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/test/smoke/common.ts b/src/test/smoke/common.ts index 5abb39cf6ae8..4c8fa12b97cd 100644 --- a/src/test/smoke/common.ts +++ b/src/test/smoke/common.ts @@ -10,7 +10,6 @@ import * as fs from 'fs-extra'; import * as glob from 'glob'; import * as path from 'path'; import * as vscode from 'vscode'; -import { LanguageServerType } from '../../client/activation/types'; import { SMOKE_TEST_EXTENSIONS_DIR } from '../constants'; import { noop, sleep } from '../core'; @@ -34,13 +33,13 @@ async function getLanguageServerFolders(): Promise { export function isJediEnabled() { const resource = vscode.workspace.workspaceFolders![0].uri; const settings = vscode.workspace.getConfiguration('python', resource); - return settings.get('languageServer') === LanguageServerType.Jedi; + return settings.get('languageServer') === 'Jedi'; } export async function enableJedi(enable: boolean | undefined) { if (isJediEnabled() === enable) { return; } - await updateSetting('languageServer', LanguageServerType.Jedi); + await updateSetting('languageServer', 'Jedi'); } export async function openFileAndWaitForLS(file: string): Promise { const textDocument = await vscode.workspace.openTextDocument(file); From 88bde1b113bc7e019e0173f352b766f50249fa14 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Wed, 20 May 2020 09:49:00 -0700 Subject: [PATCH 23/34] Use direct import in smoke tests --- src/test/smokeTest.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/test/smokeTest.ts b/src/test/smokeTest.ts index 686deec4a618..187c3d1f4f8a 100644 --- a/src/test/smokeTest.ts +++ b/src/test/smokeTest.ts @@ -31,8 +31,11 @@ class TestRunner { await this.launchTest(env); } private async enableLanguageServer(enable: boolean) { - // Work around CI issue when tests unable to resolve `../client/activation/types` - const settings = `{ "python.languageServer": ${enable ? 'Node' : 'Jedi'} }`; + // When running smoke tests, we won't have access to unbundled files. + const types = await import('../client/activation/types'); + const settings = `{ "python.languageServer": ${ + enable ? types.LanguageServerType.Node : types.LanguageServerType.Jedi + } }`; await fs.ensureDir( path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'src', 'testMultiRootWkspc', 'smokeTests', '.vscode') ); From 2d4df2f67dd44e524a6064ad42b087c9b6c31eef Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Wed, 20 May 2020 15:45:57 -0700 Subject: [PATCH 24/34] Fix smoke tests --- src/test/.vscode/settings.json | 4 ++-- src/test/smokeTest.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/test/.vscode/settings.json b/src/test/.vscode/settings.json index b8080fb6b6a2..7e5c9cc2a985 100644 --- a/src/test/.vscode/settings.json +++ b/src/test/.vscode/settings.json @@ -16,9 +16,9 @@ "python.linting.banditEnabled": false, "python.formatting.provider": "yapf", "python.linting.pylintUseMinimalCheckers": false, - "python.pythonPath": "python", + "python.pythonPath": "python" // Do not set this to "Microsoft", else it will result in LS being downloaded on CI // and that slows down tests significantly. We have other tests on CI for testing // downloading of LS with this setting enabled. - "python.languageServer": "Jedi" + // "python.languageServer": "Jedi" } diff --git a/src/test/smokeTest.ts b/src/test/smokeTest.ts index 187c3d1f4f8a..c366ea645f17 100644 --- a/src/test/smokeTest.ts +++ b/src/test/smokeTest.ts @@ -33,9 +33,9 @@ class TestRunner { private async enableLanguageServer(enable: boolean) { // When running smoke tests, we won't have access to unbundled files. const types = await import('../client/activation/types'); - const settings = `{ "python.languageServer": ${ + const settings = `{ "python.languageServer": "${ enable ? types.LanguageServerType.Node : types.LanguageServerType.Jedi - } }`; + }" }`; await fs.ensureDir( path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'src', 'testMultiRootWkspc', 'smokeTests', '.vscode') ); From c684bd17adb56062da09685dd33bb0a446536ae4 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Wed, 20 May 2020 17:09:04 -0700 Subject: [PATCH 25/34] Fix linter test --- src/test/linters/lint.multilinter.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/test/linters/lint.multilinter.test.ts b/src/test/linters/lint.multilinter.test.ts index 9d2336ab0761..20d3ea5a54f6 100644 --- a/src/test/linters/lint.multilinter.test.ts +++ b/src/test/linters/lint.multilinter.test.ts @@ -5,6 +5,7 @@ import * as assert from 'assert'; import * as path from 'path'; import { ConfigurationTarget, DiagnosticCollection, Uri, window, workspace } from 'vscode'; +import { LanguageServerType } from '../../client/activation/types'; import { ICommandManager } from '../../client/common/application/types'; import { Product } from '../../client/common/installer/productInstaller'; import { PythonToolExecutionService } from '../../client/common/process/pythonToolService'; @@ -91,6 +92,7 @@ suite('Linting - Multiple Linters Enabled Test', () => { await closeActiveWindows(); const document = await workspace.openTextDocument(path.join(pythoFilesPath, 'print.py')); await window.showTextDocument(document); + await configService.updateSetting('languageServer', LanguageServerType.Jedi, workspaceUri); await configService.updateSetting('linting.enabled', true, workspaceUri); await configService.updateSetting('linting.pylintUseMinimalCheckers', false, workspaceUri); await configService.updateSetting('linting.pylintEnabled', true, workspaceUri); From f9b183cfb310df2f5dc94c6f671064c4d1a5e97d Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Wed, 20 May 2020 17:35:04 -0700 Subject: [PATCH 26/34] Avoid imports in smoke tests --- src/test/.vscode/settings.json | 3 ++- src/test/smokeTest.ts | 5 +---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/test/.vscode/settings.json b/src/test/.vscode/settings.json index 5dbe136ee909..e06b046b67c9 100644 --- a/src/test/.vscode/settings.json +++ b/src/test/.vscode/settings.json @@ -1,5 +1,5 @@ { - "python.linting.pylintEnabled": true, + "python.linting.pylintEnabled": false, "python.linting.flake8Enabled": false, "python.workspaceSymbols.enabled": false, "python.testing.nosetestArgs": [], @@ -27,4 +27,5 @@ "python.experiments.optInto": [ "NativeNotebook - experiment" ], + "python.languageServer": "Jedi", } diff --git a/src/test/smokeTest.ts b/src/test/smokeTest.ts index c366ea645f17..8c5a08710327 100644 --- a/src/test/smokeTest.ts +++ b/src/test/smokeTest.ts @@ -32,10 +32,7 @@ class TestRunner { } private async enableLanguageServer(enable: boolean) { // When running smoke tests, we won't have access to unbundled files. - const types = await import('../client/activation/types'); - const settings = `{ "python.languageServer": "${ - enable ? types.LanguageServerType.Node : types.LanguageServerType.Jedi - }" }`; + const settings = `{ "python.languageServer": ${enable ? '"Node"' : '"Jedi"'} }`; await fs.ensureDir( path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'src', 'testMultiRootWkspc', 'smokeTests', '.vscode') ); From 1f6813c0d07171b3b5fe77b340fe814ddb70939f Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Thu, 21 May 2020 15:34:13 -0700 Subject: [PATCH 27/34] PR feedback --- .gitignore | 2 +- news/1 Enhancements/7010.md | 2 +- src/client/telemetry/index.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.gitignore b/.gitignore index 17480c9bbcbc..f3befcb98957 100644 --- a/.gitignore +++ b/.gitignore @@ -25,7 +25,7 @@ precommit.hook pythonFiles/experimental/ptvsd/** pythonFiles/lib/** debug_coverage*/** -languageServer*/** +languageServer/** languageServer.*/** bin/** obj/** diff --git a/news/1 Enhancements/7010.md b/news/1 Enhancements/7010.md index 978ac26bc97c..721269ec5e2e 100644 --- a/news/1 Enhancements/7010.md +++ b/news/1 Enhancements/7010.md @@ -1 +1 @@ -Removed `python.jediEnabled` setting. Use `"python.languageServer": "Jedi"` instead. +Removed `python.jediEnabled` setting in favor of `python.languageServer`. Instead of `"python.jediEnabled": true` please use `"python.languageServer": "Jedi"`. diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index c5a45cf738bb..37763b71e5fd 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -1121,7 +1121,7 @@ export interface IEventNamePropertyMapping { */ lsStartup?: LanguageServerType; /** - * Used to track switch between LS and Jedi. Carries the final state after the switch. + * Used to track switch between language servers. Carries the final state after the switch. */ switchTo?: LanguageServerType; }; From 54742a3f5021998919b071b7a36a6a8b2b14ea51 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Thu, 21 May 2020 17:04:24 -0700 Subject: [PATCH 28/34] Add jediEnabled/languageServer fixup with tests --- .../testing/common/updateTestSettings.ts | 44 +++++++++++++- .../checks/updateTestSettings.unit.test.ts | 58 ++++++++++++++++++- 2 files changed, 97 insertions(+), 5 deletions(-) diff --git a/src/client/testing/common/updateTestSettings.ts b/src/client/testing/common/updateTestSettings.ts index 2bc07138dcd1..cc37e4593a5b 100644 --- a/src/client/testing/common/updateTestSettings.ts +++ b/src/client/testing/common/updateTestSettings.ts @@ -5,7 +5,7 @@ import { inject, injectable } from 'inversify'; import * as path from 'path'; -import { IExtensionActivationService } from '../../activation/types'; +import { IExtensionActivationService, LanguageServerType } from '../../activation/types'; import { IApplicationEnvironment, IWorkspaceService } from '../../common/application/types'; import '../../common/extensions'; import { traceDecorators, traceError } from '../../common/logger'; @@ -13,6 +13,8 @@ import { IFileSystem } from '../../common/platform/types'; import { Resource } from '../../common/types'; import { swallowExceptions } from '../../common/utils/decorators'; +// tslint:disable-next-line:no-suspicious-comment +// TODO: rename the class since it is not used just for test settings @injectable() export class UpdateTestSettingService implements IExtensionActivationService { constructor( @@ -50,7 +52,7 @@ export class UpdateTestSettingService implements IExtensionActivationService { return result.filter((item) => item.needsFixing).map((item) => item.file); } @swallowExceptions('Failed to update settings.json') - public async fixSettingInFile(filePath: string) { + public async fixSettingInFile(filePath: string, fixLanguageServer = true): Promise { let fileContents = await this.fs.readFile(filePath); const setting = new RegExp('"python\\.unitTest', 'g'); @@ -72,8 +74,46 @@ export class UpdateTestSettingService implements IExtensionActivationService { fileContents = fileContents.replace(setting_pep8_enabled, '.pycodestyleEnabled'); fileContents = fileContents.replace(setting_pep8_path, '.pycodestylePath'); + if (fixLanguageServer) { + // `python.jediEnabled` is deprecated: + // - `true` or missing then set to `languageServer: Jedi`. + // - `false` and `languageServer` is present, do nothing. + // - `false` and `languageServer` is NOT present, set `languageServer` to `Microsoft`. + // `jediEnabled` is then removed. + try { + const settings = JSON.parse(fileContents); + let changed = false; + + if (settings[`python.jediEnabled`] === undefined) { + // `jediEnabled` is missing. Default was true, so assume Jedi. + if (!settings['python.languageServer']) { + settings['python.languageServer'] = LanguageServerType.Jedi; + changed = true; + } + } else { + if (settings[`python.jediEnabled`]) { + settings['python.languageServer'] = LanguageServerType.Jedi; + } else { + // `jediEnabled` is false. If no `languageServer`, set it to Microsoft. + if (!settings['python.languageServer']) { + settings['python.languageServer'] = LanguageServerType.Microsoft; + } + } + settings[`python.jediEnabled`] = undefined; + changed = true; + } + + if (changed) { + fileContents = JSON.stringify(settings, null, 4); + } + // tslint:disable-next-line:no-empty + } catch {} + } + await this.fs.writeFile(filePath, fileContents); + return fileContents; } + public async doesFileNeedToBeFixed(filePath: string) { try { const contents = await this.fs.readFile(filePath); diff --git a/src/test/application/diagnostics/checks/updateTestSettings.unit.test.ts b/src/test/application/diagnostics/checks/updateTestSettings.unit.test.ts index bc062f2524be..c3f44341c075 100644 --- a/src/test/application/diagnostics/checks/updateTestSettings.unit.test.ts +++ b/src/test/application/diagnostics/checks/updateTestSettings.unit.test.ts @@ -7,7 +7,7 @@ import * as assert from 'assert'; import { expect } from 'chai'; import * as path from 'path'; import * as sinon from 'sinon'; -import { anything, instance, mock, verify, when } from 'ts-mockito'; +import { anyString, anything, instance, mock, verify, when } from 'ts-mockito'; import { Uri } from 'vscode'; import { ApplicationEnvironment } from '../../../../client/common/application/applicationEnvironment'; import { IApplicationEnvironment, IWorkspaceService } from '../../../../client/common/application/types'; @@ -201,10 +201,62 @@ suite('Application Diagnostics - Check Test Settings', () => { when(fs.readFile(__filename)).thenResolve(item.contents); when(fs.writeFile(__filename, anything())).thenResolve(); - await diagnosticService.fixSettingInFile(__filename); + const actualContent = await diagnosticService.fixSettingInFile(__filename, false); verify(fs.readFile(__filename)).once(); - verify(fs.writeFile(__filename, item.expectedContents)).once(); + verify(fs.writeFile(__filename, anyString())).once(); + expect(nows(actualContent)).to.be.equal(nows(item.expectedContents)); }); }); + + [ + { + testTitle: 'No jediEnabled setting.', + contents: '{}', + expectedContent: '{ "python.languageServer": "Jedi" }' + }, + { + testTitle: 'jediEnabled: true, no languageServer setting', + contents: '{ "python.jediEnabled": true }', + expectedContent: '{"python.languageServer": "Jedi"}' + }, + { + testTitle: 'jediEnabled: true, languageServer setting present', + contents: '{ "python.jediEnabled": true }', + expectedContent: '{"python.languageServer": "Jedi"}' + }, + { + testTitle: 'jediEnabled: false, no languageServer setting', + contents: '{ "python.jediEnabled": false }', + expectedContent: '{"python.languageServer": "Microsoft"}' + }, + { + testTitle: 'jediEnabled: false, languageServer is Microsoft', + contents: '{ "python.jediEnabled": false, "python.languageServer": "Microsoft" }', + expectedContent: '{"python.languageServer": "Microsoft"}' + }, + { + testTitle: 'jediEnabled: false, languageServer is None', + contents: '{ "python.jediEnabled": false, "python.languageServer": "None" }', + expectedContent: '{"python.languageServer": "None"}' + }, + { + testTitle: 'jediEnabled: false, languageServer is Jedi', + contents: '{ "python.jediEnabled": false, "python.languageServer": "Jedi" }', + expectedContent: '{"python.languageServer": "Jedi"}' + } + ].forEach((item) => { + test(item.testTitle, async () => { + when(fs.readFile(__filename)).thenResolve(item.contents); + + const actualContent = await diagnosticService.fixSettingInFile(__filename); + + expect(nows(actualContent)).to.equal(nows(item.expectedContent)); + verify(fs.readFile(__filename)).once(); + }); + }); + + function nows(s: string): string { + return s.replace(/\s*/g, ''); + } }); From f1050728e642fb8ac13393dbe474d21ef285cd5a Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Thu, 21 May 2020 17:13:42 -0700 Subject: [PATCH 29/34] Update LS type --- .../proposeLanguageServerBanner.ts | 2 +- .../activation/activationService.unit.test.ts | 24 +++++++++---------- .../datascience/intellisense.unit.test.ts | 2 +- .../linters/linter.availability.unit.test.ts | 6 ++--- src/test/linters/linterinfo.unit.test.ts | 2 +- src/test/performanceTest.ts | 4 +++- src/test/smokeTest.ts | 2 +- 7 files changed, 22 insertions(+), 20 deletions(-) diff --git a/src/client/languageServices/proposeLanguageServerBanner.ts b/src/client/languageServices/proposeLanguageServerBanner.ts index 2c9e219859c1..9f3202779bba 100644 --- a/src/client/languageServices/proposeLanguageServerBanner.ts +++ b/src/client/languageServices/proposeLanguageServerBanner.ts @@ -115,7 +115,7 @@ export class ProposeLanguageServerBanner implements IPythonExtensionBanner { public async enableLanguageServer(): Promise { await this.configuration.updateSetting( 'languageServer', - LanguageServerType.Node, + LanguageServerType.Microsoft, undefined, ConfigurationTarget.Global ); diff --git a/src/test/activation/activationService.unit.test.ts b/src/test/activation/activationService.unit.test.ts index a568e298fcfc..6da1b9c8fe39 100644 --- a/src/test/activation/activationService.unit.test.ts +++ b/src/test/activation/activationService.unit.test.ts @@ -37,7 +37,7 @@ import { IServiceContainer } from '../../client/ioc/types'; // tslint:disable:max-func-body-length no-any suite('Language Server Activation - ActivationService', () => { - [LanguageServerType.Jedi, LanguageServerType.Node].forEach((languageServerType) => { + [LanguageServerType.Jedi, LanguageServerType.Microsoft].forEach((languageServerType) => { suite( `Test activation - ${ languageServerType === LanguageServerType.Jedi ? 'Jedi is enabled' : 'Jedi is disabled' @@ -155,7 +155,7 @@ suite('Language Server Activation - ActivationService', () => { lsSupported && activatorName !== LanguageServerType.Jedi ) { - activatorName = LanguageServerType.Node; + activatorName = LanguageServerType.Microsoft; } let diagnostics: IDiagnostic[]; @@ -200,7 +200,7 @@ suite('Language Server Activation - ActivationService', () => { .returns(() => TypeMoq.Mock.ofType().object) .verifiable(TypeMoq.Times.once()); - pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Node); + pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Microsoft); const activator = TypeMoq.Mock.ofType(); const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, @@ -229,7 +229,7 @@ suite('Language Server Activation - ActivationService', () => { // Toggle the value in the setting and invoke the callback. languageServerType = languageServerType === LanguageServerType.Jedi - ? LanguageServerType.Node + ? LanguageServerType.Microsoft : LanguageServerType.Jedi; await callbackHandler(event.object); @@ -239,7 +239,7 @@ suite('Language Server Activation - ActivationService', () => { } test('LS is supported', async () => { - pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Node); + pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Microsoft); const activator = TypeMoq.Mock.ofType(); const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, @@ -250,7 +250,7 @@ suite('Language Server Activation - ActivationService', () => { await testActivation(activationService, activator, true); }); test('LS is not supported', async () => { - pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Node); + pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Microsoft); const activator = TypeMoq.Mock.ofType(); const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, @@ -262,7 +262,7 @@ suite('Language Server Activation - ActivationService', () => { }); test('Activator must be activated', async () => { - pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Node); + pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Microsoft); const activator = TypeMoq.Mock.ofType(); const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, @@ -273,7 +273,7 @@ suite('Language Server Activation - ActivationService', () => { await testActivation(activationService, activator); }); test('Activator must be deactivated', async () => { - pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Node); + pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Microsoft); const activator = TypeMoq.Mock.ofType(); const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, @@ -311,7 +311,7 @@ suite('Language Server Activation - ActivationService', () => { .returns(() => TypeMoq.Mock.ofType().object) .verifiable(TypeMoq.Times.once()); - pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Node); + pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Microsoft); const activator = TypeMoq.Mock.ofType(); const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, @@ -798,7 +798,7 @@ suite('Language Server Activation - ActivationService', () => { .returns(() => LanguageServerType.Jedi) .verifiable(TypeMoq.Times.exactly(2)); state - .setup((s) => s.updateValue(TypeMoq.It.isValue(LanguageServerType.Node))) + .setup((s) => s.updateValue(TypeMoq.It.isValue(LanguageServerType.Microsoft))) .returns(() => Promise.resolve()) .verifiable(TypeMoq.Times.once()); @@ -807,7 +807,7 @@ suite('Language Server Activation - ActivationService', () => { stateFactory.object, experiments.object ); - await activationService.sendTelemetryForChosenLanguageServer(LanguageServerType.Node); + await activationService.sendTelemetryForChosenLanguageServer(LanguageServerType.Microsoft); state.verifyAll(); }); @@ -815,7 +815,7 @@ suite('Language Server Activation - ActivationService', () => { state.reset(); state .setup((s) => s.value) - .returns(() => LanguageServerType.Node) + .returns(() => LanguageServerType.Microsoft) .verifiable(TypeMoq.Times.exactly(2)); state .setup((s) => s.updateValue(TypeMoq.It.isValue(LanguageServerType.Jedi))) diff --git a/src/test/datascience/intellisense.unit.test.ts b/src/test/datascience/intellisense.unit.test.ts index 016282699f5e..c125513a050a 100644 --- a/src/test/datascience/intellisense.unit.test.ts +++ b/src/test/datascience/intellisense.unit.test.ts @@ -69,7 +69,7 @@ suite('DataScience Intellisense Unit Tests', () => { notebookProvider = TypeMoq.Mock.ofType(); const variableProvider = mock(JupyterVariables); - pythonSettings.languageServer = LanguageServerType.Node; + pythonSettings.languageServer = LanguageServerType.Microsoft; configService.setup((c) => c.getSettings(TypeMoq.It.isAny())).returns(() => pythonSettings); workspaceService.setup((w) => w.rootPath).returns(() => '/foo/bar'); fileSystem diff --git a/src/test/linters/linter.availability.unit.test.ts b/src/test/linters/linter.availability.unit.test.ts index bae239ea58e2..39f51fb0e4d6 100644 --- a/src/test/linters/linter.availability.unit.test.ts +++ b/src/test/linters/linter.availability.unit.test.ts @@ -62,9 +62,9 @@ suite('Linter Availability Provider tests', () => { workspaceServiceMock.verifyAll(); }); - test('Availability feature is enabled when global default for languageServer is Node.', async () => { + test('Availability feature is enabled when global default for languageServer is Microsoft.', async () => { // set expectations - const languageServerValue = LanguageServerType.Node; + const languageServerValue = LanguageServerType.Microsoft; const expectedResult = true; // arrange @@ -386,7 +386,7 @@ suite('Linter Availability Provider tests', () => { // Options to test the implementation of the IAvailableLinterActivator. // All options default to values that would otherwise allow the prompt to appear. class AvailablityTestOverallOptions { - public languageServerValue = LanguageServerType.Node; + public languageServerValue = LanguageServerType.Microsoft; public pylintUserEnabled?: boolean; public pylintWorkspaceEnabled?: boolean; public pylintWorkspaceFolderEnabled?: boolean; diff --git a/src/test/linters/linterinfo.unit.test.ts b/src/test/linters/linterinfo.unit.test.ts index 3537e8625f52..d3eab1e83033 100644 --- a/src/test/linters/linterinfo.unit.test.ts +++ b/src/test/linters/linterinfo.unit.test.ts @@ -51,7 +51,7 @@ suite('Linter Info - Pylint', () => { when(config.getSettings(anything())).thenReturn({ linting: { pylintEnabled: true }, - languageServer: LanguageServerType.Node + languageServer: LanguageServerType.Microsoft } as any); const pythonConfig = { diff --git a/src/test/performanceTest.ts b/src/test/performanceTest.ts index e5325d552643..dbc02cc3652c 100644 --- a/src/test/performanceTest.ts +++ b/src/test/performanceTest.ts @@ -73,7 +73,9 @@ class TestRunner { await this.runPerfTest(devLogFiles, releaseLogFiles, languageServerLogFiles); } private async enableLanguageServer(enable: boolean) { - const settings = `{ "python.languageServer": ${enable ? LanguageServerType.Node : LanguageServerType.Jedi} }`; + const settings = `{ "python.languageServer": "${ + enable ? LanguageServerType.Microsoft : LanguageServerType.Jedi + }" }`; await fs.writeFile(path.join(EXTENSION_ROOT_DIR, 'src', 'test', 'performance', 'settings.json'), settings); } diff --git a/src/test/smokeTest.ts b/src/test/smokeTest.ts index 8c5a08710327..79e44f1dcf3d 100644 --- a/src/test/smokeTest.ts +++ b/src/test/smokeTest.ts @@ -32,7 +32,7 @@ class TestRunner { } private async enableLanguageServer(enable: boolean) { // When running smoke tests, we won't have access to unbundled files. - const settings = `{ "python.languageServer": ${enable ? '"Node"' : '"Jedi"'} }`; + const settings = `{ "python.languageServer": ${enable ? '"Microsoft"' : '"Jedi"'} }`; await fs.ensureDir( path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'src', 'testMultiRootWkspc', 'smokeTests', '.vscode') ); From 93c2ccdab6ef5f9e32eda31d747f1ad296b04455 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Fri, 22 May 2020 10:04:23 -0700 Subject: [PATCH 30/34] Use json-parser for comments --- .../testing/common/updateTestSettings.ts | 82 +++++++++++-------- .../checks/updateTestSettings.unit.test.ts | 2 +- 2 files changed, 48 insertions(+), 36 deletions(-) diff --git a/src/client/testing/common/updateTestSettings.ts b/src/client/testing/common/updateTestSettings.ts index cc37e4593a5b..baebc61c730e 100644 --- a/src/client/testing/common/updateTestSettings.ts +++ b/src/client/testing/common/updateTestSettings.ts @@ -4,6 +4,7 @@ 'use strict'; import { inject, injectable } from 'inversify'; +import { applyEdits, Edit, findNodeAtLocation, FormattingOptions, getNodeValue, modify, parseTree } from 'jsonc-parser'; import * as path from 'path'; import { IExtensionActivationService, LanguageServerType } from '../../activation/types'; import { IApplicationEnvironment, IWorkspaceService } from '../../common/application/types'; @@ -51,8 +52,10 @@ export class UpdateTestSettingService implements IExtensionActivationService { ); return result.filter((item) => item.needsFixing).map((item) => item.file); } + // fixLanguageServerSetting provided for tests so not all tests have to + // deal with potential whitespace changes. @swallowExceptions('Failed to update settings.json') - public async fixSettingInFile(filePath: string, fixLanguageServer = true): Promise { + public async fixSettingInFile(filePath: string, fixLanguageServerSetting = true): Promise { let fileContents = await this.fs.readFile(filePath); const setting = new RegExp('"python\\.unitTest', 'g'); @@ -74,40 +77,10 @@ export class UpdateTestSettingService implements IExtensionActivationService { fileContents = fileContents.replace(setting_pep8_enabled, '.pycodestyleEnabled'); fileContents = fileContents.replace(setting_pep8_path, '.pycodestylePath'); - if (fixLanguageServer) { - // `python.jediEnabled` is deprecated: - // - `true` or missing then set to `languageServer: Jedi`. - // - `false` and `languageServer` is present, do nothing. - // - `false` and `languageServer` is NOT present, set `languageServer` to `Microsoft`. - // `jediEnabled` is then removed. - try { - const settings = JSON.parse(fileContents); - let changed = false; - - if (settings[`python.jediEnabled`] === undefined) { - // `jediEnabled` is missing. Default was true, so assume Jedi. - if (!settings['python.languageServer']) { - settings['python.languageServer'] = LanguageServerType.Jedi; - changed = true; - } - } else { - if (settings[`python.jediEnabled`]) { - settings['python.languageServer'] = LanguageServerType.Jedi; - } else { - // `jediEnabled` is false. If no `languageServer`, set it to Microsoft. - if (!settings['python.languageServer']) { - settings['python.languageServer'] = LanguageServerType.Microsoft; - } - } - settings[`python.jediEnabled`] = undefined; - changed = true; - } - - if (changed) { - fileContents = JSON.stringify(settings, null, 4); - } - // tslint:disable-next-line:no-empty - } catch {} + // tslint:disable-next-line:no-suspicious-comment + // TODO: remove when python.jediEnabled is no longer in typical user settings. + if (fixLanguageServerSetting) { + fileContents = this.fixLanguageServerSettings(fileContents); } await this.fs.writeFile(filePath, fileContents); @@ -127,4 +100,43 @@ export class UpdateTestSettingService implements IExtensionActivationService { return false; } } + + private fixLanguageServerSettings(fileContent: string): string { + // `python.jediEnabled` is deprecated: + // - `true` or missing then set to `languageServer: Jedi`. + // - `false` and `languageServer` is present, do nothing. + // - `false` and `languageServer` is NOT present, set `languageServer` to `Microsoft`. + // `jediEnabled` is then removed. + const jediEnabledPath = ['python.jediEnabled']; + const languageServerPath = ['python.languageServer']; + + try { + let ast = parseTree(fileContent); + let jediEnabledNode = findNodeAtLocation(ast, jediEnabledPath); + const jediEnabled = jediEnabledNode ? getNodeValue(jediEnabledNode) : true; + const formattingOptions: FormattingOptions = { + tabSize: 4, + insertSpaces: true + }; + let edits: Edit[] = []; + + if (!jediEnabledNode || jediEnabled) { + // `jediEnabled` is missing or is true. Default is true, so assume Jedi. + edits = modify(fileContent, languageServerPath, LanguageServerType.Jedi, { formattingOptions }); + } else { + // `jediEnabled` is false. Set `languageServer` to Microsoft. + edits = modify(fileContent, languageServerPath, LanguageServerType.Microsoft, { formattingOptions }); + } + fileContent = applyEdits(fileContent, edits); + // Remove jediEnabled + ast = parseTree(fileContent); + jediEnabledNode = findNodeAtLocation(ast, jediEnabledPath); + if (jediEnabledNode) { + edits = modify(fileContent, jediEnabledPath, undefined, { formattingOptions }); + fileContent = applyEdits(fileContent, edits); + } + // tslint:disable-next-line:no-empty + } catch {} + return fileContent; + } } diff --git a/src/test/application/diagnostics/checks/updateTestSettings.unit.test.ts b/src/test/application/diagnostics/checks/updateTestSettings.unit.test.ts index c3f44341c075..819b27b9f8cb 100644 --- a/src/test/application/diagnostics/checks/updateTestSettings.unit.test.ts +++ b/src/test/application/diagnostics/checks/updateTestSettings.unit.test.ts @@ -205,7 +205,7 @@ suite('Application Diagnostics - Check Test Settings', () => { verify(fs.readFile(__filename)).once(); verify(fs.writeFile(__filename, anyString())).once(); - expect(nows(actualContent)).to.be.equal(nows(item.expectedContents)); + expect(actualContent).to.be.equal(item.expectedContents); }); }); From 91c1b7123be66df8a8cbeba5ea99652ae7265788 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Fri, 22 May 2020 11:16:49 -0700 Subject: [PATCH 31/34] Leave existing languageServer setting value alone --- src/client/testing/common/updateTestSettings.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/client/testing/common/updateTestSettings.ts b/src/client/testing/common/updateTestSettings.ts index baebc61c730e..1f5c79faf32c 100644 --- a/src/client/testing/common/updateTestSettings.ts +++ b/src/client/testing/common/updateTestSettings.ts @@ -114,6 +114,7 @@ export class UpdateTestSettingService implements IExtensionActivationService { let ast = parseTree(fileContent); let jediEnabledNode = findNodeAtLocation(ast, jediEnabledPath); const jediEnabled = jediEnabledNode ? getNodeValue(jediEnabledNode) : true; + const languageServerNode = findNodeAtLocation(ast, languageServerPath); const formattingOptions: FormattingOptions = { tabSize: 4, insertSpaces: true @@ -124,9 +125,14 @@ export class UpdateTestSettingService implements IExtensionActivationService { // `jediEnabled` is missing or is true. Default is true, so assume Jedi. edits = modify(fileContent, languageServerPath, LanguageServerType.Jedi, { formattingOptions }); } else { - // `jediEnabled` is false. Set `languageServer` to Microsoft. - edits = modify(fileContent, languageServerPath, LanguageServerType.Microsoft, { formattingOptions }); + // `jediEnabled` is false. if languageServer is missing, set it to Microsoft. + if (!languageServerNode) { + edits = modify(fileContent, languageServerPath, LanguageServerType.Microsoft, { + formattingOptions + }); + } } + fileContent = applyEdits(fileContent, edits); // Remove jediEnabled ast = parseTree(fileContent); From 22f1e8f08afafa9d6b506a678b1a35767d28a5b4 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Tue, 26 May 2020 11:29:23 -0700 Subject: [PATCH 32/34] Update src/test/.vscode/settings.json Co-authored-by: Eric Snow --- src/test/.vscode/settings.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/.vscode/settings.json b/src/test/.vscode/settings.json index e06b046b67c9..49ca7a03c0bc 100644 --- a/src/test/.vscode/settings.json +++ b/src/test/.vscode/settings.json @@ -23,7 +23,7 @@ // Do not set this to "Microsoft", else it will result in LS being downloaded on CI // and that slows down tests significantly. We have other tests on CI for testing // downloading of LS with this setting enabled. - // "python.languageServer": "Jedi" + "python.languageServer": "Jedi" "python.experiments.optInto": [ "NativeNotebook - experiment" ], From 729c707d5d259795af58fa01563d031100ed3317 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Tue, 26 May 2020 11:30:12 -0700 Subject: [PATCH 33/34] PR feedback --- .../common/languageServerFolderService.ts | 16 ++-------------- .../languageServerFolderService.ts | 16 ++++++++++++++-- .../node/languageServerFolderService.ts | 2 +- src/client/activation/types.ts | 3 --- src/test/.vscode/settings.json | 9 ++++----- 5 files changed, 21 insertions(+), 25 deletions(-) diff --git a/src/client/activation/common/languageServerFolderService.ts b/src/client/activation/common/languageServerFolderService.ts index 591653e903e0..3aa99f8e20e4 100644 --- a/src/client/activation/common/languageServerFolderService.ts +++ b/src/client/activation/common/languageServerFolderService.ts @@ -6,7 +6,6 @@ import { inject, injectable, unmanaged } from 'inversify'; import * as path from 'path'; import * as semver from 'semver'; -import { IApplicationEnvironment } from '../../common/application/types'; import { EXTENSION_ROOT_DIR } from '../../common/constants'; import { traceDecorators } from '../../common/logger'; import { NugetPackage } from '../../common/nuget/types'; @@ -14,7 +13,6 @@ import { IFileSystem } from '../../common/platform/types'; import { IConfigurationService, Resource } from '../../common/types'; import { IServiceContainer } from '../../ioc/types'; import { - DotNetLanguageServerFolder, FolderVersionPair, IDownloadChannelRule, ILanguageServerFolderService, @@ -53,7 +51,7 @@ export abstract class LanguageServerFolderService implements ILanguageServerFold @traceDecorators.verbose('Get latest version of Language Server') public getLatestLanguageServerVersion(resource: Resource): Promise { - const minVersion = this.getMinimalLanguageServerVersion(DotNetLanguageServerFolder); + const minVersion = this.getMinimalLanguageServerVersion(); const lsPackageService = this.serviceContainer.get( ILanguageServerPackageService ); @@ -102,17 +100,7 @@ export abstract class LanguageServerFolderService implements ILanguageServerFold : semver.parse(suffix, true) || new semver.SemVer('0.0.0'); } - protected getMinimalLanguageServerVersion(key: string): string { - let minVersion = '0.0.0'; - try { - const appEnv = this.serviceContainer.get(IApplicationEnvironment); - if (appEnv) { - minVersion = appEnv.packageJson[key] as string; - } - // tslint:disable-next-line: no-empty - } catch {} - return minVersion; - } + protected abstract getMinimalLanguageServerVersion(): string; private getDownloadChannel() { const lsPackageService = this.serviceContainer.get( diff --git a/src/client/activation/languageServer/languageServerFolderService.ts b/src/client/activation/languageServer/languageServerFolderService.ts index a2c32a8df02f..03ce4e88d1b5 100644 --- a/src/client/activation/languageServer/languageServerFolderService.ts +++ b/src/client/activation/languageServer/languageServerFolderService.ts @@ -4,9 +4,13 @@ 'use strict'; import { inject, injectable } from 'inversify'; +import { IApplicationEnvironment } from '../../common/application/types'; import { IServiceContainer } from '../../ioc/types'; import { LanguageServerFolderService } from '../common/languageServerFolderService'; -import { DotNetLanguageServerFolder, DotNetLanguageServerMinVersionKey } from '../types'; +import { DotNetLanguageServerFolder } from '../types'; + +// Must match languageServerVersion* keys in package.json +const DotNetLanguageServerMinVersionKey = 'languageServerVersion'; @injectable() export class DotNetLanguageServerFolderService extends LanguageServerFolderService { @@ -15,6 +19,14 @@ export class DotNetLanguageServerFolderService extends LanguageServerFolderServi } protected getMinimalLanguageServerVersion(): string { - return super.getMinimalLanguageServerVersion(DotNetLanguageServerMinVersionKey); + let minVersion = '0.0.0'; + try { + const appEnv = this.serviceContainer.get(IApplicationEnvironment); + if (appEnv) { + minVersion = appEnv.packageJson[DotNetLanguageServerMinVersionKey] as string; + } + // tslint:disable-next-line: no-empty + } catch {} + return minVersion; } } diff --git a/src/client/activation/node/languageServerFolderService.ts b/src/client/activation/node/languageServerFolderService.ts index 205b9bf65ef7..8da206ac55c9 100644 --- a/src/client/activation/node/languageServerFolderService.ts +++ b/src/client/activation/node/languageServerFolderService.ts @@ -15,6 +15,6 @@ export class NodeLanguageServerFolderService extends LanguageServerFolderService } protected getMinimalLanguageServerVersion(): string { - return '0.0.1'; // super.getMinimalLanguageServerVersion(NodeLanguageServerMinVersionKey); + return '0.0.0'; } } diff --git a/src/client/activation/types.ts b/src/client/activation/types.ts index 3a73f75c9a70..6b53c01f60e0 100644 --- a/src/client/activation/types.ts +++ b/src/client/activation/types.ts @@ -73,9 +73,6 @@ export enum LanguageServerType { export const DotNetLanguageServerFolder = 'languageServer'; export const NodeLanguageServerFolder = 'nodeLanguageServer'; -// Must match languageServerVersion* keys in package.json -export const DotNetLanguageServerMinVersionKey = 'languageServerVersion'; - // tslint:disable-next-line: interface-name export interface DocumentHandler { handleOpen(document: TextDocument): void; diff --git a/src/test/.vscode/settings.json b/src/test/.vscode/settings.json index e06b046b67c9..7745e9cf8759 100644 --- a/src/test/.vscode/settings.json +++ b/src/test/.vscode/settings.json @@ -20,12 +20,11 @@ "python.formatting.provider": "yapf", "python.linting.pylintUseMinimalCheckers": false, "python.pythonPath": "python", - // Do not set this to "Microsoft", else it will result in LS being downloaded on CI - // and that slows down tests significantly. We have other tests on CI for testing - // downloading of LS with this setting enabled. - // "python.languageServer": "Jedi" "python.experiments.optInto": [ "NativeNotebook - experiment" ], - "python.languageServer": "Jedi", + // Do not set this to "Microsoft", else it will result in LS being downloaded on CI + // and that slows down tests significantly. We have other tests on CI for testing + // downloading of LS with this setting enabled. + "python.languageServer": "Jedi" } From 7d7890882eb7faa1ed0ebcb912e6c9d1013a6887 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Tue, 26 May 2020 12:08:49 -0700 Subject: [PATCH 34/34] Put test back --- src/test/linters/linterinfo.unit.test.ts | 36 ++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/src/test/linters/linterinfo.unit.test.ts b/src/test/linters/linterinfo.unit.test.ts index d3eab1e83033..39b2e9550677 100644 --- a/src/test/linters/linterinfo.unit.test.ts +++ b/src/test/linters/linterinfo.unit.test.ts @@ -62,4 +62,40 @@ suite('Linter Info - Pylint', () => { expect(linterInfo.isEnabled()).to.be.false; }); + const testsForisEnabled = [ + { + testName: 'When workspaceFolder setting is provided', + inspection: { workspaceFolderValue: true } + }, + { + testName: 'When workspace setting is provided', + inspection: { workspaceValue: true } + }, + { + testName: 'When global setting is provided', + inspection: { globalValue: true } + } + ]; + + suite('Test is enabled when using Language Server and Pylint is configured', () => { + testsForisEnabled.forEach((testParams) => { + test(testParams.testName, async () => { + // tslint:disable-next-line:no-shadowed-variable + const config = mock(ConfigurationService); + const workspaceService = mock(WorkspaceService); + const linterInfo = new PylintLinterInfo(instance(config), instance(workspaceService), []); + + const pythonConfig = { + inspect: () => testParams.inspection + }; + when(config.getSettings(anything())).thenReturn({ + linting: { pylintEnabled: true }, + languageServer: LanguageServerType.Microsoft + } as any); + when(workspaceService.getConfiguration('python', anything())).thenReturn(pythonConfig as any); + + expect(linterInfo.isEnabled()).to.be.true; + }); + }); + }); });