Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
feat(eslint-plugin): add new no-base-to-string rule (#1522)
  • Loading branch information
Josh Goldberg committed Feb 29, 2020
1 parent dd233b5 commit 8333d41
Show file tree
Hide file tree
Showing 6 changed files with 354 additions and 0 deletions.
1 change: 1 addition & 0 deletions packages/eslint-plugin/README.md
Expand Up @@ -107,6 +107,7 @@ Pro Tip: For larger codebases you may want to consider splitting our linting int
| [`@typescript-eslint/member-delimiter-style`](./docs/rules/member-delimiter-style.md) | Require a specific member delimiter style for interfaces and type literals | :heavy_check_mark: | :wrench: | |
| [`@typescript-eslint/member-ordering`](./docs/rules/member-ordering.md) | Require a consistent member declaration order | | | |
| [`@typescript-eslint/naming-convention`](./docs/rules/naming-convention.md) | Enforces naming conventions for everything across a codebase | | | :thought_balloon: |
| [`@typescript-eslint/no-base-to-string`](./docs/rules/no-base-to-string.md) | Requires that `.toString()` is only called on objects which provide useful information when stringified | | | :thought_balloon: |
| [`@typescript-eslint/no-dynamic-delete`](./docs/rules/no-dynamic-delete.md) | Disallow the delete operator with computed key expressions | | :wrench: | |
| [`@typescript-eslint/no-empty-interface`](./docs/rules/no-empty-interface.md) | Disallow the declaration of empty interfaces | :heavy_check_mark: | :wrench: | |
| [`@typescript-eslint/no-explicit-any`](./docs/rules/no-explicit-any.md) | Disallow usage of the `any` type | :heavy_check_mark: | :wrench: | |
Expand Down
59 changes: 59 additions & 0 deletions packages/eslint-plugin/docs/rules/no-base-to-string.md
@@ -0,0 +1,59 @@
# Requires that `.toString()` is only called on objects which provide useful information when stringified (`no-base-to-string`)

JavaScript will call `toString()` on an object when it is converted to a string, such as when `+` adding to a string or in <code>`${}`</code> template literals.

The default Object `.toString()` returns `"[object Object]"`, so this rule requires stringified objects define a more useful `.toString()` method.

Note that `Function` provides its own `.toString()` that returns the function's code.
Functions are not flagged by this rule.

This rule has some overlap with with [`restrict-plus-operands`](./restrict-plus-operands.md) and [`restrict-template-expressions`](./restrict-template-expressions.md).

## Rule Details

This rule prevents accidentally defaulting to the base Object `.toString()` method.

Examples of **incorrect** code for this rule:

```ts
// Passing an object or class instance to string concatenation:
'' + {};

class MyClass {}
const value = new MyClass();
value + '';

// Interpolation and manual .toString() calls too:
`Value: ${value}`;
({}.toString());
```

Examples of **correct** code for this rule:

```ts
// These types all have useful .toString()s
'Text' + true;
`Value: ${123}`;
`Arrays too: ${[1, 2, 3]}`;
(() => {}).toString();

// Defining a custom .toString class is considered acceptable
class CustomToString {
toString() {
return 'Hello, world!';
}
}
`Value: ${new CustomToString()}`;

const literalWithToString = {
toString: () => 'Hello, world!',
};

`Value: ${literalWithToString}`;
```

## When Not To Use It

If you don't mind `"[object Object]"` in your strings, then you will not need this rule.

- [`Object.prototype.toString()` MDN documentation](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/toString)
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/configs/all.json
Expand Up @@ -26,6 +26,7 @@
"@typescript-eslint/naming-convention": "error",
"no-array-constructor": "off",
"@typescript-eslint/no-array-constructor": "error",
"@typescript-eslint/no-base-to-string": "error",
"no-dupe-class-members": "off",
"@typescript-eslint/no-dupe-class-members": "error",
"@typescript-eslint/no-dynamic-delete": "error",
Expand Down
2 changes: 2 additions & 0 deletions packages/eslint-plugin/src/rules/index.ts
Expand Up @@ -23,6 +23,7 @@ import memberNaming from './member-naming';
import memberOrdering from './member-ordering';
import namingConvention from './naming-convention';
import noArrayConstructor from './no-array-constructor';
import noBaseToString from './no-base-to-string';
import noDupeClassMembers from './no-dupe-class-members';
import noDynamicDelete from './no-dynamic-delete';
import noEmptyFunction from './no-empty-function';
Expand Down Expand Up @@ -93,6 +94,7 @@ export default {
'ban-ts-ignore': banTsIgnore,
'ban-ts-comment': banTsComment,
'ban-types': banTypes,
'no-base-to-string': noBaseToString,
'brace-style': braceStyle,
camelcase: camelcase,
'class-name-casing': classNameCasing,
Expand Down
121 changes: 121 additions & 0 deletions packages/eslint-plugin/src/rules/no-base-to-string.ts
@@ -0,0 +1,121 @@
import {
TSESTree,
AST_NODE_TYPES,
} from '@typescript-eslint/experimental-utils';
import * as ts from 'typescript';

import * as util from '../util';

enum Usefulness {
Always,
Never = 'will',
Sometimes = 'may',
}

export default util.createRule({
name: 'no-base-to-string',
meta: {
docs: {
description:
'Requires that `.toString()` is only called on objects which provide useful information when stringified',
category: 'Best Practices',
recommended: false,
requiresTypeChecking: true,
},
messages: {
baseToString:
"'{{name}} {{certainty}} evaluate to '[Object object]' when stringified.",
},
schema: [],
type: 'suggestion',
},
defaultOptions: [],
create(context) {
const parserServices = util.getParserServices(context);
const typeChecker = parserServices.program.getTypeChecker();

function checkExpression(node: TSESTree.Expression, type?: ts.Type): void {
if (node.type === AST_NODE_TYPES.Literal) {
return;
}

const certainty = collectToStringCertainty(
type ??
typeChecker.getTypeAtLocation(
parserServices.esTreeNodeToTSNodeMap.get(node),
),
);
if (certainty === Usefulness.Always) {
return;
}

context.report({
data: {
certainty,
name: context.getSourceCode().getText(node),
},
messageId: 'baseToString',
node,
});
}

function collectToStringCertainty(type: ts.Type): Usefulness {
const toString = typeChecker.getPropertyOfType(type, 'toString');
if (toString === undefined || toString.declarations.length === 0) {
return Usefulness.Always;
}

if (
toString.declarations.every(
({ parent }) =>
!ts.isInterfaceDeclaration(parent) || parent.name.text !== 'Object',
)
) {
return Usefulness.Always;
}

if (!type.isUnion()) {
return Usefulness.Never;
}

for (const subType of type.types) {
if (collectToStringCertainty(subType) !== Usefulness.Never) {
return Usefulness.Sometimes;
}
}

return Usefulness.Never;
}

return {
'AssignmentExpression[operator = "+="], BinaryExpression[operator = "+"]'(
node: TSESTree.AssignmentExpression | TSESTree.BinaryExpression,
): void {
const leftType = typeChecker.getTypeAtLocation(
parserServices.esTreeNodeToTSNodeMap.get(node.left),
);
const rightType = typeChecker.getTypeAtLocation(
parserServices.esTreeNodeToTSNodeMap.get(node.right),
);

if (util.getTypeName(typeChecker, leftType) === 'string') {
checkExpression(node.right, rightType);
} else if (util.getTypeName(typeChecker, rightType) === 'string') {
checkExpression(node.left, leftType);
}
},
'CallExpression > MemberExpression.callee > Identifier[name = "toString"].property'(
node: TSESTree.Expression,
): void {
const memberExpr = node.parent as TSESTree.MemberExpression;
checkExpression(memberExpr.object);
},

TemplateLiteral(node: TSESTree.TemplateLiteral): void {
for (const expression of node.expressions) {
checkExpression(expression);
}
},
};
},
});
170 changes: 170 additions & 0 deletions packages/eslint-plugin/tests/rules/no-base-to-string.test.ts
@@ -0,0 +1,170 @@
import rule from '../../src/rules/no-base-to-string';
import { RuleTester, getFixturesRootDir } from '../RuleTester';

const rootDir = getFixturesRootDir();
const ruleTester = new RuleTester({
parser: '@typescript-eslint/parser',
parserOptions: {
ecmaVersion: 2018,
tsconfigRootDir: rootDir,
project: './tsconfig.json',
},
});

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()`,
`function someFunction() { }
someFunction.toString();`,
'unknownObject.toString()',
'unknownObject.someOtherMethod()',
'(() => {}).toString();',
`class CustomToString { toString() { return "Hello, world!"; } }
"" + (new CustomToString());`,
`const literalWithToString = {
toString: () => "Hello, world!",
};
"" + literalToString;`,
`let _ = {} * {}`,
`let _ = {} / {}`,
`let _ = {} *= {}`,
`let _ = {} /= {}`,
`let _ = {} = {}`,
`let _ = {} == {}`,
`let _ = {} === {}`,
`let _ = {} in {}`,
`let _ = {} & {}`,
`let _ = {} ^ {}`,
`let _ = {} << {}`,
`let _ = {} >> {}`,
],
invalid: [
{
code: `\`\${{}})\``,
errors: [
{
data: {
certainty: 'will',
name: '{}',
},
messageId: 'baseToString',
},
],
},
{
code: `({}).toString()`,
errors: [
{
data: {
certainty: 'will',
name: '{}',
},
messageId: 'baseToString',
},
],
},
{
code: `"" + {}`,
errors: [
{
data: {
certainty: 'will',
name: '{}',
},
messageId: 'baseToString',
},
],
},
{
code: `"" += {}`,
errors: [
{
data: {
certainty: 'will',
name: '{}',
},
messageId: 'baseToString',
},
],
},
{
code: `
let someObjectOrString = Math.random() ? { a: true } : "text";
someObjectOrString.toString();
`,
errors: [
{
data: {
certainty: 'may',
name: 'someObjectOrString',
},
messageId: 'baseToString',
},
],
},
{
code: `
let someObjectOrString = Math.random() ? { a: true } : "text";
someObjectOrString + "";
`,
errors: [
{
data: {
certainty: 'may',
name: 'someObjectOrString',
},
messageId: 'baseToString',
},
],
},
{
code: `
let someObjectOrObject = Math.random() ? { a: true, b: true } : { a: true };
someObjectOrObject.toString();
`,
errors: [
{
data: {
certainty: 'will',
name: 'someObjectOrObject',
},
messageId: 'baseToString',
},
],
},
{
code: `
let someObjectOrObject = Math.random() ? { a: true, b: true } : { a: true };
someObjectOrObject + "";
`,
errors: [
{
data: {
certainty: 'will',
name: 'someObjectOrObject',
},
messageId: 'baseToString',
},
],
},
],
});

0 comments on commit 8333d41

Please sign in to comment.