Skip to content
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

fix(eslint-plugin): [no-unsafe-*] special case handling for the empty map constructor with no generics #3394

Merged
merged 2 commits into from May 15, 2021
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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();
});
});
});