From fbeb9996bcc0ff8d4f4fd53644ea3684a8b6a71c Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Wed, 8 Jan 2025 00:50:41 +0200 Subject: [PATCH 1/5] don't crash for recursive array or tuple types --- .../src/rules/no-base-to-string.ts | 63 ++++++++++++++----- .../tests/rules/no-base-to-string.test.ts | 58 +++++++++++++++++ 2 files changed, 104 insertions(+), 17 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-base-to-string.ts b/packages/eslint-plugin/src/rules/no-base-to-string.ts index f7afc31d1ecd..dce69a48457b 100644 --- a/packages/eslint-plugin/src/rules/no-base-to-string.ts +++ b/packages/eslint-plugin/src/rules/no-base-to-string.ts @@ -74,6 +74,7 @@ export default createRule({ } const certainty = collectToStringCertainty( type ?? services.getTypeAtLocation(node), + new Set(), ); if (certainty === Usefulness.Always) { return; @@ -92,8 +93,9 @@ export default createRule({ function checkExpressionForArrayJoin( node: TSESTree.Node, type: ts.Type, + visited: Set, ): void { - const certainty = collectJoinCertainty(type); + const certainty = collectJoinCertainty(type, visited); if (certainty === Usefulness.Always) { return; @@ -140,9 +142,14 @@ export default createRule({ return Usefulness.Never; } - function collectTupleCertainty(type: ts.TypeReference): Usefulness { + function collectTupleCertainty( + type: ts.TypeReference, + visited: Set, + ): Usefulness { const typeArgs = checker.getTypeArguments(type); - const certainties = typeArgs.map(t => collectToStringCertainty(t)); + const certainties = typeArgs.map(t => + collectToStringCertainty(t, visited), + ); if (certainties.some(certainty => certainty === Usefulness.Never)) { return Usefulness.Never; } @@ -154,39 +161,57 @@ export default createRule({ return Usefulness.Always; } - function collectArrayCertainty(type: ts.Type): Usefulness { + function collectArrayCertainty( + type: ts.Type, + visited: Set, + ): Usefulness { const elemType = nullThrows( type.getNumberIndexType(), 'array should have number index type', ); - return collectToStringCertainty(elemType); + return collectToStringCertainty(elemType, visited); } - function collectJoinCertainty(type: ts.Type): Usefulness { + function collectJoinCertainty( + type: ts.Type, + visited: Set, + ): Usefulness { if (tsutils.isUnionType(type)) { - return collectUnionTypeCertainty(type, collectJoinCertainty); + return collectUnionTypeCertainty(type, t => + collectJoinCertainty(t, visited), + ); } if (tsutils.isIntersectionType(type)) { - return collectIntersectionTypeCertainty(type, collectJoinCertainty); + return collectIntersectionTypeCertainty(type, t => + collectJoinCertainty(t, visited), + ); } if (checker.isTupleType(type)) { - return collectTupleCertainty(type); + return collectTupleCertainty(type, visited); } if (checker.isArrayType(type)) { - return collectArrayCertainty(type); + return collectArrayCertainty(type, visited); } return Usefulness.Always; } - function collectToStringCertainty(type: ts.Type): Usefulness { + function collectToStringCertainty( + type: ts.Type, + visited: Set, + ): Usefulness { + if (visited.has(type)) { + // don't report if this is a self referencing array or tuple type + return Usefulness.Always; + } + if (tsutils.isTypeParameter(type)) { const constraint = type.getConstraint(); if (constraint) { - return collectToStringCertainty(constraint); + return collectToStringCertainty(constraint, visited); } // unconstrained generic means `unknown` return Usefulness.Always; @@ -205,19 +230,23 @@ export default createRule({ } if (type.isIntersection()) { - return collectIntersectionTypeCertainty(type, collectToStringCertainty); + return collectIntersectionTypeCertainty(type, t => + collectToStringCertainty(t, visited), + ); } if (type.isUnion()) { - return collectUnionTypeCertainty(type, collectToStringCertainty); + return collectUnionTypeCertainty(type, t => + collectToStringCertainty(t, visited), + ); } if (checker.isTupleType(type)) { - return collectTupleCertainty(type); + return collectTupleCertainty(type, new Set([...visited, type])); } if (checker.isArrayType(type)) { - return collectArrayCertainty(type); + return collectArrayCertainty(type, new Set([...visited, type])); } const toString = @@ -290,7 +319,7 @@ export default createRule({ ): void { const memberExpr = node.parent as TSESTree.MemberExpression; const type = getConstrainedTypeAtLocation(services, memberExpr.object); - checkExpressionForArrayJoin(memberExpr.object, type); + checkExpressionForArrayJoin(memberExpr.object, type, new Set()); }, 'CallExpression > MemberExpression.callee > Identifier[name = /^(toLocaleString|toString)$/].property'( node: TSESTree.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 index 6ac4b14bd345..a7e2e82fc4ec 100644 --- a/packages/eslint-plugin/tests/rules/no-base-to-string.test.ts +++ b/packages/eslint-plugin/tests/rules/no-base-to-string.test.ts @@ -483,6 +483,34 @@ function foo(x: T) { declare const u: unknown; String(u); `, + ` +type Value = boolean | Value[]; + +declare const v: Value; + +String(v); + `, + ` +type Value = (boolean | Value)[]; + +declare const v: Value; + +String(v); + `, + ` +type Value = [Value]; + +declare const v: Value; + +String(v); + `, + ` +type Value = [Value | number]; + +declare const v: Value; + +String(v); + `, ], invalid: [ { @@ -1811,5 +1839,35 @@ foo.toString(); }, ], }, + { + code: ` +type Value = + | boolean + | number + | string + | Date + | Struct + | Uint8Array + | Value[] + | undefined; + +interface Struct { + [key: string]: Value; +} + +function foo(v: Value) { + return \`Hi \${v}\`; +} + `, + errors: [ + { + data: { + certainty: 'may', + name: 'v', + }, + messageId: 'baseToString', + }, + ], + }, ], }); From 5b74c6291f8bdc9d0bccbee4eefe7186c92eea23 Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Wed, 8 Jan 2025 18:54:22 +0200 Subject: [PATCH 2/5] testing adjustments --- .../tests/rules/no-base-to-string.test.ts | 97 ++++++++++++++----- 1 file changed, 72 insertions(+), 25 deletions(-) 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 index a7e2e82fc4ec..aac26b1edc4d 100644 --- a/packages/eslint-plugin/tests/rules/no-base-to-string.test.ts +++ b/packages/eslint-plugin/tests/rules/no-base-to-string.test.ts @@ -484,31 +484,25 @@ declare const u: unknown; String(u); `, ` -type Value = boolean | Value[]; - +type Value = string | Value[]; declare const v: Value; String(v); `, ` -type Value = (boolean | Value)[]; - +type Value = (string | Value)[]; declare const v: Value; String(v); `, ` -type Value = [Value]; - +type Value = [Value, Value]; declare const v: Value; String(v); `, ` -type Value = [Value | number]; - -declare const v: Value; - +declare const v: ('foo' | 'bar')[][]; String(v); `, ], @@ -1841,23 +1835,27 @@ foo.toString(); }, { code: ` -type Value = - | boolean - | number - | string - | Date - | Struct - | Uint8Array - | Value[] - | undefined; +type Value = { foo: string } | Value[]; +declare const v: Value; -interface Struct { - [key: string]: Value; -} +String(v); + `, + errors: [ + { + data: { + certainty: 'may', + name: 'v', + }, + messageId: 'baseToString', + }, + ], + }, + { + code: ` +type Value = ({ foo: string } | Value)[]; +declare const v: Value; -function foo(v: Value) { - return \`Hi \${v}\`; -} +String(v); `, errors: [ { @@ -1869,5 +1867,54 @@ function foo(v: Value) { }, ], }, + { + code: ` +type Value = [{ foo: string }, Value]; +declare const v: Value; + +String(v); + `, + errors: [ + { + data: { + certainty: 'will', + name: 'v', + }, + messageId: 'baseToString', + }, + ], + }, + { + code: ` +declare const v: { foo: string }[][]; +v.join(); + `, + errors: [ + { + data: { + certainty: 'will', + name: 'v', + }, + messageId: 'baseArrayJoin', + }, + ], + }, + { + code: ` +type Value = [{ value: Vallue }]; +declare const v: Value; + +String(v); + `, + errors: [ + { + data: { + certainty: 'will', + name: 'v', + }, + messageId: 'baseToString', + }, + ], + }, ], }); From ea7e77c71e6da867f8069e0bff8f4035f36b6eef Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Wed, 8 Jan 2025 19:02:52 +0200 Subject: [PATCH 3/5] change test --- .../eslint-plugin/tests/rules/no-base-to-string.test.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) 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 index aac26b1edc4d..4f96a7501639 100644 --- a/packages/eslint-plugin/tests/rules/no-base-to-string.test.ts +++ b/packages/eslint-plugin/tests/rules/no-base-to-string.test.ts @@ -496,7 +496,13 @@ declare const v: Value; String(v); `, ` -type Value = [Value, Value]; +type Value = Value[]; +declare const v: Value; + +String(v); + `, + ` +type Value = [Value]; declare const v: Value; String(v); From 24da033a4572e5207ca77b905b194260e91bb4f4 Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Wed, 8 Jan 2025 19:08:00 +0200 Subject: [PATCH 4/5] change test --- .../tests/rules/no-base-to-string.test.ts | 17 ----------------- 1 file changed, 17 deletions(-) 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 index 4f96a7501639..f0fe39979c4d 100644 --- a/packages/eslint-plugin/tests/rules/no-base-to-string.test.ts +++ b/packages/eslint-plugin/tests/rules/no-base-to-string.test.ts @@ -1905,22 +1905,5 @@ v.join(); }, ], }, - { - code: ` -type Value = [{ value: Vallue }]; -declare const v: Value; - -String(v); - `, - errors: [ - { - data: { - certainty: 'will', - name: 'v', - }, - messageId: 'baseToString', - }, - ], - }, ], }); From 75d2265b81f4373cd8d25d43e7901779a9e9ae23 Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Sun, 19 Jan 2025 19:46:48 +0200 Subject: [PATCH 5/5] simplify passing of 'visited' for array-join --- packages/eslint-plugin/src/rules/no-base-to-string.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-base-to-string.ts b/packages/eslint-plugin/src/rules/no-base-to-string.ts index dce69a48457b..6617c8d1dea2 100644 --- a/packages/eslint-plugin/src/rules/no-base-to-string.ts +++ b/packages/eslint-plugin/src/rules/no-base-to-string.ts @@ -93,9 +93,8 @@ export default createRule({ function checkExpressionForArrayJoin( node: TSESTree.Node, type: ts.Type, - visited: Set, ): void { - const certainty = collectJoinCertainty(type, visited); + const certainty = collectJoinCertainty(type, new Set()); if (certainty === Usefulness.Always) { return; @@ -319,7 +318,7 @@ export default createRule({ ): void { const memberExpr = node.parent as TSESTree.MemberExpression; const type = getConstrainedTypeAtLocation(services, memberExpr.object); - checkExpressionForArrayJoin(memberExpr.object, type, new Set()); + checkExpressionForArrayJoin(memberExpr.object, type); }, 'CallExpression > MemberExpression.callee > Identifier[name = /^(toLocaleString|toString)$/].property'( node: TSESTree.Expression,