From 2fc9dc26bc2b1e70c5d6a98ab027de2113dc7209 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sat, 25 Jul 2020 15:23:56 -0400 Subject: [PATCH 1/4] Ignore all unknown editor settings in conversion --- .../convertEditorSettings.test.ts | 79 +++++++++++-------- src/editorSettings/convertEditorSettings.ts | 11 ++- 2 files changed, 55 insertions(+), 35 deletions(-) diff --git a/src/editorSettings/convertEditorSettings.test.ts b/src/editorSettings/convertEditorSettings.test.ts index b1d5bd890..1c4a0b2d2 100644 --- a/src/editorSettings/convertEditorSettings.test.ts +++ b/src/editorSettings/convertEditorSettings.test.ts @@ -13,29 +13,26 @@ describe("convertEditorSettings", () => { }; // Act - const { converted, missing, failed } = convertEditorSettings( - { converters }, - editorConfiguration, - ); + const result = convertEditorSettings({ converters }, editorConfiguration); // Assert - expect(converted.size).toEqual(0); - expect(missing.length).toEqual(0); - expect(failed.length).toEqual(0); + expect(result).toEqual({ + converted: new Map(), + failed: [], + missing: [], + }); }); it("skips a configuration if not an editor setting", () => { // Arrange - const conversionResult: EditorSettingConversionResult = { + const { editorSetting, converters } = setupConversionEnvironment({ settings: [ { editorSettingName: "editor.eslint-setting-a", value: "a", }, ], - }; - - const { editorSetting, converters } = setupConversionEnvironment(conversionResult); + }); const editorConfiguration = { notAnEditorSetting: "a", @@ -44,15 +41,22 @@ describe("convertEditorSettings", () => { }; // Act - const { converted, missing, failed } = convertEditorSettings( - { converters }, - editorConfiguration, - ); + const result = convertEditorSettings({ converters }, editorConfiguration); // Assert - expect(converted.size).toEqual(1); - expect(missing.length).toEqual(0); - expect(failed.length).toEqual(0); + expect(result).toEqual({ + converted: new Map([ + [ + "editor.eslint-setting-a", + { + editorSettingName: "editor.eslint-setting-a", + value: "a", + }, + ], + ]), + failed: [], + missing: [], + }); }); it("marks a setting as missing when its converter returns undefined", () => { @@ -60,13 +64,17 @@ describe("convertEditorSettings", () => { const { editorSetting, converters } = setupConversionEnvironment(); // Act - const { missing } = convertEditorSettings( + const result = convertEditorSettings( { converters }, { [editorSetting.editorSettingName]: editorSetting }, ); // Assert - expect(missing).toEqual([{ editorSettingName: editorSetting.editorSettingName }]); + expect(result).toEqual({ + converted: new Map(), + failed: [], + missing: [{ editorSettingName: editorSetting.editorSettingName }], + }); }); it("marks a conversion as failed when returned a conversion error", () => { @@ -76,45 +84,50 @@ describe("convertEditorSettings", () => { converters.set(editorSetting.editorSettingName, () => conversionError); // Act - const { failed } = convertEditorSettings( + const result = convertEditorSettings( { converters }, { [editorSetting.editorSettingName]: editorSetting }, ); // Assert - expect(failed).toEqual([conversionError]); + expect(result).toEqual({ + converted: new Map(), + failed: [conversionError], + missing: [], + }); }); it("marks a converted setting name as converted when a conversion has settings", () => { // Arrange - const conversionResult: EditorSettingConversionResult = { + const { editorSetting, converters } = setupConversionEnvironment({ settings: [ { - editorSettingName: "editor.eslint-setting-a", + editorSettingName: "eslint.configFile", value: "a", }, ], - }; - const { editorSetting, converters } = setupConversionEnvironment(conversionResult); + }); // Act - const { converted } = convertEditorSettings( + const result = convertEditorSettings( { converters }, { [editorSetting.editorSettingName]: editorSetting.value }, ); // Assert - expect(converted).toEqual( - new Map([ + expect(result).toEqual({ + converted: new Map([ [ - "editor.eslint-setting-a", + "eslint.configFile", { - editorSettingName: "editor.eslint-setting-a", + editorSettingName: "eslint.configFile", value: "a", }, ], ]), - ); + failed: [], + missing: [], + }); }); }); @@ -127,7 +140,7 @@ function setupConversionEnvironment(conversionResult?: EditorSettingConversionRe function createSampleEditorSetting(): EditorSetting { return { - editorSettingName: "editor.tslint-editor-setting-a", + editorSettingName: "tslint.configFile", value: "a", }; } diff --git a/src/editorSettings/convertEditorSettings.ts b/src/editorSettings/convertEditorSettings.ts index 809adaec8..043b76fad 100644 --- a/src/editorSettings/convertEditorSettings.ts +++ b/src/editorSettings/convertEditorSettings.ts @@ -4,7 +4,14 @@ import { convertEditorSetting } from "./convertEditorSetting"; import { EditorSettingConverter } from "./converter"; import { EditorSetting } from "./types"; -const EDITOR_SETTINGS_PREFIX = "editor."; +const knownEditorSettings = new Set([ + "tslint.configFile", + "tslint.jsEnable", + "tslint.ignoreDefinitionFiles", + "tslint.exclude", + "tslint.alwaysShowRuleFailuresAsWarnings", + "tslint.suppressWhileTypeErrorsPresent", +]); export type ConvertEditorSettingsDependencies = { converters: Map; @@ -30,7 +37,7 @@ export const convertEditorSettings = ( for (const [configurationName, value] of Object.entries(rawEditorConfiguration)) { // Configurations other than editor settings will be ignored. // See: https://marketplace.visualstudio.com/items?itemName=ms-vscode.vscode-typescript-tslint-plugin#configuration - if (!configurationName.startsWith(EDITOR_SETTINGS_PREFIX)) { + if (!knownEditorSettings.has(configurationName)) { continue; } From 8f23bff4067622de13ea853b3cf9a613582de93f Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sat, 25 Jul 2020 15:59:28 -0400 Subject: [PATCH 2/4] Added editor settings converter for tslint.configFile, with full settings access --- src/conversion/convertEditorConfig.test.ts | 6 ++- src/conversion/convertEditorConfig.ts | 5 ++- .../convertEditorSetting.test.ts | 9 +++- src/editorSettings/convertEditorSetting.ts | 4 +- .../convertEditorSettings.test.ts | 16 ++++++- src/editorSettings/convertEditorSettings.ts | 4 +- src/editorSettings/converter.ts | 4 +- .../tests/editor-code-actions-on-save.test.ts | 44 ++++++++++++------- .../tests/tslint-config-file.test.ts | 34 ++++++++++++++ .../converters/tslint-config-file.ts | 24 ++++++++++ .../editorSettingsConverters.ts | 2 + src/settings.stubs.ts | 3 ++ 12 files changed, 130 insertions(+), 25 deletions(-) create mode 100644 src/editorSettings/converters/tests/tslint-config-file.test.ts create mode 100644 src/editorSettings/converters/tslint-config-file.ts create mode 100644 src/settings.stubs.ts diff --git a/src/conversion/convertEditorConfig.test.ts b/src/conversion/convertEditorConfig.test.ts index d734e48a8..08572cc65 100644 --- a/src/conversion/convertEditorConfig.test.ts +++ b/src/conversion/convertEditorConfig.test.ts @@ -1,4 +1,5 @@ import { EditorSetting } from "../editorSettings/types"; +import { createStubTSLintToESLintSettings } from "../settings.stubs"; import { FailedResult, ResultStatus } from "../types"; import { createEmptySettingConversionResults } from "./conversionResults.stubs"; import { convertEditorConfig, ConvertEditorConfigDependencies } from "./convertEditorConfig"; @@ -89,7 +90,10 @@ describe("convertEditorConfig", () => { await convertEditorConfig(dependencies, stubSettings); // Assert - expect(dependencies.convertEditorSettings).toHaveBeenCalledWith(originalConfig); + expect(dependencies.convertEditorSettings).toHaveBeenCalledWith( + originalConfig, + createStubTSLintToESLintSettings(), + ); }); it("reports conversion results when settings are converted successfully", async () => { diff --git a/src/conversion/convertEditorConfig.ts b/src/conversion/convertEditorConfig.ts index 3218ffb55..c1e8ec170 100644 --- a/src/conversion/convertEditorConfig.ts +++ b/src/conversion/convertEditorConfig.ts @@ -33,7 +33,10 @@ export const convertEditorConfig = async ( }; } - const settingConversionResults = dependencies.convertEditorSettings(conversion.result); + const settingConversionResults = dependencies.convertEditorSettings( + conversion.result, + settings, + ); const fileWriteError = await dependencies.writeConversionResults( conversion.configPath, diff --git a/src/editorSettings/convertEditorSetting.test.ts b/src/editorSettings/convertEditorSetting.test.ts index e5b60c939..557d84329 100644 --- a/src/editorSettings/convertEditorSetting.test.ts +++ b/src/editorSettings/convertEditorSetting.test.ts @@ -2,6 +2,7 @@ import { ConversionError } from "../errors/conversionError"; import { convertEditorSetting } from "./convertEditorSetting"; import { EditorSettingConverter } from "./converter"; import { EditorSetting } from "./types"; +import { createStubTSLintToESLintSettings } from "../settings.stubs"; describe("convertEditorSetting", () => { it("returns undefined when no converter exists for a setting", () => { @@ -15,6 +16,7 @@ describe("convertEditorSetting", () => { value: "any value", }, converters, + createStubTSLintToESLintSettings(), ); // Assert @@ -42,6 +44,7 @@ describe("convertEditorSetting", () => { value: "existing value", }, converters, + createStubTSLintToESLintSettings(), ); // Assert @@ -65,7 +68,11 @@ describe("convertEditorSetting", () => { }; // Act - const result = convertEditorSetting(tsLintSetting, converters); + const result = convertEditorSetting( + tsLintSetting, + converters, + createStubTSLintToESLintSettings(), + ); // Assert expect(result).toEqual(ConversionError.forSettingError(error, tsLintSetting)); diff --git a/src/editorSettings/convertEditorSetting.ts b/src/editorSettings/convertEditorSetting.ts index 0843a51c9..8ceb0c02f 100644 --- a/src/editorSettings/convertEditorSetting.ts +++ b/src/editorSettings/convertEditorSetting.ts @@ -1,10 +1,12 @@ import { ConversionError } from "../errors/conversionError"; +import { TSLintToESLintSettings } from "../types"; import { EditorSettingConverter } from "./converter"; import { EditorSetting } from "./types"; export const convertEditorSetting = ( editorSetting: EditorSetting, converters: Map, + settings: TSLintToESLintSettings, ) => { const converter = converters.get(editorSetting.editorSettingName); if (converter === undefined) { @@ -12,7 +14,7 @@ export const convertEditorSetting = ( } try { - return converter(editorSetting); + return converter(editorSetting, settings); } catch (error) { return ConversionError.forSettingError(error, editorSetting); } diff --git a/src/editorSettings/convertEditorSettings.test.ts b/src/editorSettings/convertEditorSettings.test.ts index 1c4a0b2d2..9f9b4d05b 100644 --- a/src/editorSettings/convertEditorSettings.test.ts +++ b/src/editorSettings/convertEditorSettings.test.ts @@ -1,4 +1,5 @@ import { ConversionError } from "../errors/conversionError"; +import { createStubTSLintToESLintSettings } from "../settings.stubs"; import { convertEditorSettings } from "./convertEditorSettings"; import { EditorSettingConversionResult, EditorSettingConverter } from "./converter"; import { EditorSetting } from "./types"; @@ -13,7 +14,11 @@ describe("convertEditorSettings", () => { }; // Act - const result = convertEditorSettings({ converters }, editorConfiguration); + const result = convertEditorSettings( + { converters }, + editorConfiguration, + createStubTSLintToESLintSettings(), + ); // Assert expect(result).toEqual({ @@ -41,7 +46,11 @@ describe("convertEditorSettings", () => { }; // Act - const result = convertEditorSettings({ converters }, editorConfiguration); + const result = convertEditorSettings( + { converters }, + editorConfiguration, + createStubTSLintToESLintSettings(), + ); // Assert expect(result).toEqual({ @@ -67,6 +76,7 @@ describe("convertEditorSettings", () => { const result = convertEditorSettings( { converters }, { [editorSetting.editorSettingName]: editorSetting }, + createStubTSLintToESLintSettings(), ); // Assert @@ -87,6 +97,7 @@ describe("convertEditorSettings", () => { const result = convertEditorSettings( { converters }, { [editorSetting.editorSettingName]: editorSetting }, + createStubTSLintToESLintSettings(), ); // Assert @@ -112,6 +123,7 @@ describe("convertEditorSettings", () => { const result = convertEditorSettings( { converters }, { [editorSetting.editorSettingName]: editorSetting.value }, + createStubTSLintToESLintSettings(), ); // Assert diff --git a/src/editorSettings/convertEditorSettings.ts b/src/editorSettings/convertEditorSettings.ts index 043b76fad..129aa8d2b 100644 --- a/src/editorSettings/convertEditorSettings.ts +++ b/src/editorSettings/convertEditorSettings.ts @@ -1,5 +1,6 @@ import { ConversionError } from "../errors/conversionError"; import { ErrorSummary } from "../errors/errorSummary"; +import { TSLintToESLintSettings } from "../types"; import { convertEditorSetting } from "./convertEditorSetting"; import { EditorSettingConverter } from "./converter"; import { EditorSetting } from "./types"; @@ -29,6 +30,7 @@ export type EditorConfiguration = Record; export const convertEditorSettings = ( dependencies: ConvertEditorSettingsDependencies, rawEditorConfiguration: EditorConfiguration, + settings: TSLintToESLintSettings, ): EditorSettingConversionResults => { const converted = new Map(); const failed: ConversionError[] = []; @@ -42,7 +44,7 @@ export const convertEditorSettings = ( } const editorSetting = { editorSettingName: configurationName, value }; - const conversion = convertEditorSetting(editorSetting, dependencies.converters); + const conversion = convertEditorSetting(editorSetting, dependencies.converters, settings); if (conversion === undefined) { const { editorSettingName } = editorSetting; diff --git a/src/editorSettings/converter.ts b/src/editorSettings/converter.ts index b3121aa03..441208e51 100644 --- a/src/editorSettings/converter.ts +++ b/src/editorSettings/converter.ts @@ -1,4 +1,5 @@ import { ConversionError } from "../errors/conversionError"; +import { TSLintToESLintSettings } from "../types"; import { EditorSetting } from "./types"; /** @@ -6,7 +7,8 @@ import { EditorSetting } from "./types"; */ export type EditorSettingConverter = ( tslintEditorSetting: EditorSetting, -) => ConversionError | EditorSettingConversionResult; + settings: TSLintToESLintSettings, +) => ConversionError | EditorSettingConversionResult | undefined; /** * Successful result from converting a TSLint editor setting to its ESLint equivalents. diff --git a/src/editorSettings/converters/tests/editor-code-actions-on-save.test.ts b/src/editorSettings/converters/tests/editor-code-actions-on-save.test.ts index 425de1a6e..dd7fa0c60 100644 --- a/src/editorSettings/converters/tests/editor-code-actions-on-save.test.ts +++ b/src/editorSettings/converters/tests/editor-code-actions-on-save.test.ts @@ -1,13 +1,17 @@ +import { createStubTSLintToESLintSettings } from "../../../settings.stubs"; import { convertEditorCodeActionsOnSave } from "../editor-code-actions-on-save"; describe(convertEditorCodeActionsOnSave, () => { test("conversion of 'source.fixAll.tslint' when value is true", () => { - const result = convertEditorCodeActionsOnSave({ - editorSettingName: "editor.codeActionsOnSave", - value: { - "source.fixAll.tslint": true, + const result = convertEditorCodeActionsOnSave( + { + editorSettingName: "editor.codeActionsOnSave", + value: { + "source.fixAll.tslint": true, + }, }, - }); + createStubTSLintToESLintSettings(), + ); expect(result).toEqual({ settings: [ @@ -24,12 +28,15 @@ describe(convertEditorCodeActionsOnSave, () => { }); test("conversion of 'source.fixAll.tslint' when value is false", () => { - const result = convertEditorCodeActionsOnSave({ - editorSettingName: "editor.codeActionsOnSave", - value: { - "source.fixAll.tslint": false, + const result = convertEditorCodeActionsOnSave( + { + editorSettingName: "editor.codeActionsOnSave", + value: { + "source.fixAll.tslint": false, + }, }, - }); + createStubTSLintToESLintSettings(), + ); expect(result).toEqual({ settings: [ @@ -46,14 +53,17 @@ describe(convertEditorCodeActionsOnSave, () => { }); test("conversion of 'source.fixAll.tslint' without touching any other 'editor.codeActionsOnSave'", () => { - const result = convertEditorCodeActionsOnSave({ - editorSettingName: "editor.codeActionsOnSave", - value: { - "one-property": 42, - "source.fixAll.tslint": true, - "another-property": "foo", + const result = convertEditorCodeActionsOnSave( + { + editorSettingName: "editor.codeActionsOnSave", + value: { + "one-property": 42, + "source.fixAll.tslint": true, + "another-property": "foo", + }, }, - }); + createStubTSLintToESLintSettings(), + ); expect(result).toEqual({ settings: [ diff --git a/src/editorSettings/converters/tests/tslint-config-file.test.ts b/src/editorSettings/converters/tests/tslint-config-file.test.ts new file mode 100644 index 000000000..9bb3317ff --- /dev/null +++ b/src/editorSettings/converters/tests/tslint-config-file.test.ts @@ -0,0 +1,34 @@ +import { convertTSLintConfigFile } from "../tslint-config-file"; + +describe(convertTSLintConfigFile, () => { + test("conversion of 'tslint.configFile' when it roughly equals the ESLint file", () => { + const result = convertTSLintConfigFile( + { + editorSettingName: "tslint.configFile", + value: "./custom/tslint.json", + }, + { config: "./custom/eslintrc.js" }, + ); + + expect(result).toEqual({ + settings: [ + { + editorSettingName: "eslint.options", + value: { configFile: "./custom/eslintrc.js" }, + }, + ], + }); + }); + + test("conversion of 'tslint.configFile' when it does not roughly equal the ESLint file", () => { + const result = convertTSLintConfigFile( + { + editorSettingName: "tslint.configFile", + value: "./custom/tslint.json", + }, + { config: "./eslintrc.js" }, + ); + + expect(result).toEqual(undefined); + }); +}); diff --git a/src/editorSettings/converters/tslint-config-file.ts b/src/editorSettings/converters/tslint-config-file.ts new file mode 100644 index 000000000..53b8c3c65 --- /dev/null +++ b/src/editorSettings/converters/tslint-config-file.ts @@ -0,0 +1,24 @@ +import * as path from "path"; + +import { EditorSettingConverter } from "../converter"; + +export const convertTSLintConfigFile: EditorSettingConverter = ( + originalTSLintConfigFile, + settings, +) => { + // If the output ESLint config path doesn't roughly match the original TSLint path, skip this. + const tslintPath = originalTSLintConfigFile.value; + const eslintPath = settings.config; + if (path.relative(path.dirname(tslintPath), path.dirname(eslintPath))) { + return undefined; + } + + return { + settings: [ + { + editorSettingName: "eslint.options", + value: { configFile: settings.config }, + }, + ], + }; +}; diff --git a/src/editorSettings/editorSettingsConverters.ts b/src/editorSettings/editorSettingsConverters.ts index ec36c0e3a..c49181025 100644 --- a/src/editorSettings/editorSettingsConverters.ts +++ b/src/editorSettings/editorSettingsConverters.ts @@ -1,8 +1,10 @@ import { convertEditorCodeActionsOnSave } from "./converters/editor-code-actions-on-save"; +import { convertTSLintConfigFile } from "./converters/tslint-config-file"; /** * Keys TSLint property names in editor settings to their ESLint editor settings converters. */ export const editorSettingsConverters = new Map([ ["editor.codeActionsOnSave", convertEditorCodeActionsOnSave], + ["tslint.configFile", convertTSLintConfigFile], ]); diff --git a/src/settings.stubs.ts b/src/settings.stubs.ts new file mode 100644 index 000000000..f2dcc5339 --- /dev/null +++ b/src/settings.stubs.ts @@ -0,0 +1,3 @@ +export const createStubTSLintToESLintSettings = () => ({ + config: "./eslintrc.js", +}); From d72f2b8afec096ce236fd8fcaea3e56ff625b34f Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sun, 9 Aug 2020 13:23:28 -0400 Subject: [PATCH 3/4] Unit test fix --- src/conversion/convertEditorConfig.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/conversion/convertEditorConfig.test.ts b/src/conversion/convertEditorConfig.test.ts index 08572cc65..4eee87675 100644 --- a/src/conversion/convertEditorConfig.test.ts +++ b/src/conversion/convertEditorConfig.test.ts @@ -92,7 +92,7 @@ describe("convertEditorConfig", () => { // Assert expect(dependencies.convertEditorSettings).toHaveBeenCalledWith( originalConfig, - createStubTSLintToESLintSettings(), + stubSettings, ); }); From 56f2d13b4f244392ce2bdb019a7a4be23ea7bef8 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sun, 9 Aug 2020 13:30:32 -0400 Subject: [PATCH 4/4] unused variable --- src/conversion/convertEditorConfig.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/conversion/convertEditorConfig.test.ts b/src/conversion/convertEditorConfig.test.ts index 4eee87675..9ba0859a8 100644 --- a/src/conversion/convertEditorConfig.test.ts +++ b/src/conversion/convertEditorConfig.test.ts @@ -1,5 +1,4 @@ import { EditorSetting } from "../editorSettings/types"; -import { createStubTSLintToESLintSettings } from "../settings.stubs"; import { FailedResult, ResultStatus } from "../types"; import { createEmptySettingConversionResults } from "./conversionResults.stubs"; import { convertEditorConfig, ConvertEditorConfigDependencies } from "./convertEditorConfig";