Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(eslint-plugin): fix no-base-to-string boolean literal check #1850

Merged
merged 6 commits into from Apr 27, 2020
Merged
35 changes: 35 additions & 0 deletions packages/eslint-plugin/docs/rules/no-base-to-string.md
Expand Up @@ -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 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.
Expand Down
19 changes: 18 additions & 1 deletion packages/eslint-plugin/src/rules/no-base-to-string.ts
Expand Up @@ -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';
Expand All @@ -42,16 +43,23 @@ export default util.createRule<Options, MessageIds>({
type: 'boolean',
default: true,
},
ignoredTypeNames: {
type: 'array',
items: {
type: 'string',
},
},
},
additionalProperties: false,
},
],
type: 'suggestion',
},
defaultOptions: [{ ignoreTaggedTemplateExpressions: true }],
bradzacher marked this conversation as resolved.
Show resolved Hide resolved
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) {
Expand Down Expand Up @@ -84,6 +92,15 @@ export default util.createRule<Options, MessageIds>({
return Usefulness.Always;
}

// Patch for old version TypeScript, the Boolean type definition missing toString()
if (type.flags & ts.TypeFlags.BooleanLiteral) {
return Usefulness.Always;
}

if (ignoredTypeNames.includes(util.getTypeName(typeChecker, type))) {
return Usefulness.Always;
}

if (
toString.declarations.every(
({ parent }) =>
Expand Down
84 changes: 59 additions & 25 deletions packages/eslint-plugin/tests/rules/no-base-to-string.test.ts
Expand Up @@ -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() {
Expand Down Expand Up @@ -85,6 +104,21 @@ tag\`\${{}}\`;
},
],
},
{
code: `
\`\${/regex/}\`;
'' + /regex/;
/regex/.toString();
let value = /regex/;
value.toString();
let text = \`\${value}\`;
`,
options: [
{
ignoredTypeNames: ['RegExp'],
},
],
},
],
invalid: [
{
Expand Down