Improved error message and issue template for missing rule mergers

.eslintrc.js

@@ -18,10 +18,11 @@ module.exports = {
     },
     plugins: ["@typescript-eslint"],
     rules: {
-        "@typescript-eslint/consistent-type-definitions": ["error", "type"],
+        "@typescript-eslint/consistent-type-definitions": 0,
         "@typescript-eslint/explicit-function-return-type": 0,
         "@typescript-eslint/generic-type-naming": 0,
         "@typescript-eslint/indent": 0,
+        "@typescript-eslint/member-ordering": 0,
         "@typescript-eslint/no-explicit-any": 0,
         "@typescript-eslint/no-extra-parens": 0,
         "@typescript-eslint/no-magic-numbers": 0,

.github/ISSUE_TEMPLATE/missing_merger.md

@@ -0,0 +1,31 @@
+---
+name: Missing Merger
+about: Report a missing ESLint rule merger in tslint-to-eslint-config
+---
+
+<!--
+👋 Hi, thanks for filing an issue on tslint-to-eslint-config! 💖
+Please fill out all fields below to ensure your issue is addressed.
+
+If your issue doesn't provide enough info to fully explain or reproduce your bug, it will be closed. 😦
+-->
+
+### 💥 Missing Merger
+
+-   `tslint-to-eslint-config` version: X.X.X
+-   ESLint version: X.X.X
+
+#### CLI Output
+
+<!-- What did tslint-to-eslint-config print to the screen?  -->
+
+#### File Output
+
+<!-- What did tslint-to-eslint-config print to tslint-to-eslint-config.log?  -->
+
+#### Reproduction
+
+<!--
+Please paste a link to a repository, Gist, or other means of reproducing your error here.
+Note that `tslint-to-eslint-config` includes information from `package.json`, `tsconfig.json`, and other files, so just your TSLint configuration may not be enough.
+-->

src/errors/configurationError.test.ts

@@ -0,0 +1,20 @@
+import { EOL } from "os";
+
+import { ConfigurationError } from "./configurationError";
+
+describe(ConfigurationError, () => {
+    describe("getSummary", () => {
+        it("creates a summary for the error", () => {
+            // Arrange
+            const rawError = new Error("Oh no!");
+            const complaint = "It broke";
+            const error = new ConfigurationError(rawError, complaint);
+
+            // Act
+            const summary = error.getSummary();
+
+            // Assert
+            expect(summary).toEqual(`It broke: ${rawError.stack}${EOL}`);
+        });
+    });
+});

src/errors/configurationError.ts

@@ -1,3 +1,11 @@
-export class ConfigurationError {
-    public constructor(public readonly error: Error, public readonly complaint: string) {}
+import { EOL } from "os";
+
+import { ErrorSummary } from "./errorSummary";
+
+export class ConfigurationError implements ErrorSummary {
+    public constructor(private readonly error: Error, private readonly complaint: string) {}
+
+    public getSummary(): string {
+        return `${this.complaint}: ${this.error.stack}${EOL}`;
+    }
 }

src/errors/conversionError.test.ts

@@ -0,0 +1,40 @@
+import { EOL } from "os";
+
+import { ConversionError } from "./conversionError";
+import { TSLintRuleOptions } from "../rules/types";
+
+describe(ConversionError, () => {
+    describe("getSummary", () => {
+        it("prints the error stack when created for an error", () => {
+            // Arrange
+            const error = new Error("Oh no!");
+            const tslintRule: TSLintRuleOptions = {
+                ruleArguments: [],
+                ruleName: "rule-a",
+                ruleSeverity: "warning",
+            };
+            const conversionError = ConversionError.forRuleError(error, tslintRule);
+
+            // Act
+            const summary = conversionError.getSummary();
+
+            // Assert
+            expect(summary).toEqual(
+                `rule-a threw an error during conversion: ${error.stack}${EOL}`,
+            );
+        });
+
+        it("prints a merger complaint when created for a merger", () => {
+            // Arrange
+            const conversionError = ConversionError.forMerger("eslint-rule");
+
+            // Act
+            const summary = conversionError.getSummary();
+
+            // Assert
+            expect(summary).toEqual(
+                `Error: multiple output eslint-rule ESLint rule options were generated, but tslint-to-eslint-config doesn't have "merger" logic to deal with this.${EOL}Please file an issue at https://github.com/typescript-eslint/tslint-to-eslint-config/issues/new?template=bug_report.md!`,
+            );
+        });
+    });
+});

src/errors/conversionError.ts

@@ -1,8 +1,27 @@
+import { EOL } from "os";
+
 import { TSLintRuleOptions } from "../rules/types";
+import { ErrorSummary } from "./errorSummary";
+
+export class ConversionError implements ErrorSummary {
+    private constructor(private readonly summary: string) {}
+
+    public static forMerger = (eslintRule: string) => {
+        return new ConversionError(
+            [
+                `Error: multiple output ${eslintRule} ESLint rule options were generated, but tslint-to-eslint-config doesn't have "merger" logic to deal with this.`,
+                `Please file an issue at https://github.com/typescript-eslint/tslint-to-eslint-config/issues/new?template=missing_merger.md 🙏`,
+            ].join(EOL),
+        );
+    };
+
+    public getSummary(): string {
+        return this.summary;
+    }
 
-export class ConversionError {
-    public constructor(
-        public readonly error: Error,
-        public readonly tslintRule: TSLintRuleOptions,
-    ) {}
+    public static forRuleError = (error: Error, tslintRule: TSLintRuleOptions) => {
+        return new ConversionError(
+            `${tslintRule.ruleName} threw an error during conversion: ${error.stack}${EOL}`,
+        );
+    };
 }

src/errors/errorSummary.ts

@@ -0,0 +1,3 @@
+export interface ErrorSummary {
+    getSummary(): string;
+}

src/reporting/reportConversionResults.test.ts

@@ -1,10 +1,9 @@
 import { EOL } from "os";
-import { ESLintRuleOptions } from "../rules/types";
-import { reportConversionResults } from "./reportConversionResults";
+
 import { createStubLogger, expectEqualWrites } from "../adapters/logger.stubs";
 import { createEmptyConversionResults } from "../conversion/conversionResults.stubs";
-import { ConversionError } from "../errors/conversionError";
-import { ConfigurationError } from "../errors/configurationError";
+import { ESLintRuleOptions } from "../rules/types";
+import { reportConversionResults } from "./reportConversionResults";
 
 describe("reportConversionResults", () => {
     it("logs a successful conversion without notices when there is one converted rule without notices", () => {
@@ -110,35 +109,10 @@ describe("reportConversionResults", () => {
         );
     });
 
-    it("logs a failed configuration when there is one failed configuration error", () => {
-        // Arrange
-        const conversionResults = createEmptyConversionResults({
-            failed: [new ConfigurationError(new Error("and a one"), "some complaint")],
-        });
-
-        const logger = createStubLogger();
-
-        // Act
-        reportConversionResults({ logger }, conversionResults);
-
-        // Assert
-        expectEqualWrites(
-            logger.stderr.write,
-            "💀 1 error thrown. 💀",
-            `Check ${logger.debugFileName} for details.`,
-        );
-    });
-
     it("logs a failed conversion when there is one failed conversion", () => {
         // Arrange
         const conversionResults = createEmptyConversionResults({
-            failed: [
-                new ConversionError(new Error("and a one"), {
-                    ruleArguments: ["a", "b"],
-                    ruleName: "tslint-rule-one",
-                    ruleSeverity: "error",
-                }),
-            ],
+            failed: [{ getSummary: () => "It broke." }],
         });
 
         const logger = createStubLogger();
@@ -157,18 +131,7 @@ describe("reportConversionResults", () => {
     it("logs failed conversions when there are multiple failed conversions", () => {
         // Arrange
         const conversionResults = createEmptyConversionResults({
-            failed: [
-                new ConversionError(new Error("and a one"), {
-                    ruleArguments: ["a", "b"],
-                    ruleName: "tslint-rule-one",
-                    ruleSeverity: "error",
-                }),
-                new ConversionError(new Error("and a two"), {
-                    ruleArguments: ["c", "d"],
-                    ruleName: "tslint-rule-two",
-                    ruleSeverity: "warning",
-                }),
-            ],
+            failed: [{ getSummary: () => "It broke." }, { getSummary: () => "It really broke." }],
         });
 
         const logger = createStubLogger();

src/reporting/reportConversionResults.ts

@@ -2,8 +2,7 @@ import chalk from "chalk";
 import { EOL } from "os";
 
 import { Logger } from "../adapters/logger";
-import { ConfigurationError } from "../errors/configurationError";
-import { ConversionError } from "../errors/conversionError";
+import { ErrorSummary } from "../errors/errorSummary";
 import { RuleConversionResults } from "../rules/convertRules";
 import { TSLintRuleOptions, ESLintRuleOptions } from "../rules/types";
 
@@ -43,21 +42,13 @@ const logSuccessfulConversions = (converted: Map<string, ESLintRuleOptions>, log
     logger.stdout.write(chalk.greenBright(` ✨${EOL}`));
 };
 
-const logFailedConversions = (failed: (ConfigurationError | ConversionError)[], logger: Logger) => {
+const logFailedConversions = (failed: ErrorSummary[], logger: Logger) => {
     logger.stderr.write(`${chalk.redBright(`💀 ${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(failed =>
-                failed instanceof ConfigurationError
-                    ? `${failed.complaint}: ${failed.error.stack}${EOL}`
-                    : `${failed.tslintRule.ruleName} threw an error during conversion: ${failed.error.stack}${EOL}`,
-            )
-            .join(""),
-    );
+    logger.info.write(failed.map(failed => failed.getSummary()).join(""));
 
     logger.stderr.write(chalk.gray(`Check ${logger.debugFileName} for details.${EOL}`));
 };

src/rules/convertRule.test.ts

@@ -68,6 +68,6 @@ describe("convertRule", () => {
         const result = convertRule(tslintRule, converters);
 
         // Assert
-        expect(result).toEqual(new ConversionError(error, tslintRule));
+        expect(result).toEqual(ConversionError.forRuleError(error, tslintRule));
     });
 });

src/rules/convertRule.ts

@@ -14,6 +14,6 @@ export const convertRule = (
     try {
         return converter(tslintRule);
     } catch (error) {
-        return new ConversionError(error, tslintRule);
+        return ConversionError.forRuleError(error, tslintRule);
     }
 };

src/rules/convertRules.test.ts

@@ -50,7 +50,7 @@ describe("convertRules", () => {
             ruleName: "tslint-rule-a",
             ruleSeverity: "error",
         };
-        const conversionError = new ConversionError(new Error(), tslintRule);
+        const conversionError = ConversionError.forRuleError(new Error(), tslintRule);
         const converters = new Map([[tslintRule.ruleName, () => conversionError]]);
         const mergers = new Map();
 
@@ -128,16 +128,7 @@ describe("convertRules", () => {
         );
 
         // Assert
-        expect(failed).toEqual(
-            jasmine.arrayContaining([
-                jasmine.objectContaining({
-                    error: jasmine.objectContaining({
-                        message: `No merger for multiple output eslint-rule-a rule configurations.`,
-                    }),
-                    tslintRule,
-                }),
-            ]),
-        );
+        expect(failed).toEqual([ConversionError.forMerger("eslint-rule-a")]);
     });
 
     it("merges rule arguments two outputs exist for a converted rule with a merger", () => {

src/rules/convertRules.ts

@@ -1,5 +1,5 @@
-import { ConfigurationError } from "../errors/configurationError";
 import { ConversionError } from "../errors/conversionError";
+import { ErrorSummary } from "../errors/errorSummary";
 import { TSLintConfigurationRules } from "../input/findTSLintConfiguration";
 import { RuleConverter } from "./converter";
 import { convertRule } from "./convertRule";
@@ -15,7 +15,7 @@ export type ConvertRulesDependencies = {
 
 export type RuleConversionResults = {
     converted: Map<string, ESLintRuleOptions>;
-    failed: (ConfigurationError | ConversionError)[];
+    failed: ErrorSummary[];
     missing: TSLintRuleOptions[];
     plugins: Set<string>;
 };
@@ -60,14 +60,7 @@ export const convertRules = (
 
             const merger = dependencies.mergers.get(changes.ruleName);
             if (merger === undefined) {
-                failed.push(
-                    new ConversionError(
-                        new Error(
-                            `No merger for multiple output ${changes.ruleName} rule configurations.`,
-                        ),
-                        tslintRule,
-                    ),
-                );
+                failed.push(ConversionError.forMerger(changes.ruleName));
             } else {
                 const existingNotices = existingConversion.notices || [];
                 const newNotices = newConversion.notices || [];

src/rules/converter.ts

@@ -1,5 +1,5 @@
-import { ConversionError } from "../errors/conversionError";
 import { TSLintRuleOptions } from "./types";
+import { ConversionError } from "../errors/conversionError";
 
 /**
  * Section of a TSLint rule's options used for conversion.

src/rules/converters/member-ordering.ts

@@ -4,7 +4,7 @@ export const convertMemberOrdering: RuleConverter = () => {
     return {
         rules: [
             {
-                ruleName: "@typescript-eslint/member-ordering",
+                ruleName: "some-rule",
             },
         ],
     };

src/rules/converters/no-eval.ts

@@ -4,7 +4,7 @@ export const convertNoEval: RuleConverter = () => {
     return {
         rules: [
             {
-                ruleName: "no-eval",
+                ruleName: "some-rule",
             },
         ],
     };