diff --git a/CHANGELOG.md b/CHANGELOG.md index 5146a6dcc7b2..dcfebd354904 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,14 @@ # Changelog +## 2020.6.2 (25 June 2020) + +### Fixes + +1. Fix `linting.pylintEnabled` setting check. + ([#12285](https://github.com/Microsoft/vscode-python/issues/12285)) +1. Don't modify LS settings if jediEnabled does not exist. + ([#12429](https://github.com/Microsoft/vscode-python/issues/12429)) + ## 2020.6.1 (17 June 2020) ### Fixes diff --git a/package-lock.json b/package-lock.json index 2ee1538ffeef..231ac1d03356 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "python", - "version": "2020.6.1", + "version": "2020.6.2", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index fc652563216c..c9763b31385a 100644 --- a/package.json +++ b/package.json @@ -2,7 +2,7 @@ "name": "python", "displayName": "Python", "description": "Linting, Debugging (multi-threaded, remote), Intellisense, Jupyter Notebooks, code formatting, refactoring, unit tests, snippets, and more.", - "version": "2020.6.1", + "version": "2020.6.2", "featureFlags": { "usingNewInterpreterStorage": true }, diff --git a/src/client/linters/linterInfo.ts b/src/client/linters/linterInfo.ts index 21ead3b13c70..ee7842a204e6 100644 --- a/src/client/linters/linterInfo.ts +++ b/src/client/linters/linterInfo.ts @@ -99,7 +99,7 @@ export class PylintLinterInfo extends LinterInfo { // 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); + const inspection = configuration.inspect(`linting.${this.enabledSettingName}`); if ( !inspection || (inspection.globalValue === undefined && diff --git a/src/client/testing/common/updateTestSettings.ts b/src/client/testing/common/updateTestSettings.ts index b7b60aad8ad8..dc549bdd4fae 100644 --- a/src/client/testing/common/updateTestSettings.ts +++ b/src/client/testing/common/updateTestSettings.ts @@ -4,7 +4,7 @@ 'use strict'; import { inject, injectable } from 'inversify'; -import { applyEdits, Edit, findNodeAtLocation, FormattingOptions, getNodeValue, modify, parseTree } from 'jsonc-parser'; +import { applyEdits, findNodeAtLocation, getNodeValue, ModificationOptions, modify, parseTree } from 'jsonc-parser'; import * as path from 'path'; import { IExtensionActivationService, LanguageServerType } from '../../activation/types'; import { IApplicationEnvironment, IWorkspaceService } from '../../common/application/types'; @@ -104,7 +104,8 @@ export class UpdateTestSettingService implements IExtensionActivationService { private fixLanguageServerSettings(fileContent: string): string { // `python.jediEnabled` is deprecated: - // - `true` or missing then set to `languageServer: Jedi`. + // - If missing, do nothing. + // - `true`, then set to `languageServer: Jedi`. // - `false` and `languageServer` is present, do nothing. // - `false` and `languageServer` is NOT present, set `languageServer` to `Microsoft`. // `jediEnabled` is NOT removed since JSONC parser may also remove comments. @@ -113,28 +114,40 @@ export class UpdateTestSettingService implements IExtensionActivationService { try { const ast = parseTree(fileContent); + const jediEnabledNode = findNodeAtLocation(ast, jediEnabledPath); - const jediEnabled = jediEnabledNode ? getNodeValue(jediEnabledNode) : true; const languageServerNode = findNodeAtLocation(ast, languageServerPath); - 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. if languageServer is missing, set it to Microsoft. - if (!languageServerNode) { - edits = modify(fileContent, languageServerPath, LanguageServerType.Microsoft, { - formattingOptions - }); + + // If missing, do nothing. + if (!jediEnabledNode) { + return fileContent; + } + + const jediEnabled = getNodeValue(jediEnabledNode); + + const modificationOptions: ModificationOptions = { + formattingOptions: { + tabSize: 4, + insertSpaces: true } + }; + + // `jediEnabled` is true, set it to Jedi. + if (jediEnabled) { + return applyEdits( + fileContent, + modify(fileContent, languageServerPath, LanguageServerType.Jedi, modificationOptions) + ); + } + + // `jediEnabled` is false. if languageServer is missing, set it to Microsoft. + if (!languageServerNode) { + return applyEdits( + fileContent, + modify(fileContent, languageServerPath, LanguageServerType.Microsoft, modificationOptions) + ); } - 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 d70dd05728ef..039086a404a3 100644 --- a/src/test/application/diagnostics/checks/updateTestSettings.unit.test.ts +++ b/src/test/application/diagnostics/checks/updateTestSettings.unit.test.ts @@ -217,9 +217,14 @@ suite('Application Diagnostics - Check Test Settings', () => { [ { - testTitle: 'No jediEnabled setting.', + testTitle: 'No jediEnabled setting', contents: '{}', - expectedContent: '{ "python.languageServer": "Jedi" }' + expectedContent: '{}' + }, + { + testTitle: 'jediEnabled setting in comment', + contents: '{\n // "python.jediEnabled": true\n }', + expectedContent: '{\n // "python.jediEnabled": true\n }' }, { testTitle: 'jediEnabled: true, no languageServer setting', @@ -250,6 +255,11 @@ suite('Application Diagnostics - Check Test Settings', () => { testTitle: 'jediEnabled: false, languageServer is Jedi', contents: '{ "python.jediEnabled": false, "python.languageServer": "Jedi" }', expectedContent: '{ "python.jediEnabled": false, "python.languageServer": "Jedi"}' + }, + { + testTitle: 'jediEnabled not present, languageServer is Microsoft', + contents: '{ "python.languageServer": "Microsoft" }', + expectedContent: '{ "python.languageServer": "Microsoft" }' } ].forEach((item) => { test(item.testTitle, async () => { diff --git a/src/test/linters/linterinfo.unit.test.ts b/src/test/linters/linterinfo.unit.test.ts index 39b2e9550677..4ec1f5f5e03f 100644 --- a/src/test/linters/linterinfo.unit.test.ts +++ b/src/test/linters/linterinfo.unit.test.ts @@ -6,6 +6,7 @@ // tslint:disable:chai-vague-errors no-unused-expression max-func-body-length no-any import { expect } from 'chai'; +import * as sinon from 'sinon'; import { anything, instance, mock, when } from 'ts-mockito'; import { LanguageServerType } from '../../client/activation/types'; import { WorkspaceService } from '../../client/common/application/workspace'; @@ -62,6 +63,22 @@ suite('Linter Info - Pylint', () => { expect(linterInfo.isEnabled()).to.be.false; }); + test('Should inspect the value of linting.pylintEnabled when using Language Server', async () => { + const linterInfo = new PylintLinterInfo(instance(config), instance(workspace), []); + const inspectStub = sinon.stub(); + const pythonConfig = { + inspect: inspectStub + }; + + when(config.getSettings(anything())).thenReturn({ + linting: { pylintEnabled: true }, + languageServer: LanguageServerType.Microsoft + } as any); + when(workspace.getConfiguration('python', anything())).thenReturn(pythonConfig as any); + + expect(linterInfo.isEnabled()).to.be.false; + expect(inspectStub.calledOnceWith('linting.pylintEnabled')).to.be.true; + }); const testsForisEnabled = [ { testName: 'When workspaceFolder setting is provided',