From 9a78a60ee0a35397a764392824a24f5cac8f94f4 Mon Sep 17 00:00:00 2001 From: Budi Irawan Date: Thu, 17 Oct 2019 21:01:35 +1100 Subject: [PATCH 1/2] add converter trailing-comma and unit test (#186) --- src/rules/converters.ts | 2 + .../converters/tests/trailing-comma.test.ts | 275 ++++++++++++++++++ src/rules/converters/trailing-comma.ts | 120 ++++++++ 3 files changed, 397 insertions(+) create mode 100644 src/rules/converters/tests/trailing-comma.test.ts create mode 100644 src/rules/converters/trailing-comma.ts diff --git a/src/rules/converters.ts b/src/rules/converters.ts index d21e68bea..cee212ff5 100644 --- a/src/rules/converters.ts +++ b/src/rules/converters.ts @@ -113,6 +113,7 @@ import { convertSemicolon } from "./converters/semicolon"; import { convertSpaceBeforeFunctionParen } from "./converters/space-before-function-paren"; import { convertSpaceWithinParens } from "./converters/space-within-parens"; import { convertSwitchDefault } from "./converters/switch-default"; +import { convertTrailingComma } from "./converters/trailing-comma"; import { convertTripleEquals } from "./converters/triple-equals"; import { convertTypedefWhitespace } from "./converters/typedef-whitespace"; import { convertTypeLiteralDelimiter } from "./converters/type-literal-delimiter"; @@ -243,6 +244,7 @@ export const converters = new Map([ ["space-before-function-paren", convertSpaceBeforeFunctionParen], ["space-within-parens", convertSpaceWithinParens], ["switch-default", convertSwitchDefault], + ["trailing-comma", convertTrailingComma], ["triple-equals", convertTripleEquals], ["type-literal-delimiter", convertTypeLiteralDelimiter], ["typedef-whitespace", convertTypedefWhitespace], diff --git a/src/rules/converters/tests/trailing-comma.test.ts b/src/rules/converters/tests/trailing-comma.test.ts new file mode 100644 index 000000000..329238869 --- /dev/null +++ b/src/rules/converters/tests/trailing-comma.test.ts @@ -0,0 +1,275 @@ +import { convertTrailingComma } from "../trailing-comma"; + +describe(convertTrailingComma, () => { + test("conversion without arguments", () => { + const result = convertTrailingComma({ + ruleArguments: [], + }); + + expect(result).toEqual({ + rules: [ + { + ruleName: "comma-dangle", + }, + ], + }); + }); + + describe("conversion with arguments using string values", () => { + const testCases = [ + { + argument: { + singleline: "never", + }, + expectedRuleArguments: [], + }, + { + argument: { + singleline: "always", + }, + expectedRuleArguments: [], + }, + { + argument: { + multiline: "never", + }, + expectedRuleArguments: [], + }, + { + argument: { + multiline: "always", + }, + expectedRuleArguments: ["always-multiline"], + }, + { + argument: { + singleline: "never", + multiline: "never", + }, + expectedRuleArguments: [], + }, + { + argument: { + singleline: "never", + multiline: "always", + }, + expectedRuleArguments: ["always-multiline"], + }, + { + argument: { + singleline: "always", + multiline: "never", + }, + expectedRuleArguments: [], + }, + { + argument: { + singleline: "always", + multiline: "always", + }, + expectedRuleArguments: ["always"], + }, + ]; + + testCases.forEach(testCase => { + test(`conversion with arguments ${JSON.stringify(testCase.argument)}`, () => { + const result = convertTrailingComma({ + ruleArguments: [testCase.argument], + }); + + expect(result).toEqual({ + rules: [ + { + ruleName: "comma-dangle", + ...(testCase.expectedRuleArguments.length !== 0 && { + ruleArguments: testCase.expectedRuleArguments, + }), + }, + ], + }); + }); + }); + }); + + describe("conversion with arguments using object values", () => { + const testCases = [ + { + argument: { + singleline: "never", + multiline: { + objects: "always", + arrays: "always", + functions: "always", + imports: "always", + exports: "always", + typeLiterals: "ignore", + }, + }, + expectedRuleArguments: [ + { + arrays: "always-multiline", + objects: "always-multiline", + functions: "always-multiline", + imports: "always-multiline", + exports: "always-multiline", + }, + ], + }, + { + argument: { + singleline: "always", + multiline: { + objects: "always", + arrays: "always", + functions: "always", + imports: "always", + exports: "always", + typeLiterals: "ignore", + }, + }, + expectedRuleArguments: [ + { + arrays: "always", + objects: "always", + functions: "always", + imports: "always", + exports: "always", + }, + ], + }, + { + argument: { + singleline: { + objects: "never", + arrays: "never", + functions: "never", + imports: "never", + exports: "never", + typeLiterals: "ignore", + }, + }, + expectedRuleArguments: [ + { + arrays: "never", + objects: "never", + functions: "never", + imports: "never", + exports: "never", + }, + ], + }, + { + argument: { + singleline: { + objects: "never", + arrays: "never", + functions: "never", + typeLiterals: "ignore", + }, + multiline: { + objects: "never", + arrays: "never", + functions: "never", + typeLiterals: "ignore", + }, + }, + expectedRuleArguments: [ + { + arrays: "never", + objects: "never", + functions: "never", + }, + ], + }, + { + argument: { + multiline: { + objects: "always", + arrays: "always", + functions: "always", + imports: "always", + exports: "always", + typeLiterals: "ignore", + }, + }, + expectedRuleArguments: [ + { + arrays: "always-multiline", + objects: "always-multiline", + functions: "always-multiline", + imports: "always-multiline", + exports: "always-multiline", + }, + ], + }, + { + argument: { + singleline: { + objects: "always", + arrays: "always", + functions: "always", + imports: "always", + exports: "always", + typeLiterals: "ignore", + }, + multiline: { + objects: "always", + arrays: "always", + functions: "always", + imports: "always", + exports: "always", + typeLiterals: "ignore", + }, + }, + expectedRuleArguments: [ + { + arrays: "always", + objects: "always", + functions: "always", + imports: "always", + exports: "always", + }, + ], + }, + { + argument: { + singleline: { + objects: "always", + arrays: "always", + }, + multiline: { + objects: "always", + arrays: "always", + functions: "always", + }, + }, + expectedRuleArguments: [ + { + arrays: "always", + objects: "always", + functions: "always-multiline", + }, + ], + }, + ]; + + testCases.forEach(testCase => { + test(`conversion with arguments ${JSON.stringify(testCase.argument)}`, () => { + const result = convertTrailingComma({ + ruleArguments: [testCase.argument], + }); + + expect(result).toEqual({ + rules: [ + { + ruleName: "comma-dangle", + ...(testCase.expectedRuleArguments.length !== 0 && { + ruleArguments: testCase.expectedRuleArguments, + }), + }, + ], + }); + }); + }); + }); +}); diff --git a/src/rules/converters/trailing-comma.ts b/src/rules/converters/trailing-comma.ts new file mode 100644 index 000000000..641e6f325 --- /dev/null +++ b/src/rules/converters/trailing-comma.ts @@ -0,0 +1,120 @@ +import { RuleConverter } from "../converter"; + +export const convertTrailingComma: RuleConverter = tslintRule => { + const eslintArgs = + tslintRule.ruleArguments.length !== 0 + ? collectArguments(tslintRule.ruleArguments) + : undefined; + + return { + rules: [ + { + ruleName: "comma-dangle", + ...(eslintArgs && { ruleArguments: [eslintArgs] }), + }, + ], + }; +}; + +function collectArguments(args: any[]) { + const tslintArg: any = args[0]; + const { multiline, singleline } = tslintArg; + + if (typeof multiline === "object" || typeof singleline === "object") { + const fields = mergePropertyKeys(singleline, multiline); + const single = getFieldValue(singleline); + const multi = getFieldValue(multiline); + + return fields.reduce( + (acc, field) => ({ + ...acc, + ...collectFields(field, single, multi), + }), + {}, + ); + } + + if ( + multiline === TSLintValue.Always && + (singleline === undefined || singleline === TSLintValue.Never) + ) { + return ESLintValue.AlwaysMultiline; + } + + if (multiline === TSLintValue.Always && singleline === TSLintValue.Always) { + return ESLintValue.Always; + } + + return; +} + +function mergePropertyKeys(singleline: any, multiline: any) { + const getKeysIfObject = (field: any) => (typeof field === "object" ? Object.keys(field) : []); + const singlelineKeys = getKeysIfObject(singleline); + const multilineKeys = getKeysIfObject(multiline); + + const uniqueKeys = [...new Set([...singlelineKeys, ...multilineKeys])]; + const unsupportedKeyInEsLint = "typeLiterals"; + + return uniqueKeys.filter(field => field !== unsupportedKeyInEsLint); +} + +function collectFields(fieldName: string, singleline: any, multiline: any) { + const hasSingleline = Boolean(singleline); + const hasSinglelineAndFieldExist = Boolean(singleline && singleline[fieldName]); + const hasSinglelineAlways = Boolean(singleline && singleline[fieldName] === TSLintValue.Always); + const hasMultilineAlways = Boolean(multiline && multiline[fieldName] === TSLintValue.Always); + + if (!hasSingleline && hasMultilineAlways) { + return { + [fieldName]: ESLintValue.AlwaysMultiline, + }; + } + + if (!hasSinglelineAndFieldExist && hasMultilineAlways) { + return { + [fieldName]: ESLintValue.AlwaysMultiline, + }; + } + + if (!hasSinglelineAlways && hasMultilineAlways) { + return { + [fieldName]: ESLintValue.AlwaysMultiline, + }; + } + + if (hasSinglelineAlways && hasMultilineAlways) { + return { + [fieldName]: ESLintValue.Always, + }; + } + + return { + [fieldName]: ESLintValue.Never, + }; +} + +function getFieldValue(value: string | object) { + return typeof value === "string" + ? { + arrays: value, + objects: value, + functions: value, + imports: value, + exports: value, + } + : value; +} + +enum TSLintValue { + Always = "always", + Never = "never", +} + +enum ESLintValue { + Never = "never", + Always = "always", + AlwaysMultiline = "always-multiline", + OnlyMultiline = "only-multiline", + Ignore = "ignore", +} From b07a0e0f6ad9ca7fea421119543323069ac7ccf5 Mon Sep 17 00:00:00 2001 From: Budi Irawan Date: Sun, 3 Nov 2019 08:02:34 +0700 Subject: [PATCH 2/2] add more typings and display notices --- .../converters/tests/trailing-comma.test.ts | 102 +++++++++++-- src/rules/converters/trailing-comma.ts | 144 ++++++++++++------ 2 files changed, 194 insertions(+), 52 deletions(-) diff --git a/src/rules/converters/tests/trailing-comma.test.ts b/src/rules/converters/tests/trailing-comma.test.ts index 329238869..244146be6 100644 --- a/src/rules/converters/tests/trailing-comma.test.ts +++ b/src/rules/converters/tests/trailing-comma.test.ts @@ -102,7 +102,6 @@ describe(convertTrailingComma, () => { functions: "always", imports: "always", exports: "always", - typeLiterals: "ignore", }, }, expectedRuleArguments: [ @@ -124,7 +123,6 @@ describe(convertTrailingComma, () => { functions: "always", imports: "always", exports: "always", - typeLiterals: "ignore", }, }, expectedRuleArguments: [ @@ -145,7 +143,6 @@ describe(convertTrailingComma, () => { functions: "never", imports: "never", exports: "never", - typeLiterals: "ignore", }, }, expectedRuleArguments: [ @@ -164,13 +161,11 @@ describe(convertTrailingComma, () => { objects: "never", arrays: "never", functions: "never", - typeLiterals: "ignore", }, multiline: { objects: "never", arrays: "never", functions: "never", - typeLiterals: "ignore", }, }, expectedRuleArguments: [ @@ -189,7 +184,6 @@ describe(convertTrailingComma, () => { functions: "always", imports: "always", exports: "always", - typeLiterals: "ignore", }, }, expectedRuleArguments: [ @@ -210,7 +204,6 @@ describe(convertTrailingComma, () => { functions: "always", imports: "always", exports: "always", - typeLiterals: "ignore", }, multiline: { objects: "always", @@ -218,7 +211,6 @@ describe(convertTrailingComma, () => { functions: "always", imports: "always", exports: "always", - typeLiterals: "ignore", }, }, expectedRuleArguments: [ @@ -263,9 +255,101 @@ describe(convertTrailingComma, () => { rules: [ { ruleName: "comma-dangle", - ...(testCase.expectedRuleArguments.length !== 0 && { + ...(testCase.expectedRuleArguments.length && { + ruleArguments: testCase.expectedRuleArguments, + }), + }, + ], + }); + }); + }); + }); + + describe("conversion with not supported config", () => { + const testCases = [ + { + argument: { + esSpecCompliant: true, + }, + expectedRuleArguments: [], + expectedNotices: ["ESLint does not support config property esSpecCompliant"], + }, + { + argument: { + singleline: { + typeLiterals: "ignore", + }, + }, + expectedRuleArguments: [{}], + expectedNotices: ["ESLint does not support config property typeLiterals"], + }, + { + argument: { + multiline: { + typeLiterals: "ignore", + }, + }, + expectedRuleArguments: [{}], + expectedNotices: ["ESLint does not support config property typeLiterals"], + }, + { + argument: { + esSpecCompliant: true, + singleline: { + typeLiterals: "always", + }, + }, + expectedRuleArguments: [{}], + expectedNotices: [ + "ESLint does not support config property esSpecCompliant", + "ESLint does not support config property typeLiterals", + ], + }, + { + argument: { + esSpecCompliant: false, + multiline: { + typeLiterals: "always-multiline", + }, + }, + expectedRuleArguments: [{}], + expectedNotices: [ + "ESLint does not support config property esSpecCompliant", + "ESLint does not support config property typeLiterals", + ], + }, + { + argument: { + esSpecCompliant: false, + singleline: { + typeLiterals: "ignore", + }, + multiline: { + typeLiterals: "ignore", + }, + }, + expectedRuleArguments: [{}], + expectedNotices: [ + "ESLint does not support config property esSpecCompliant", + "ESLint does not support config property typeLiterals", + ], + }, + ]; + + testCases.forEach(testCase => { + test(`conversion with arguments ${JSON.stringify(testCase.argument)}`, () => { + const result = convertTrailingComma({ + ruleArguments: [testCase.argument], + }); + + expect(result).toEqual({ + rules: [ + { + ruleName: "comma-dangle", + ...(testCase.expectedRuleArguments.length && { ruleArguments: testCase.expectedRuleArguments, }), + notices: testCase.expectedNotices, }, ], }); diff --git a/src/rules/converters/trailing-comma.ts b/src/rules/converters/trailing-comma.ts index 641e6f325..e94d455ed 100644 --- a/src/rules/converters/trailing-comma.ts +++ b/src/rules/converters/trailing-comma.ts @@ -1,100 +1,110 @@ import { RuleConverter } from "../converter"; +const unsupportedKeyInEsLint = "typeLiterals"; + export const convertTrailingComma: RuleConverter = tslintRule => { - const eslintArgs = - tslintRule.ruleArguments.length !== 0 - ? collectArguments(tslintRule.ruleArguments) - : undefined; + const eslintArgs = tslintRule.ruleArguments.length + ? collectArguments(tslintRule.ruleArguments) + : undefined; + + const notices = tslintRule.ruleArguments.length + ? collectNotices(tslintRule.ruleArguments) + : undefined; return { rules: [ { ruleName: "comma-dangle", ...(eslintArgs && { ruleArguments: [eslintArgs] }), + ...(notices && notices.length && { notices }), }, ], }; }; -function collectArguments(args: any[]) { - const tslintArg: any = args[0]; - const { multiline, singleline } = tslintArg; +function collectArguments(args: TSLintArg[]): ESLintArgValue | undefined { + const tslintArg = args[0]; + const { singleline, multiline } = tslintArg; - if (typeof multiline === "object" || typeof singleline === "object") { - const fields = mergePropertyKeys(singleline, multiline); - const single = getFieldValue(singleline); - const multi = getFieldValue(multiline); + if (typeof singleline === "object" || typeof multiline === "object") { + const keys = mergePropertyKeys(singleline, multiline); + const single = singleline && mapToObjectConfig(singleline); + const multi = multiline && mapToObjectConfig(multiline); - return fields.reduce( - (acc, field) => ({ + return keys.reduce( + (acc, key) => ({ ...acc, - ...collectFields(field, single, multi), + ...collectKeys(key as TSLintObjectKey, single, multi), }), {}, ); } - if ( - multiline === TSLintValue.Always && - (singleline === undefined || singleline === TSLintValue.Never) - ) { - return ESLintValue.AlwaysMultiline; + if ((singleline === undefined || singleline === "never") && multiline === "always") { + return "always-multiline"; } - if (multiline === TSLintValue.Always && singleline === TSLintValue.Always) { - return ESLintValue.Always; + if (singleline === "always" && multiline === "always") { + return "always"; } return; } -function mergePropertyKeys(singleline: any, multiline: any) { - const getKeysIfObject = (field: any) => (typeof field === "object" ? Object.keys(field) : []); +function mergePropertyKeys( + singleline: TSLintArgValue | undefined, + multiline: TSLintArgValue | undefined, +): string[] { + const getKeysIfObject = (field: TSLintArgValue | undefined): string[] => + typeof field === "object" ? Object.keys(field) : []; const singlelineKeys = getKeysIfObject(singleline); const multilineKeys = getKeysIfObject(multiline); const uniqueKeys = [...new Set([...singlelineKeys, ...multilineKeys])]; - const unsupportedKeyInEsLint = "typeLiterals"; return uniqueKeys.filter(field => field !== unsupportedKeyInEsLint); } -function collectFields(fieldName: string, singleline: any, multiline: any) { +function collectKeys( + key: TSLintObjectKey, + singleline: TSLintObject | undefined, + multiline: TSLintObject | undefined, +): { [key: string]: ESLintStringValue } { const hasSingleline = Boolean(singleline); - const hasSinglelineAndFieldExist = Boolean(singleline && singleline[fieldName]); - const hasSinglelineAlways = Boolean(singleline && singleline[fieldName] === TSLintValue.Always); - const hasMultilineAlways = Boolean(multiline && multiline[fieldName] === TSLintValue.Always); + const hasSinglelineAndFieldExist = Boolean(singleline && singleline[key]); + const hasSinglelineAlways = Boolean(singleline && singleline[key] === "always"); + const hasMultilineAlways = Boolean(multiline && multiline[key] === "always"); if (!hasSingleline && hasMultilineAlways) { return { - [fieldName]: ESLintValue.AlwaysMultiline, + [key]: "always-multiline", }; } if (!hasSinglelineAndFieldExist && hasMultilineAlways) { return { - [fieldName]: ESLintValue.AlwaysMultiline, + [key]: "always-multiline", }; } if (!hasSinglelineAlways && hasMultilineAlways) { return { - [fieldName]: ESLintValue.AlwaysMultiline, + [key]: "always-multiline", }; } if (hasSinglelineAlways && hasMultilineAlways) { return { - [fieldName]: ESLintValue.Always, + [key]: "always", }; } return { - [fieldName]: ESLintValue.Never, + [key]: "never", }; } -function getFieldValue(value: string | object) { +function mapToObjectConfig(value: TSLintArgValue): TSLintObject { return typeof value === "string" ? { arrays: value, @@ -106,15 +116,63 @@ function getFieldValue(value: string | object) { : value; } -enum TSLintValue { - Always = "always", - Never = "never", +function collectNotices(args: TSLintArg[]): string[] { + const tslintArg = args[0]; + + return [buildNoticeForEsSpecCompliant(tslintArg), buildNoticeForTypeLiterals(tslintArg)].filter( + Boolean, + ); } -enum ESLintValue { - Never = "never", - Always = "always", - AlwaysMultiline = "always-multiline", - OnlyMultiline = "only-multiline", - Ignore = "ignore", +function buildNoticeForEsSpecCompliant(arg: TSLintArg): string { + const unsupportedConfigKey = "esSpecCompliant"; + + if (Object.keys(arg).includes(unsupportedConfigKey)) { + return `ESLint does not support config property ${unsupportedConfigKey}`; + } + + return ""; } + +function buildNoticeForTypeLiterals(arg: TSLintArg): string { + const { singleline, multiline } = arg; + const hasTypeLiterals = (field: any) => + typeof field === "object" && Object.keys(field).includes(unsupportedKeyInEsLint); + + if (hasTypeLiterals(singleline) || hasTypeLiterals(multiline)) { + return `ESLint does not support config property ${unsupportedKeyInEsLint}`; + } + + return ""; +} + +type TSLintArg = { + singleline?: TSLintArgValue; + multiline?: TSLintArgValue; + esSpecCompliant?: boolean; +}; + +type TSLintArgValue = TSLintStringValue | TSLintObject; +type TSLintObjectKey = keyof TSLintObject; + +type TSLintObject = { + arrays?: TSLintStringValueForObject; + objects?: TSLintStringValueForObject; + functions?: TSLintStringValueForObject; + imports?: TSLintStringValueForObject; + exports?: TSLintStringValueForObject; + typeLiterals?: TSLintStringValueForObject; +}; +type TSLintStringValue = "always" | "never"; +type TSLintStringValueForObject = TSLintStringValue | "ignore"; + +// ESLint +type ESLintArgValue = ESLintStringValue | ESLintObject; +type ESLintStringValue = "never" | "always" | "always-multiline" | "only-multiline" | "ignore"; +type ESLintObject = { + arrays?: ESLintStringValue; + objects?: ESLintStringValue; + functions?: ESLintStringValue; + imports?: ESLintStringValue; + exports?: ESLintStringValue; +};