Skip to content

Commit

Permalink
fix(eslint-plugin): [no-base-to-string] handle more robustly when mul…
Browse files Browse the repository at this point in the history 
…tiple `toString()` declarations are present for a type (#10432)

* remove confusing spread element test cases

* fix intersection checking

* fix for old versions of TS

* lintfix

* readability

* isTypeParameter comment

* derp

* ignored types

* expression
  • Loading branch information
@kirkwaiblinger
kirkwaiblinger authored Dec 7, 2024
1 parent 47f1ab3 commit 24a1510
Show file tree
Hide file tree
Showing 2 changed files with 141 additions and 41 deletions.
63 changes: 42 additions & 21 deletions packages/eslint-plugin/src/rules/no-base-to-string.ts
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
/* eslint-disable @typescript-eslint/internal/prefer-ast-types-enum */
import type { TSESTree } from '@typescript-eslint/utils';

import { AST_NODE_TYPES } from '@typescript-eslint/utils';
Expand Down Expand Up @@ -69,7 +68,7 @@ export default createRule<Options, MessageIds>({
const checker = services.program.getTypeChecker();
const ignoredTypeNames = option.ignoredTypeNames ?? [];

function checkExpression(node: TSESTree.Node, type?: ts.Type): void {
function checkExpression(node: TSESTree.Expression, type?: ts.Type): void {
if (node.type === AST_NODE_TYPES.Literal) {
return;
}
Expand Down Expand Up @@ -176,15 +175,17 @@ export default createRule<Options, MessageIds>({
}

function collectToStringCertainty(type: ts.Type): Usefulness {
const toString =
checker.getPropertyOfType(type, 'toString') ??
checker.getPropertyOfType(type, 'toLocaleString');
const declarations = toString?.getDeclarations();
if (!toString || !declarations || declarations.length === 0) {
// https://github.com/JoshuaKGoldberg/ts-api-utils/issues/382
if ((tsutils.isTypeParameter as (t: ts.Type) => boolean)(type)) {
const constraint = type.getConstraint();
if (constraint) {
return collectToStringCertainty(constraint);
}
// unconstrained generic means `unknown`
return Usefulness.Always;
}

// Patch for old version TypeScript, the Boolean type definition missing toString()
// the Boolean type definition missing toString()
if (
type.flags & ts.TypeFlags.Boolean ||
type.flags & ts.TypeFlags.BooleanLiteral
Expand All @@ -196,32 +197,49 @@ export default createRule<Options, MessageIds>({
return Usefulness.Always;
}

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

if (type.isIntersection()) {
return collectIntersectionTypeCertainty(type, collectToStringCertainty);
}

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

const toString =
checker.getPropertyOfType(type, 'toString') ??
checker.getPropertyOfType(type, 'toLocaleString');
if (!toString) {
// e.g. any/unknown
return Usefulness.Always;
}
return collectUnionTypeCertainty(type, collectToStringCertainty);

const declarations = toString.getDeclarations();

if (declarations == null || declarations.length !== 1) {
// If there are multiple declarations, at least one of them must not be
// the default object toString.
//
// This may only matter for older versions of TS
// see https://github.com/typescript-eslint/typescript-eslint/issues/8585
return Usefulness.Always;
}

const declaration = declarations[0];
const isBaseToString =
ts.isInterfaceDeclaration(declaration.parent) &&
declaration.parent.name.text === 'Object';
return isBaseToString ? Usefulness.Never : Usefulness.Always;
}

function isBuiltInStringCall(node: TSESTree.CallExpression): boolean {
if (
node.callee.type === AST_NODE_TYPES.Identifier &&
// eslint-disable-next-line @typescript-eslint/internal/prefer-ast-types-enum
node.callee.name === 'String' &&
node.arguments[0]
) {
const scope = context.sourceCode.getScope(node);
// eslint-disable-next-line @typescript-eslint/internal/prefer-ast-types-enum
const variable = scope.set.get('String');
return !variable?.defs.length;
}
Expand All @@ -245,7 +263,10 @@ export default createRule<Options, MessageIds>({
}
},
CallExpression(node: TSESTree.CallExpression): void {
if (isBuiltInStringCall(node)) {
if (
isBuiltInStringCall(node) &&
node.arguments[0].type !== AST_NODE_TYPES.SpreadElement
) {
checkExpression(node.arguments[0]);
}
},
Expand Down
119 changes: 99 additions & 20 deletions packages/eslint-plugin/tests/rules/no-base-to-string.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,16 +135,20 @@ tag\`\${{}}\`;
"'' += new URL();",
"'' += new URLSearchParams();",
`
let numbers = [1, 2, 3];
String(...a);
`,
`
Number(1);
`,
{
code: 'String(/regex/);',
options: [{ ignoredTypeNames: ['RegExp'] }],
},
{
code: `
type Foo = { a: string } | { b: string };
declare const foo: Foo;
String(foo);
`,
options: [{ ignoredTypeNames: ['Foo'] }],
},
`
function String(value) {
return value;
Expand Down Expand Up @@ -215,6 +219,46 @@ class Foo {}
declare const tuple: [string] & [Foo];
tuple.join('');
`,
// don't bother trying to interpret spread args.
`
let objects = [{}, {}];
String(...objects);
`,
// https://github.com/typescript-eslint/typescript-eslint/issues/8585
`
type Constructable<Entity> = abstract new (...args: any[]) => Entity;
interface GuildChannel {
toString(): \`<#\${string}>\`;
}
declare const foo: Constructable<GuildChannel & { bar: 1 }>;
class ExtendedGuildChannel extends foo {}
declare const bb: ExtendedGuildChannel;
bb.toString();
`,
// https://github.com/typescript-eslint/typescript-eslint/issues/8585 with intersection order reversed.
`
type Constructable<Entity> = abstract new (...args: any[]) => Entity;
interface GuildChannel {
toString(): \`<#\${string}>\`;
}
declare const foo: Constructable<{ bar: 1 } & GuildChannel>;
class ExtendedGuildChannel extends foo {}
declare const bb: ExtendedGuildChannel;
bb.toString();
`,
`
function foo<T>(x: T) {
String(x);
}
`,
`
declare const u: unknown;
String(u);
`,
],
invalid: [
{
Expand Down Expand Up @@ -277,21 +321,6 @@ tuple.join('');
},
],
},
{
code: `
let objects = [{}, {}];
String(...objects);
`,
errors: [
{
data: {
certainty: 'will',
name: '...objects',
},
messageId: 'baseToString',
},
],
},
{
code: "'' += {};",
errors: [
Expand Down Expand Up @@ -682,13 +711,63 @@ declare const foo: Bar & Foo;
errors: [
{
data: {
certainty: 'will',
certainty: 'may',
name: 'array',
},
messageId: 'baseArrayJoin',
},
],
},
{
code: `
type Bar = Record<string, string>;
function foo<T extends string | Bar>(array: T[]) {
array[0].toString();
}
`,
errors: [
{
data: {
certainty: 'may',
name: 'array[0]',
},
messageId: 'baseToString',
},
],
},
{
code: `
type Bar = Record<string, string>;
function foo<T extends string | Bar>(value: T) {
value.toString();
}
`,
errors: [
{
data: {
certainty: 'may',
name: 'value',
},
messageId: 'baseToString',
},
],
},
{
code: `
type Bar = Record<string, string>;
declare const foo: Bar | string;
foo.toString();
`,
errors: [
{
data: {
certainty: 'may',
name: 'foo',
},
messageId: 'baseToString',
},
],
},
{
code: `
type Bar = Record<string, string>;
Expand Down

0 comments on commit 24a1510

Please sign in to comment.