Don't output ESLint rules that match extended configurations (#448)

* WIP: logic for deduplicating ESLint rules' extensions

* Whoa, it actually works

* Moved trimming into the existing summarization section

* Reverted some unnecessary changes

* Fixed up edge cases

* Split extension merging and rule normalization

* Fixed compile complaint

Author: Josh Goldberg

docs/Architecture.md

@@ -13,9 +13,10 @@ Within `src/conversion/convertConfig.ts`, the following steps occur:
 
 1. Existing configurations are read from disk
 2. TSLint rules are converted into their ESLint configurations
-3. ESLint configurations are simplified based on extended ESLint and TSLint presets
+3. ESLint configurations are summarized based on extended ESLint and TSLint presets
     - 3a. If no output rules conflict with `eslint-config-prettier`, it's added in
-4. The simplified configuration is written to the output config file
+    - 3b. Any ESLint rules that are configured the same as an extended preset are trimmed
+4. The summarized configuration is written to the output config file
 5. Files to transform comments in have source text rewritten using the same rule conversion logic
 6. A summary of the results is printed to the user's console
 

src/cli/main.ts

@@ -16,16 +16,16 @@ import {
     convertEditorConfig,
     ConvertEditorConfigDependencies,
 } from "../conversion/convertEditorConfig";
