Respect ESLint extensions in deduplicating rules and config writing (#78)

* Respect ESLint extensions in deduplicating rules and config writing

The original ESLint configuration is now included in the `writeConversionResults` written file with a single `...`.

Additionally skipped outputting rules that match an existing ESLint plugin. If an extended ESLint plugin defines a rule with the same values as the resultant config, the config will skip printing it.
This necessitated expanding "failures" to allow a new `ConfigurationError` type of error for unfound ESLint configurations.

Renames "packages" in the configuration reading/converting logic areas to "plugins", as that's what ESLint calls them.

* Used formatJsonOutput in merged test

Author: Josh Goldberg

.eslintrc.js

@@ -17,18 +17,19 @@ module.exports = {
         "@typescript-eslint/no-use-before-define": 0,
         "@typescript-eslint/prefer-interface": 0,
         "default-case": 0,
+        "guard-for-in": 0,
         "import/no-extraneous-dependencies": [
             "error",
             { devDependencies: ["**/*.stubs.ts", "**/*.test.*"] },
         ],
         "import/first": 0,
-        "import/newline-after-import": 0,
         "import/no-unresolved": 0,
         "import/prefer-default-export": 0,
         "no-console": 0,
         "no-continue": 0,
         "no-empty-function": 0,
         "no-restricted-syntax": 0,
+        "no-param-reassign": 0,
         "no-shadow": 0,
         "no-undef": 0,
         "no-useless-constructor": 0,

README.md

@@ -66,7 +66,7 @@ npx tslint-to-eslint-config --eslint ./path/to/eslintrc.js
 _Default: `--config`'s value_
 
 Path to an ESLint configuration file to read settings from.
-This isn't yet used for anything, but will eventually inform settings for the generated ESLint configuration file.
+The generated ESLint configuration file will include any settings `import`ed from this file.
 
 #### `package`
 

src/adapters/importer.ts

@@ -0,0 +1,9 @@
+export type Importer = (moduleName: string) => unknown | Error;
+
+export const nativeImporter = async (moduleName: string) => {
+    try {
+        return await import(moduleName);
+    } catch (error) {
+        return error;
+    }
+};

src/cli/main.ts

@@ -1,11 +1,20 @@
 import { EOL } from "os";
 
-import { bind } from "../binding";
-import { runCli, RunCliDependencies } from "./runCli";
+import { nativeImporter } from "../adapters/importer";
 import { processLogger } from "../adapters/processLogger";
 import { childProcessExec } from "../adapters/childProcessExec";
 import { fsFileSystem } from "../adapters/fsFileSystem";
+import { bind } from "../binding";
 import { ConvertConfigDependencies, convertConfig } from "../conversion/convertConfig";
+import { removeExtendsDuplicatedRules } from "../creation/simplification/removeExtendsDuplicatedRules";
+import {
+    RetrieveExtendsValuesDependencies,
+    retrieveExtendsValues,
+} from "../creation/simplification/retrieveExtendsValues";
+import {
+    simplifyPackageRules,
+    SimplifyPackageRulesDependencies,
+} from "../creation/simplification/simplifyPackageRules";
 import {
     writeConversionResults,
     WriteConversionResultsDependencies,
@@ -14,6 +23,7 @@ import {
     findOriginalConfigurations,
     FindOriginalConfigurationsDependencies,
 } from "../input/findOriginalConfigurations";
+import { findPackagesConfiguration } from "../input/findPackagesConfiguration";
 import { findESLintConfiguration } from "../input/findESLintConfiguration";
 import { findTSLintConfiguration } from "../input/findTSLintConfiguration";
 import { findTypeScriptConfiguration } from "../input/findTypeScriptConfiguration";
@@ -24,7 +34,7 @@ import {
 import { converters } from "../rules/converters";
 import { convertRules } from "../rules/convertRules";
 import { mergers } from "../rules/mergers";
-import { findPackagesConfiguration } from "../input/findPackagesConfiguration";
+import { runCli, RunCliDependencies } from "./runCli";
 
 const convertRulesDependencies = {
     converters,
@@ -46,6 +56,15 @@ const reportConversionResultsDependencies: ReportConversionResultsDependencies =
     logger: processLogger,
 };
 
+const retrieveExtendsValuesDependencies: RetrieveExtendsValuesDependencies = {
+    importer: nativeImporter,
+};
+
+const simplifyPackageRulesDependencies: SimplifyPackageRulesDependencies = {
+    removeExtendsDuplicatedRules,
+    retrieveExtendsValues: bind(retrieveExtendsValues, retrieveExtendsValuesDependencies),
+};
+
 const writeConversionResultsDependencies: WriteConversionResultsDependencies = {
     fileSystem: fsFileSystem,
 };
@@ -57,6 +76,7 @@ const convertConfigDependencies: ConvertConfigDependencies = {
         findOriginalConfigurationsDependencies,
     ),
     reportConversionResults: bind(reportConversionResults, reportConversionResultsDependencies),
+    simplifyPackageRules: bind(simplifyPackageRules, simplifyPackageRulesDependencies),
     writeConversionResults: bind(writeConversionResults, writeConversionResultsDependencies),
 };
 

src/conversion/conversionResults.stubs.ts

@@ -1,11 +1,11 @@
 import { RuleConversionResults } from "../rules/convertRules";
 
 export const createEmptyConversionResults = (
-    overrides: Partial<RuleConversionResults>,
+    overrides: Partial<RuleConversionResults> = {},
 ): RuleConversionResults => ({
     converted: new Map(),
     failed: [],
     missing: [],
-    packages: new Set(),
+    plugins: new Set(),
     ...overrides,
 });

src/conversion/convertConfig.test.ts

@@ -4,9 +4,10 @@ import { OriginalConfigurations } from "../input/findOriginalConfigurations";
 
 const createStubDependencies = (
     overrides: Pick<ConvertConfigDependencies, "findOriginalConfigurations">,
-) => ({
+): ConvertConfigDependencies => ({
     convertRules: jest.fn(),
     reportConversionResults: jest.fn(),
+    simplifyPackageRules: async (_configurations, data) => data,
     writeConversionResults: jest.fn().mockReturnValue(Promise.resolve()),
     ...overrides,
 });

src/conversion/convertConfig.ts

@@ -1,4 +1,5 @@
 import { SansDependencies } from "../binding";
+import { simplifyPackageRules } from "../creation/simplification/simplifyPackageRules";
 import { writeConversionResults } from "../creation/writeConversionResults";
 import { findOriginalConfigurations } from "../input/findOriginalConfigurations";
 import { reportConversionResults } from "../reporting/reportConversionResults";
@@ -9,6 +10,7 @@ export type ConvertConfigDependencies = {
     convertRules: SansDependencies<typeof convertRules>;
     findOriginalConfigurations: SansDependencies<typeof findOriginalConfigurations>;
     reportConversionResults: SansDependencies<typeof reportConversionResults>;
+    simplifyPackageRules: SansDependencies<typeof simplifyPackageRules>;
     writeConversionResults: SansDependencies<typeof writeConversionResults>;
 };
 
@@ -24,13 +26,20 @@ export const convertConfig = async (
     const ruleConversionResults = dependencies.convertRules(
         originalConfigurations.data.tslint.rules,
     );
+    const mergedConfiguration = {
+        ...ruleConversionResults,
+        ...(await dependencies.simplifyPackageRules(
+            originalConfigurations.data.eslint,
+            ruleConversionResults,
+        )),
+    };
 
     await dependencies.writeConversionResults(
         settings.config,
-        ruleConversionResults,
+        mergedConfiguration,
         originalConfigurations.data,
     );
-    dependencies.reportConversionResults(ruleConversionResults);
+    dependencies.reportConversionResults(mergedConfiguration);
 
     return {
         status: ResultStatus.Succeeded,

src/creation/simplification/removeExtendsDuplicatedRules.test.ts

@@ -0,0 +1,165 @@
+import {
+    ESLintConfiguration,
+    ESLintConfigurationRuleValue,
+} from "../../input/findESLintConfiguration";
+import { ESLintRuleOptions } from "../../rules/types";
+import { removeExtendsDuplicatedRules } from "./removeExtendsDuplicatedRules";
+
+const prepareTestRule = (
+    ruleOptions: Partial<ESLintRuleOptions>,
+    extensionConfiguration: ESLintConfigurationRuleValue = 2,
+) => {
+    const ruleName = "rule-a";
+    const allRules = new Map<string, ESLintRuleOptions>([
+        [
+            ruleName,
+            {
+                ruleArguments: [],
+                ruleName,
+                ruleSeverity: "off",
+                ...ruleOptions,
+            },
+        ],
+    ]);
+    const extensions: Partial<ESLintConfiguration>[] = [
+        {
+            rules: {
+                [ruleName]: extensionConfiguration,
+            },
+        },
+    ];
+
+    return { ruleName, allRules, extensions };
+};
+
+describe("removeExtendsDuplicatedRules", () => {
+    it("keeps a rule when there are no rules in the extension", () => {
+        // Arrange
+        const { allRules } = prepareTestRule(
+            {
+                ruleName: "mismatched",
+            },
+            2,
+        );
+
+        // Act
+        const differentRules = removeExtendsDuplicatedRules(allRules, [{}]);
+
+        // Assert
+        expect(differentRules.size).toBe(1);
+    });
+
+    it("keeps a rule when it doesn't match any existing rule", () => {
+        // Arrange
+        const { allRules, extensions } = prepareTestRule(
+            {
+                ruleName: "mismatched",
+            },
+            2,
+        );
+
+        // Act
+        const differentRules = removeExtendsDuplicatedRules(allRules, extensions);
+
+        // Assert
+        expect(differentRules.size).toBe(1);
+    });
+
+    it("removes a rule when it matches an existing rule as numbers", () => {
+        // Arrange
+        const { allRules, extensions } = prepareTestRule(
+            {
+                ruleSeverity: "warn",
+            },
+            1,
+        );
+
+        // Act
+        const differentRules = removeExtendsDuplicatedRules(allRules, extensions);
+
+        // Assert
+        expect(differentRules.size).toBe(0);
+    });
+
+    it("keeps a rule when it conflicts with an existing rule as numbers", () => {
+        // Arrange
+        const { allRules, extensions } = prepareTestRule(
+            {
+                ruleSeverity: "warn",
+            },
+            2,
+        );
+
+        // Act
+        const differentRules = removeExtendsDuplicatedRules(allRules, extensions);
+
+        // Assert
+        expect(differentRules.size).toBe(1);
+    });
+
+    it("removes a rule when it matches an existing rule as strings", () => {
+        // Arrange
+        const { allRules, extensions } = prepareTestRule(
+            {
+                ruleSeverity: "warn",
+            },
+            "warn",
+        );
+
+        // Act
+        const differentRules = removeExtendsDuplicatedRules(allRules, extensions);
+
+        // Assert
+        expect(differentRules.size).toBe(0);
+    });
+
+    it("keeps a rule when it conflicts with an existing rule as strings", () => {
+        // Arrange
+        const { allRules, extensions } = prepareTestRule(
+            {
+                ruleSeverity: "warn",
+            },
+            "error",
+        );
+
+        // Act
+        const differentRules = removeExtendsDuplicatedRules(allRules, extensions);
+
+        // Assert
+        expect(differentRules.size).toBe(1);
+    });
+
+    it("removes a rule when it matches an existing rule as objects", () => {
+        // Arrange
+        const { allRules, extensions } = prepareTestRule(
+            {
+                ruleArguments: ["some-argument"],
+                ruleSeverity: "warn",
+            },
+            ["warn", "some-argument"],
+        );
+
+        // Act
+        const differentRules = removeExtendsDuplicatedRules(allRules, extensions);
+
+        // Assert
+        expect(differentRules.size).toBe(0);
+    });
+
+    it("keeps a rule when it conflicts with an existing rule as objects", () => {
+        // Arrange
+        const { allRules, extensions } = prepareTestRule(
+            {
+                ruleArguments: ["some-argument-one"],
+                ruleSeverity: "warn",
+            },
+            ["warn", "some-argument-modified"],
+        );
+
+        // Act
+        const differentRules = removeExtendsDuplicatedRules(allRules, extensions);
+
+        // Assert
+        expect(differentRules.size).toBe(1);
+    });
+});