From f799a522c1ba7c28e9626fdd06e020ef28c08fb6 Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Fri, 12 Jan 2024 01:02:59 +0900 Subject: [PATCH 1/2] fix(type-utils): preventing isUnsafeAssignment infinite recursive calls --- .../tests/rules/no-unsafe-argument.test.ts | 33 +++++++++++++++++ .../tests/rules/no-unsafe-assignment.test.ts | 18 ++++++++++ packages/type-utils/src/isUnsafeAssignment.ts | 20 ++++++++++- .../tests/isUnsafeAssignment.test.ts | 36 +++++++++++++++++-- 4 files changed, 104 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin/tests/rules/no-unsafe-argument.test.ts b/packages/eslint-plugin/tests/rules/no-unsafe-argument.test.ts index 3b17a756c426..e525ac428730 100644 --- a/packages/eslint-plugin/tests/rules/no-unsafe-argument.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unsafe-argument.test.ts @@ -92,6 +92,19 @@ toHaveBeenCalledWith(1 as any); declare function acceptsMap(arg: Map): void; acceptsMap(new Map()); `, + ` +type T = [number, T[]]; +declare function foo(t: T): void; +declare const t: T; + +foo(t); + `, + ` +type T = Array; +declare function foo(t: T): T; +const t: T = []; +foo(t); + `, ], invalid: [ { @@ -352,5 +365,25 @@ foo('a', 1 as any, 'a' as any, 1 as any); }, ], }, + { + code: ` +type T = [number, T[]]; +declare function foo(t: T): void; +declare const t: T; +foo(t as any); + `, + errors: [ + { + messageId: 'unsafeArgument', + line: 5, + column: 5, + endColumn: 13, + data: { + sender: 'any', + receiver: 'T', + }, + }, + ], + }, ], }); diff --git a/packages/eslint-plugin/tests/rules/no-unsafe-assignment.test.ts b/packages/eslint-plugin/tests/rules/no-unsafe-assignment.test.ts index fb2d713e647f..56da125a97ca 100644 --- a/packages/eslint-plugin/tests/rules/no-unsafe-assignment.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unsafe-assignment.test.ts @@ -114,6 +114,10 @@ class Foo { 'const x: { y: number } = { y: 1 };', 'const x = [...[1, 2, 3]];', 'const [{ [`x${1}`]: x }] = [{ [`x`]: 1 }] as [{ [`x`]: any }];', + ` +type T = [string, T[]]; +const test: T = ['string', []] as T; + `, { code: ` type Props = { a: string }; @@ -371,5 +375,19 @@ function foo() { }, ], }, + { + code: ` +type T = [string, T[]]; +const test: T = ['string', []] as any; + `, + errors: [ + { + messageId: 'anyAssignment', + line: 3, + column: 7, + endColumn: 38, + }, + ], + }, ], }); diff --git a/packages/type-utils/src/isUnsafeAssignment.ts b/packages/type-utils/src/isUnsafeAssignment.ts index 606fadfdd32c..3e3898955707 100644 --- a/packages/type-utils/src/isUnsafeAssignment.ts +++ b/packages/type-utils/src/isUnsafeAssignment.ts @@ -20,6 +20,7 @@ export function isUnsafeAssignment( receiver: ts.Type, checker: ts.TypeChecker, senderNode: TSESTree.Node | null, + visited = new Map>(), ): false | { sender: ts.Type; receiver: ts.Type } { if (isTypeAnyType(type)) { // Allow assignment of any ==> unknown. @@ -32,6 +33,17 @@ export function isUnsafeAssignment( } } + const typeAlreadyVisited = visited.get(type); + + if (typeAlreadyVisited) { + if (typeAlreadyVisited.has(receiver)) { + return false; + } + typeAlreadyVisited.add(receiver); + } else { + visited.set(type, new Set([receiver])); + } + if (tsutils.isTypeReference(type) && tsutils.isTypeReference(receiver)) { // TODO - figure out how to handle cases like this, // where the types are assignable, but not the same type @@ -72,7 +84,13 @@ export function isUnsafeAssignment( const arg = typeArguments[i]; const receiverArg = receiverTypeArguments[i]; - const unsafe = isUnsafeAssignment(arg, receiverArg, checker, senderNode); + const unsafe = isUnsafeAssignment( + arg, + receiverArg, + checker, + senderNode, + visited, + ); if (unsafe) { return { sender: type, receiver }; } diff --git a/packages/type-utils/tests/isUnsafeAssignment.test.ts b/packages/type-utils/tests/isUnsafeAssignment.test.ts index 55e2195f93af..733682b5a559 100644 --- a/packages/type-utils/tests/isUnsafeAssignment.test.ts +++ b/packages/type-utils/tests/isUnsafeAssignment.test.ts @@ -9,7 +9,10 @@ import { expectToHaveParserServices } from './test-utils/expectToHaveParserServi describe('isUnsafeAssignment', () => { const rootDir = path.join(__dirname, 'fixtures'); - function getTypes(code: string): { + function getTypes( + code: string, + declarationIndex = 0, + ): { sender: ts.Type; senderNode: TSESTree.Node; receiver: ts.Type; @@ -23,7 +26,9 @@ describe('isUnsafeAssignment', () => { expectToHaveParserServices(services); const checker = services.program.getTypeChecker(); - const declaration = ast.body[0] as TSESTree.VariableDeclaration; + const declaration = ast.body[ + declarationIndex + ] as TSESTree.VariableDeclaration; const declarator = declaration.declarations[0]; return { receiver: services.getTypeAtLocation(declarator.id), @@ -111,6 +116,21 @@ describe('isUnsafeAssignment', () => { 'Set>>', ); }); + + it('circular reference', () => { + const { sender, senderNode, receiver, checker } = getTypes( + `type T = [string, T[]]; + const test: T = ["string", []] as any;`, + 1, + ); + + expectTypesAre( + isUnsafeAssignment(sender, receiver, checker, senderNode), + checker, + 'any', + 'T', + ); + }); }); describe('safe', () => { @@ -202,5 +222,17 @@ describe('isUnsafeAssignment', () => { isUnsafeAssignment(sender, receiver, checker, senderNode), ).toBeFalsy(); }); + + it('circular reference', () => { + const { sender, senderNode, receiver, checker } = getTypes( + `type T = [string, T[]]; + const test: T = ["string", []] as T;`, + 1, + ); + + expect( + isUnsafeAssignment(sender, receiver, checker, senderNode), + ).toBeFalsy(); + }); }); }); From a84ed0484b535e6be764187a59ed11aaf5e9a886 Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Sat, 13 Jan 2024 15:00:04 +0900 Subject: [PATCH 2/2] chore: apply reviews --- packages/type-utils/src/isUnsafeAssignment.ts | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/packages/type-utils/src/isUnsafeAssignment.ts b/packages/type-utils/src/isUnsafeAssignment.ts index 3e3898955707..3f462610a909 100644 --- a/packages/type-utils/src/isUnsafeAssignment.ts +++ b/packages/type-utils/src/isUnsafeAssignment.ts @@ -20,7 +20,22 @@ export function isUnsafeAssignment( receiver: ts.Type, checker: ts.TypeChecker, senderNode: TSESTree.Node | null, - visited = new Map>(), +): false | { sender: ts.Type; receiver: ts.Type } { + return isUnsafeAssignmentWorker( + type, + receiver, + checker, + senderNode, + new Map(), + ); +} + +function isUnsafeAssignmentWorker( + type: ts.Type, + receiver: ts.Type, + checker: ts.TypeChecker, + senderNode: TSESTree.Node | null, + visited: Map>, ): false | { sender: ts.Type; receiver: ts.Type } { if (isTypeAnyType(type)) { // Allow assignment of any ==> unknown. @@ -84,7 +99,7 @@ export function isUnsafeAssignment( const arg = typeArguments[i]; const receiverArg = receiverTypeArguments[i]; - const unsafe = isUnsafeAssignment( + const unsafe = isUnsafeAssignmentWorker( arg, receiverArg, checker,