Skip to content

Commit

Permalink
fix(eslint-plugin): [no-unsafe-*] special case handling for the empty…
Browse files Browse the repository at this point in the history
… map constructor with no generics (#3394)

Fixes #2109

Sucks that it had to be this way, but oh well.
  • Loading branch information
bradzacher committed May 15, 2021
1 parent 37ec2c2 commit cae4f4a
Show file tree
Hide file tree
Showing 8 changed files with 76 additions and 18 deletions.
4 changes: 4 additions & 0 deletions packages/eslint-plugin/src/rules/no-unsafe-argument.ts
Expand Up @@ -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({
Expand Down Expand Up @@ -258,6 +261,7 @@ export default util.createRule<[], MessageIds>({
argumentType,
parameterType,
checker,
argument,
);
if (result) {
context.report({
Expand Down
7 changes: 6 additions & 1 deletion packages/eslint-plugin/src/rules/no-unsafe-assignment.ts
Expand Up @@ -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;
}
Expand Down
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/rules/no-unsafe-return.ts
Expand Up @@ -151,6 +151,7 @@ export default util.createRule({
returnNodeType,
functionReturnType,
checker,
returnNode,
);
if (!result) {
return;
Expand Down
20 changes: 19 additions & 1 deletion 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,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -451,14 +456,27 @@ 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<any, any>` :(
// https://github.com/typescript-eslint/typescript-eslint/issues/2109#issuecomment-634144396
return false;
}

const typeArguments = type.typeArguments ?? [];
const receiverTypeArguments = receiver.typeArguments ?? [];

for (let i = 0; i < typeArguments.length; i += 1) {
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 };
}
Expand Down
5 changes: 5 additions & 0 deletions packages/eslint-plugin/tests/rules/no-unsafe-argument.test.ts
Expand Up @@ -85,6 +85,11 @@ foo('a', 'b', 1 as any);
declare function toHaveBeenCalledWith<E extends any[]>(...params: E): void;
toHaveBeenCalledWith(1 as any);
`,
// https://github.com/typescript-eslint/typescript-eslint/issues/2109
`
declare function acceptsMap(arg: Map<string, string>): void;
acceptsMap(new Map());
`,
],
invalid: [
{
Expand Down
Expand Up @@ -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<unknown> = y as Set<any>;',
// https://github.com/typescript-eslint/typescript-eslint/issues/2109
'const x: Map<string, string> = new Map();',
],
invalid: [
...batchedSingleLineTests({
Expand Down
6 changes: 6 additions & 0 deletions packages/eslint-plugin/tests/rules/no-unsafe-return.test.ts
Expand Up @@ -92,6 +92,12 @@ function foo(): Set<number> {
return x as Set<any>;
}
`,
// https://github.com/typescript-eslint/typescript-eslint/issues/2109
`
function test(): Map<string, string> {
return new Map();
}
`,
],
invalid: [
...batchedSingleLineTests({
Expand Down
49 changes: 33 additions & 16 deletions packages/eslint-plugin/tests/util/isUnsafeAssignment.test.ts
Expand Up @@ -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'),
Expand All @@ -28,6 +33,7 @@ describe('isUnsafeAssignment', () => {
sender: checker.getTypeAtLocation(
esTreeNodeToTSNodeMap.get(declarator.init!),
),
senderNode: declarator.init!,
checker,
};
}
Expand All @@ -52,7 +58,7 @@ describe('isUnsafeAssignment', () => {
);

expectTypesAre(
isUnsafeAssignment(sender, receiver, checker),
isUnsafeAssignment(sender, receiver, checker, null),
checker,
'any',
'string',
Expand All @@ -65,7 +71,7 @@ describe('isUnsafeAssignment', () => {
);

expectTypesAre(
isUnsafeAssignment(sender, receiver, checker),
isUnsafeAssignment(sender, receiver, checker, null),
checker,
'Set<any>',
'Set<string>',
Expand All @@ -78,7 +84,7 @@ describe('isUnsafeAssignment', () => {
);

expectTypesAre(
isUnsafeAssignment(sender, receiver, checker),
isUnsafeAssignment(sender, receiver, checker, null),
checker,
'Map<string, any>',
'Map<string, string>',
Expand All @@ -91,7 +97,7 @@ describe('isUnsafeAssignment', () => {
);

expectTypesAre(
isUnsafeAssignment(sender, receiver, checker),
isUnsafeAssignment(sender, receiver, checker, null),
checker,
'Set<any[]>',
'Set<string[]>',
Expand All @@ -104,7 +110,7 @@ describe('isUnsafeAssignment', () => {
);

expectTypesAre(
isUnsafeAssignment(sender, receiver, checker),
isUnsafeAssignment(sender, receiver, checker, null),
checker,
'Set<Set<Set<any>>>',
'Set<Set<Set<string>>>',
Expand All @@ -118,77 +124,88 @@ 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', () => {
const { sender, receiver, checker } = getTypes(
'const test: Set<string> = new Set<string>();',
);

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)', () => {
const { sender, receiver, checker } = getTypes(
'const test: Map<string, string> = new Map<string, string>();',
);

expect(isUnsafeAssignment(sender, receiver, checker)).toBeFalsy();
expect(isUnsafeAssignment(sender, receiver, checker, null)).toBeFalsy();
});

it('non-any[] in a generic position to a non-any[]', () => {
const { sender, receiver, checker } = getTypes(
'const test: Set<string[]> = new Set<string[]>();',
);

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)', () => {
const { sender, receiver, checker } = getTypes(
'const test: Set<Set<Set<string>>> = new Set<Set<Set<string>>>();',
);

expect(isUnsafeAssignment(sender, receiver, checker)).toBeFalsy();
expect(isUnsafeAssignment(sender, receiver, checker, null)).toBeFalsy();
});

it('non-any in a generic position to a any (nested)', () => {
const { sender, receiver, checker } = getTypes(
'const test: Set<Set<Set<any>>> = new Set<Set<Set<string>>>();',
);

expect(isUnsafeAssignment(sender, receiver, checker)).toBeFalsy();
expect(isUnsafeAssignment(sender, receiver, checker, null)).toBeFalsy();
});

it('any to a unknown', () => {
const { sender, receiver, checker } = getTypes(
'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[]', () => {
const { sender, receiver, checker } = getTypes(
'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)', () => {
const { sender, receiver, checker } = getTypes(
'const test: Set<Set<Set<unknown>>> = new Set<Set<Set<any>>>();',
);

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<string, string> = new Map();',
);

expect(
isUnsafeAssignment(sender, receiver, checker, senderNode),
).toBeFalsy();
});
});
});

0 comments on commit cae4f4a

Please sign in to comment.