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
28 changes: 28 additions & 0 deletions packages/eslint-plugin/docs/rules/no-base-to-string.md
Expand Up @@ -52,6 +52,34 @@ const literalWithToString = {
`Value: ${literalWithToString}`;
```

## Options

```ts
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`, `URLSearchParams` etc.

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.
Expand Down
26 changes: 24 additions & 2 deletions 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,28 @@ export default util.createRule<Options, MessageIds>({
type: 'boolean',
default: true,
},
ignoredTypeNames: {
type: 'array',
items: {
type: 'string',
},
},
},
additionalProperties: false,
},
],
type: 'suggestion',
},
defaultOptions: [{ ignoreTaggedTemplateExpressions: true }],
create(context) {
defaultOptions: [
{
ignoreTaggedTemplateExpressions: true,
ignoredTypeNames: ['RegExp'],
},
],
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 +97,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
70 changes: 45 additions & 25 deletions packages/eslint-plugin/tests/rules/no-base-to-string.test.ts
Expand Up @@ -11,39 +11,59 @@ const ruleTester = new RuleTester({
},
});

const literalListBasic: string[] = [
"''",
"'text'",
'true',
'false',
'1',
'1n',
'[]',
'/regex/',
];

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