-import { addPrettierExtensions } from "../creation/simplification/prettier/addPrettierExtensions";
-import { removeExtendsDuplicatedRules } from "../creation/simplification/removeExtendsDuplicatedRules";
+import { addPrettierExtensions } from "../creation/summarization/prettier/addPrettierExtensions";
+import { removeExtendsDuplicatedRules } from "../creation/pruning/removeExtendsDuplicatedRules";
 import {
     retrieveExtendsValues,
     RetrieveExtendsValuesDependencies,
-} from "../creation/simplification/retrieveExtendsValues";
+} from "../creation/summarization/retrieveExtendsValues";
 import {
-    simplifyPackageRules,
-    SimplifyPackageRulesDependencies,
-} from "../creation/simplification/simplifyPackageRules";
+    summarizePackageRules,
+    SummarizePackageRulesDependencies,
+} from "../creation/summarization/summarizePackageRules";
 import {
     writeConversionResults,
     WriteConversionResultsDependencies,
@@ -138,7 +138,7 @@ const retrieveExtendsValuesDependencies: RetrieveExtendsValuesDependencies = {
     importer: boundImporter,
 };
 
-const simplifyPackageRulesDependencies: SimplifyPackageRulesDependencies = {
+const summarizePackageRulesDependencies: SummarizePackageRulesDependencies = {
     addPrettierExtensions,
     removeExtendsDuplicatedRules,
     retrieveExtendsValues: bind(retrieveExtendsValues, retrieveExtendsValuesDependencies),
@@ -175,7 +175,7 @@ const convertConfigDependencies: ConvertConfigDependencies = {
     logMissingPackages: bind(logMissingPackages, logMissingPackagesDependencies),
     reportCommentResults: bind(reportCommentResults, reportCommentResultsDependencies),
     reportConversionResults: bind(reportConversionResults, reportConversionResultsDependencies),
-    simplifyPackageRules: bind(simplifyPackageRules, simplifyPackageRulesDependencies),
+    summarizePackageRules: bind(summarizePackageRules, summarizePackageRulesDependencies),
     writeConversionResults: bind(writeConversionResults, writeConversionResultsDependencies),
 };
 

src/conversion/conversionResults.stubs.ts

@@ -1,10 +1,12 @@
+import { SummarizedResultsConfiguration } from "../creation/summarization/types";
 import { EditorSettingConversionResults } from "../editorSettings/convertEditorSettings";
-import { SimplifiedResultsConfiguration } from "../creation/simplification/simplifyPackageRules";
 
 export const createEmptyConversionResults = (
-    overrides: Partial<SimplifiedResultsConfiguration> = {},
-): SimplifiedResultsConfiguration => ({
+    overrides: Partial<SummarizedResultsConfiguration> = {},
+): SummarizedResultsConfiguration => ({
     converted: new Map(),
+    extends: [],
+    extensionRules: new Map(),
     failed: [],
     missing: [],
     plugins: new Set(),

src/conversion/convertConfig.test.ts

@@ -16,10 +16,11 @@ const createStubDependencies = (
     }),
     reportCommentResults: jest.fn(),
     reportConversionResults: jest.fn(),
-    simplifyPackageRules: async (_configurations, data) => ({
+    summarizePackageRules: async (_configurations, data) => ({
         ...data,
         converted: new Map(),
         extends: [],
+        extensionRules: new Map(),
         failed: [],
         missing: [],
         plugins: new Set(),

src/conversion/convertConfig.ts

@@ -1,6 +1,6 @@
 import { SansDependencies } from "../binding";
 import { convertComments } from "../comments/convertComments";
-import { simplifyPackageRules } from "../creation/simplification/simplifyPackageRules";
+import { summarizePackageRules } from "../creation/summarization/summarizePackageRules";
 import { writeConversionResults } from "../creation/writeConversionResults";
 import { findOriginalConfigurations } from "../input/findOriginalConfigurations";
 import { logMissingPackages } from "../reporting/packages/logMissingPackages";
@@ -16,7 +16,7 @@ export type ConvertConfigDependencies = {
     logMissingPackages: SansDependencies<typeof logMissingPackages>;
     reportCommentResults: SansDependencies<typeof reportCommentResults>;
     reportConversionResults: SansDependencies<typeof reportConversionResults>;
-    simplifyPackageRules: SansDependencies<typeof simplifyPackageRules>;
+    summarizePackageRules: SansDependencies<typeof summarizePackageRules>;
     writeConversionResults: SansDependencies<typeof writeConversionResults>;
 };
 
@@ -39,18 +39,18 @@ export const convertConfig = async (
         originalConfigurations.data.tslint.full.rules,
     );
 
-    // 3. ESLint configurations are simplified based on extended ESLint and TSLint presets
-    const simplifiedConfiguration = await dependencies.simplifyPackageRules(
+    // 3. ESLint configurations are summarized based on extended ESLint and TSLint presets
+    const summarizedConfiguration = await dependencies.summarizePackageRules(
         originalConfigurations.data.eslint,
         originalConfigurations.data.tslint,
         ruleConversionResults,
         settings.prettier,
     );
 
-    // 4. The simplified configuration is written to the output config file
+    // 4. The summarized configuration is written to the output config file
     const fileWriteError = await dependencies.writeConversionResults(
         settings.config,
-        simplifiedConfiguration,
+        summarizedConfiguration,
         originalConfigurations.data,
     );
     if (fileWriteError !== undefined) {
@@ -64,10 +64,10 @@ export const convertConfig = async (
     const commentsResult = await dependencies.convertComments(settings.comments);
 
     // 6. A summary of the results is printed to the user's console
-    await dependencies.reportConversionResults(settings.config, simplifiedConfiguration);
+    await dependencies.reportConversionResults(settings.config, summarizedConfiguration);
     dependencies.reportCommentResults(commentsResult);
     await dependencies.logMissingPackages(
-        simplifiedConfiguration,
+        summarizedConfiguration,
         originalConfigurations.data.packages,
     );
 

src/creation/pruning/normalizeExtensions.test.ts

@@ -0,0 +1,73 @@
+import { normalizeExtensions } from "./normalizeExtensions";
+import { ESLintConfigurationRuleValue } from "../../input/findESLintConfiguration";
+
+const createStubExtension = (ruleName: string, ruleValue: ESLintConfigurationRuleValue) => {
+    return {
+        rules: {
+            [ruleName]: ruleValue,
+        },
+    };
+};
+
+describe("removeExtendsDuplicatedRules", () => {
+    it("ignores an empty extension", () => {
+        // Arrange
+        const extensions = [{}];
+
+        // Act
+        const results = normalizeExtensions(extensions);
+
+        // Assert
+        expect(results).toEqual(new Map());
+    });
+
+    it("overrides a rule's first value when a second extension contains it", () => {
+        // Arrange
+        const ruleName = "rule-a";
+        const extensions = [
+            createStubExtension(ruleName, "error"),
+            createStubExtension(ruleName, "warn"),
+        ];
+
+        // Act
+        const results = normalizeExtensions(extensions);
+
+        // Assert
+        expect(results).toEqual(
+            new Map([
+                [
+                    ruleName,
+                    {
+                        ruleArguments: [],
+                        ruleName,
+                        ruleSeverity: "warn",
+                    },
+                ],
+            ]),
+        );
+    });
+
+    it("normalizes a configuration when an array", () => {
+        // Arrange
+        const ruleName = "rule-a";
+        const ruleArgument = { value: true };
+        const extensions = [createStubExtension(ruleName, ["error", ruleArgument])];
+
+        // Act
+        const results = normalizeExtensions(extensions);
+
+        // Assert
+        expect(results).toEqual(
+            new Map([
+                [
+                    ruleName,
+                    {
+                        ruleArguments: [ruleArgument],
+                        ruleName,
+                        ruleSeverity: "error",
+                    },
+                ],
+            ]),
+        );
+    });
+});

src/creation/pruning/normalizeExtensions.ts

@@ -0,0 +1,41 @@
+import {
+    ESLintConfiguration,
+    ESLintConfigurationRuleValue,
+} from "../../input/findESLintConfiguration";
+import { ESLintRuleOptionsWithArguments } from "../../rules/types";
+import { normalizeRawESLintRuleSeverity } from "./normalizeRawESLintRuleSeverity";
+
+export const normalizeExtensions = (extensions: Partial<ESLintConfiguration>[]) => {
+    const mergedRules = new Map<string, ESLintRuleOptionsWithArguments>();
+
+    for (const extension of extensions) {
+        if (extension.rules === undefined) {
+            continue;
+        }
+
+        for (const ruleName in extension.rules) {
+            mergedRules.set(ruleName, formatRuleArguments(ruleName, extension.rules[ruleName]));
+        }
+    }
+
+    return mergedRules;
+};
+
+const formatRuleArguments = (
+    ruleName: string,
+    originalValue: ESLintConfigurationRuleValue,
+): ESLintRuleOptionsWithArguments => {
+    if (originalValue instanceof Array) {
+        return {
+            ruleArguments: originalValue.slice(1),
+            ruleName,
+            ruleSeverity: normalizeRawESLintRuleSeverity(originalValue[0]),
+        };
+    }
+
+    return {
+        ruleArguments: [],
+        ruleName,
+        ruleSeverity: normalizeRawESLintRuleSeverity(originalValue),
+    };
+};

src/creation/pruning/normalizeRawESLintRuleSeverity.test.ts

@@ -0,0 +1,47 @@
+import { normalizeRawESLintRuleSeverity } from "./normalizeRawESLintRuleSeverity";
+
+describe("normalizeRawESLintRuleSeverity", () => {
+    it("converts the severity to off when equal to 0", () => {
+        // Arrange
+        const rawSeverity = 0;
+
+        // Act
+        const result = normalizeRawESLintRuleSeverity(rawSeverity);
+
+        // Assert
+        expect(result).toEqual("off");
+    });
+
+    it("converts the severity to warn when equal to 1", () => {
+        // Arrange
+        const rawSeverity = 1;
+
+        // Act
+        const result = normalizeRawESLintRuleSeverity(rawSeverity);
+
+        // Assert
+        expect(result).toEqual("warn");
+    });
+
+    it("converts the severity to error when equal to 2", () => {
+        // Arrange
+        const rawSeverity = 2;
+
+        // Act
+        const result = normalizeRawESLintRuleSeverity(rawSeverity);
+
+        // Assert
+        expect(result).toEqual("error");
+    });
+
+    it("returns the same severity when already a string", () => {
+        // Arrange
+        const rawSeverity = "error";
+
+        // Act
+        const result = normalizeRawESLintRuleSeverity(rawSeverity);
+
+        // Assert
+        expect(result).toEqual(rawSeverity);
+    });
+});

src/creation/pruning/normalizeRawESLintRuleSeverity.ts

@@ -0,0 +1,19 @@
+import { ESLintRuleSeverity, RawESLintRuleSeverity } from "../../rules/types";
+
+export const normalizeRawESLintRuleSeverity = (
+    rawSeverity: RawESLintRuleSeverity,
+): ESLintRuleSeverity => {
+    switch (rawSeverity) {
+        case 0:
+            return "off";
+
+        case 1:
+            return "warn";
+
+        case 2:
+            return "error";
+
+        default:
+            return rawSeverity;
+    }
+};