Skip to content

Commit

Permalink
fix(type-utils): preventing isUnsafeAssignment infinite recursive cal…
Browse files Browse the repository at this point in the history
…ls (#8237)

* fix(type-utils): preventing isUnsafeAssignment infinite recursive calls

* chore: apply reviews
  • Loading branch information
yeonjuan committed Jan 16, 2024
1 parent 7c673a1 commit 01556f5
Show file tree
Hide file tree
Showing 4 changed files with 119 additions and 3 deletions.
33 changes: 33 additions & 0 deletions packages/eslint-plugin/tests/rules/no-unsafe-argument.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,19 @@ toHaveBeenCalledWith(1 as any);
declare function acceptsMap(arg: Map<string, string>): void;
acceptsMap(new Map());
`,
`
type T = [number, T[]];
declare function foo(t: T): void;
declare const t: T;
foo(t);
`,
`
type T = Array<T>;
declare function foo<T>(t: T): T;
const t: T = [];
foo(t);
`,
],
invalid: [
{
Expand Down Expand Up @@ -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',
},
},
],
},
],
});
18 changes: 18 additions & 0 deletions packages/eslint-plugin/tests/rules/no-unsafe-assignment.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
Expand Down Expand Up @@ -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,
},
],
},
],
});
35 changes: 34 additions & 1 deletion packages/type-utils/src/isUnsafeAssignment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,22 @@ export function isUnsafeAssignment(
receiver: ts.Type,
checker: ts.TypeChecker,
senderNode: TSESTree.Node | null,
): 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<ts.Type, Set<ts.Type>>,
): false | { sender: ts.Type; receiver: ts.Type } {
if (isTypeAnyType(type)) {
// Allow assignment of any ==> unknown.
Expand All @@ -32,6 +48,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
Expand Down Expand Up @@ -72,7 +99,13 @@ export function isUnsafeAssignment(
const arg = typeArguments[i];
const receiverArg = receiverTypeArguments[i];

const unsafe = isUnsafeAssignment(arg, receiverArg, checker, senderNode);
const unsafe = isUnsafeAssignmentWorker(
arg,
receiverArg,
checker,
senderNode,
visited,
);
if (unsafe) {
return { sender: type, receiver };
}
Expand Down
36 changes: 34 additions & 2 deletions packages/type-utils/tests/isUnsafeAssignment.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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),
Expand Down Expand Up @@ -111,6 +116,21 @@ describe('isUnsafeAssignment', () => {
'Set<Set<Set<string>>>',
);
});

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', () => {
Expand Down Expand Up @@ -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();
});
});
});

0 comments on commit 01556f5

Please sign in to comment.