diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index c560ebb6761..778e159d6ea 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -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: | | diff --git a/packages/eslint-plugin/docs/rules/no-base-to-string.md b/packages/eslint-plugin/docs/rules/no-base-to-string.md new file mode 100644 index 00000000000..4050047673b --- /dev/null +++ b/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 `${}` 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) diff --git a/packages/eslint-plugin/src/configs/all.json b/packages/eslint-plugin/src/configs/all.json index 23e9db2d974..bf5844653ef 100644 --- a/packages/eslint-plugin/src/configs/all.json +++ b/packages/eslint-plugin/src/configs/all.json @@ -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", diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index 16a4c705ca5..0c3a5d9f0d1 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -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'; @@ -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, diff --git a/packages/eslint-plugin/src/rules/no-base-to-string.ts b/packages/eslint-plugin/src/rules/no-base-to-string.ts new file mode 100644 index 00000000000..fa404272895 --- /dev/null +++ b/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); + } + }, + }; + }, +}); 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 new file mode 100644 index 00000000000..3be0828791d --- /dev/null +++ b/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', + }, + ], + }, + ], +});