From a98d6d89182aa686d2e49f24a66905f98440eb9b Mon Sep 17 00:00:00 2001 From: duduluu Date: Sat, 25 Apr 2020 20:57:40 +0800 Subject: [PATCH 1/6] fix(eslint-plugin): fix no-base-to-string for old ts verion boolean type --- packages/eslint-plugin/src/rules/no-base-to-string.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/eslint-plugin/src/rules/no-base-to-string.ts b/packages/eslint-plugin/src/rules/no-base-to-string.ts index a1121f37abd..ea1aebeff23 100644 --- a/packages/eslint-plugin/src/rules/no-base-to-string.ts +++ b/packages/eslint-plugin/src/rules/no-base-to-string.ts @@ -84,6 +84,11 @@ export default util.createRule({ return Usefulness.Always; } + // Patch for old version TypeScript, the Boolean type definition missing toString() + if (type.flags & ts.TypeFlags.BooleanLiteral) { + return Usefulness.Always; + } + if ( toString.declarations.every( ({ parent }) => From c83950cb222d458c7572a6c52972488ed4eebb5e Mon Sep 17 00:00:00 2001 From: duduluu Date: Sat, 25 Apr 2020 20:58:29 +0800 Subject: [PATCH 2/6] feat(eslint-plugin): rule no-base-to-string add option ignoredTypeNames --- .../eslint-plugin/src/rules/no-base-to-string.ts | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/no-base-to-string.ts b/packages/eslint-plugin/src/rules/no-base-to-string.ts index ea1aebeff23..ea7c59ca5c1 100644 --- a/packages/eslint-plugin/src/rules/no-base-to-string.ts +++ b/packages/eslint-plugin/src/rules/no-base-to-string.ts @@ -16,6 +16,7 @@ type Options = [ { /** @deprecated This option is now ignored and treated as always true, it will be removed in 3.0 */ ignoreTaggedTemplateExpressions?: boolean; + ignoredTypeNames?: string[]; }, ]; type MessageIds = 'baseToString'; @@ -42,6 +43,12 @@ export default util.createRule({ type: 'boolean', default: true, }, + ignoredTypeNames: { + type: 'array', + items: { + type: 'string', + }, + }, }, additionalProperties: false, }, @@ -49,9 +56,10 @@ export default util.createRule({ type: 'suggestion', }, defaultOptions: [{ ignoreTaggedTemplateExpressions: true }], - create(context) { + create(context, [option]) { const parserServices = util.getParserServices(context); const typeChecker = parserServices.program.getTypeChecker(); + const ignoredTypeNames = option.ignoredTypeNames ?? []; function checkExpression(node: TSESTree.Expression, type?: ts.Type): void { if (node.type === AST_NODE_TYPES.Literal) { @@ -89,6 +97,10 @@ export default util.createRule({ return Usefulness.Always; } + if (ignoredTypeNames.includes(util.getTypeName(typeChecker, type))) { + return Usefulness.Always; + } + if ( toString.declarations.every( ({ parent }) => From c8b6838c98abdfb896ee05abe0c57448f9ac3330 Mon Sep 17 00:00:00 2001 From: duduluu Date: Sat, 25 Apr 2020 20:58:59 +0800 Subject: [PATCH 3/6] test(eslint-plugin): full tests for rule no-base-to-string --- .../tests/rules/no-base-to-string.test.ts | 84 +++++++++++++------ 1 file changed, 59 insertions(+), 25 deletions(-) diff --git a/packages/eslint-plugin/tests/rules/no-base-to-string.test.ts b/packages/eslint-plugin/tests/rules/no-base-to-string.test.ts index 82bd4e0302e..dac36952ddd 100644 --- a/packages/eslint-plugin/tests/rules/no-base-to-string.test.ts +++ b/packages/eslint-plugin/tests/rules/no-base-to-string.test.ts @@ -11,39 +11,58 @@ const ruleTester = new RuleTester({ }, }); +const literalListBasic: string[] = [ + "''", + "'text'", + 'true', + 'false', + '1', + '1n', + '[]', +]; + +const literalListNeedParen: string[] = [ + '{}.constructor()', + '() => {}', + 'function() {}', +]; + +const literalList = [...literalListBasic, ...literalListNeedParen]; + +const literalListWrapped = [ + ...literalListBasic, + ...literalListNeedParen.map(i => `(${i})`), +]; + ruleTester.run('no-base-to-string', rule, { valid: [ - "`${''}`;", - '`${true}`;', - '`${[]}`;', - '`${function() {}}`;', - "'' + '';", - "'' + true;", - "'' + [];", - 'true + true;', - "true + '';", - 'true + [];', - '[] + [];', - '[] + true;', - "[] + '';", - '({}.constructor());', - "'text'.toString();", - 'false.toString();', - ` -let value = 1; -value.toString(); - `, - ` -let value = 1n; -value.toString(); - `, + // template + ...literalList.map(i => `\`\${${i}}\`;`), + + // operator + += + ...literalListWrapped + .map(l => literalListWrapped.map(r => `${l} + ${r};`)) + .reduce((pre, cur) => [...pre, ...cur]), + + // toString() + ...literalListWrapped.map(i => `${i === '1' ? `(${i})` : i}.toString();`), + + // variable toString() and template + ...literalList.map( + i => ` + let value = ${i}; + value.toString(); + let text = \`\${value}\`; + `, + ), + ` function someFunction() {} someFunction.toString(); +let text = \`\${someFunction}\`; `, 'unknownObject.toString();', 'unknownObject.someOtherMethod();', - '(() => {}).toString();', ` class CustomToString { toString() { @@ -85,6 +104,21 @@ tag\`\${{}}\`; }, ], }, + { + code: ` + \`\${/regex/}\`; + '' + /regex/; + /regex/.toString(); + let value = /regex/; + value.toString(); + let text = \`\${value}\`; + `, + options: [ + { + ignoredTypeNames: ['RegExp'], + }, + ], + }, ], invalid: [ { From 939fa10329882b241a30e07888bb4ca6c12cf1fd Mon Sep 17 00:00:00 2001 From: duduluu Date: Sat, 25 Apr 2020 21:20:11 +0800 Subject: [PATCH 4/6] docs(eslint-plugin): update doc for rule no-base-to-string --- .../docs/rules/no-base-to-string.md | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/packages/eslint-plugin/docs/rules/no-base-to-string.md b/packages/eslint-plugin/docs/rules/no-base-to-string.md index 569ffe0e6d9..1a7220a09c8 100644 --- a/packages/eslint-plugin/docs/rules/no-base-to-string.md +++ b/packages/eslint-plugin/docs/rules/no-base-to-string.md @@ -52,6 +52,41 @@ const literalWithToString = { `Value: ${literalWithToString}`; ``` +## Options + +```ts +type Options = { + ignoredTypeNames?: string[]; +}; +``` + +### `ignoreTypeNames` + +A string array of type names to ignore, this is useful for types missing `toString()` (but actually has `toString()`). +There are some types missing `toString()` in old version TypeScript, like `RegExp`, `URL`, etc. + +The following patterns are considered incorrect code if no options are provided (old TypeScript version) + +```ts +`${/regex/}`; +'' + /regex/; +/regex/.toString(); +let value = /regex/; +value.toString(); +let text = `${value}`; +``` + +The following patterns are considered correct with the default options `{ ignoreTypeNames: ["RegExp"] }`: + +```ts +`${/regex/}`; +'' + /regex/; +/regex/.toString(); +let value = /regex/; +value.toString(); +let text = `${value}`; +``` + ## When Not To Use It If you don't mind `"[object Object]"` in your strings, then you will not need this rule. From ef8d7823787194ff2e586674098a5131ade81277 Mon Sep 17 00:00:00 2001 From: duduluu Date: Sun, 26 Apr 2020 13:18:30 +0800 Subject: [PATCH 5/6] docs(eslint-plugin): fix typo for no-base-to-string document --- packages/eslint-plugin/docs/rules/no-base-to-string.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/docs/rules/no-base-to-string.md b/packages/eslint-plugin/docs/rules/no-base-to-string.md index 1a7220a09c8..bcfc249b0b0 100644 --- a/packages/eslint-plugin/docs/rules/no-base-to-string.md +++ b/packages/eslint-plugin/docs/rules/no-base-to-string.md @@ -76,7 +76,7 @@ value.toString(); let text = `${value}`; ``` -The following patterns are considered correct with the default options `{ ignoreTypeNames: ["RegExp"] }`: +The following patterns are considered correct with the options `{ ignoreTypeNames: ["RegExp"] }`: ```ts `${/regex/}`; From abdf4c929e0e1d728106932c4ef05c8cd7d98ab3 Mon Sep 17 00:00:00 2001 From: duduluu Date: Mon, 27 Apr 2020 15:15:13 +0800 Subject: [PATCH 6/6] feat(eslint-plugin): no-base-to-string default ignore regex --- .../docs/rules/no-base-to-string.md | 19 ++++++------------- .../src/rules/no-base-to-string.ts | 7 ++++++- .../tests/rules/no-base-to-string.test.ts | 16 +--------------- 3 files changed, 13 insertions(+), 29 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/no-base-to-string.md b/packages/eslint-plugin/docs/rules/no-base-to-string.md index bcfc249b0b0..5c74cb4d84c 100644 --- a/packages/eslint-plugin/docs/rules/no-base-to-string.md +++ b/packages/eslint-plugin/docs/rules/no-base-to-string.md @@ -58,25 +58,18 @@ const literalWithToString = { type Options = { ignoredTypeNames?: string[]; }; + +const defaultOptions: Options = { + ignoredTypeNames: ['RegExp'], +}; ``` ### `ignoreTypeNames` A string array of type names to ignore, this is useful for types missing `toString()` (but actually has `toString()`). -There are some types missing `toString()` in old version TypeScript, like `RegExp`, `URL`, etc. - -The following patterns are considered incorrect code if no options are provided (old TypeScript version) - -```ts -`${/regex/}`; -'' + /regex/; -/regex/.toString(); -let value = /regex/; -value.toString(); -let text = `${value}`; -``` +There are some types missing `toString()` in old version TypeScript, like `RegExp`, `URL`, `URLSearchParams` etc. -The following patterns are considered correct with the options `{ ignoreTypeNames: ["RegExp"] }`: +The following patterns are considered correct with the default options `{ ignoreTypeNames: ["RegExp"] }`: ```ts `${/regex/}`; diff --git a/packages/eslint-plugin/src/rules/no-base-to-string.ts b/packages/eslint-plugin/src/rules/no-base-to-string.ts index ea7c59ca5c1..2e06bdb870d 100644 --- a/packages/eslint-plugin/src/rules/no-base-to-string.ts +++ b/packages/eslint-plugin/src/rules/no-base-to-string.ts @@ -55,7 +55,12 @@ export default util.createRule({ ], type: 'suggestion', }, - defaultOptions: [{ ignoreTaggedTemplateExpressions: true }], + defaultOptions: [ + { + ignoreTaggedTemplateExpressions: true, + ignoredTypeNames: ['RegExp'], + }, + ], create(context, [option]) { const parserServices = util.getParserServices(context); const typeChecker = parserServices.program.getTypeChecker(); diff --git a/packages/eslint-plugin/tests/rules/no-base-to-string.test.ts b/packages/eslint-plugin/tests/rules/no-base-to-string.test.ts index dac36952ddd..842ead488c1 100644 --- a/packages/eslint-plugin/tests/rules/no-base-to-string.test.ts +++ b/packages/eslint-plugin/tests/rules/no-base-to-string.test.ts @@ -19,6 +19,7 @@ const literalListBasic: string[] = [ '1', '1n', '[]', + '/regex/', ]; const literalListNeedParen: string[] = [ @@ -104,21 +105,6 @@ tag\`\${{}}\`; }, ], }, - { - code: ` - \`\${/regex/}\`; - '' + /regex/; - /regex/.toString(); - let value = /regex/; - value.toString(); - let text = \`\${value}\`; - `, - options: [ - { - ignoredTypeNames: ['RegExp'], - }, - ], - }, ], invalid: [ {