Eslint rule merging logic (#30)

Josh Goldberg · web-flow · commit 691d3ed911dd · 2019-06-26T07:54:18.000-04:00
* Always included typescript-eslint parser Fixes #12. * Added ruleArguments mergers support Starts off with ban-types and no-unnecessary-type-assertion.
diff --git 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 = (
@@ -51,6 +52,7 @@ export const convertConfig = async ({
             ...value,
         })),
         converters,
+        mergers,
     );
 
     await createNewConfiguration(convertedRules, originalConfiguration);
diff --git 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
@@ -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) {
diff --git a/src/rules/merger.ts 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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"] }]);
+    });
+});
