Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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();
});
});
});