From 57a63601dd1d955f6a5dcbbd3921adda5c5d565f Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Wed, 24 Jun 2020 14:41:41 -0700 Subject: [PATCH 1/2] Don't modify LS settings if jediEnabled does not exist --- src/client/testing/common/updateTestSettings.ts | 12 +++++++++--- .../checks/updateTestSettings.unit.test.ts | 14 ++++++++++++-- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/src/client/testing/common/updateTestSettings.ts b/src/client/testing/common/updateTestSettings.ts index b7b60aad8ad8..f68fad07fb56 100644 --- a/src/client/testing/common/updateTestSettings.ts +++ b/src/client/testing/common/updateTestSettings.ts @@ -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,8 +114,13 @@ export class UpdateTestSettingService implements IExtensionActivationService { try { const ast = parseTree(fileContent); + const jediEnabledNode = findNodeAtLocation(ast, jediEnabledPath); - const jediEnabled = jediEnabledNode ? getNodeValue(jediEnabledNode) : true; + if (!jediEnabledNode) { + return fileContent; + } + + const jediEnabled = getNodeValue(jediEnabledNode); const languageServerNode = findNodeAtLocation(ast, languageServerPath); const formattingOptions: FormattingOptions = { tabSize: 4, @@ -122,7 +128,7 @@ export class UpdateTestSettingService implements IExtensionActivationService { }; let edits: Edit[] = []; - if (!jediEnabledNode || jediEnabled) { + if (jediEnabled) { // `jediEnabled` is missing or is true. Default is true, so assume Jedi. edits = modify(fileContent, languageServerPath, LanguageServerType.Jedi, { formattingOptions }); } else { 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 () => { From 80868a2de8bb0cb24534d37dfba776970ef8e689 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Wed, 24 Jun 2020 15:17:36 -0700 Subject: [PATCH 2/2] Cleanup --- .../testing/common/updateTestSettings.ts | 39 +++++++++++-------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/src/client/testing/common/updateTestSettings.ts b/src/client/testing/common/updateTestSettings.ts index f68fad07fb56..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'; @@ -116,31 +116,38 @@ export class UpdateTestSettingService implements IExtensionActivationService { const ast = parseTree(fileContent); const jediEnabledNode = findNodeAtLocation(ast, jediEnabledPath); + const languageServerNode = findNodeAtLocation(ast, languageServerPath); + + // If missing, do nothing. if (!jediEnabledNode) { return fileContent; } const jediEnabled = getNodeValue(jediEnabledNode); - const languageServerNode = findNodeAtLocation(ast, languageServerPath); - const formattingOptions: FormattingOptions = { - tabSize: 4, - insertSpaces: true + + const modificationOptions: ModificationOptions = { + formattingOptions: { + tabSize: 4, + insertSpaces: true + } }; - let edits: Edit[] = []; + // `jediEnabled` is true, set it to Jedi. if (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 - }); - } + 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;