diff --git a/src/convertConfig.ts b/src/convertConfig.ts index 15e9b3030..348a239b5 100644 --- a/src/convertConfig.ts +++ b/src/convertConfig.ts @@ -3,6 +3,7 @@ import { ProcessLogger } from "./logger"; import { reportConversionResults } from "./reportConversionResults"; import { convertRules, ConfigConversionResults } from "./rules/convertRules"; import { converters } from "./rules/converters"; +import { mergers } from "./rules/mergers"; import { TSLintToESLintSettings, TSLintToESLintResult, ResultStatus } from "./types"; export type CreateNewConfiguration = (conversionResults: ConfigConversionResults) => Promise; @@ -48,6 +49,7 @@ export const convertConfig = async ({ ...value, })), converters, + mergers, ); await createNewConfiguration(convertedRules); diff --git a/src/creation/createNewConfiguration.test.ts b/src/creation/createNewConfiguration.test.ts index c6843057d..1813313e9 100644 --- a/src/creation/createNewConfiguration.test.ts +++ b/src/creation/createNewConfiguration.test.ts @@ -3,7 +3,7 @@ import { createNewConfiguration } from "./createNewConfiguration"; import { ConfigConversionResults } from "../rules/convertRules"; describe("createNewConfiguration", () => { - it("writes only formatted rules when there are no missing rules", async () => { + it("excludes the tslint plugin when there are no missing rules", async () => { // Arrange const conversionResults: ConfigConversionResults = { ...emptyConversionResults, @@ -17,7 +17,17 @@ describe("createNewConfiguration", () => { // Assert expect(writeFile).toHaveBeenLastCalledWith( ".eslintrc.json", - JSON.stringify({ rules: {} }, undefined, 4), + JSON.stringify( + { + parser: "@typescript-eslint/parser", + parserOptions: { + project: "tsconfig.json", + }, + rules: {}, + }, + undefined, + 4, + ), ); }); @@ -44,11 +54,11 @@ describe("createNewConfiguration", () => { ".eslintrc.json", JSON.stringify( { - plugins: ["@typescript-eslint/tslint"], parser: "@typescript-eslint/parser", parserOptions: { project: "tsconfig.json", }, + plugins: ["@typescript-eslint/tslint"], rules: { "@typescript-eslint/tslint/config": [ "error", diff --git a/src/creation/createNewConfiguration.ts b/src/creation/createNewConfiguration.ts index e637dcefc..c02cfa26c 100644 --- a/src/creation/createNewConfiguration.ts +++ b/src/creation/createNewConfiguration.ts @@ -8,12 +8,12 @@ export const createNewConfiguration = async ( writeFile: WriteFile, ) => { const output = { + parser: "@typescript-eslint/parser", + parserOptions: { + project: "tsconfig.json", + }, ...(conversionResults.missing.length && { plugins: ["@typescript-eslint/tslint"], - parser: "@typescript-eslint/parser", - parserOptions: { - project: "tsconfig.json", - }, }), rules: formatConvertedRules(conversionResults), }; diff --git a/src/rules/convertRules.test.ts b/src/rules/convertRules.test.ts index 7dd246d2a..e27bcaa13 100644 --- a/src/rules/convertRules.test.ts +++ b/src/rules/convertRules.test.ts @@ -11,9 +11,10 @@ describe("convertRules", () => { ruleSeverity: "off", }; const converters = new Map(); + const mergers = new Map(); // Act - const { missing } = convertRules([tslintRule], converters); + const { missing } = convertRules([tslintRule], converters, mergers); // Assert expect(missing).toEqual([]); @@ -27,9 +28,10 @@ describe("convertRules", () => { ruleSeverity: "error", }; const converters = new Map(); + const mergers = new Map(); // Act - const { missing } = convertRules([tslintRule], converters); + const { missing } = convertRules([tslintRule], converters, mergers); // Assert expect(missing).toEqual([tslintRule]); @@ -44,9 +46,10 @@ describe("convertRules", () => { }; const conversionError = new ConversionError(new Error(), tslintRule); const converters = new Map([[tslintRule.ruleName, () => conversionError]]); + const mergers = new Map(); // Act - const { failed } = convertRules([tslintRule], converters); + const { failed } = convertRules([tslintRule], converters, mergers); // Assert expect(failed).toEqual([conversionError]); @@ -67,9 +70,10 @@ describe("convertRules", () => { ], }; const converters = new Map([[tslintRule.ruleName, () => conversionResult]]); + const mergers = new Map(); // Act - const { converted } = convertRules([tslintRule], converters); + const { converted } = convertRules([tslintRule], converters, mergers); // Assert expect(converted).toEqual( @@ -85,6 +89,81 @@ describe("convertRules", () => { ); }); + it("reports a failure when two outputs exist for a converted rule without a merger", () => { + // Arrange + const tslintRule: TSLintRuleOptions = { + ruleArguments: [], + ruleName: "tslint-rule-a", + ruleSeverity: "error", + }; + const conversionResult = { + rules: [ + { + ruleName: "eslint-rule-a", + }, + { + ruleName: "eslint-rule-a", + }, + ], + }; + const converters = new Map([[tslintRule.ruleName, () => conversionResult]]); + const mergers = new Map(); + + // Act + const { failed } = convertRules([tslintRule], converters, mergers); + + // Assert + expect(failed).toEqual( + jasmine.arrayContaining([ + jasmine.objectContaining({ + error: jasmine.objectContaining({ + message: `No merger for multiple output eslint-rule-a rule configurations.`, + }), + tslintRule, + }), + ]), + ); + }); + + it("merges rule arguments two outputs exist for a converted rule with a merger", () => { + // Arrange + const tslintRule: TSLintRuleOptions = { + ruleArguments: [], + ruleName: "tslint-rule-a", + ruleSeverity: "error", + }; + const conversionResult = { + rules: [ + { + ruleName: "eslint-rule-a", + }, + { + ruleName: "eslint-rule-a", + }, + ], + }; + const mergedArguments = [{ merged: true }]; + const converters = new Map([[tslintRule.ruleName, () => conversionResult]]); + const mergers = new Map([[conversionResult.rules[0].ruleName, () => mergedArguments]]); + + // Act + const { converted } = convertRules([tslintRule], converters, mergers); + + // Assert + expect(converted).toEqual( + new Map([ + [ + "eslint-rule-a", + { + ruleArguments: mergedArguments, + ruleName: "eslint-rule-a", + ruleSeverity: "error", + }, + ], + ]), + ); + }); + it("marks a new package when a conversion has a new package", () => { // Arrange const tslintRule: TSLintRuleOptions = { @@ -97,9 +176,10 @@ describe("convertRules", () => { rules: [], }; const converters = new Map([[tslintRule.ruleName, () => conversionResult]]); + const mergers = new Map(); // Act - const { packages } = convertRules([tslintRule], converters); + const { packages } = convertRules([tslintRule], converters, mergers); // Assert expect(packages).toEqual(new Set(["extra-package"])); diff --git a/src/rules/convertRules.ts b/src/rules/convertRules.ts index fcbfc344b..da6928ab9 100644 --- a/src/rules/convertRules.ts +++ b/src/rules/convertRules.ts @@ -3,6 +3,7 @@ import { convertRule } from "./convertRule"; import { convertRuleSeverity } from "./convertRuleSeverity"; import { TSLintRuleOptions, ESLintRuleOptions } from "./types"; import { RuleConverter } from "./converter"; +import { RuleMerger } from "./merger"; export type ConfigConversionResults = { converted: Map; @@ -14,6 +15,7 @@ export type ConfigConversionResults = { export const convertRules = ( tslintRules: TSLintRuleOptions[], converters: Map, + mergers: Map, ): ConfigConversionResults => { const converted = new Map(); const failed: ConversionError[] = []; @@ -36,10 +38,36 @@ export const convertRules = ( } for (const changes of conversion.rules) { - converted.set(changes.ruleName, { + const existingConversion = converted.get(changes.ruleName); + const newConversion = { ...changes, ruleSeverity: convertRuleSeverity(tslintRule.ruleSeverity), - }); + }; + + if (existingConversion === undefined) { + converted.set(changes.ruleName, newConversion); + continue; + } + + const merger = mergers.get(changes.ruleName); + if (merger === undefined) { + failed.push( + new ConversionError( + new Error( + `No merger for multiple output ${changes.ruleName} rule configurations.`, + ), + tslintRule, + ), + ); + } else { + converted.set(changes.ruleName, { + ...existingConversion, + ruleArguments: merger( + existingConversion.ruleArguments, + newConversion.ruleArguments, + ), + }); + } } if (conversion.packages !== undefined) { diff --git a/src/rules/merger.ts b/src/rules/merger.ts new file mode 100644 index 000000000..3e54122e6 --- /dev/null +++ b/src/rules/merger.ts @@ -0,0 +1,7 @@ +/** + * Merges two ESLint rule result outputs to a single result. + */ +export type RuleMerger = ( + existingOptions: any[] | undefined, + newOptions: any[] | undefined, +) => any[]; diff --git a/src/rules/mergers.ts b/src/rules/mergers.ts new file mode 100644 index 000000000..583f3d41b --- /dev/null +++ b/src/rules/mergers.ts @@ -0,0 +1,7 @@ +import { mergeBanTypes } from "./mergers/ban-types"; +import { mergeNoUnnecessaryTypeAssertion } from "./mergers/no-unnecessary-type-assertion"; + +export const mergers = new Map([ + ["@typescript-eslint/ban-types", mergeBanTypes], + ["@typescript-eslint/no-unnecessary-type-assertion", mergeNoUnnecessaryTypeAssertion], +]); diff --git a/src/rules/mergers/ban-types.ts b/src/rules/mergers/ban-types.ts new file mode 100644 index 000000000..d5c06e338 --- /dev/null +++ b/src/rules/mergers/ban-types.ts @@ -0,0 +1,16 @@ +import { RuleMerger } from "../merger"; + +export const mergeBanTypes: RuleMerger = (existingOptions, newOptions) => { + if (existingOptions === undefined && newOptions === undefined) { + return []; + } + + return [ + { + types: { + ...(existingOptions && existingOptions[0].types), + ...(newOptions && newOptions[0].types), + }, + }, + ]; +}; diff --git a/src/rules/mergers/no-unnecessary-type-assertion.ts b/src/rules/mergers/no-unnecessary-type-assertion.ts new file mode 100644 index 000000000..fc230e17c --- /dev/null +++ b/src/rules/mergers/no-unnecessary-type-assertion.ts @@ -0,0 +1,31 @@ +import { RuleMerger } from "../merger"; + +export const mergeNoUnnecessaryTypeAssertion: RuleMerger = (existingOptions, newOptions) => { + if (existingOptions === undefined && newOptions === undefined) { + return []; + } + + let typesToIgnore: string[] | undefined; + + for (const options of [existingOptions, newOptions]) { + if ( + options === undefined || + options.length === 0 || + options[0].typesToIgnore === undefined + ) { + continue; + } + + if (typesToIgnore === undefined) { + typesToIgnore = []; + } + + typesToIgnore.push(...options[0].typesToIgnore); + } + + return [ + { + ...(typesToIgnore !== undefined && { typesToIgnore }), + }, + ]; +}; diff --git a/src/rules/mergers/tests/ban-types.test.ts b/src/rules/mergers/tests/ban-types.test.ts new file mode 100644 index 000000000..e08f7809f --- /dev/null +++ b/src/rules/mergers/tests/ban-types.test.ts @@ -0,0 +1,27 @@ +import { mergeBanTypes } from "../ban-types"; + +describe(mergeBanTypes, () => { + test("neither types existing", () => { + const result = mergeBanTypes(undefined, undefined); + + expect(result).toEqual([]); + }); + + test("original type existing", () => { + const result = mergeBanTypes([{ types: { a: "original" } }], undefined); + + expect(result).toEqual([{ types: { a: "original" } }]); + }); + + test("new type existing", () => { + const result = mergeBanTypes(undefined, [{ types: { b: "new" } }]); + + expect(result).toEqual([{ types: { b: "new" } }]); + }); + + test("both types existing", () => { + const result = mergeBanTypes([{ types: { a: "original" } }], [{ types: { b: "new" } }]); + + expect(result).toEqual([{ types: { a: "original", b: "new" } }]); + }); +}); diff --git a/src/rules/mergers/tests/no-unnecessary-type-assertion.test.ts b/src/rules/mergers/tests/no-unnecessary-type-assertion.test.ts new file mode 100644 index 000000000..ff9f747f0 --- /dev/null +++ b/src/rules/mergers/tests/no-unnecessary-type-assertion.test.ts @@ -0,0 +1,60 @@ +import { mergeNoUnnecessaryTypeAssertion } from "../no-unnecessary-type-assertion"; + +describe(mergeNoUnnecessaryTypeAssertion, () => { + test("neither options existing", () => { + const result = mergeNoUnnecessaryTypeAssertion(undefined, undefined); + + expect(result).toEqual([]); + }); + + test("neither typesToIgnore existing", () => { + const result = mergeNoUnnecessaryTypeAssertion([{}], [{}]); + + expect(result).toEqual([{}]); + }); + + test("original typesToIgnore existing but empty", () => { + const result = mergeNoUnnecessaryTypeAssertion([{ typesToIgnore: [] }], undefined); + + expect(result).toEqual([{ typesToIgnore: [] }]); + }); + + test("original typesToIgnore existing", () => { + const result = mergeNoUnnecessaryTypeAssertion( + [{ typesToIgnore: ["original"] }], + undefined, + ); + + expect(result).toEqual([{ typesToIgnore: ["original"] }]); + }); + + test("new typesToIgnore existing but empty", () => { + const result = mergeNoUnnecessaryTypeAssertion(undefined, [{ typesToIgnore: [] }]); + + expect(result).toEqual([{ typesToIgnore: [] }]); + }); + + test("new typesToIgnore existing", () => { + const result = mergeNoUnnecessaryTypeAssertion(undefined, [{ typesToIgnore: ["new"] }]); + + expect(result).toEqual([{ typesToIgnore: ["new"] }]); + }); + + test("both typesToIgnore existing but empty", () => { + const result = mergeNoUnnecessaryTypeAssertion( + [{ typesToIgnore: [] }], + [{ typesToIgnore: [] }], + ); + + expect(result).toEqual([{ typesToIgnore: [] }]); + }); + + test("both typesToIgnore existing", () => { + const result = mergeNoUnnecessaryTypeAssertion( + [{ typesToIgnore: ["original"] }], + [{ typesToIgnore: ["new"] }], + ); + + expect(result).toEqual([{ typesToIgnore: ["original", "new"] }]); + }); +});