Eslint rule merging logic

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<void>;
@@ -48,6 +49,7 @@ export const convertConfig = async ({
             ...value,
         })),
         converters,
+        mergers,
     );
 
     await createNewConfiguration(convertedRules);

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",

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),
     };

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"]));

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<string, ESLintRuleOptions>;
@@ -14,6 +15,7 @@ export type ConfigConversionResults = {
 export const convertRules = (
     tslintRules: TSLintRuleOptions[],
     converters: Map<string, RuleConverter>,
+    mergers: Map<string, RuleMerger>,
 ): ConfigConversionResults => {
     const converted = new Map<string, ESLintRuleOptions>();
     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) {

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[];

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],
+]);

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),
+            },
+        },
+    ];
+};

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 }),
+        },
+    ];
+};

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" } }]);
+    });
+});