From 8b84a3de209224d2bbc5ddef1c68c4b5ca3a1cf7 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Thu, 1 Oct 2020 23:48:38 -0400 Subject: [PATCH 1/4] WIP: Atom editor conversion --- package-lock.json | 13 ++ package.json | 1 + src/cli/main.ts | 53 +++--- .../editorConfigs/convertEditorConfig.test.ts | 137 -------------- .../editorConfigs/convertEditorConfig.ts | 73 ++------ .../editorConfigs/convertEditorConfigs.ts | 57 ++++++ .../convertEditorSetting.test.ts | 80 -------- .../editorConfigs/convertEditorSetting.ts | 21 --- .../convertEditorSettings.test.ts | 171 ------------------ .../editorConfigs/convertEditorSettings.ts | 66 ------- src/converters/editorConfigs/converter.ts | 36 ---- .../converters/convertAtomConfig.ts | 16 ++ .../converters/convertVSCodeConfig.ts | 31 ++++ .../converters/editor-code-actions-on-save.ts | 26 --- .../tests/editor-code-actions-on-save.test.ts | 84 --------- .../tests/tslint-config-file.test.ts | 34 ---- .../converters/tslint-config-file.ts | 24 --- .../editorConversionResults.stubs.ts | 10 - .../editorConfigs/editorSettingsConverters.ts | 10 - ...portEditorConfigConversionResults.test.ts} | 16 +- ...=> reportEditorConfigConversionResults.ts} | 12 +- src/converters/editorConfigs/types.ts | 12 +- src/input/importer.ts | 4 +- src/types.ts | 4 +- src/utils.ts | 4 + 25 files changed, 189 insertions(+), 806 deletions(-) delete mode 100644 src/converters/editorConfigs/convertEditorConfig.test.ts create mode 100644 src/converters/editorConfigs/convertEditorConfigs.ts delete mode 100644 src/converters/editorConfigs/convertEditorSetting.test.ts delete mode 100644 src/converters/editorConfigs/convertEditorSetting.ts delete mode 100644 src/converters/editorConfigs/convertEditorSettings.test.ts delete mode 100644 src/converters/editorConfigs/convertEditorSettings.ts delete mode 100644 src/converters/editorConfigs/converter.ts create mode 100644 src/converters/editorConfigs/converters/convertAtomConfig.ts create mode 100644 src/converters/editorConfigs/converters/convertVSCodeConfig.ts delete mode 100644 src/converters/editorConfigs/converters/editor-code-actions-on-save.ts delete mode 100644 src/converters/editorConfigs/converters/tests/editor-code-actions-on-save.test.ts delete mode 100644 src/converters/editorConfigs/converters/tests/tslint-config-file.test.ts delete mode 100644 src/converters/editorConfigs/converters/tslint-config-file.ts delete mode 100644 src/converters/editorConfigs/editorConversionResults.stubs.ts delete mode 100644 src/converters/editorConfigs/editorSettingsConverters.ts rename src/converters/editorConfigs/reporting/{reportEditorSettingConversionResults.test.ts => reportEditorConfigConversionResults.test.ts} (89%) rename src/converters/editorConfigs/reporting/{reportEditorSettingConversionResults.ts => reportEditorConfigConversionResults.ts} (74%) diff --git a/package-lock.json b/package-lock.json index 4e2742ddf..5140a8f8a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -3775,6 +3775,11 @@ "integrity": "sha1-bqa989hTrlTMuOR7+gvz+QMfsYQ=", "dev": true }, + "coffeescript": { + "version": "1.12.7", + "resolved": "https://registry.npmjs.org/coffeescript/-/coffeescript-1.12.7.tgz", + "integrity": "sha512-pLXHFxQMPklVoEekowk8b3erNynC+DVJzChxS/LCBBgR6/8AJkHivkm//zbowcfc7BTCAjryuhx6gPqPRfsFoA==" + }, "collect-v8-coverage": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/collect-v8-coverage/-/collect-v8-coverage-1.0.1.tgz", @@ -3911,6 +3916,14 @@ } } }, + "cson-parser": { + "version": "4.0.5", + "resolved": "https://registry.npmjs.org/cson-parser/-/cson-parser-4.0.5.tgz", + "integrity": "sha512-XgloWiJcHy3TeuonPJseMfZOuwDpfczIQ12xw+DS2D8TCFxbj61gVlu8Cfs6lOwucQlrJFCvBTiovBqYMwPfQw==", + "requires": { + "coffeescript": "^1.10.0" + } + }, "cssom": { "version": "0.4.4", "resolved": "https://registry.npmjs.org/cssom/-/cssom-0.4.4.tgz", diff --git a/package.json b/package.json index f9ff9dab9..2347d3960 100644 --- a/package.json +++ b/package.json @@ -12,6 +12,7 @@ "dependencies": { "chalk": "4.1.0", "commander": "6.1.0", + "cson-parser": "^4.0.5", "eslint-config-prettier": "6.12.0", "glob": "7.1.6", "minimatch": "3.0.4", diff --git a/src/cli/main.ts b/src/cli/main.ts index b925dc656..35a1895fd 100644 --- a/src/cli/main.ts +++ b/src/cli/main.ts @@ -14,15 +14,7 @@ import { ReportCommentResultsDependencies, reportCommentResults, } from "../converters/comments/reporting/reportCommentResults"; -import { - ConvertEditorConfigDependencies, - convertEditorConfig, -} from "../converters/editorConfigs/convertEditorConfig"; -import { - ConvertEditorSettingsDependencies, - convertEditorSettings, -} from "../converters/editorConfigs/convertEditorSettings"; -import { editorSettingsConverters } from "../converters/editorConfigs/editorSettingsConverters"; +import { convertEditorConfig } from "../converters/editorConfigs/convertEditorConfig"; import { reportEditorSettingConversionResults } from "../converters/editorConfigs/reporting/reportEditorSettingConversionResults"; import { ConvertLintConfigDependencies, @@ -67,17 +59,12 @@ import { } from "../converters/lintConfigs/reporting/packages/logMissingPackages"; import { runCli, RunCliDependencies } from "./runCli"; import { ruleMergers } from "../converters/lintConfigs/rules/ruleMergers"; -import { writeEditorConfigConversionResults } from "../converters/lintConfigs/writeEditorConfigConversionResults"; import { addPrettierExtensions } from "../converters/lintConfigs/summarization/prettier/addPrettierExtensions"; import { removeExtendsDuplicatedRules } from "../converters/lintConfigs/pruning/removeExtendsDuplicatedRules"; import { ExtractGlobPathsDependencies, extractGlobPaths, } from "../converters/comments/extractGlobPaths"; -import { - findEditorConfiguration, - FindEditorConfigurationDependencies, -} from "../input/findEditorConfiguration"; import { findESLintConfiguration } from "../input/findESLintConfiguration"; import { findOriginalConfigurations, @@ -88,6 +75,13 @@ import { findTSLintConfiguration } from "../input/findTSLintConfiguration"; import { findTypeScriptConfiguration } from "../input/findTypeScriptConfiguration"; import { importer, ImporterDependencies } from "../input/importer"; import { mergeLintConfigurations } from "../input/mergeLintConfigurations"; +import { convertVSCodeConfig } from "../converters/editorConfigs/converters/convertVSCodeConfig"; +import { convertAtomConfig } from "../converters/editorConfigs/converters/convertAtomConfig"; +import { EditorConfigDescriptor } from "../converters/editorConfigs/types"; +import { + convertEditorConfigs, + ConvertEditorConfigsDependencies, +} from "../converters/editorConfigs/convertEditorConfigs"; const convertFileCommentsDependencies: ConvertFileCommentsDependencies = { converters: ruleConverters, @@ -103,10 +97,6 @@ const convertRulesDependencies: ConvertRulesDependencies = { ruleMergers, }; -const convertEditorSettingsDependencies: ConvertEditorSettingsDependencies = { - converters: editorSettingsConverters, -}; - const nativeImporterDependencies: ImporterDependencies = { fileSystem: fsFileSystem, getCwd: () => process.cwd(), @@ -120,11 +110,6 @@ const findConfigurationDependencies = { importer: boundImporter, }; -const findEditorConfigurationDependencies: FindEditorConfigurationDependencies = { - fileSystem: fsFileSystem, - importer: boundImporter, -}; - const findOriginalConfigurationsDependencies: FindOriginalConfigurationsDependencies = { findESLintConfiguration: bind(findESLintConfiguration, findConfigurationDependencies), findPackagesConfiguration: bind(findPackagesConfiguration, findConfigurationDependencies), @@ -179,17 +164,23 @@ const reportEditorSettingConversionResultsDependencies = { logger: processLogger, }; -const convertEditorConfigDependencies: ConvertEditorConfigDependencies = { - findEditorConfiguration: bind(findEditorConfiguration, findEditorConfigurationDependencies), - convertEditorSettings: bind(convertEditorSettings, convertEditorSettingsDependencies), +const editorConfigDescriptors: EditorConfigDescriptor[] = [ + [".atom/config.cson", convertAtomConfig], + [".vscode/settings.json", convertVSCodeConfig], +]; + +const convertEditorConfigDependencies = { + editorConfigDescriptors, + fileSystem: fsFileSystem, +}; + +const convertEditorConfigsDependencies: ConvertEditorConfigsDependencies = { + convertEditorConfig: bind(convertEditorConfig, convertEditorConfigDependencies), + editorConfigDescriptors, reportEditorSettingConversionResults: bind( reportEditorSettingConversionResults, reportEditorSettingConversionResultsDependencies, ), - writeEditorConfigConversionResults: bind( - writeEditorConfigConversionResults, - writeConversionResultsDependencies, - ), }; const convertLintConfigDependencies: ConvertLintConfigDependencies = { @@ -209,7 +200,7 @@ const convertLintConfigDependencies: ConvertLintConfigDependencies = { const runCliDependencies: RunCliDependencies = { converters: [ bind(convertLintConfig, convertLintConfigDependencies), - bind(convertEditorConfig, convertEditorConfigDependencies), + bind(convertEditorConfigs, convertEditorConfigsDependencies), bind(convertComments, convertCommentsDependencies), ], findOriginalConfigurations: bind( diff --git a/src/converters/editorConfigs/convertEditorConfig.test.ts b/src/converters/editorConfigs/convertEditorConfig.test.ts deleted file mode 100644 index 2b2e926b4..000000000 --- a/src/converters/editorConfigs/convertEditorConfig.test.ts +++ /dev/null @@ -1,137 +0,0 @@ -import { ResultStatus, FailedResult } from "../../types"; -import { convertEditorConfig, ConvertEditorConfigDependencies } from "./convertEditorConfig"; -import { createEmptyEditorSettingConversionResults } from "./editorConversionResults.stubs"; -import { EditorSetting } from "./types"; - -const stubSettings = { - config: "./eslintrc.js", - editor: "./my-editor/settings.json", -}; - -const createStubDependencies = ( - overrides: Partial = {}, -): ConvertEditorConfigDependencies => ({ - convertEditorSettings: jest.fn(), - findEditorConfiguration: jest.fn().mockResolvedValue({}), - reportEditorSettingConversionResults: jest.fn(), - writeEditorConfigConversionResults: jest.fn().mockReturnValue(Promise.resolve()), - ...overrides, -}); - -describe("convertEditorConfig", () => { - it("returns a success result when there is no original configuration", async () => { - // Arrange - const dependencies = createStubDependencies({ - findEditorConfiguration: async () => undefined, - }); - - // Act - const result = await convertEditorConfig(dependencies, stubSettings); - - // Assert - expect(result).toEqual({ - status: ResultStatus.Succeeded, - }); - }); - - it("returns the failure result when finding the original configurations fails", async () => { - // Arrange - const error = new Error(); - const findError: FailedResult = { - errors: [error], - status: ResultStatus.Failed, - }; - - const dependencies = createStubDependencies({ - findEditorConfiguration: async () => ({ - configPath: "", - result: error, - }), - }); - - // Act - const result = await convertEditorConfig(dependencies, stubSettings); - - // Assert - expect(result).toEqual(findError); - }); - - it("returns the failure result when writing to the configuration file fails", async () => { - // Arrange - const fileWriteError = new Error(); - const dependencies = createStubDependencies({ - writeEditorConfigConversionResults: jest.fn().mockResolvedValueOnce(fileWriteError), - }); - - // Act - const result = await convertEditorConfig(dependencies, stubSettings); - - // Assert - expect(result).toEqual({ - errors: [fileWriteError], - status: ResultStatus.Failed, - }); - }); - - it("converts conversion results when finding the original configurations succeeds", async () => { - // Arrange - const originalConfig = { - "typescript.tsdk": "node_modules/typescript/lib", - }; - - const dependencies = createStubDependencies({ - findEditorConfiguration: jest.fn().mockResolvedValue({ - result: originalConfig, - }), - }); - - // Act - await convertEditorConfig(dependencies, stubSettings); - - // Assert - expect(dependencies.convertEditorSettings).toHaveBeenCalledWith( - originalConfig, - stubSettings, - ); - }); - - it("reports conversion results when settings are converted successfully", async () => { - // Arrange - const conversionResults = createEmptyEditorSettingConversionResults({ - converted: new Map([ - [ - "tslint-editor-setting-one", - { - editorSettingName: "tslint-editor-setting-one", - value: 42, - }, - ], - ]), - }); - - const dependencies = createStubDependencies({ - convertEditorSettings: jest.fn().mockReturnValue(conversionResults), - }); - - // Act - await convertEditorConfig(dependencies, stubSettings); - - // Assert - expect(dependencies.reportEditorSettingConversionResults).toHaveBeenCalledWith( - conversionResults, - ); - }); - - it("returns a successful result when finding the original configurations succeeds", async () => { - // Arrange - const dependencies = createStubDependencies(); - - // Act - const result = await convertEditorConfig(dependencies, stubSettings); - - // Assert - expect(result).toEqual({ - status: ResultStatus.Succeeded, - }); - }); -}); diff --git a/src/converters/editorConfigs/convertEditorConfig.ts b/src/converters/editorConfigs/convertEditorConfig.ts index c490222bb..dc02bd6b7 100644 --- a/src/converters/editorConfigs/convertEditorConfig.ts +++ b/src/converters/editorConfigs/convertEditorConfig.ts @@ -1,66 +1,31 @@ -import { SansDependencies } from "../../binding"; -import { findEditorConfiguration } from "../../input/findEditorConfiguration"; -import { TSLintToESLintSettings, ResultWithStatus, ResultStatus } from "../../types"; -import { writeEditorConfigConversionResults } from "../lintConfigs/writeEditorConfigConversionResults"; -import { convertEditorSettings } from "./convertEditorSettings"; -import { reportEditorSettingConversionResults } from "./reporting/reportEditorSettingConversionResults"; +import { FileSystem } from "../../adapters/fileSystem"; +import { TSLintToESLintSettings } from "../../types"; +import { EditorConfigDescriptor } from "./types"; export type ConvertEditorConfigDependencies = { - convertEditorSettings: SansDependencies; - findEditorConfiguration: SansDependencies; - reportEditorSettingConversionResults: SansDependencies< - typeof reportEditorSettingConversionResults - >; - writeEditorConfigConversionResults: SansDependencies; + editorConfigDescriptors: EditorConfigDescriptor[]; + fileSystem: Pick; }; -/** - * Root-level driver to convert an editor configuration. - * @see `/docs/Architecture/Editors.md` for documentation. - */ export const convertEditorConfig = async ( dependencies: ConvertEditorConfigDependencies, + requestedPath: string, settings: TSLintToESLintSettings, -): Promise => { - // 1. An existing editor configuration is read from disk. - const configuration = await dependencies.findEditorConfiguration(settings.editor); - - // 2. If the existing configuration is not found or errored, nothing else needs to be done. - if (configuration === undefined) { - return { - status: ResultStatus.Succeeded, - }; - } - if (configuration.result instanceof Error) { - return { - errors: [configuration.result], - status: ResultStatus.Failed, - }; - } - - // 3. Configuration settings are converted to their ESLint equivalents. - const settingConversionResults = dependencies.convertEditorSettings( - configuration.result, - settings, +) => { + const editorConfigDescriptor = dependencies.editorConfigDescriptors.find(([defaultPath]) => + requestedPathMatchesDefault(defaultPath, requestedPath), ); - - // 4. Those ESLint equivalents are written to the configuration file. - const fileWriteError = await dependencies.writeEditorConfigConversionResults( - configuration.configPath, - settingConversionResults, - configuration.result, - ); - if (fileWriteError !== undefined) { - return { - errors: [fileWriteError], - status: ResultStatus.Failed, - }; + if (!editorConfigDescriptor) { + return new Error(`Could not find a matching editor config for '${requestedPath}'.`); } - // 5. Results from converting are reported to the user. - dependencies.reportEditorSettingConversionResults(settingConversionResults); + const fileContents = await dependencies.fileSystem.readFile(requestedPath); + if (fileContents instanceof Error) { + return fileContents; + } - return { - status: ResultStatus.Succeeded, - }; + return editorConfigDescriptor[1](fileContents, settings); }; + +const requestedPathMatchesDefault = (defaultPath: string, requestedPath: string) => + defaultPath.replace(/\W+/g, "") === requestedPath.replace(/\W+/g, ""); diff --git a/src/converters/editorConfigs/convertEditorConfigs.ts b/src/converters/editorConfigs/convertEditorConfigs.ts new file mode 100644 index 000000000..a98557a8b --- /dev/null +++ b/src/converters/editorConfigs/convertEditorConfigs.ts @@ -0,0 +1,57 @@ +import { SansDependencies } from "../../binding"; +import { TSLintToESLintSettings } from "../../types"; +import { uniqueFromSources } from "../../utils"; +import { convertEditorConfig } from "./convertEditorConfig"; +import { reportEditorConfigConversionResults } from "./reporting/reportEditorConfigConversionResults"; +import { EditorConfigDescriptor } from "./types"; + +export type ConvertEditorConfigsDependencies = { + convertEditorConfig: SansDependencies; + editorConfigDescriptors: EditorConfigDescriptor[]; + reportEditorConfigConversionResults: SansDependencies< + typeof reportEditorConfigConversionResults + >; +}; + +export type EditorSettingConversionResults = { + errors: string[]; + successes: string[]; +}; + +export const convertEditorConfigs = async ( + dependencies: ConvertEditorConfigsDependencies, + settings: TSLintToESLintSettings, +) => { + const errors: Error[] = []; + const successes: string[] = []; + const requestedPaths = uniqueFromSources(settings.editor); + + await Promise.all( + requestedPaths.map(async (requestedPath) => { + const result = await dependencies.convertEditorConfig(requestedPath, settings); + + if (result) { + errors.push(result); + } else { + successes.push(requestedPath); + } + }), + ); + + await Promise.all( + dependencies.editorConfigDescriptors + .filter( + ([defaultPath]) => + !successes.some((success) => defaultPathMatches(defaultPath, success)), + ) + .map( + async ([defaultPath, converter]) => + await dependencies.convertEditorConfig(defaultPath, settings, converter), + ), + ); + + return { errors, successes }; +}; + +const defaultPathMatches = (defaultPath: string, otherPath: string) => + defaultPath.replace(/\W+/g, "") === otherPath.replace(/\W+/g, ""); diff --git a/src/converters/editorConfigs/convertEditorSetting.test.ts b/src/converters/editorConfigs/convertEditorSetting.test.ts deleted file mode 100644 index cdd42e0ba..000000000 --- a/src/converters/editorConfigs/convertEditorSetting.test.ts +++ /dev/null @@ -1,80 +0,0 @@ -import { ConversionError } from "../../errors/conversionError"; -import { createStubTSLintToESLintSettings } from "../../settings.stubs"; -import { convertEditorSetting } from "./convertEditorSetting"; -import { EditorSettingConverter } from "./converter"; -import { EditorSetting } from "./types"; - -describe("convertEditorSetting", () => { - it("returns undefined when no converter exists for a setting", () => { - // Arrange - const converters = new Map(); - - // Act - const result = convertEditorSetting( - { - editorSettingName: "tslint-editor-setting", - value: "any value", - }, - converters, - createStubTSLintToESLintSettings(), - ); - - // Assert - expect(result).toEqual(undefined); - }); - - it("returns converter results when the converter does not throw an error", () => { - // Arrange - const converted = { - settings: [ - { - editorSettingName: "eslint-setting", - value: "new value", - }, - ], - }; - const converters = new Map([ - ["tslint-editor-setting", () => converted], - ]); - - // Act - const result = convertEditorSetting( - { - editorSettingName: "tslint-editor-setting", - value: "existing value", - }, - converters, - createStubTSLintToESLintSettings(), - ); - - // Assert - expect(result).toEqual(converted); - }); - - it("returns a conversion error when the converter throws an error", () => { - // Arrange - const error = new Error("oh no"); - const converters = new Map([ - [ - "tslint-editor-setting", - () => { - throw error; - }, - ], - ]); - const tsLintSetting: EditorSetting = { - editorSettingName: "tslint-editor-setting", - value: "existing value", - }; - - // Act - const result = convertEditorSetting( - tsLintSetting, - converters, - createStubTSLintToESLintSettings(), - ); - - // Assert - expect(result).toEqual(ConversionError.forSettingError(error, tsLintSetting)); - }); -}); diff --git a/src/converters/editorConfigs/convertEditorSetting.ts b/src/converters/editorConfigs/convertEditorSetting.ts deleted file mode 100644 index 66a4cd52f..000000000 --- a/src/converters/editorConfigs/convertEditorSetting.ts +++ /dev/null @@ -1,21 +0,0 @@ -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) { - return undefined; - } - - try { - return converter(editorSetting, settings); - } catch (error) { - return ConversionError.forSettingError(error, editorSetting); - } -}; diff --git a/src/converters/editorConfigs/convertEditorSettings.test.ts b/src/converters/editorConfigs/convertEditorSettings.test.ts deleted file mode 100644 index fd1a9bf0a..000000000 --- a/src/converters/editorConfigs/convertEditorSettings.test.ts +++ /dev/null @@ -1,171 +0,0 @@ -import { ConversionError } from "../../errors/conversionError"; -import { createStubTSLintToESLintSettings } from "../../settings.stubs"; -import { convertEditorSettings } from "./convertEditorSettings"; -import { EditorSettingConversionResult, EditorSettingConverter } from "./converter"; -import { EditorSetting } from "./types"; - -describe("convertEditorSettings", () => { - it("skips entire conversion if none of the configurations is an editor setting", () => { - // Arrange - const { converters } = setupConversionEnvironment(); - - const editorConfiguration = { - notAnEditorSetting: "a", - }; - - // Act - const result = convertEditorSettings( - { converters }, - editorConfiguration, - createStubTSLintToESLintSettings(), - ); - - // Assert - expect(result).toEqual({ - converted: new Map(), - failed: [], - missing: [], - }); - }); - - it("skips a configuration if not an editor setting", () => { - // Arrange - const { editorSetting, converters } = setupConversionEnvironment({ - settings: [ - { - editorSettingName: "editor.eslint-setting-a", - value: "a", - }, - ], - }); - - const editorConfiguration = { - notAnEditorSetting: "a", - [editorSetting.editorSettingName]: editorSetting, - notAnEditorSettingEither: "b", - }; - - // Act - const result = convertEditorSettings( - { converters }, - editorConfiguration, - createStubTSLintToESLintSettings(), - ); - - // Assert - 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", () => { - // Arrange - const { editorSetting, converters } = setupConversionEnvironment(); - - // Act - const result = convertEditorSettings( - { converters }, - { [editorSetting.editorSettingName]: editorSetting }, - createStubTSLintToESLintSettings(), - ); - - // Assert - expect(result).toEqual({ - converted: new Map(), - failed: [], - missing: [{ editorSettingName: editorSetting.editorSettingName }], - }); - }); - - it("marks a conversion as failed when returned a conversion error", () => { - // Arrange - const { editorSetting, converters } = setupConversionEnvironment(); - const conversionError = ConversionError.forSettingError(new Error(), editorSetting); - converters.set(editorSetting.editorSettingName, () => conversionError); - - // Act - const result = convertEditorSettings( - { converters }, - { [editorSetting.editorSettingName]: editorSetting }, - createStubTSLintToESLintSettings(), - ); - - // Assert - expect(result).toEqual({ - converted: new Map(), - failed: [conversionError], - missing: [], - }); - }); - - it("marks a converted setting name as converted when a conversion has settings", () => { - // Arrange - const { editorSetting, converters } = setupConversionEnvironment({ - settings: [ - { - editorSettingName: "eslint.configFile", - value: "a", - }, - ], - }); - - // Act - const result = convertEditorSettings( - { converters }, - { [editorSetting.editorSettingName]: editorSetting.value }, - createStubTSLintToESLintSettings(), - ); - - // Assert - expect(result).toEqual({ - converted: new Map([ - [ - "eslint.configFile", - { - editorSettingName: "eslint.configFile", - value: "a", - }, - ], - ]), - failed: [], - missing: [], - }); - }); -}); - -function setupConversionEnvironment(conversionResult?: EditorSettingConversionResult) { - const editorSetting = createSampleEditorSetting(); - const converters = createConverters(editorSetting, conversionResult); - - return { editorSetting, converters }; -} - -function createSampleEditorSetting(): EditorSetting { - return { - editorSettingName: "tslint.configFile", - value: "a", - }; -} - -function createConverters( - tslintSetting: EditorSetting, - conversionResult?: EditorSettingConversionResult, -): Map { - const converters = new Map(); - - if (conversionResult !== undefined) { - converters.set(tslintSetting.editorSettingName, () => conversionResult); - } - - return converters; -} diff --git a/src/converters/editorConfigs/convertEditorSettings.ts b/src/converters/editorConfigs/convertEditorSettings.ts deleted file mode 100644 index 9cf4cc25e..000000000 --- a/src/converters/editorConfigs/convertEditorSettings.ts +++ /dev/null @@ -1,66 +0,0 @@ -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"; - -const knownEditorSettings = new Set([ - "tslint.configFile", - "tslint.jsEnable", - "tslint.ignoreDefinitionFiles", - "tslint.exclude", - "tslint.alwaysShowRuleFailuresAsWarnings", - "tslint.suppressWhileTypeErrorsPresent", -]); - -export type ConvertEditorSettingsDependencies = { - converters: Map; -}; - -export type EditorSettingConversionResults = { - converted: Map; - failed: ErrorSummary[]; - missing: Pick[]; -}; - -// The entire editor configuration of any keys and values. -export type EditorConfiguration = Record; - -export const convertEditorSettings = ( - dependencies: ConvertEditorSettingsDependencies, - rawEditorConfiguration: EditorConfiguration, - settings: TSLintToESLintSettings, -): EditorSettingConversionResults => { - const converted = new Map(); - const failed: ConversionError[] = []; - const missing: Pick[] = []; - - 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 (!knownEditorSettings.has(configurationName)) { - continue; - } - - const editorSetting = { editorSettingName: configurationName, value }; - const conversion = convertEditorSetting(editorSetting, dependencies.converters, settings); - - if (conversion === undefined) { - const { editorSettingName } = editorSetting; - missing.push({ editorSettingName }); - continue; - } - - if (conversion instanceof ConversionError) { - failed.push(conversion); - continue; - } - - for (const changes of conversion.settings) { - converted.set(changes.editorSettingName, { ...changes }); - } - } - - return { converted, failed, missing }; -}; diff --git a/src/converters/editorConfigs/converter.ts b/src/converters/editorConfigs/converter.ts deleted file mode 100644 index 481c5ac61..000000000 --- a/src/converters/editorConfigs/converter.ts +++ /dev/null @@ -1,36 +0,0 @@ -import { ConversionError } from "../../errors/conversionError"; -import { TSLintToESLintSettings } from "../../types"; -import { EditorSetting } from "./types"; - -/** - * Attempts to convert a TSLint editor setting into the ESLint equivalents. - */ -export type EditorSettingConverter = ( - tslintEditorSetting: EditorSetting, - settings: TSLintToESLintSettings, -) => ConversionError | EditorSettingConversionResult | undefined; - -/** - * Successful result from converting a TSLint editor setting to its ESLint equivalents. - */ -export type EditorSettingConversionResult = { - /** - * At least one equivalent ESLint setting. - */ - settings: ConvertedEditorSettingChanges[]; -}; - -/** - * An ESLint editor setting equivalent to a previously enabled TSLint editor setting. - */ -export type ConvertedEditorSettingChanges = { - /** - * Any values for that ESLint editor setting. - */ - value: any; - - /** - * Equivalent ESLint editor setting name that should be enabled. - */ - editorSettingName: string; -}; diff --git a/src/converters/editorConfigs/converters/convertAtomConfig.ts b/src/converters/editorConfigs/converters/convertAtomConfig.ts new file mode 100644 index 000000000..2daa4f462 --- /dev/null +++ b/src/converters/editorConfigs/converters/convertAtomConfig.ts @@ -0,0 +1,16 @@ +import * as CsonParser from "cson-parser"; + +import { EditorConfigConverter } from "../types"; + +export const convertAtomConfig: EditorConfigConverter = (rawEditorSettings) => { + const editorSettings = CsonParser.parse(rawEditorSettings); + const useLocalTslint = editorSettings?.["linter-tslint"]?.useLocalTslint; + + return mergeCson({ + "linter-eslint": { + global: { + ...(useLocalTslint !== undefined && { useGlobalEslint: !useLocalTslint }), + }, + }, + }); +}; diff --git a/src/converters/editorConfigs/converters/convertVSCodeConfig.ts b/src/converters/editorConfigs/converters/convertVSCodeConfig.ts new file mode 100644 index 000000000..6e848ef5b --- /dev/null +++ b/src/converters/editorConfigs/converters/convertVSCodeConfig.ts @@ -0,0 +1,31 @@ +import * as path from "path"; + +import { parseJson } from "../../../utils"; +import { EditorConfigConverter } from "../types"; + +export const convertVSCodeConfig: EditorConfigConverter = (rawEditorSettings, settings) => { + const editorSettings = parseJson(rawEditorSettings); + const autoFixOnSave = editorSettings["editor.codeActionsOnSave"]?.["source.fixAll.tslint"]; + + // Only create a new config file path if the input and output configs roughly match + const eslintPath = + editorSettings["tslint.configFile"] && + !path.relative( + path.dirname(editorSettings["tslint.configFile"]), + path.dirname(settings.config), + ) && + settings.config; + + return mergeJson(rawEditorSettings, { + ...(autoFixOnSave !== undefined && { + "editor.codeActionsOnSave": { + "eslint.autoFixOnSave": autoFixOnSave, + }, + }), + ...(eslintPath && { + "eslint.options": { + configFile: eslintPath, + }, + }), + }); +}; diff --git a/src/converters/editorConfigs/converters/editor-code-actions-on-save.ts b/src/converters/editorConfigs/converters/editor-code-actions-on-save.ts deleted file mode 100644 index f9e0b25a3..000000000 --- a/src/converters/editorConfigs/converters/editor-code-actions-on-save.ts +++ /dev/null @@ -1,26 +0,0 @@ -import { EditorSettingConverter } from "../converter"; - -const SUB_SETTING_SOURCE_FIXALL = "source.fixAll.tslint"; - -export const convertEditorCodeActionsOnSave: EditorSettingConverter = ( - originalCodeActionsOnSave, -) => { - // Split properties to replace (into parent) and original ones. - const { - [SUB_SETTING_SOURCE_FIXALL]: originalSourceFixAllTsLint, - ...codeActionsOnSaveWithoutReplacedProperties - } = originalCodeActionsOnSave.value; - - return { - settings: [ - { - editorSettingName: "editor.codeActionsOnSave", - value: codeActionsOnSaveWithoutReplacedProperties, - }, - { - editorSettingName: "eslint.autoFixOnSave", - value: originalSourceFixAllTsLint, - }, - ], - }; -}; diff --git a/src/converters/editorConfigs/converters/tests/editor-code-actions-on-save.test.ts b/src/converters/editorConfigs/converters/tests/editor-code-actions-on-save.test.ts deleted file mode 100644 index 4e39f38a5..000000000 --- a/src/converters/editorConfigs/converters/tests/editor-code-actions-on-save.test.ts +++ /dev/null @@ -1,84 +0,0 @@ -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, - }, - }, - createStubTSLintToESLintSettings(), - ); - - expect(result).toEqual({ - settings: [ - { - editorSettingName: "editor.codeActionsOnSave", - value: {}, - }, - { - editorSettingName: "eslint.autoFixOnSave", - value: true, - }, - ], - }); - }); - - test("conversion of 'source.fixAll.tslint' when value is false", () => { - const result = convertEditorCodeActionsOnSave( - { - editorSettingName: "editor.codeActionsOnSave", - value: { - "source.fixAll.tslint": false, - }, - }, - createStubTSLintToESLintSettings(), - ); - - expect(result).toEqual({ - settings: [ - { - editorSettingName: "editor.codeActionsOnSave", - value: {}, - }, - { - editorSettingName: "eslint.autoFixOnSave", - value: false, - }, - ], - }); - }); - - 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", - }, - }, - createStubTSLintToESLintSettings(), - ); - - expect(result).toEqual({ - settings: [ - { - editorSettingName: "editor.codeActionsOnSave", - value: { - "one-property": 42, - "another-property": "foo", - }, - }, - { - editorSettingName: "eslint.autoFixOnSave", - value: true, - }, - ], - }); - }); -}); diff --git a/src/converters/editorConfigs/converters/tests/tslint-config-file.test.ts b/src/converters/editorConfigs/converters/tests/tslint-config-file.test.ts deleted file mode 100644 index 9bb3317ff..000000000 --- a/src/converters/editorConfigs/converters/tests/tslint-config-file.test.ts +++ /dev/null @@ -1,34 +0,0 @@ -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/converters/editorConfigs/converters/tslint-config-file.ts b/src/converters/editorConfigs/converters/tslint-config-file.ts deleted file mode 100644 index 53b8c3c65..000000000 --- a/src/converters/editorConfigs/converters/tslint-config-file.ts +++ /dev/null @@ -1,24 +0,0 @@ -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/converters/editorConfigs/editorConversionResults.stubs.ts b/src/converters/editorConfigs/editorConversionResults.stubs.ts deleted file mode 100644 index 83d755733..000000000 --- a/src/converters/editorConfigs/editorConversionResults.stubs.ts +++ /dev/null @@ -1,10 +0,0 @@ -import { EditorSettingConversionResults } from "./convertEditorSettings"; - -export const createEmptyEditorSettingConversionResults = ( - overrides: Partial = {}, -): EditorSettingConversionResults => ({ - converted: new Map(), - failed: [], - missing: [], - ...overrides, -}); diff --git a/src/converters/editorConfigs/editorSettingsConverters.ts b/src/converters/editorConfigs/editorSettingsConverters.ts deleted file mode 100644 index c49181025..000000000 --- a/src/converters/editorConfigs/editorSettingsConverters.ts +++ /dev/null @@ -1,10 +0,0 @@ -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/converters/editorConfigs/reporting/reportEditorSettingConversionResults.test.ts b/src/converters/editorConfigs/reporting/reportEditorConfigConversionResults.test.ts similarity index 89% rename from src/converters/editorConfigs/reporting/reportEditorSettingConversionResults.test.ts rename to src/converters/editorConfigs/reporting/reportEditorConfigConversionResults.test.ts index d0c399fca..05c1d9fe2 100644 --- a/src/converters/editorConfigs/reporting/reportEditorSettingConversionResults.test.ts +++ b/src/converters/editorConfigs/reporting/reportEditorConfigConversionResults.test.ts @@ -4,9 +4,9 @@ import { createStubLogger, expectEqualWrites } from "../../../adapters/logger.st import { createEmptyEditorSettingConversionResults } from "../editorConversionResults.stubs"; import { EditorSetting } from "../types"; -import { reportEditorSettingConversionResults } from "./reportEditorSettingConversionResults"; +import { reportEditorConfigConversionResults } from "./reportEditorConfigConversionResults"; -describe("reportEditorSettingConversionResults", () => { +describe("reportEditorConfigConversionResults", () => { it("logs a successful conversion when there is one converted editor setting", () => { // Arrange const conversionResults = createEmptyEditorSettingConversionResults({ @@ -24,7 +24,7 @@ describe("reportEditorSettingConversionResults", () => { const logger = createStubLogger(); // Act - reportEditorSettingConversionResults({ logger }, conversionResults); + reportEditorConfigConversionResults({ logger }, conversionResults); // Assert expectEqualWrites( @@ -57,7 +57,7 @@ describe("reportEditorSettingConversionResults", () => { const logger = createStubLogger(); // Act - reportEditorSettingConversionResults({ logger }, conversionResults); + reportEditorConfigConversionResults({ logger }, conversionResults); // Assert expectEqualWrites( @@ -75,7 +75,7 @@ describe("reportEditorSettingConversionResults", () => { const logger = createStubLogger(); // Act - reportEditorSettingConversionResults({ logger }, conversionResults); + reportEditorConfigConversionResults({ logger }, conversionResults); // Assert expectEqualWrites( @@ -94,7 +94,7 @@ describe("reportEditorSettingConversionResults", () => { const logger = createStubLogger(); // Act - reportEditorSettingConversionResults({ logger }, conversionResults); + reportEditorConfigConversionResults({ logger }, conversionResults); // Assert expectEqualWrites( @@ -117,7 +117,7 @@ describe("reportEditorSettingConversionResults", () => { const logger = createStubLogger(); // Act - reportEditorSettingConversionResults({ logger }, conversionResults); + reportEditorConfigConversionResults({ logger }, conversionResults); // Assert expectEqualWrites( @@ -148,7 +148,7 @@ describe("reportEditorSettingConversionResults", () => { const logger = createStubLogger(); // Act - reportEditorSettingConversionResults({ logger }, conversionResults); + reportEditorConfigConversionResults({ logger }, conversionResults); // Assert expectEqualWrites( diff --git a/src/converters/editorConfigs/reporting/reportEditorSettingConversionResults.ts b/src/converters/editorConfigs/reporting/reportEditorConfigConversionResults.ts similarity index 74% rename from src/converters/editorConfigs/reporting/reportEditorSettingConversionResults.ts rename to src/converters/editorConfigs/reporting/reportEditorConfigConversionResults.ts index d7da61dcd..cc6e4274b 100644 --- a/src/converters/editorConfigs/reporting/reportEditorSettingConversionResults.ts +++ b/src/converters/editorConfigs/reporting/reportEditorConfigConversionResults.ts @@ -4,20 +4,20 @@ import { logFailedConversions, logMissingConversionTarget, } from "../../../reporting"; -import { EditorSettingConversionResults } from "../convertEditorSettings"; +import { EditorSettingConversionResults } from "../convertEditorConfigs"; -export type ReportEditorSettingConversionResultsDependencies = { +export type ReportEditorConfigConversionResultsDependencies = { logger: Logger; }; -export const reportEditorSettingConversionResults = ( - dependencies: ReportEditorSettingConversionResultsDependencies, +export const reportEditorConfigConversionResults = ( + dependencies: ReportEditorConfigConversionResultsDependencies, editorSettingConversionResults: EditorSettingConversionResults, ) => { - if (editorSettingConversionResults.converted.size !== 0) { + if (editorSettingConversionResults.successes.length !== 0) { logSuccessfulConversions( "editor setting", - editorSettingConversionResults.converted, + editorSettingConversionResults.successes, dependencies.logger, ); } diff --git a/src/converters/editorConfigs/types.ts b/src/converters/editorConfigs/types.ts index 2c8ff4d54..524ea798d 100644 --- a/src/converters/editorConfigs/types.ts +++ b/src/converters/editorConfigs/types.ts @@ -1,4 +1,8 @@ -export type EditorSetting = { - editorSettingName: string; - value: any; -}; +import { TSLintToESLintSettings } from "../../types"; + +export type EditorConfigConverter = ( + rawEditorSettings: string, + settings: TSLintToESLintSettings, +) => any; + +export type EditorConfigDescriptor = [string, EditorConfigConverter]; diff --git a/src/input/importer.ts b/src/input/importer.ts index be3a622bf..aa2af57e6 100644 --- a/src/input/importer.ts +++ b/src/input/importer.ts @@ -1,8 +1,8 @@ import * as path from "path"; -import stripJsonComments from "strip-json-comments"; import { FileSystem } from "../adapters/fileSystem"; import { NativeImporter } from "../adapters/nativeImporter"; +import { parseJson } from "../utils"; export type ImporterDependencies = { fileSystem: Pick; @@ -31,7 +31,7 @@ export const importer = async ( } try { - return JSON.parse(stripJsonComments(rawJsonContents)); + return parseJson(rawJsonContents); } catch (error) { return error; } diff --git a/src/types.ts b/src/types.ts index a25919786..8763f2659 100644 --- a/src/types.ts +++ b/src/types.ts @@ -10,9 +10,9 @@ export type TSLintToESLintSettings = { comments?: true | string | string[]; /** - * Original Editor configuration file path, such as `.vscode/settings.json`. + * Original Editor configuration file path(s), such as `.vscode/settings.json`. */ - editor?: string; + editor?: string | string[]; /** * Original ESLint configuration file path, such as `.eslintrc.js`. diff --git a/src/utils.ts b/src/utils.ts index 2adbd4447..798c4c0e2 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -1,3 +1,5 @@ +import stripJsonComments from "strip-json-comments"; + export const isDefined = (item: Item | undefined): item is Item => item !== undefined; export const isError = (item: Item | Error): item is Error => item instanceof Error; @@ -52,3 +54,5 @@ export const uniqueFromSources = (...sources: (T | T[] | undefined)[]) => { return Array.from(new Set(items)); }; + +export const parseJson = (text: string) => JSON.parse(stripJsonComments(text)); From b6affc88095b2458f61a574fb20c47988be2ab0b Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 13 Oct 2020 16:55:30 -0400 Subject: [PATCH 2/4] Continue refactor: merging, and reporting updates --- package-lock.json | 13 +- package.json | 2 + src/cli/main.ts | 16 +-- .../editorConfigs/convertEditorConfig.test.ts | 44 ++++++ .../editorConfigs/convertEditorConfig.ts | 24 ++-- .../convertEditorConfigs.test.ts | 107 +++++++++++++++ .../editorConfigs/convertEditorConfigs.ts | 64 ++++++--- .../converters/convertAtomConfig.test.ts | 44 ++++++ .../converters/convertAtomConfig.ts | 24 ++-- .../converters/convertVSCodeConfig.test.ts | 78 +++++++++++ .../converters/convertVSCodeConfig.ts | 30 ++-- ...eportEditorConfigConversionResults.test.ts | 117 +++------------- .../reportEditorConfigConversionResults.ts | 33 ++--- src/converters/editorConfigs/types.ts | 4 +- .../reportConfigConversionResults.ts | 12 +- ...writeEditorConfigConversionResults.test.ts | 128 ------------------ .../writeEditorConfigConversionResults.ts | 36 ----- src/errors/conversionError.ts | 13 +- src/reporting.ts | 18 +-- 19 files changed, 435 insertions(+), 372 deletions(-) create mode 100644 src/converters/editorConfigs/convertEditorConfig.test.ts create mode 100644 src/converters/editorConfigs/convertEditorConfigs.test.ts create mode 100644 src/converters/editorConfigs/converters/convertAtomConfig.test.ts create mode 100644 src/converters/editorConfigs/converters/convertVSCodeConfig.test.ts delete mode 100644 src/converters/lintConfigs/writeEditorConfigConversionResults.test.ts delete mode 100644 src/converters/lintConfigs/writeEditorConfigConversionResults.ts diff --git a/package-lock.json b/package-lock.json index 5140a8f8a..50592b320 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2969,6 +2969,12 @@ "integrity": "sha512-3c+yGKvVP5Y9TYBEibGNR+kLtijnj7mYrXRg+WpFb2X9xm04g/DXYkfg4hmzJQosc9snFNUPkbYIhu+KAm6jJw==", "dev": true }, + "@types/lodash": { + "version": "4.14.162", + "resolved": "https://registry.npmjs.org/@types/lodash/-/lodash-4.14.162.tgz", + "integrity": "sha512-alvcho1kRUnnD1Gcl4J+hK0eencvzq9rmzvFPRmP5rPHx9VVsJj6bKLTATPVf9ktgv4ujzh7T+XWKp+jhuODig==", + "dev": true + }, "@types/minimatch": { "version": "3.0.3", "resolved": "https://registry.npmjs.org/@types/minimatch/-/minimatch-3.0.3.tgz", @@ -7157,10 +7163,9 @@ } }, "lodash": { - "version": "4.17.19", - "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.19.tgz", - "integrity": "sha512-JNvd8XER9GQX0v2qJgsaN/mzFCNA5BRe/j8JN9d+tWyGLSodKQHKFicdwNYzWwI3wjRnaKPsGj1XkBjx/F96DQ==", - "dev": true + "version": "4.17.20", + "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.20.tgz", + "integrity": "sha512-PlhdFcillOINfeV7Ni6oF1TAEayyZBoZ8bcshTHqOYJYlrqzRK5hagpagky5o4HfCzzd1TRkXPMFq6cKk9rGmA==" }, "lodash.sortby": { "version": "4.7.0", diff --git a/package.json b/package.json index 2347d3960..8f1362d13 100644 --- a/package.json +++ b/package.json @@ -15,6 +15,7 @@ "cson-parser": "^4.0.5", "eslint-config-prettier": "6.12.0", "glob": "7.1.6", + "lodash": "^4.17.20", "minimatch": "3.0.4", "strip-json-comments": "3.1.1", "tslint": "6.1.3", @@ -29,6 +30,7 @@ "@types/eslint-config-prettier": "6.11.0", "@types/glob": "7.1.3", "@types/jest": "26.0.14", + "@types/lodash": "^4.14.162", "@types/minimatch": "3.0.3", "@types/node": "12.12.21", "@typescript-eslint/eslint-plugin": "4.2.0", diff --git a/src/cli/main.ts b/src/cli/main.ts index 35a1895fd..5e8de1500 100644 --- a/src/cli/main.ts +++ b/src/cli/main.ts @@ -15,7 +15,6 @@ import { reportCommentResults, } from "../converters/comments/reporting/reportCommentResults"; import { convertEditorConfig } from "../converters/editorConfigs/convertEditorConfig"; -import { reportEditorSettingConversionResults } from "../converters/editorConfigs/reporting/reportEditorSettingConversionResults"; import { ConvertLintConfigDependencies, convertLintConfig, @@ -82,6 +81,7 @@ import { convertEditorConfigs, ConvertEditorConfigsDependencies, } from "../converters/editorConfigs/convertEditorConfigs"; +import { reportEditorConfigConversionResults } from "../converters/editorConfigs/reporting/reportEditorConfigConversionResults"; const convertFileCommentsDependencies: ConvertFileCommentsDependencies = { converters: ruleConverters, @@ -160,10 +160,6 @@ const writeConversionResultsDependencies: WriteConversionResultsDependencies = { fileSystem: fsFileSystem, }; -const reportEditorSettingConversionResultsDependencies = { - logger: processLogger, -}; - const editorConfigDescriptors: EditorConfigDescriptor[] = [ [".atom/config.cson", convertAtomConfig], [".vscode/settings.json", convertVSCodeConfig], @@ -174,12 +170,16 @@ const convertEditorConfigDependencies = { fileSystem: fsFileSystem, }; +const reportEditorConfigConversionResultsDependencies = { + logger: processLogger, +}; + const convertEditorConfigsDependencies: ConvertEditorConfigsDependencies = { convertEditorConfig: bind(convertEditorConfig, convertEditorConfigDependencies), editorConfigDescriptors, - reportEditorSettingConversionResults: bind( - reportEditorSettingConversionResults, - reportEditorSettingConversionResultsDependencies, + reportEditorConfigConversionResults: bind( + reportEditorConfigConversionResults, + reportEditorConfigConversionResultsDependencies, ), }; diff --git a/src/converters/editorConfigs/convertEditorConfig.test.ts b/src/converters/editorConfigs/convertEditorConfig.test.ts new file mode 100644 index 000000000..f3a95ae80 --- /dev/null +++ b/src/converters/editorConfigs/convertEditorConfig.test.ts @@ -0,0 +1,44 @@ +import { convertEditorConfig } from "./convertEditorConfig"; + +const stubPath = "./vscode/settings.json"; + +const stubSettings = { + config: ".eslintrc.js", +}; + +describe("convertEditorConfig", () => { + it("returns an error when reading the file fails", async () => { + // Arrange + const error = new Error("Oh no"); + const dependencies = { + fileSystem: { + readFile: jest.fn().mockResolvedValue(error), + writeFile: jest.fn(), + }, + }; + + // Act + const result = await convertEditorConfig(dependencies, jest.fn(), stubPath, stubSettings); + + // Assert + expect(result).toEqual(error); + }); + + it("writes the file when conversion succeeds", async () => { + // Arrange + const originalFileContents = "Hello"; + const converter = (input: string) => `${input} world!`; + const dependencies = { + fileSystem: { + readFile: jest.fn().mockResolvedValue(originalFileContents), + writeFile: jest.fn(), + }, + }; + + // Act + await convertEditorConfig(dependencies, converter, stubPath, stubSettings); + + // Assert + expect(dependencies.fileSystem.writeFile).toHaveBeenCalledWith(stubPath, "Hello world!"); + }); +}); diff --git a/src/converters/editorConfigs/convertEditorConfig.ts b/src/converters/editorConfigs/convertEditorConfig.ts index dc02bd6b7..24d131447 100644 --- a/src/converters/editorConfigs/convertEditorConfig.ts +++ b/src/converters/editorConfigs/convertEditorConfig.ts @@ -1,31 +1,23 @@ import { FileSystem } from "../../adapters/fileSystem"; import { TSLintToESLintSettings } from "../../types"; -import { EditorConfigDescriptor } from "./types"; +import { EditorConfigConverter } from "./types"; export type ConvertEditorConfigDependencies = { - editorConfigDescriptors: EditorConfigDescriptor[]; - fileSystem: Pick; + fileSystem: Pick; }; export const convertEditorConfig = async ( dependencies: ConvertEditorConfigDependencies, + converter: EditorConfigConverter, requestedPath: string, settings: TSLintToESLintSettings, ) => { - const editorConfigDescriptor = dependencies.editorConfigDescriptors.find(([defaultPath]) => - requestedPathMatchesDefault(defaultPath, requestedPath), - ); - if (!editorConfigDescriptor) { - return new Error(`Could not find a matching editor config for '${requestedPath}'.`); + const originalFileContents = await dependencies.fileSystem.readFile(requestedPath); + if (originalFileContents instanceof Error) { + return originalFileContents; } - const fileContents = await dependencies.fileSystem.readFile(requestedPath); - if (fileContents instanceof Error) { - return fileContents; - } + const updatedFileContents = converter(originalFileContents, settings); - return editorConfigDescriptor[1](fileContents, settings); + return await dependencies.fileSystem.writeFile(requestedPath, updatedFileContents); }; - -const requestedPathMatchesDefault = (defaultPath: string, requestedPath: string) => - defaultPath.replace(/\W+/g, "") === requestedPath.replace(/\W+/g, ""); diff --git a/src/converters/editorConfigs/convertEditorConfigs.test.ts b/src/converters/editorConfigs/convertEditorConfigs.test.ts new file mode 100644 index 000000000..57097249e --- /dev/null +++ b/src/converters/editorConfigs/convertEditorConfigs.test.ts @@ -0,0 +1,107 @@ +import { convertEditorConfigs, ConvertEditorConfigsDependencies } from "./convertEditorConfigs"; + +const stubConfigPath = "stub.json"; + +const stubEditorConfigDescriptors = [[stubConfigPath, jest.fn()]] as const; + +const createStubDependencies = (overrides: Partial = {}) => ({ + convertEditorConfig: jest.fn(), + editorConfigDescriptors: stubEditorConfigDescriptors, + reportEditorConfigConversionResults: jest.fn(), + ...overrides, +}); + +const createSettings = (requestedPath?: string) => ({ + config: ".eslintrc.js", + editor: requestedPath, +}); + +describe("convertEditorConfigs", () => { + it("reports an error when an unknown editor config path is requested", async () => { + // Arrange + const dependencies = createStubDependencies(); + const settings = createSettings("unknown/path.txt"); + + // Act + await convertEditorConfigs(dependencies, settings); + + // Assert + expect(dependencies.reportEditorConfigConversionResults).toHaveBeenCalledWith({ + failed: [ + expect.objectContaining({ + message: `Unknown editor config path requested: 'unknown/path.txt'.`, + }), + ], + successes: [stubConfigPath], + }); + }); + + it("reports an error when converting a requested editor config reports an error", async () => { + // Arrange + const error = new Error("Oh no!"); + const dependencies = createStubDependencies({ + convertEditorConfig: jest.fn().mockResolvedValue(error), + }); + const settings = createSettings(stubConfigPath); + + // Act + await convertEditorConfigs(dependencies, settings); + + // Assert + expect(dependencies.reportEditorConfigConversionResults).toHaveBeenCalledWith({ + failed: [error], + successes: [], + }); + }); + + it("reports a success when converting a requested editor config reports a success", async () => { + // Arrange + const dependencies = createStubDependencies({ + convertEditorConfig: jest.fn().mockResolvedValue(undefined), + }); + const settings = createSettings(stubConfigPath); + + // Act + await convertEditorConfigs(dependencies, settings); + + // Assert + expect(dependencies.reportEditorConfigConversionResults).toHaveBeenCalledWith({ + failed: [], + successes: [stubConfigPath], + }); + }); + + it("does not report an error when converting a default editor config reports an error", async () => { + // Arrange + const dependencies = createStubDependencies({ + convertEditorConfig: jest.fn().mockResolvedValue(new Error("Oh no!")), + }); + const settings = createSettings(); + + // Act + await convertEditorConfigs(dependencies, settings); + + // Assert + expect(dependencies.reportEditorConfigConversionResults).toHaveBeenCalledWith({ + failed: [], + successes: [], + }); + }); + + it("reports a success when converting a default editor config reports a success", async () => { + // Arrange + const dependencies = createStubDependencies({ + convertEditorConfig: jest.fn().mockResolvedValue(undefined), + }); + const settings = createSettings(); + + // Act + await convertEditorConfigs(dependencies, settings); + + // Assert + expect(dependencies.reportEditorConfigConversionResults).toHaveBeenCalledWith({ + failed: [], + successes: [stubConfigPath], + }); + }); +}); diff --git a/src/converters/editorConfigs/convertEditorConfigs.ts b/src/converters/editorConfigs/convertEditorConfigs.ts index a98557a8b..9a61ca2c0 100644 --- a/src/converters/editorConfigs/convertEditorConfigs.ts +++ b/src/converters/editorConfigs/convertEditorConfigs.ts @@ -1,5 +1,5 @@ import { SansDependencies } from "../../binding"; -import { TSLintToESLintSettings } from "../../types"; +import { ResultStatus, ResultWithStatus, TSLintToESLintSettings } from "../../types"; import { uniqueFromSources } from "../../utils"; import { convertEditorConfig } from "./convertEditorConfig"; import { reportEditorConfigConversionResults } from "./reporting/reportEditorConfigConversionResults"; @@ -7,50 +7,76 @@ import { EditorConfigDescriptor } from "./types"; export type ConvertEditorConfigsDependencies = { convertEditorConfig: SansDependencies; - editorConfigDescriptors: EditorConfigDescriptor[]; + editorConfigDescriptors: readonly EditorConfigDescriptor[]; reportEditorConfigConversionResults: SansDependencies< typeof reportEditorConfigConversionResults >; }; - -export type EditorSettingConversionResults = { - errors: string[]; - successes: string[]; -}; - export const convertEditorConfigs = async ( dependencies: ConvertEditorConfigsDependencies, settings: TSLintToESLintSettings, -) => { - const errors: Error[] = []; +): Promise => { + const failed: Error[] = []; const successes: string[] = []; const requestedPaths = uniqueFromSources(settings.editor); + // 1. Requested paths ??? with error reporting ??? await Promise.all( requestedPaths.map(async (requestedPath) => { - const result = await dependencies.convertEditorConfig(requestedPath, settings); + const descriptor = dependencies.editorConfigDescriptors.find(([defaultPath]) => + defaultPathMatches(defaultPath, requestedPath), + ); + if (!descriptor) { + failed.push(new Error(`Unknown editor config path requested: '${requestedPath}'.`)); + return; + } - if (result) { - errors.push(result); + const error = await dependencies.convertEditorConfig( + descriptor[1], + requestedPath, + settings, + ); + + if (error) { + failed.push(error); } else { successes.push(requestedPath); } }), ); + // 2. Default paths ??? without error reporting ??? + // (yes, this works, .convertEditorConfig has the readFile attempt) + // (todo, change to only if requested paths is true bool?) await Promise.all( dependencies.editorConfigDescriptors .filter( ([defaultPath]) => - !successes.some((success) => defaultPathMatches(defaultPath, success)), + !requestedPaths.some((success) => defaultPathMatches(defaultPath, success)), ) - .map( - async ([defaultPath, converter]) => - await dependencies.convertEditorConfig(defaultPath, settings, converter), - ), + .map(async ([defaultPath, converter]) => { + const error = await dependencies.convertEditorConfig( + converter, + defaultPath, + settings, + ); + + if (!error) { + successes.push(defaultPath); + } + }), ); - return { errors, successes }; + dependencies.reportEditorConfigConversionResults({ failed, successes }); + + return failed.length === 0 + ? { + status: ResultStatus.Succeeded, + } + : { + errors: failed, + status: ResultStatus.Failed, + }; }; const defaultPathMatches = (defaultPath: string, otherPath: string) => diff --git a/src/converters/editorConfigs/converters/convertAtomConfig.test.ts b/src/converters/editorConfigs/converters/convertAtomConfig.test.ts new file mode 100644 index 000000000..cfda14eb8 --- /dev/null +++ b/src/converters/editorConfigs/converters/convertAtomConfig.test.ts @@ -0,0 +1,44 @@ +import * as CsonParser from "cson-parser"; + +import { convertAtomConfig } from "./convertAtomConfig"; + +describe("convertAtomConfig", () => { + it("preserves the original config when no linter-tslint settings exist", () => { + // Arrange + const editorSettings = { unrelated: true }; + + // Act + const result = convertAtomConfig(CsonParser.stringify(editorSettings)); + + // Assert + expect(result).toEqual(CsonParser.stringify(editorSettings)); + }); + + it("includes useGlobalEslint when useLocalTslint exists", () => { + // Arrange + const editorSettings = { + "linter-tslint": { + useLocalTslint: true, + }, + unrelated: true, + }; + + // Act + const result = convertAtomConfig(CsonParser.stringify(editorSettings)); + + // Assert + expect(result).toEqual( + CsonParser.stringify({ + "linter-tslint": { + useLocalTslint: true, + }, + unrelated: true, + "linter-eslint": { + global: { + useGlobalEslint: false, + }, + }, + }), + ); + }); +}); diff --git a/src/converters/editorConfigs/converters/convertAtomConfig.ts b/src/converters/editorConfigs/converters/convertAtomConfig.ts index 2daa4f462..d09a6ba0f 100644 --- a/src/converters/editorConfigs/converters/convertAtomConfig.ts +++ b/src/converters/editorConfigs/converters/convertAtomConfig.ts @@ -1,16 +1,20 @@ import * as CsonParser from "cson-parser"; +import { merge } from "lodash"; -import { EditorConfigConverter } from "../types"; - -export const convertAtomConfig: EditorConfigConverter = (rawEditorSettings) => { +export const convertAtomConfig = (rawEditorSettings: string) => { const editorSettings = CsonParser.parse(rawEditorSettings); - const useLocalTslint = editorSettings?.["linter-tslint"]?.useLocalTslint; + const useLocalTslint = editorSettings["linter-tslint"]?.useLocalTslint; - return mergeCson({ - "linter-eslint": { - global: { - ...(useLocalTslint !== undefined && { useGlobalEslint: !useLocalTslint }), + return CsonParser.stringify( + merge( + editorSettings, + useLocalTslint !== undefined && { + "linter-eslint": { + global: { + ...{ useGlobalEslint: !useLocalTslint }, + }, + }, }, - }, - }); + ), + ); }; diff --git a/src/converters/editorConfigs/converters/convertVSCodeConfig.test.ts b/src/converters/editorConfigs/converters/convertVSCodeConfig.test.ts new file mode 100644 index 000000000..8227aab13 --- /dev/null +++ b/src/converters/editorConfigs/converters/convertVSCodeConfig.test.ts @@ -0,0 +1,78 @@ +import { convertVSCodeConfig } from "./convertVSCodeConfig"; + +const stubSettings = { + config: ".eslintrc.js", +}; + +describe("convertVSCodeConfig", () => { + it("preserves original settings when no TSLint settings exist", () => { + // Arrange + const editorSettings = { unrelated: true }; + + // Act + const result = convertVSCodeConfig(JSON.stringify(editorSettings), stubSettings); + + // Assert + expect(result).toEqual(JSON.stringify(editorSettings)); + }); + + it("includes eslint.autoFixOnSave when source.fixAll.tslint exists", () => { + // Arrange + const editorSettings = { + "editor.codeActionsOnSave": { + "source.fixAll.tslint": true, + }, + unrelated: true, + }; + + // Act + const result = convertVSCodeConfig(JSON.stringify(editorSettings), stubSettings); + + // Assert + expect(result).toEqual( + JSON.stringify({ + "editor.codeActionsOnSave": { + "source.fixAll.tslint": true, + "eslint.autoFixOnSave": true, + }, + unrelated: true, + }), + ); + }); + + it("does not include configFile when tslint.configFile does not match the output config", () => { + // Arrange + const editorSettings = { + "tslint.configFile": "unrelated/path/tsconfig.json", + unrelated: true, + }; + + // Act + const result = convertVSCodeConfig(JSON.stringify(editorSettings), stubSettings); + + // Assert + expect(result).toEqual(JSON.stringify(editorSettings)); + }); + + it("includes configFile when tslint.configFile matches", () => { + // Arrange + const editorSettings = { + "tslint.configFile": "./tslint.json", + unrelated: true, + }; + + // Act + const result = convertVSCodeConfig(JSON.stringify(editorSettings), stubSettings); + + // Assert + expect(result).toEqual( + JSON.stringify({ + "tslint.configFile": "./tslint.json", + unrelated: true, + "eslint.options": { + configFile: stubSettings.config, + }, + }), + ); + }); +}); diff --git a/src/converters/editorConfigs/converters/convertVSCodeConfig.ts b/src/converters/editorConfigs/converters/convertVSCodeConfig.ts index 6e848ef5b..c089e0c4d 100644 --- a/src/converters/editorConfigs/converters/convertVSCodeConfig.ts +++ b/src/converters/editorConfigs/converters/convertVSCodeConfig.ts @@ -1,3 +1,4 @@ +import { merge } from "lodash"; import * as path from "path"; import { parseJson } from "../../../utils"; @@ -8,24 +9,27 @@ export const convertVSCodeConfig: EditorConfigConverter = (rawEditorSettings, se const autoFixOnSave = editorSettings["editor.codeActionsOnSave"]?.["source.fixAll.tslint"]; // Only create a new config file path if the input and output configs roughly match - const eslintPath = + const eslintPathMatches = editorSettings["tslint.configFile"] && !path.relative( path.dirname(editorSettings["tslint.configFile"]), path.dirname(settings.config), - ) && - settings.config; + ); - return mergeJson(rawEditorSettings, { - ...(autoFixOnSave !== undefined && { - "editor.codeActionsOnSave": { - "eslint.autoFixOnSave": autoFixOnSave, + return JSON.stringify( + merge( + {}, + editorSettings, + autoFixOnSave !== undefined && { + "editor.codeActionsOnSave": { + "eslint.autoFixOnSave": autoFixOnSave, + }, }, - }), - ...(eslintPath && { - "eslint.options": { - configFile: eslintPath, + eslintPathMatches && { + "eslint.options": { + configFile: settings.config, + }, }, - }), - }); + ), + ); }; diff --git a/src/converters/editorConfigs/reporting/reportEditorConfigConversionResults.test.ts b/src/converters/editorConfigs/reporting/reportEditorConfigConversionResults.test.ts index 05c1d9fe2..9c92bdbe8 100644 --- a/src/converters/editorConfigs/reporting/reportEditorConfigConversionResults.test.ts +++ b/src/converters/editorConfigs/reporting/reportEditorConfigConversionResults.test.ts @@ -1,24 +1,22 @@ import { EOL } from "os"; import { createStubLogger, expectEqualWrites } from "../../../adapters/logger.stubs"; -import { createEmptyEditorSettingConversionResults } from "../editorConversionResults.stubs"; -import { EditorSetting } from "../types"; - -import { reportEditorConfigConversionResults } from "./reportEditorConfigConversionResults"; +import { + EditorSettingConversionResults, + reportEditorConfigConversionResults, +} from "./reportEditorConfigConversionResults"; + +const createStubConversionResults = (overrides: Partial = {}) => ({ + failed: [], + successes: [], + ...overrides, +}); describe("reportEditorConfigConversionResults", () => { it("logs a successful conversion when there is one converted editor setting", () => { // Arrange - const conversionResults = createEmptyEditorSettingConversionResults({ - converted: new Map([ - [ - "tslint-editor-setting-one", - { - editorSettingName: "tslint-editor-setting-one", - value: 42, - }, - ], - ]), + const conversionResults = createStubConversionResults({ + successes: ["./hooray"], }); const logger = createStubLogger(); @@ -29,29 +27,14 @@ describe("reportEditorConfigConversionResults", () => { // Assert expectEqualWrites( logger.stdout.write, - `${EOL}✨ 1 editor setting replaced with its ESLint equivalent. ✨${EOL}`, + `${EOL}✨ 1 editor file augmented with its ESLint equivalent. ✨${EOL}`, ); }); it("logs successful conversions when there are multiple converted settings", () => { // Arrange - const conversionResults = createEmptyEditorSettingConversionResults({ - converted: new Map([ - [ - "tslint-editor-setting-one", - { - editorSettingName: "tslint-editor-setting-one", - value: 42, - }, - ], - [ - "tslint-editor-setting-two", - { - editorSettingName: "tslint-editor-setting-two", - value: 4711, - }, - ], - ]), + const conversionResults = createStubConversionResults({ + successes: ["./one", "./two"], }); const logger = createStubLogger(); @@ -62,14 +45,14 @@ describe("reportEditorConfigConversionResults", () => { // Assert expectEqualWrites( logger.stdout.write, - `${EOL}✨ 2 editor settings replaced with their ESLint equivalents. ✨${EOL}`, + `${EOL}✨ 2 editor files augmented with their ESLint equivalents. ✨${EOL}`, ); }); it("logs a failed conversion when there is one failed conversion", () => { // Arrange - const conversionResults = createEmptyEditorSettingConversionResults({ - failed: [{ getSummary: () => "It broke." }], + const conversionResults = createStubConversionResults({ + failed: [new Error("It broke.")], }); const logger = createStubLogger(); @@ -87,8 +70,8 @@ describe("reportEditorConfigConversionResults", () => { it("logs failed conversions when there are multiple failed conversions", () => { // Arrange - const conversionResults = createEmptyEditorSettingConversionResults({ - failed: [{ getSummary: () => "It broke." }, { getSummary: () => "It really broke." }], + const conversionResults = createStubConversionResults({ + failed: [new Error("It broke."), new Error("It really broke.")], }); const logger = createStubLogger(); @@ -103,64 +86,4 @@ describe("reportEditorConfigConversionResults", () => { ` Check ${logger.debugFileName} for details.`, ); }); - - it("logs a missing editor setting when there is a missing setting", () => { - // Arrange - const conversionResults = createEmptyEditorSettingConversionResults({ - missing: [ - { - editorSettingName: "tslint-editor-setting-one", - }, - ], - }); - - const logger = createStubLogger(); - - // Act - reportEditorConfigConversionResults({ logger }, conversionResults); - - // Assert - expectEqualWrites( - logger.stdout.write, - `❓ 1 editor setting is not known by tslint-to-eslint-config to have an ESLint equivalent. ❓`, - ` Check ${logger.debugFileName} for details.`, - ); - expectEqualWrites( - logger.info.write, - `1 editor setting is not known by tslint-to-eslint-config to have an ESLint equivalent:`, - ' * tslint-to-eslint-config does not know the ESLint equivalent for TSLint\'s "tslint-editor-setting-one".', - ); - }); - - it("logs missing settings when there are missing settings", () => { - // Arrange - const conversionResults = createEmptyEditorSettingConversionResults({ - missing: [ - { - editorSettingName: "tslint-editor-setting-one", - }, - { - editorSettingName: "tslint-editor-setting-two", - }, - ], - }); - - const logger = createStubLogger(); - - // Act - reportEditorConfigConversionResults({ logger }, conversionResults); - - // Assert - expectEqualWrites( - logger.stdout.write, - `❓ 2 editor settings are not known by tslint-to-eslint-config to have ESLint equivalents. ❓`, - ` Check ${logger.debugFileName} for details.`, - ); - expectEqualWrites( - logger.info.write, - `2 editor settings are not known by tslint-to-eslint-config to have ESLint equivalents:`, - ' * tslint-to-eslint-config does not know the ESLint equivalent for TSLint\'s "tslint-editor-setting-one".', - ' * tslint-to-eslint-config does not know the ESLint equivalent for TSLint\'s "tslint-editor-setting-two".', - ); - }); }); diff --git a/src/converters/editorConfigs/reporting/reportEditorConfigConversionResults.ts b/src/converters/editorConfigs/reporting/reportEditorConfigConversionResults.ts index cc6e4274b..3374e554f 100644 --- a/src/converters/editorConfigs/reporting/reportEditorConfigConversionResults.ts +++ b/src/converters/editorConfigs/reporting/reportEditorConfigConversionResults.ts @@ -1,10 +1,10 @@ import { Logger } from "../../../adapters/logger"; -import { - logSuccessfulConversions, - logFailedConversions, - logMissingConversionTarget, -} from "../../../reporting"; -import { EditorSettingConversionResults } from "../convertEditorConfigs"; +import { logSuccessfulConversions, logFailedConversions } from "../../../reporting"; + +export type EditorSettingConversionResults = { + failed: Error[]; + successes: string[]; +}; export type ReportEditorConfigConversionResultsDependencies = { logger: Logger; @@ -12,25 +12,20 @@ export type ReportEditorConfigConversionResultsDependencies = { export const reportEditorConfigConversionResults = ( dependencies: ReportEditorConfigConversionResultsDependencies, - editorSettingConversionResults: EditorSettingConversionResults, + results: EditorSettingConversionResults, ) => { - if (editorSettingConversionResults.successes.length !== 0) { + if (results.successes.length !== 0) { logSuccessfulConversions( - "editor setting", - editorSettingConversionResults.successes, + "editor file", + "augmented", + results.successes.length, dependencies.logger, ); } - if (editorSettingConversionResults.failed.length !== 0) { - logFailedConversions(editorSettingConversionResults.failed, dependencies.logger); - } - - if (editorSettingConversionResults.missing.length !== 0) { - logMissingConversionTarget( - "editor setting", - (editorSetting) => editorSetting.editorSettingName, - editorSettingConversionResults.missing, + if (results.failed.length !== 0) { + logFailedConversions( + results.failed.map((fail) => fail.message), dependencies.logger, ); } diff --git a/src/converters/editorConfigs/types.ts b/src/converters/editorConfigs/types.ts index 524ea798d..a3c85edf2 100644 --- a/src/converters/editorConfigs/types.ts +++ b/src/converters/editorConfigs/types.ts @@ -3,6 +3,6 @@ import { TSLintToESLintSettings } from "../../types"; export type EditorConfigConverter = ( rawEditorSettings: string, settings: TSLintToESLintSettings, -) => any; +) => string; -export type EditorConfigDescriptor = [string, EditorConfigConverter]; +export type EditorConfigDescriptor = readonly [string, EditorConfigConverter]; diff --git a/src/converters/lintConfigs/reporting/reportConfigConversionResults.ts b/src/converters/lintConfigs/reporting/reportConfigConversionResults.ts index d60613379..287a06e17 100644 --- a/src/converters/lintConfigs/reporting/reportConfigConversionResults.ts +++ b/src/converters/lintConfigs/reporting/reportConfigConversionResults.ts @@ -20,12 +20,20 @@ export const reportConfigConversionResults = async ( ruleConversionResults: SummarizedConfigResultsConfiguration, ) => { if (ruleConversionResults.converted.size !== 0) { - logSuccessfulConversions("rule", ruleConversionResults.converted, dependencies.logger); + logSuccessfulConversions( + "rule", + "replaced", + ruleConversionResults.converted.size, + dependencies.logger, + ); logNotices(ruleConversionResults.converted, dependencies.logger); } if (ruleConversionResults.failed.length !== 0) { - logFailedConversions(ruleConversionResults.failed, dependencies.logger); + logFailedConversions( + ruleConversionResults.failed.map((fail) => fail.getSummary()), + dependencies.logger, + ); } if (ruleConversionResults.missing.length !== 0) { diff --git a/src/converters/lintConfigs/writeEditorConfigConversionResults.test.ts b/src/converters/lintConfigs/writeEditorConfigConversionResults.test.ts deleted file mode 100644 index 7900e2e4c..000000000 --- a/src/converters/lintConfigs/writeEditorConfigConversionResults.test.ts +++ /dev/null @@ -1,128 +0,0 @@ -import { createStubFileSystem } from "../../adapters/fileSystem.stub"; -import { EditorConfiguration } from "../../input/editorConfiguration"; -import { DeepPartial } from "../../input/findReportedConfiguration"; -import { EditorSettingConversionResults } from "../editorConfigs/convertEditorSettings"; -import { createEmptyEditorSettingConversionResults } from "../editorConfigs/editorConversionResults.stubs"; -import { EditorSetting } from "../editorConfigs/types"; -import { formatJsonOutput } from "./formatting/formatters/formatJsonOutput"; -import { - writeEditorConfigConversionResults, - WriteConversionResultsDependencies, -} from "./writeEditorConfigConversionResults"; - -const createStubDependencies = (overrides: Partial = {}) => ({ - fileSystem: createStubFileSystem(), - ...overrides, -}); - -describe("writeConversionResults", () => { - it("writes to correct output path with file system", async () => { - // Arrange - const dependencies = createStubDependencies(); - const outputPath = "/temp"; - const { originalConfig, conversionResults } = setupConversionEnvironment(); - - // Act - await writeEditorConfigConversionResults( - dependencies, - outputPath, - conversionResults, - originalConfig, - ); - - // Assert - expect(dependencies.fileSystem.writeFile).toHaveBeenCalledWith( - outputPath, - expect.anything(), - ); - }); - - it("writes formatted output with sorted keys to file system", async () => { - // Arrange - const dependencies = createStubDependencies(); - const outputPath = "/temp"; - - const { originalConfig, conversionResults } = setupConversionEnvironment({ - originalConfig: { - "property.a": "someValue", - "property.c": 123, - "property.b": { - "unsorted.sub.property.b": false, - "unsorted.sub.property.a": false, - }, - }, - conversionResults: createEmptyEditorSettingConversionResults({ - converted: new Map([ - [ - "eslint-setting-b", - { - editorSettingName: "eslint-setting-b", - value: 42, - }, - ], - [ - "eslint-setting-a", - { - editorSettingName: "eslint-setting-a", - value: 4711, - }, - ], - ]), - }), - }); - - const expectedSortedOutput = formatJsonOutput({ - "eslint-setting-a": 4711, - "eslint-setting-b": 42, - "property.a": "someValue", - "property.b": { - "unsorted.sub.property.b": false, - "unsorted.sub.property.a": false, - }, - "property.c": 123, - }); - - // Act - await writeEditorConfigConversionResults( - dependencies, - outputPath, - conversionResults, - originalConfig, - ); - - // Assert - expect(dependencies.fileSystem.writeFile).toHaveBeenCalledWith( - expect.anything(), - expectedSortedOutput, - ); - }); -}); - -function setupConversionEnvironment( - overrides: { - conversionResults?: EditorSettingConversionResults; - originalConfig?: DeepPartial; - } = {}, -) { - return { - originalConfig: { - "typescript.tsdk": "node_modules/typescript/lib", - "editor.tabSize": 4, - "editor.codeActionsOnSave": { - "source.organizeImports": false, - }, - }, - conversionResults: createEmptyEditorSettingConversionResults({ - converted: new Map([ - [ - "tslint-editor-setting-one", - { - editorSettingName: "tslint-editor-setting-one", - value: 42, - }, - ], - ]), - }), - ...overrides, - }; -} diff --git a/src/converters/lintConfigs/writeEditorConfigConversionResults.ts b/src/converters/lintConfigs/writeEditorConfigConversionResults.ts deleted file mode 100644 index 7c4bd9dc9..000000000 --- a/src/converters/lintConfigs/writeEditorConfigConversionResults.ts +++ /dev/null @@ -1,36 +0,0 @@ -import { FileSystem } from "../../adapters/fileSystem"; -import { EditorConfiguration } from "../../input/editorConfiguration"; -import { DeepPartial } from "../../input/findReportedConfiguration"; -import { EditorSettingConversionResults } from "../editorConfigs/convertEditorSettings"; -import { formatOutput } from "./formatting/formatOutput"; - -export type WriteConversionResultsDependencies = { - fileSystem: Pick; -}; - -export const writeEditorConfigConversionResults = async ( - dependencies: WriteConversionResultsDependencies, - outputPath: string, - conversionResults: EditorSettingConversionResults, - originalConfiguration: DeepPartial, -) => { - const output = { - ...originalConfiguration, - ...formatConvertedSettings(conversionResults), - }; - - return await dependencies.fileSystem.writeFile(outputPath, formatOutput(outputPath, output)); -}; - -export const formatConvertedSettings = (conversionResults: EditorSettingConversionResults) => { - const output: { [i: string]: string | any[] } = {}; - const sortedEntries = Array.from(conversionResults.converted).sort(([nameA], [nameB]) => - nameA.localeCompare(nameB), - ); - - for (const [name, setting] of sortedEntries) { - output[name] = setting.value; - } - - return output; -}; diff --git a/src/errors/conversionError.ts b/src/errors/conversionError.ts index 472d0203d..e37195004 100644 --- a/src/errors/conversionError.ts +++ b/src/errors/conversionError.ts @@ -1,11 +1,12 @@ import { EOL } from "os"; -import { EditorSetting } from "../converters/editorConfigs/types"; import { TSLintRuleOptions } from "../converters/lintConfigs/rules/types"; import { ErrorSummary } from "./errorSummary"; -export class ConversionError implements ErrorSummary { - private constructor(private readonly summary: string) {} +export class ConversionError extends Error implements ErrorSummary { + private constructor(private readonly summary: string) { + super(summary); + } public static forMerger(eslintRule: string) { return new ConversionError( @@ -22,12 +23,6 @@ export class ConversionError implements ErrorSummary { ); } - public static forSettingError(error: Error, editorSetting: EditorSetting) { - return new ConversionError( - `${editorSetting.editorSettingName} threw an error during conversion: ${error.stack}${EOL}`, - ); - } - public getSummary(): string { return this.summary; } diff --git a/src/reporting.ts b/src/reporting.ts index b41f04875..5b296e604 100644 --- a/src/reporting.ts +++ b/src/reporting.ts @@ -2,7 +2,6 @@ import chalk from "chalk"; import { EOL } from "os"; import { Logger } from "./adapters/logger"; -import { ErrorSummary } from "./errors/errorSummary"; import { ResultWithStatus, ResultStatus } from "./types"; export const logErrorResult = (result: ResultWithStatus, logger: Logger) => { @@ -31,24 +30,25 @@ export const logErrorResult = (result: ResultWithStatus, logger: Logger) => { export const logSuccessfulConversions = ( conversionTypeName: string, - converted: Map, + action: string, + quantity: number, logger: Logger, ) => { - logger.stdout.write(chalk.greenBright(`${EOL}✨ ${converted.size}`)); + logger.stdout.write(chalk.greenBright(`${EOL}✨ ${quantity}`)); logger.stdout.write( - converted.size === 1 - ? chalk.green(` ${conversionTypeName} replaced with its ESLint equivalent.`) - : chalk.green(` ${conversionTypeName}s replaced with their ESLint equivalents.`), + quantity === 1 + ? chalk.green(` ${conversionTypeName} ${action} with its ESLint equivalent.`) + : chalk.green(` ${conversionTypeName}s ${action} with their ESLint equivalents.`), ); logger.stdout.write(chalk.greenBright(` ✨${EOL}`)); }; -export const logFailedConversions = (failed: ErrorSummary[], logger: Logger) => { +export const logFailedConversions = (failed: string[], logger: Logger) => { logger.stderr.write(`${chalk.redBright(`${EOL}❌ ${failed.length}`)}`); logger.stderr.write(chalk.red(` error${failed.length === 1 ? "" : "s"}`)); logger.stderr.write(chalk.red(" thrown.")); logger.stderr.write(chalk.redBright(` ❌${EOL}`)); - logger.info.write(failed.map((fail) => fail.getSummary()).join("\n\n") + "\n\n"); + logger.info.write(failed.join("\n\n") + "\n\n"); logger.stderr.write(chalk.red(` Check ${logger.debugFileName} for details.${EOL}`)); }; @@ -57,7 +57,7 @@ export const logMissingConversionTarget = ( missingOutputMapping: (missing: T) => string, missing: T[], logger: Logger, - additionalWarnings: string[] = [], + additionalWarnings: string[], ) => { const headline = missing.length === 1 From 1d5106fd07fffa70a8e062e57f87d275b1c9b0dd Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Thu, 15 Oct 2020 15:03:02 -0400 Subject: [PATCH 3/4] Added back missing reporting --- src/cli/main.ts | 16 ++-- .../editorConfigs/convertEditorConfig.test.ts | 35 ++++++-- .../editorConfigs/convertEditorConfig.ts | 5 +- .../convertEditorConfigs.test.ts | 82 ++++++++----------- .../editorConfigs/convertEditorConfigs.ts | 50 ++++------- .../converters/convertAtomConfig.test.ts | 51 +++++++++--- .../converters/convertAtomConfig.ts | 13 ++- .../converters/convertVSCodeConfig.test.ts | 78 ++++++++++++++---- .../converters/convertVSCodeConfig.ts | 16 +++- ...eportEditorConfigConversionResults.test.ts | 55 ++++++++++--- .../reportEditorConfigConversionResults.ts | 33 +++++--- src/converters/editorConfigs/types.ts | 12 ++- src/reporting.ts | 2 +- 13 files changed, 291 insertions(+), 157 deletions(-) diff --git a/src/cli/main.ts b/src/cli/main.ts index 5e8de1500..57a4b3c07 100644 --- a/src/cli/main.ts +++ b/src/cli/main.ts @@ -35,6 +35,14 @@ import { ConvertFileCommentsDependencies, convertFileComments, } from "../converters/comments/convertFileComments"; +import { + convertEditorConfigs, + ConvertEditorConfigsDependencies, +} from "../converters/editorConfigs/convertEditorConfigs"; +import { convertAtomConfig } from "../converters/editorConfigs/converters/convertAtomConfig"; +import { convertVSCodeConfig } from "../converters/editorConfigs/converters/convertVSCodeConfig"; +import { reportEditorConfigConversionResults } from "../converters/editorConfigs/reporting/reportEditorConfigConversionResults"; +import { EditorConfigDescriptor } from "../converters/editorConfigs/types"; import { ConvertRulesDependencies, convertRules, @@ -74,14 +82,6 @@ import { findTSLintConfiguration } from "../input/findTSLintConfiguration"; import { findTypeScriptConfiguration } from "../input/findTypeScriptConfiguration"; import { importer, ImporterDependencies } from "../input/importer"; import { mergeLintConfigurations } from "../input/mergeLintConfigurations"; -import { convertVSCodeConfig } from "../converters/editorConfigs/converters/convertVSCodeConfig"; -import { convertAtomConfig } from "../converters/editorConfigs/converters/convertAtomConfig"; -import { EditorConfigDescriptor } from "../converters/editorConfigs/types"; -import { - convertEditorConfigs, - ConvertEditorConfigsDependencies, -} from "../converters/editorConfigs/convertEditorConfigs"; -import { reportEditorConfigConversionResults } from "../converters/editorConfigs/reporting/reportEditorConfigConversionResults"; const convertFileCommentsDependencies: ConvertFileCommentsDependencies = { converters: ruleConverters, diff --git a/src/converters/editorConfigs/convertEditorConfig.test.ts b/src/converters/editorConfigs/convertEditorConfig.test.ts index f3a95ae80..76f50d815 100644 --- a/src/converters/editorConfigs/convertEditorConfig.test.ts +++ b/src/converters/editorConfigs/convertEditorConfig.test.ts @@ -24,21 +24,46 @@ describe("convertEditorConfig", () => { expect(result).toEqual(error); }); - it("writes the file when conversion succeeds", async () => { + it("returns an error when writing to a file fails", async () => { // Arrange const originalFileContents = "Hello"; - const converter = (input: string) => `${input} world!`; + const error = new Error("Oh no!"); + const converter = (input: string) => ({ + contents: `${input} world!`, + missing: [], + }); const dependencies = { fileSystem: { readFile: jest.fn().mockResolvedValue(originalFileContents), - writeFile: jest.fn(), + writeFile: jest.fn().mockResolvedValue(error), + }, + }; + + // Act + const result = await convertEditorConfig(dependencies, converter, stubPath, stubSettings); + + // Assert + expect(result).toEqual(error); + }); + + it("returns the conversion data when writing to a file succeeds", async () => { + // Arrange + const originalFileContents = "Hello"; + const converter = (input: string) => ({ + contents: `${input} world!`, + missing: [], + }); + const dependencies = { + fileSystem: { + readFile: jest.fn().mockResolvedValue(originalFileContents), + writeFile: jest.fn().mockResolvedValue(undefined), }, }; // Act - await convertEditorConfig(dependencies, converter, stubPath, stubSettings); + const result = await convertEditorConfig(dependencies, converter, stubPath, stubSettings); // Assert - expect(dependencies.fileSystem.writeFile).toHaveBeenCalledWith(stubPath, "Hello world!"); + expect(result).toEqual(converter(originalFileContents)); }); }); diff --git a/src/converters/editorConfigs/convertEditorConfig.ts b/src/converters/editorConfigs/convertEditorConfig.ts index 24d131447..0654c55a4 100644 --- a/src/converters/editorConfigs/convertEditorConfig.ts +++ b/src/converters/editorConfigs/convertEditorConfig.ts @@ -17,7 +17,8 @@ export const convertEditorConfig = async ( return originalFileContents; } - const updatedFileContents = converter(originalFileContents, settings); + const conversion = converter(originalFileContents, settings); + const error = await dependencies.fileSystem.writeFile(requestedPath, conversion.contents); - return await dependencies.fileSystem.writeFile(requestedPath, updatedFileContents); + return error ?? conversion; }; diff --git a/src/converters/editorConfigs/convertEditorConfigs.test.ts b/src/converters/editorConfigs/convertEditorConfigs.test.ts index 57097249e..2ae8a1f78 100644 --- a/src/converters/editorConfigs/convertEditorConfigs.test.ts +++ b/src/converters/editorConfigs/convertEditorConfigs.test.ts @@ -1,3 +1,4 @@ +import { ResultStatus } from "../../types"; import { convertEditorConfigs, ConvertEditorConfigsDependencies } from "./convertEditorConfigs"; const stubConfigPath = "stub.json"; @@ -19,24 +20,28 @@ const createSettings = (requestedPath?: string) => ({ describe("convertEditorConfigs", () => { it("reports an error when an unknown editor config path is requested", async () => { // Arrange + const unknownPath = "unknown/path.txt"; const dependencies = createStubDependencies(); - const settings = createSettings("unknown/path.txt"); + const settings = createSettings(unknownPath); // Act - await convertEditorConfigs(dependencies, settings); + const result = await convertEditorConfigs(dependencies, settings); // Assert + const error = expect.objectContaining({ + message: `Unknown editor config path requested: 'unknown/path.txt'.`, + }); expect(dependencies.reportEditorConfigConversionResults).toHaveBeenCalledWith({ - failed: [ - expect.objectContaining({ - message: `Unknown editor config path requested: 'unknown/path.txt'.`, - }), - ], - successes: [stubConfigPath], + failed: new Map([[unknownPath, error]]), + successes: new Map(), + }); + expect(result).toEqual({ + errors: [error], + status: ResultStatus.Failed, }); }); - it("reports an error when converting a requested editor config reports an error", async () => { + it("reports an error when converting an editor config reports an error", async () => { // Arrange const error = new Error("Oh no!"); const dependencies = createStubDependencies({ @@ -45,63 +50,40 @@ describe("convertEditorConfigs", () => { const settings = createSettings(stubConfigPath); // Act - await convertEditorConfigs(dependencies, settings); + const result = await convertEditorConfigs(dependencies, settings); // Assert expect(dependencies.reportEditorConfigConversionResults).toHaveBeenCalledWith({ - failed: [error], - successes: [], - }); - }); - - it("reports a success when converting a requested editor config reports a success", async () => { - // Arrange - const dependencies = createStubDependencies({ - convertEditorConfig: jest.fn().mockResolvedValue(undefined), + failed: new Map([[stubConfigPath, error]]), + successes: new Map(), }); - const settings = createSettings(stubConfigPath); - - // Act - await convertEditorConfigs(dependencies, settings); - - // Assert - expect(dependencies.reportEditorConfigConversionResults).toHaveBeenCalledWith({ - failed: [], - successes: [stubConfigPath], + expect(result).toEqual({ + errors: [error], + status: ResultStatus.Failed, }); }); - it("does not report an error when converting a default editor config reports an error", async () => { + it("reports a success when converting an editor config reports a success", async () => { // Arrange + const success = { + contents: "Hello, world!", + missing: [], + }; const dependencies = createStubDependencies({ - convertEditorConfig: jest.fn().mockResolvedValue(new Error("Oh no!")), + convertEditorConfig: jest.fn().mockResolvedValue(success), }); - const settings = createSettings(); + const settings = createSettings(stubConfigPath); // Act - await convertEditorConfigs(dependencies, settings); + const result = await convertEditorConfigs(dependencies, settings); // Assert expect(dependencies.reportEditorConfigConversionResults).toHaveBeenCalledWith({ - failed: [], - successes: [], - }); - }); - - it("reports a success when converting a default editor config reports a success", async () => { - // Arrange - const dependencies = createStubDependencies({ - convertEditorConfig: jest.fn().mockResolvedValue(undefined), + failed: new Map(), + successes: new Map([[stubConfigPath, success]]), }); - const settings = createSettings(); - - // Act - await convertEditorConfigs(dependencies, settings); - - // Assert - expect(dependencies.reportEditorConfigConversionResults).toHaveBeenCalledWith({ - failed: [], - successes: [stubConfigPath], + expect(result).toEqual({ + status: ResultStatus.Succeeded, }); }); }); diff --git a/src/converters/editorConfigs/convertEditorConfigs.ts b/src/converters/editorConfigs/convertEditorConfigs.ts index 9a61ca2c0..d2aec5d3d 100644 --- a/src/converters/editorConfigs/convertEditorConfigs.ts +++ b/src/converters/editorConfigs/convertEditorConfigs.ts @@ -3,7 +3,7 @@ import { ResultStatus, ResultWithStatus, TSLintToESLintSettings } from "../../ty import { uniqueFromSources } from "../../utils"; import { convertEditorConfig } from "./convertEditorConfig"; import { reportEditorConfigConversionResults } from "./reporting/reportEditorConfigConversionResults"; -import { EditorConfigDescriptor } from "./types"; +import { EditorConfigDescriptor, EditorConfigsConversionResults } from "./types"; export type ConvertEditorConfigsDependencies = { convertEditorConfig: SansDependencies; @@ -16,65 +16,47 @@ export const convertEditorConfigs = async ( dependencies: ConvertEditorConfigsDependencies, settings: TSLintToESLintSettings, ): Promise => { - const failed: Error[] = []; - const successes: string[] = []; + const results: EditorConfigsConversionResults = { + failed: new Map(), + successes: new Map(), + }; const requestedPaths = uniqueFromSources(settings.editor); - // 1. Requested paths ??? with error reporting ??? await Promise.all( requestedPaths.map(async (requestedPath) => { const descriptor = dependencies.editorConfigDescriptors.find(([defaultPath]) => defaultPathMatches(defaultPath, requestedPath), ); if (!descriptor) { - failed.push(new Error(`Unknown editor config path requested: '${requestedPath}'.`)); + results.failed.set( + requestedPath, + new Error(`Unknown editor config path requested: '${requestedPath}'.`), + ); return; } - const error = await dependencies.convertEditorConfig( + const result = await dependencies.convertEditorConfig( descriptor[1], requestedPath, settings, ); - if (error) { - failed.push(error); + if (result instanceof Error) { + results.failed.set(requestedPath, result); } else { - successes.push(requestedPath); + results.successes.set(requestedPath, result); } }), ); - // 2. Default paths ??? without error reporting ??? - // (yes, this works, .convertEditorConfig has the readFile attempt) - // (todo, change to only if requested paths is true bool?) - await Promise.all( - dependencies.editorConfigDescriptors - .filter( - ([defaultPath]) => - !requestedPaths.some((success) => defaultPathMatches(defaultPath, success)), - ) - .map(async ([defaultPath, converter]) => { - const error = await dependencies.convertEditorConfig( - converter, - defaultPath, - settings, - ); - - if (!error) { - successes.push(defaultPath); - } - }), - ); - - dependencies.reportEditorConfigConversionResults({ failed, successes }); + dependencies.reportEditorConfigConversionResults(results); - return failed.length === 0 + return results.failed.size === 0 ? { status: ResultStatus.Succeeded, } : { - errors: failed, + errors: Array.from(results.failed.values()), status: ResultStatus.Failed, }; }; diff --git a/src/converters/editorConfigs/converters/convertAtomConfig.test.ts b/src/converters/editorConfigs/converters/convertAtomConfig.test.ts index cfda14eb8..7f83bf503 100644 --- a/src/converters/editorConfigs/converters/convertAtomConfig.test.ts +++ b/src/converters/editorConfigs/converters/convertAtomConfig.test.ts @@ -11,7 +11,10 @@ describe("convertAtomConfig", () => { const result = convertAtomConfig(CsonParser.stringify(editorSettings)); // Assert - expect(result).toEqual(CsonParser.stringify(editorSettings)); + expect(result).toEqual({ + contents: CsonParser.stringify(editorSettings, null, 4), + missing: [], + }); }); it("includes useGlobalEslint when useLocalTslint exists", () => { @@ -27,18 +30,42 @@ describe("convertAtomConfig", () => { const result = convertAtomConfig(CsonParser.stringify(editorSettings)); // Assert - expect(result).toEqual( - CsonParser.stringify({ - "linter-tslint": { - useLocalTslint: true, - }, - unrelated: true, - "linter-eslint": { - global: { - useGlobalEslint: false, + expect(result).toEqual({ + contents: CsonParser.stringify( + { + "linter-tslint": { + useLocalTslint: true, + }, + unrelated: true, + "linter-eslint": { + global: { + useGlobalEslint: false, + }, }, }, - }), - ); + null, + 4, + ), + missing: [], + }); + }); + + it("includes missing notices when known missing settings are included", () => { + // Arrange + const editorSettings = { + "linter-tslint": { + enableSemanticRules: true, + rulesDirectory: true, + }, + }; + + // Act + const result = convertAtomConfig(CsonParser.stringify(editorSettings)); + + // Assert + expect(result).toEqual({ + contents: CsonParser.stringify(editorSettings, null, 4), + missing: ["enableSemanticRules", "rulesDirectory"], + }); }); }); diff --git a/src/converters/editorConfigs/converters/convertAtomConfig.ts b/src/converters/editorConfigs/converters/convertAtomConfig.ts index d09a6ba0f..b32ec4d97 100644 --- a/src/converters/editorConfigs/converters/convertAtomConfig.ts +++ b/src/converters/editorConfigs/converters/convertAtomConfig.ts @@ -1,11 +1,14 @@ import * as CsonParser from "cson-parser"; import { merge } from "lodash"; +const knownMissingSettings = ["enableSemanticRules", "rulesDirectory"]; + export const convertAtomConfig = (rawEditorSettings: string) => { const editorSettings = CsonParser.parse(rawEditorSettings); - const useLocalTslint = editorSettings["linter-tslint"]?.useLocalTslint; + const linterSettings = editorSettings["linter-tslint"]; + const useLocalTslint = linterSettings?.useLocalTslint; - return CsonParser.stringify( + const contents = CsonParser.stringify( merge( editorSettings, useLocalTslint !== undefined && { @@ -16,5 +19,11 @@ export const convertAtomConfig = (rawEditorSettings: string) => { }, }, ), + null, + 4, ); + + const missing = knownMissingSettings.filter((setting) => linterSettings?.[setting]); + + return { contents, missing }; }; diff --git a/src/converters/editorConfigs/converters/convertVSCodeConfig.test.ts b/src/converters/editorConfigs/converters/convertVSCodeConfig.test.ts index 8227aab13..1b3852e9d 100644 --- a/src/converters/editorConfigs/converters/convertVSCodeConfig.test.ts +++ b/src/converters/editorConfigs/converters/convertVSCodeConfig.test.ts @@ -13,7 +13,10 @@ describe("convertVSCodeConfig", () => { const result = convertVSCodeConfig(JSON.stringify(editorSettings), stubSettings); // Assert - expect(result).toEqual(JSON.stringify(editorSettings)); + expect(result).toEqual({ + contents: JSON.stringify(editorSettings, null, 4), + missing: [], + }); }); it("includes eslint.autoFixOnSave when source.fixAll.tslint exists", () => { @@ -29,15 +32,20 @@ describe("convertVSCodeConfig", () => { const result = convertVSCodeConfig(JSON.stringify(editorSettings), stubSettings); // Assert - expect(result).toEqual( - JSON.stringify({ - "editor.codeActionsOnSave": { - "source.fixAll.tslint": true, - "eslint.autoFixOnSave": true, + expect(result).toEqual({ + contents: JSON.stringify( + { + "editor.codeActionsOnSave": { + "source.fixAll.tslint": true, + "eslint.autoFixOnSave": true, + }, + unrelated: true, }, - unrelated: true, - }), - ); + null, + 4, + ), + missing: [], + }); }); it("does not include configFile when tslint.configFile does not match the output config", () => { @@ -51,7 +59,10 @@ describe("convertVSCodeConfig", () => { const result = convertVSCodeConfig(JSON.stringify(editorSettings), stubSettings); // Assert - expect(result).toEqual(JSON.stringify(editorSettings)); + expect(result).toEqual({ + contents: JSON.stringify(editorSettings, null, 4), + missing: [], + }); }); it("includes configFile when tslint.configFile matches", () => { @@ -65,14 +76,45 @@ describe("convertVSCodeConfig", () => { const result = convertVSCodeConfig(JSON.stringify(editorSettings), stubSettings); // Assert - expect(result).toEqual( - JSON.stringify({ - "tslint.configFile": "./tslint.json", - unrelated: true, - "eslint.options": { - configFile: stubSettings.config, + expect(result).toEqual({ + contents: JSON.stringify( + { + "tslint.configFile": "./tslint.json", + unrelated: true, + "eslint.options": { + configFile: stubSettings.config, + }, }, - }), - ); + null, + 4, + ), + missing: [], + }); + }); + + it("includes missing notices when known missing settings are included", () => { + // Arrange + const editorSettings = { + "tslint.alwaysShowRuleFailuresAsWarnings": true, + "tslint.exclude": true, + "tslint.ignoreDefinitionFiles": true, + "tslint.jsEnable": true, + "tslint.suppressWhileTypeErrorsPresent": true, + }; + + // Act + const result = convertVSCodeConfig(JSON.stringify(editorSettings), stubSettings); + + // Assert + expect(result).toEqual({ + contents: JSON.stringify(editorSettings, null, 4), + missing: [ + "tslint.alwaysShowRuleFailuresAsWarnings", + "tslint.exclude", + "tslint.ignoreDefinitionFiles", + "tslint.jsEnable", + "tslint.suppressWhileTypeErrorsPresent", + ], + }); }); }); diff --git a/src/converters/editorConfigs/converters/convertVSCodeConfig.ts b/src/converters/editorConfigs/converters/convertVSCodeConfig.ts index c089e0c4d..f47694be8 100644 --- a/src/converters/editorConfigs/converters/convertVSCodeConfig.ts +++ b/src/converters/editorConfigs/converters/convertVSCodeConfig.ts @@ -4,6 +4,14 @@ import * as path from "path"; import { parseJson } from "../../../utils"; import { EditorConfigConverter } from "../types"; +const knownMissingSettings = [ + "tslint.alwaysShowRuleFailuresAsWarnings", + "tslint.exclude", + "tslint.ignoreDefinitionFiles", + "tslint.jsEnable", + "tslint.suppressWhileTypeErrorsPresent", +]; + export const convertVSCodeConfig: EditorConfigConverter = (rawEditorSettings, settings) => { const editorSettings = parseJson(rawEditorSettings); const autoFixOnSave = editorSettings["editor.codeActionsOnSave"]?.["source.fixAll.tslint"]; @@ -16,7 +24,7 @@ export const convertVSCodeConfig: EditorConfigConverter = (rawEditorSettings, se path.dirname(settings.config), ); - return JSON.stringify( + const contents = JSON.stringify( merge( {}, editorSettings, @@ -31,5 +39,11 @@ export const convertVSCodeConfig: EditorConfigConverter = (rawEditorSettings, se }, }, ), + null, + 4, ); + + const missing = knownMissingSettings.filter((setting) => editorSettings[setting]); + + return { contents, missing }; }; diff --git a/src/converters/editorConfigs/reporting/reportEditorConfigConversionResults.test.ts b/src/converters/editorConfigs/reporting/reportEditorConfigConversionResults.test.ts index 9c92bdbe8..9a88d0061 100644 --- a/src/converters/editorConfigs/reporting/reportEditorConfigConversionResults.test.ts +++ b/src/converters/editorConfigs/reporting/reportEditorConfigConversionResults.test.ts @@ -1,22 +1,25 @@ import { EOL } from "os"; import { createStubLogger, expectEqualWrites } from "../../../adapters/logger.stubs"; -import { - EditorSettingConversionResults, - reportEditorConfigConversionResults, -} from "./reportEditorConfigConversionResults"; - -const createStubConversionResults = (overrides: Partial = {}) => ({ - failed: [], - successes: [], +import { EditorConfigsConversionResults } from "../types"; +import { reportEditorConfigConversionResults } from "./reportEditorConfigConversionResults"; + +const createStubConversionResults = (overrides: Partial = {}) => ({ + failed: new Map(), + successes: new Map(), ...overrides, }); +const stubSuccess = { + contents: "Hello, world!", + missing: [], +}; + describe("reportEditorConfigConversionResults", () => { it("logs a successful conversion when there is one converted editor setting", () => { // Arrange const conversionResults = createStubConversionResults({ - successes: ["./hooray"], + successes: new Map([["./one", stubSuccess]]), }); const logger = createStubLogger(); @@ -34,7 +37,10 @@ describe("reportEditorConfigConversionResults", () => { it("logs successful conversions when there are multiple converted settings", () => { // Arrange const conversionResults = createStubConversionResults({ - successes: ["./one", "./two"], + successes: new Map([ + ["./one", stubSuccess], + ["./two", stubSuccess], + ]), }); const logger = createStubLogger(); @@ -49,10 +55,32 @@ describe("reportEditorConfigConversionResults", () => { ); }); + it("logs missing setting when there a successful conversion includes them", () => { + // Arrange + const missing = ["setting"]; + const conversionResults = createStubConversionResults({ + successes: new Map([["./one", { ...stubSuccess, missing }]]), + }); + + const logger = createStubLogger(); + + // Act + reportEditorConfigConversionResults({ logger }, conversionResults); + + // Assert + expectEqualWrites( + logger.stdout.write, + `${EOL}✨ 1 editor file augmented with its ESLint equivalent. ✨`, + ``, + `❓ 1 ./one editor setting is not known by tslint-to-eslint-config to have an ESLint equivalent. ❓`, + ` Check stub-output.log for details.`, + ); + }); + it("logs a failed conversion when there is one failed conversion", () => { // Arrange const conversionResults = createStubConversionResults({ - failed: [new Error("It broke.")], + failed: new Map([["./one", new Error("It broke.")]]), }); const logger = createStubLogger(); @@ -71,7 +99,10 @@ describe("reportEditorConfigConversionResults", () => { it("logs failed conversions when there are multiple failed conversions", () => { // Arrange const conversionResults = createStubConversionResults({ - failed: [new Error("It broke."), new Error("It really broke.")], + failed: new Map([ + ["./one", new Error("It broke.")], + ["./two", new Error("It really broke.")], + ]), }); const logger = createStubLogger(); diff --git a/src/converters/editorConfigs/reporting/reportEditorConfigConversionResults.ts b/src/converters/editorConfigs/reporting/reportEditorConfigConversionResults.ts index 3374e554f..537ddb690 100644 --- a/src/converters/editorConfigs/reporting/reportEditorConfigConversionResults.ts +++ b/src/converters/editorConfigs/reporting/reportEditorConfigConversionResults.ts @@ -1,10 +1,10 @@ import { Logger } from "../../../adapters/logger"; -import { logSuccessfulConversions, logFailedConversions } from "../../../reporting"; - -export type EditorSettingConversionResults = { - failed: Error[]; - successes: string[]; -}; +import { + logSuccessfulConversions, + logFailedConversions, + logMissingConversionTarget, +} from "../../../reporting"; +import { EditorConfigsConversionResults } from "../types"; export type ReportEditorConfigConversionResultsDependencies = { logger: Logger; @@ -12,20 +12,31 @@ export type ReportEditorConfigConversionResultsDependencies = { export const reportEditorConfigConversionResults = ( dependencies: ReportEditorConfigConversionResultsDependencies, - results: EditorSettingConversionResults, + results: EditorConfigsConversionResults, ) => { - if (results.successes.length !== 0) { + if (results.successes.size !== 0) { logSuccessfulConversions( "editor file", "augmented", - results.successes.length, + results.successes.size, dependencies.logger, ); + + for (const [filePath, success] of results.successes) { + if (success.missing.length) { + logMissingConversionTarget( + `${filePath} editor setting`, + (editorSetting) => editorSetting, + success.missing, + dependencies.logger, + ); + } + } } - if (results.failed.length !== 0) { + if (results.failed.size !== 0) { logFailedConversions( - results.failed.map((fail) => fail.message), + Array.from(results.failed.values()).map((fail) => fail.message), dependencies.logger, ); } diff --git a/src/converters/editorConfigs/types.ts b/src/converters/editorConfigs/types.ts index a3c85edf2..a1e7b30ec 100644 --- a/src/converters/editorConfigs/types.ts +++ b/src/converters/editorConfigs/types.ts @@ -3,6 +3,16 @@ import { TSLintToESLintSettings } from "../../types"; export type EditorConfigConverter = ( rawEditorSettings: string, settings: TSLintToESLintSettings, -) => string; +) => EditorConfigConversionResults; + +export type EditorConfigConversionResults = { + contents: string; + missing: string[]; +}; + +export type EditorConfigsConversionResults = { + failed: Map; + successes: Map; +}; export type EditorConfigDescriptor = readonly [string, EditorConfigConverter]; diff --git a/src/reporting.ts b/src/reporting.ts index 5b296e604..579ff8cf0 100644 --- a/src/reporting.ts +++ b/src/reporting.ts @@ -57,7 +57,7 @@ export const logMissingConversionTarget = ( missingOutputMapping: (missing: T) => string, missing: T[], logger: Logger, - additionalWarnings: string[], + additionalWarnings: string[] = [], ) => { const headline = missing.length === 1 From 1c20b2e0429a315f71da41b6a7e262a03cbbaa53 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Thu, 15 Oct 2020 16:52:25 -0400 Subject: [PATCH 4/4] Use a more tolerant endsWith --- src/converters/editorConfigs/convertEditorConfigs.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/converters/editorConfigs/convertEditorConfigs.ts b/src/converters/editorConfigs/convertEditorConfigs.ts index d2aec5d3d..7a0f1dc7e 100644 --- a/src/converters/editorConfigs/convertEditorConfigs.ts +++ b/src/converters/editorConfigs/convertEditorConfigs.ts @@ -61,5 +61,5 @@ export const convertEditorConfigs = async ( }; }; -const defaultPathMatches = (defaultPath: string, otherPath: string) => - defaultPath.replace(/\W+/g, "") === otherPath.replace(/\W+/g, ""); +const defaultPathMatches = (defaultPath: string, requestedPath: string) => + requestedPath.replace(/\W+/g, "").endsWith(defaultPath.replace(/\W+/g, ""));