From cae4f4a0f33f8c954b1670d0abcfc8edd6193a06 Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Sat, 15 May 2021 14:52:27 -0700 Subject: [PATCH] fix(eslint-plugin): [no-unsafe-*] special case handling for the empty map constructor with no generics (#3394) Fixes #2109 Sucks that it had to be this way, but oh well. --- .../src/rules/no-unsafe-argument.ts | 4 ++ .../src/rules/no-unsafe-assignment.ts | 7 ++- .../src/rules/no-unsafe-return.ts | 1 + packages/eslint-plugin/src/util/types.ts | 20 +++++++- .../tests/rules/no-unsafe-argument.test.ts | 5 ++ .../tests/rules/no-unsafe-assignment.test.ts | 2 + .../tests/rules/no-unsafe-return.test.ts | 6 +++ .../tests/util/isUnsafeAssignment.test.ts | 49 +++++++++++++------ 8 files changed, 76 insertions(+), 18 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unsafe-argument.ts b/packages/eslint-plugin/src/rules/no-unsafe-argument.ts index 160519e2768..84887bd8d65 100644 --- a/packages/eslint-plugin/src/rules/no-unsafe-argument.ts +++ b/packages/eslint-plugin/src/rules/no-unsafe-argument.ts @@ -220,6 +220,9 @@ export default util.createRule<[], MessageIds>({ tupleType, parameterType, checker, + // we can't pass the individual tuple members in here as this will most likely be a spread variable + // not a spread array + null, ); if (result) { context.report({ @@ -258,6 +261,7 @@ export default util.createRule<[], MessageIds>({ argumentType, parameterType, checker, + argument, ); if (result) { context.report({ diff --git a/packages/eslint-plugin/src/rules/no-unsafe-assignment.ts b/packages/eslint-plugin/src/rules/no-unsafe-assignment.ts index ac815208544..ae900b14f0c 100644 --- a/packages/eslint-plugin/src/rules/no-unsafe-assignment.ts +++ b/packages/eslint-plugin/src/rules/no-unsafe-assignment.ts @@ -283,7 +283,12 @@ export default util.createRule({ return false; } - const result = util.isUnsafeAssignment(senderType, receiverType, checker); + const result = util.isUnsafeAssignment( + senderType, + receiverType, + checker, + senderNode, + ); if (!result) { return false; } diff --git a/packages/eslint-plugin/src/rules/no-unsafe-return.ts b/packages/eslint-plugin/src/rules/no-unsafe-return.ts index a818be4ef4e..4d65ce529af 100644 --- a/packages/eslint-plugin/src/rules/no-unsafe-return.ts +++ b/packages/eslint-plugin/src/rules/no-unsafe-return.ts @@ -151,6 +151,7 @@ export default util.createRule({ returnNodeType, functionReturnType, checker, + returnNode, ); if (!result) { return; diff --git a/packages/eslint-plugin/src/util/types.ts b/packages/eslint-plugin/src/util/types.ts index 920ce66de02..089620e702a 100644 --- a/packages/eslint-plugin/src/util/types.ts +++ b/packages/eslint-plugin/src/util/types.ts @@ -1,3 +1,7 @@ +import { + AST_NODE_TYPES, + TSESTree, +} from '@typescript-eslint/experimental-utils'; import debug from 'debug'; import { isCallExpression, @@ -419,6 +423,7 @@ export function isUnsafeAssignment( type: ts.Type, receiver: ts.Type, checker: ts.TypeChecker, + senderNode: TSESTree.Node | null, ): false | { sender: ts.Type; receiver: ts.Type } { if (isTypeAnyType(type)) { // Allow assignment of any ==> unknown. @@ -451,6 +456,19 @@ export function isUnsafeAssignment( return false; } + if ( + senderNode?.type === AST_NODE_TYPES.NewExpression && + senderNode.callee.type === AST_NODE_TYPES.Identifier && + senderNode.callee.name === 'Map' && + senderNode.arguments.length === 0 && + senderNode.typeParameters == null + ) { + // special case to handle `new Map()` + // unfortunately Map's default empty constructor is typed to return `Map` :( + // https://github.com/typescript-eslint/typescript-eslint/issues/2109#issuecomment-634144396 + return false; + } + const typeArguments = type.typeArguments ?? []; const receiverTypeArguments = receiver.typeArguments ?? []; @@ -458,7 +476,7 @@ export function isUnsafeAssignment( const arg = typeArguments[i]; const receiverArg = receiverTypeArguments[i]; - const unsafe = isUnsafeAssignment(arg, receiverArg, checker); + const unsafe = isUnsafeAssignment(arg, receiverArg, checker, senderNode); if (unsafe) { return { sender: type, receiver }; } 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 a9a6bc3eacc..6205a371a14 100644 --- a/packages/eslint-plugin/tests/rules/no-unsafe-argument.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unsafe-argument.test.ts @@ -85,6 +85,11 @@ foo('a', 'b', 1 as any); declare function toHaveBeenCalledWith(...params: E): void; toHaveBeenCalledWith(1 as any); `, + // https://github.com/typescript-eslint/typescript-eslint/issues/2109 + ` +declare function acceptsMap(arg: Map): void; +acceptsMap(new Map()); + `, ], invalid: [ { 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 b0f8ec61233..f9594fc9348 100644 --- a/packages/eslint-plugin/tests/rules/no-unsafe-assignment.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unsafe-assignment.test.ts @@ -141,6 +141,8 @@ declare function Foo(props: { a: string }): never; 'const x: unknown = y as any;', 'const x: unknown[] = y as any[];', 'const x: Set = y as Set;', + // https://github.com/typescript-eslint/typescript-eslint/issues/2109 + 'const x: Map = new Map();', ], invalid: [ ...batchedSingleLineTests({ diff --git a/packages/eslint-plugin/tests/rules/no-unsafe-return.test.ts b/packages/eslint-plugin/tests/rules/no-unsafe-return.test.ts index 5cfd965ab8e..0f533917d54 100644 --- a/packages/eslint-plugin/tests/rules/no-unsafe-return.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unsafe-return.test.ts @@ -92,6 +92,12 @@ function foo(): Set { return x as Set; } `, + // https://github.com/typescript-eslint/typescript-eslint/issues/2109 + ` + function test(): Map { + return new Map(); + } + `, ], invalid: [ ...batchedSingleLineTests({ diff --git a/packages/eslint-plugin/tests/util/isUnsafeAssignment.test.ts b/packages/eslint-plugin/tests/util/isUnsafeAssignment.test.ts index efe1fe9d1ba..ece61a1c149 100644 --- a/packages/eslint-plugin/tests/util/isUnsafeAssignment.test.ts +++ b/packages/eslint-plugin/tests/util/isUnsafeAssignment.test.ts @@ -10,7 +10,12 @@ describe('isUnsafeAssignment', () => { function getTypes( code: string, - ): { sender: ts.Type; receiver: ts.Type; checker: ts.TypeChecker } { + ): { + sender: ts.Type; + senderNode: TSESTree.Node; + receiver: ts.Type; + checker: ts.TypeChecker; + } { const { ast, services } = parseForESLint(code, { project: './tsconfig.json', filePath: path.join(rootDir, 'file.ts'), @@ -28,6 +33,7 @@ describe('isUnsafeAssignment', () => { sender: checker.getTypeAtLocation( esTreeNodeToTSNodeMap.get(declarator.init!), ), + senderNode: declarator.init!, checker, }; } @@ -52,7 +58,7 @@ describe('isUnsafeAssignment', () => { ); expectTypesAre( - isUnsafeAssignment(sender, receiver, checker), + isUnsafeAssignment(sender, receiver, checker, null), checker, 'any', 'string', @@ -65,7 +71,7 @@ describe('isUnsafeAssignment', () => { ); expectTypesAre( - isUnsafeAssignment(sender, receiver, checker), + isUnsafeAssignment(sender, receiver, checker, null), checker, 'Set', 'Set', @@ -78,7 +84,7 @@ describe('isUnsafeAssignment', () => { ); expectTypesAre( - isUnsafeAssignment(sender, receiver, checker), + isUnsafeAssignment(sender, receiver, checker, null), checker, 'Map', 'Map', @@ -91,7 +97,7 @@ describe('isUnsafeAssignment', () => { ); expectTypesAre( - isUnsafeAssignment(sender, receiver, checker), + isUnsafeAssignment(sender, receiver, checker, null), checker, 'Set', 'Set', @@ -104,7 +110,7 @@ describe('isUnsafeAssignment', () => { ); expectTypesAre( - isUnsafeAssignment(sender, receiver, checker), + isUnsafeAssignment(sender, receiver, checker, null), checker, 'Set>>', 'Set>>', @@ -118,13 +124,13 @@ describe('isUnsafeAssignment', () => { 'const test: string = "";', ); - expect(isUnsafeAssignment(sender, receiver, checker)).toBeFalsy(); + expect(isUnsafeAssignment(sender, receiver, checker, null)).toBeFalsy(); }); it('non-any to a any', () => { const { sender, receiver, checker } = getTypes('const test: any = "";'); - expect(isUnsafeAssignment(sender, receiver, checker)).toBeFalsy(); + expect(isUnsafeAssignment(sender, receiver, checker, null)).toBeFalsy(); }); it('non-any in a generic position to a non-any', () => { @@ -132,7 +138,7 @@ describe('isUnsafeAssignment', () => { 'const test: Set = new Set();', ); - expect(isUnsafeAssignment(sender, receiver, checker)).toBeFalsy(); + expect(isUnsafeAssignment(sender, receiver, checker, null)).toBeFalsy(); }); it('non-any in a generic position to a non-any (multiple generics)', () => { @@ -140,7 +146,7 @@ describe('isUnsafeAssignment', () => { 'const test: Map = new Map();', ); - expect(isUnsafeAssignment(sender, receiver, checker)).toBeFalsy(); + expect(isUnsafeAssignment(sender, receiver, checker, null)).toBeFalsy(); }); it('non-any[] in a generic position to a non-any[]', () => { @@ -148,7 +154,7 @@ describe('isUnsafeAssignment', () => { 'const test: Set = new Set();', ); - expect(isUnsafeAssignment(sender, receiver, checker)).toBeFalsy(); + expect(isUnsafeAssignment(sender, receiver, checker, null)).toBeFalsy(); }); it('non-any in a generic position to a non-any (nested)', () => { @@ -156,7 +162,7 @@ describe('isUnsafeAssignment', () => { 'const test: Set>> = new Set>>();', ); - expect(isUnsafeAssignment(sender, receiver, checker)).toBeFalsy(); + expect(isUnsafeAssignment(sender, receiver, checker, null)).toBeFalsy(); }); it('non-any in a generic position to a any (nested)', () => { @@ -164,7 +170,7 @@ describe('isUnsafeAssignment', () => { 'const test: Set>> = new Set>>();', ); - expect(isUnsafeAssignment(sender, receiver, checker)).toBeFalsy(); + expect(isUnsafeAssignment(sender, receiver, checker, null)).toBeFalsy(); }); it('any to a unknown', () => { @@ -172,7 +178,7 @@ describe('isUnsafeAssignment', () => { 'const test: unknown = [] as any;', ); - expect(isUnsafeAssignment(sender, receiver, checker)).toBeFalsy(); + expect(isUnsafeAssignment(sender, receiver, checker, null)).toBeFalsy(); }); it('any[] in a generic position to a unknown[]', () => { @@ -180,7 +186,7 @@ describe('isUnsafeAssignment', () => { 'const test: unknown[] = [] as any[]', ); - expect(isUnsafeAssignment(sender, receiver, checker)).toBeFalsy(); + expect(isUnsafeAssignment(sender, receiver, checker, null)).toBeFalsy(); }); it('any in a generic position to a unknown (nested)', () => { @@ -188,7 +194,18 @@ describe('isUnsafeAssignment', () => { 'const test: Set>> = new Set>>();', ); - expect(isUnsafeAssignment(sender, receiver, checker)).toBeFalsy(); + expect(isUnsafeAssignment(sender, receiver, checker, null)).toBeFalsy(); + }); + + // https://github.com/typescript-eslint/typescript-eslint/issues/2109 + it('special cases the empty map constructor with no generics', () => { + const { sender, senderNode, receiver, checker } = getTypes( + 'const test: Map = new Map();', + ); + + expect( + isUnsafeAssignment(sender, receiver, checker, senderNode), + ).toBeFalsy(); }); }); });