From 3afd2d229c2b405a0a36f87feb6ffc1b2c98be01 Mon Sep 17 00:00:00 2001 From: moznion Date: Mon, 30 Mar 2020 01:14:21 +0900 Subject: [PATCH 1/6] Respect `ban-types` configuration of TSLint In the past, this tool didn't respect the TSLint configuration of `ban-types`, it means even if TSLint specified `ban-types` with the argument, generated ESLint configuration doesn't contain the detailed configuration of that. For example, when the TSLint configuration is like the following: ``` { "rules": { "ban-types": [true, ["Object", "Use {} instead."], ] } } ``` then, older tool generates the following ESLint configuration: ``` "rules": { "@typescript-eslint/ban-types": "error" } ``` According to the official documentation, probably this generated result doesn't make sense because each rule requires the type name (and the message). https://github.com/typescript-eslint/typescript-eslint/blob/f3160b471f8247e157555b6cf5b40a1f6ccdc233/packages/eslint-plugin/docs/rules/ban-types.md So this commit makes the generated ESLint configuration to be suitable to the certainly working configuration, like so: ``` "rules": { "@typescript-eslint/ban-types": [ "error", { "types": { "Object": { "message": "Use {} instead." } } } ] } ``` See also: https://palantir.github.io/tslint/rules/ban-types/ Related ticket: https://github.com/typescript-eslint/tslint-to-eslint-config/issues/352 --- src/rules/converters/ban-types.ts | 25 ++++++++- src/rules/converters/tests/ban-types.test.ts | 55 ++++++++++++++++++++ 2 files changed, 79 insertions(+), 1 deletion(-) diff --git a/src/rules/converters/ban-types.ts b/src/rules/converters/ban-types.ts index b3b2886bc..b8944236b 100644 --- a/src/rules/converters/ban-types.ts +++ b/src/rules/converters/ban-types.ts @@ -1,10 +1,33 @@ import { RuleConverter } from "../converter"; -export const convertBanTypes: RuleConverter = () => { +export const convertBanTypes: RuleConverter = tslintRule => { + type ConvertBanTypeArgument = { + message: string; + }; + + const bannedTypesObj: { [key: string]: ConvertBanTypeArgument | null } = {}; + + for (const rule of tslintRule.ruleArguments) { + const typ = rule[0]; + if (!typ) { + break; + } + + bannedTypesObj[typ] = rule[1] + ? { + message: rule[1], + } + : null; + } + + const ruleArguments = + Object.keys(bannedTypesObj).length === 0 ? undefined : [{ types: bannedTypesObj }]; + return { rules: [ { ruleName: "@typescript-eslint/ban-types", + ...{ ruleArguments }, }, ], }; diff --git a/src/rules/converters/tests/ban-types.test.ts b/src/rules/converters/tests/ban-types.test.ts index a5b08b851..5cb39aa8d 100644 --- a/src/rules/converters/tests/ban-types.test.ts +++ b/src/rules/converters/tests/ban-types.test.ts @@ -14,4 +14,59 @@ describe(convertBanTypes, () => { ], }); }); + + test("conversion with arguments", () => { + const result = convertBanTypes({ + ruleArguments: [ + ["Object", "Use {} instead."], + ["String"], + ["Number"], + ["Boolean", "Use 'boolean' instead."], + [], // empty array: this should be ignored + ], + }); + + expect(result).toEqual({ + rules: [ + { + ruleName: "@typescript-eslint/ban-types", + ruleArguments: [ + { + types: { + Object: { + message: "Use {} instead.", + }, + String: null, + Number: null, + Boolean: { + message: "Use 'boolean' instead.", + }, + }, + }, + ], + }, + ], + }); + }); + + test("conversion with duplicated arguments", () => { + const result = convertBanTypes({ + ruleArguments: [["Object", "Use {} instead."], ["Object"]], + }); + + expect(result).toEqual({ + rules: [ + { + ruleName: "@typescript-eslint/ban-types", + ruleArguments: [ + { + types: { + Object: null, + }, + }, + ], + }, + ], + }); + }); }); From 3874d8793f6b2d3925f52f1e010b8a74fe9452b4 Mon Sep 17 00:00:00 2001 From: moznion Date: Tue, 31 Mar 2020 01:01:12 +0900 Subject: [PATCH 2/6] Use the Record type instead of traditional dict decralation Fix: https://github.com/typescript-eslint/tslint-to-eslint-config/pull/353#discussion_r400301362 --- src/rules/converters/ban-types.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rules/converters/ban-types.ts b/src/rules/converters/ban-types.ts index b8944236b..71b8601bf 100644 --- a/src/rules/converters/ban-types.ts +++ b/src/rules/converters/ban-types.ts @@ -5,7 +5,7 @@ export const convertBanTypes: RuleConverter = tslintRule => { message: string; }; - const bannedTypesObj: { [key: string]: ConvertBanTypeArgument | null } = {}; + const bannedTypesObj: Record = {}; for (const rule of tslintRule.ruleArguments) { const typ = rule[0]; From 8c833da2875ed349e84b9f9a28a2afe10a395c52 Mon Sep 17 00:00:00 2001 From: moznion Date: Tue, 31 Mar 2020 01:02:28 +0900 Subject: [PATCH 3/6] Simplify a variable name for banned-types Fix: https://github.com/typescript-eslint/tslint-to-eslint-config/pull/353#discussion_r400303609 --- src/rules/converters/ban-types.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/rules/converters/ban-types.ts b/src/rules/converters/ban-types.ts index 71b8601bf..d69bb9d49 100644 --- a/src/rules/converters/ban-types.ts +++ b/src/rules/converters/ban-types.ts @@ -5,7 +5,7 @@ export const convertBanTypes: RuleConverter = tslintRule => { message: string; }; - const bannedTypesObj: Record = {}; + const bannedTypes: Record = {}; for (const rule of tslintRule.ruleArguments) { const typ = rule[0]; @@ -13,7 +13,7 @@ export const convertBanTypes: RuleConverter = tslintRule => { break; } - bannedTypesObj[typ] = rule[1] + bannedTypes[typ] = rule[1] ? { message: rule[1], } @@ -21,7 +21,7 @@ export const convertBanTypes: RuleConverter = tslintRule => { } const ruleArguments = - Object.keys(bannedTypesObj).length === 0 ? undefined : [{ types: bannedTypesObj }]; + Object.keys(bannedTypes).length === 0 ? undefined : [{ types: bannedTypes }]; return { rules: [ From 4af2d55ae28f1dc1d8c1b8bc97b0c180e2fe21ab Mon Sep 17 00:00:00 2001 From: moznion Date: Tue, 31 Mar 2020 01:04:04 +0900 Subject: [PATCH 4/6] Rename an unclear variable name for banned-type to self-descriptive name Fix: https://github.com/typescript-eslint/tslint-to-eslint-config/pull/353#discussion_r400303609 --- src/rules/converters/ban-types.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/rules/converters/ban-types.ts b/src/rules/converters/ban-types.ts index d69bb9d49..cbe7e2f33 100644 --- a/src/rules/converters/ban-types.ts +++ b/src/rules/converters/ban-types.ts @@ -8,12 +8,12 @@ export const convertBanTypes: RuleConverter = tslintRule => { const bannedTypes: Record = {}; for (const rule of tslintRule.ruleArguments) { - const typ = rule[0]; - if (!typ) { + const bannedType = rule[0]; + if (!bannedType) { break; } - bannedTypes[typ] = rule[1] + bannedTypes[bannedType] = rule[1] ? { message: rule[1], } From e6b2f615c1413040a8527a7c150fde51db1651fb Mon Sep 17 00:00:00 2001 From: moznion Date: Tue, 31 Mar 2020 01:07:46 +0900 Subject: [PATCH 5/6] Use short-hand notation for variable substitution Fix: https://github.com/typescript-eslint/tslint-to-eslint-config/pull/353#discussion_r400304253 --- src/rules/converters/ban-types.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/rules/converters/ban-types.ts b/src/rules/converters/ban-types.ts index cbe7e2f33..f8fbaa5ff 100644 --- a/src/rules/converters/ban-types.ts +++ b/src/rules/converters/ban-types.ts @@ -8,16 +8,12 @@ export const convertBanTypes: RuleConverter = tslintRule => { const bannedTypes: Record = {}; for (const rule of tslintRule.ruleArguments) { - const bannedType = rule[0]; + const [bannedType, message] = rule; if (!bannedType) { break; } - bannedTypes[bannedType] = rule[1] - ? { - message: rule[1], - } - : null; + bannedTypes[bannedType] = message ? { message } : null; } const ruleArguments = From 3d8ded3cf9dfd01f32185e80925d373a4fdd6d26 Mon Sep 17 00:00:00 2001 From: moznion Date: Tue, 31 Mar 2020 01:25:51 +0900 Subject: [PATCH 6/6] Check whether a rule is array object or not on `ban-types` converter --- src/rules/converters/ban-types.ts | 4 ++++ src/rules/converters/tests/ban-types.test.ts | 14 ++++++++++++++ 2 files changed, 18 insertions(+) diff --git a/src/rules/converters/ban-types.ts b/src/rules/converters/ban-types.ts index f8fbaa5ff..7c5e200cc 100644 --- a/src/rules/converters/ban-types.ts +++ b/src/rules/converters/ban-types.ts @@ -8,6 +8,10 @@ export const convertBanTypes: RuleConverter = tslintRule => { const bannedTypes: Record = {}; for (const rule of tslintRule.ruleArguments) { + if (!Array.isArray(rule)) { + break; + } + const [bannedType, message] = rule; if (!bannedType) { break; diff --git a/src/rules/converters/tests/ban-types.test.ts b/src/rules/converters/tests/ban-types.test.ts index 5cb39aa8d..832d76ed8 100644 --- a/src/rules/converters/tests/ban-types.test.ts +++ b/src/rules/converters/tests/ban-types.test.ts @@ -69,4 +69,18 @@ describe(convertBanTypes, () => { ], }); }); + + test("conversion with duplicated arguments", () => { + const result = convertBanTypes({ + ruleArguments: ["!!!this-is-not-array!!!"], + }); + + expect(result).toEqual({ + rules: [ + { + ruleName: "@typescript-eslint/ban-types", + }, + ], + }); + }); });