From e48fa14dc37a0662b9ec3b768addc72e29f733e9 Mon Sep 17 00:00:00 2001 From: Edwin Kofler Date: Sat, 30 Apr 2022 08:37:05 -0700 Subject: [PATCH 1/4] fix: Properly use `ruleSeverity` properties of rule converters during conversion --- .../lintConfigs/rules/convertRules.test.ts | 41 ++++++++++++++++++- .../lintConfigs/rules/convertRules.ts | 3 +- 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/src/converters/lintConfigs/rules/convertRules.test.ts b/src/converters/lintConfigs/rules/convertRules.test.ts index cac48852c..4e247048d 100644 --- a/src/converters/lintConfigs/rules/convertRules.test.ts +++ b/src/converters/lintConfigs/rules/convertRules.test.ts @@ -4,7 +4,7 @@ import { ConversionError } from "../../../errors/conversionError"; import { convertRules } from "./convertRules"; import { ConversionResult, RuleConverter } from "./ruleConverter"; import { RuleMerger } from "./ruleMerger"; -import { TSLintRuleOptions, TSLintRuleSeverity } from "./types"; +import { ESLintRuleSeverity, TSLintRuleOptions, TSLintRuleSeverity } from "./types"; describe("convertRules", () => { it("doesn't crash when passed an undefined configuration", () => { @@ -353,6 +353,45 @@ describe("convertRules", () => { ]).toContainEqual(converted); }); + it("merges when a rule's ruleSeverity explicitly differs from its TSLint equivalent", () => { + // Arrange + const conversionResult: { + rules: { ruleName: string; ruleSeverity: ESLintRuleSeverity }[]; + } = { + rules: [ + { + ruleName: "eslint-rule-a", + ruleSeverity: "off", + }, + ], + }; + const { tslintRule, converters, mergers } = setupConversionEnvironment({ + ruleSeverity: "error", + conversionResult, + ruleToMerge: conversionResult.rules[0].ruleName, + }); + + // Act + const { converted } = convertRules( + { ruleConverters: converters, ruleMergers: mergers }, + { [tslintRule.ruleName]: tslintRule }, + new Map(), + ); + + // Assert + expect([ + new Map([ + [ + "eslint-rule-a", + { + ruleName: "eslint-rule-a", + ruleSeverity: "off", + }, + ], + ]), + ]).toContainEqual(converted); + }); + it("marks a new plugin when a conversion has a new plugin", () => { // Arrange const conversionResult = { diff --git a/src/converters/lintConfigs/rules/convertRules.ts b/src/converters/lintConfigs/rules/convertRules.ts index bbfe0b494..117630d6b 100644 --- a/src/converters/lintConfigs/rules/convertRules.ts +++ b/src/converters/lintConfigs/rules/convertRules.ts @@ -80,7 +80,8 @@ export const convertRules = ( const existingConversion = converted.get(changes.ruleName); const newConversion = { ...changes, - ruleSeverity: convertTSLintRuleSeverity(tslintRule.ruleSeverity), + ruleSeverity: + changes.ruleSeverity || convertTSLintRuleSeverity(tslintRule.ruleSeverity), }; // 4c. If this is the first time the output ESLint rule is seen, it's directly marked as converted. From bb23d2302539d4858b1d6f5c243382ccd08f59ad Mon Sep 17 00:00:00 2001 From: Edwin Kofler Date: Sat, 30 Apr 2022 17:40:03 -0700 Subject: [PATCH 2/4] style: Use nullish coalescing operator over logical or --- src/converters/lintConfigs/rules/convertRules.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/converters/lintConfigs/rules/convertRules.ts b/src/converters/lintConfigs/rules/convertRules.ts index 117630d6b..ae9be00f7 100644 --- a/src/converters/lintConfigs/rules/convertRules.ts +++ b/src/converters/lintConfigs/rules/convertRules.ts @@ -81,7 +81,7 @@ export const convertRules = ( const newConversion = { ...changes, ruleSeverity: - changes.ruleSeverity || convertTSLintRuleSeverity(tslintRule.ruleSeverity), + changes.ruleSeverity ?? convertTSLintRuleSeverity(tslintRule.ruleSeverity), }; // 4c. If this is the first time the output ESLint rule is seen, it's directly marked as converted. From 8f4296d4f5fa19ef2946b32403ec6215b31de5dc Mon Sep 17 00:00:00 2001 From: Edwin Kofler Date: Mon, 2 May 2022 22:37:53 -0700 Subject: [PATCH 3/4] style: Miscellaneous improvements --- .../lintConfigs/rules/convertRules.test.ts | 26 ++++++++----------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/src/converters/lintConfigs/rules/convertRules.test.ts b/src/converters/lintConfigs/rules/convertRules.test.ts index 4e247048d..bdfc2598a 100644 --- a/src/converters/lintConfigs/rules/convertRules.test.ts +++ b/src/converters/lintConfigs/rules/convertRules.test.ts @@ -2,9 +2,9 @@ import { describe, expect, it, test } from "@jest/globals"; import { ConversionError } from "../../../errors/conversionError"; import { convertRules } from "./convertRules"; -import { ConversionResult, RuleConverter } from "./ruleConverter"; +import { ConversionResult, ConvertedRuleChanges, RuleConverter } from "./ruleConverter"; import { RuleMerger } from "./ruleMerger"; -import { ESLintRuleSeverity, TSLintRuleOptions, TSLintRuleSeverity } from "./types"; +import { TSLintRuleOptions, TSLintRuleSeverity } from "./types"; describe("convertRules", () => { it("doesn't crash when passed an undefined configuration", () => { @@ -355,27 +355,23 @@ describe("convertRules", () => { it("merges when a rule's ruleSeverity explicitly differs from its TSLint equivalent", () => { // Arrange - const conversionResult: { - rules: { ruleName: string; ruleSeverity: ESLintRuleSeverity }[]; - } = { - rules: [ - { - ruleName: "eslint-rule-a", - ruleSeverity: "off", - }, - ], - }; + const rules: ConvertedRuleChanges[] = [ + { + ruleName: "eslint-rule-a", + ruleSeverity: "off", + }, + ]; const { tslintRule, converters, mergers } = setupConversionEnvironment({ ruleSeverity: "error", - conversionResult, - ruleToMerge: conversionResult.rules[0].ruleName, + conversionResult: { rules }, + ruleToMerge: rules[0].ruleName, }); // Act const { converted } = convertRules( { ruleConverters: converters, ruleMergers: mergers }, { [tslintRule.ruleName]: tslintRule }, - new Map(), + new Map(), ); // Assert From 297df35f4b3b8161471781289ec00c3377d35c34 Mon Sep 17 00:00:00 2001 From: Edwin Kofler Date: Tue, 3 May 2022 00:59:15 -0700 Subject: [PATCH 4/4] test: Ensure multiple output rules are correctly treated This additionally ensures that the ruleSeverities during the conversion aren't always treated exactly the same. Moreover, it tests the case that if ruleSeverity isn't specified, the default (its respective TSLint equivalent) is used instead. --- .../lintConfigs/rules/convertRules.test.ts | 47 ++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-) diff --git a/src/converters/lintConfigs/rules/convertRules.test.ts b/src/converters/lintConfigs/rules/convertRules.test.ts index bdfc2598a..879a29a50 100644 --- a/src/converters/lintConfigs/rules/convertRules.test.ts +++ b/src/converters/lintConfigs/rules/convertRules.test.ts @@ -353,7 +353,7 @@ describe("convertRules", () => { ]).toContainEqual(converted); }); - it("merges when a rule's ruleSeverity explicitly differs from its TSLint equivalent", () => { + it("merges when a rule severity differs from its TSLint equivalent", () => { // Arrange const rules: ConvertedRuleChanges[] = [ { @@ -388,6 +388,51 @@ describe("convertRules", () => { ]).toContainEqual(converted); }); + it("merges when rule severities inconsistently differ from their TSLint equivalent", () => { + // Arrange + const rules: ConvertedRuleChanges[] = [ + { + ruleName: "eslint-rule-a", + ruleSeverity: "off", + }, + { + ruleName: "eslint-rule-b", + }, + ]; + const { tslintRule, converters, mergers } = setupConversionEnvironment({ + ruleSeverity: "error", + conversionResult: { rules }, + ruleToMerge: rules[0].ruleName, + }); + + // Act + const { converted } = convertRules( + { ruleConverters: converters, ruleMergers: mergers }, + { [tslintRule.ruleName]: tslintRule }, + new Map(), + ); + + // Assert + expect([ + new Map([ + [ + "eslint-rule-a", + { + ruleName: "eslint-rule-a", + ruleSeverity: "off", + }, + ], + [ + "eslint-rule-b", + { + ruleName: "eslint-rule-b", + ruleSeverity: "error", + }, + ], + ]), + ]).toContainEqual(converted); + }); + it("marks a new plugin when a conversion has a new plugin", () => { // Arrange const conversionResult = {