8000 fix(eslint-plugin): use `isTypeArrayTypeOrUnionOfArrayTypes` util for… · ryanwang520/typescript-eslint@05030f8 · GitHub
[go: up one dir, main page]

Skip to content

Commit 05030f8

Browse files
authored
fix(eslint-plugin): use isTypeArrayTypeOrUnionOfArrayTypes util for checking if type is array (typescript-eslint#1728)
1 parent c92d240 commit 05030f8

10 files changed

+106
-45
lines changed

packages/eslint-plugin/src/rules/no-for-in-array.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,13 @@ export default util.createRule({
2525
const checker = parserServices.program.getTypeChecker();
2626
const originalNode = parserServices.esTreeNodeToTSNodeMap.get(node);
2727

28-
const type = checker.getTypeAtLocation(originalNode.expression);
28+
const type = util.getConstrainedTypeAtLocation(
29+
checker,
30+
originalNode.expression,
31+
);
2932

3033
if (
31-
(typeof type.symbol !== 'undefined' &&
32-
type.symbol.name === 'Array') ||
34+
util.isTypeArrayTypeOrUnionOfArrayTypes(type, checker) ||
3335
(type.flags & ts.TypeFlags.StringLike) !== 0
3436
) {
3537
context.report({

packages/eslint-plugin/src/rules/no-unsafe-call.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,8 @@ export default util.createRule<[], MessageIds>({
3131
messageId: MessageIds,
3232
): void {
3333
const tsNode = esTreeNodeToTSNodeMap.get(node);
34-
const type = checker.getTypeAtLocation(tsNode);
34+
const type = util.getConstrainedTypeAtLocation(checker, tsNode);
35+
3536
if (util.isTypeAnyType(type)) {
3637
context.report({
3738
node: reportingNode,

packages/eslint-plugin/src/rules/no-unsafe-return.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,8 @@ export default util.createRule({
7474
}
7575

7676
// function has an explicit return type, so ensure it's a safe return
77-
const returnNodeType = checker.getTypeAtLocation(
77+
const returnNodeType = util.getConstrainedTypeAtLocation(
78+
checker,
7879
esTreeNodeToTSNodeMap.get(returnNode),
7980
);
8081
const functionTSNode = esTreeNodeToTSNodeMap.get(functionNode);

packages/eslint-plugin/src/rules/require-array-sort-compare.ts

Lines changed: 8 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import { TSESTree } from '@typescript-eslint/experimental-utils';
2-
import * as ts from 'typescript';
32
import * as util from '../util';
43

54
export default util.createRule({
@@ -27,26 +26,16 @@ export default util.createRule({
2726

2827
return {
2928
":matches(CallExpression, OptionalCallExpression)[arguments.length=0] > :matches(MemberExpression, OptionalMemberExpression)[property.name='sort'][computed=false]"(
30-
node: TSESTree.MemberExpression | TSESTree.OptionalMemberExpression,
29+
callee: TSESTree.MemberExpression | TSESTree.OptionalMemberExpression,
3130
): void {
32-
// Get the symbol of the `sort` method.
33-
const tsNode = service.esTreeNodeToTSNodeMap.get(node);
34-
const sortSymbol = checker.getSymbolAtLocation(tsNode);
35-
if (sortSymbol == null) {
36-
return;
37-
}
31+
const tsNode = service.esTreeNodeToTSNodeMap.get(callee.object);
32+
const calleeObjType = util.getConstrainedTypeAtLocation(
33+
checker,
34+
tsNode,
35+
);
3836

39-
// Check the owner type of the `sort` method.
40-
for (const methodDecl of sortSymbol.declarations) {
41-
const typeDecl = methodDecl.parent;
42-
if (
43-
ts.isInterfaceDeclaration(typeDecl) &&
44-
ts.isSourceFile(typeDecl.parent) &&
45-
typeDecl.name.escapedText === 'Array'
46-
) {
47-
context.report({ node: node.parent!, messageId: 'requireCompare' });
48-
return;
49-
}
37+
if (util.isTypeArrayTypeOrUnionOfArrayTypes(calleeObjType, checker)) {
38+
context.report({ node: callee.parent!, messageId: 'requireCompare' });
5039
}
5140
},
5241
};

packages/eslint-plugin/src/rules/restrict-plus-operands.ts

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -54,16 +54,6 @@ export default util.createRule<Options, MessageIds>({
5454
* Helper function to get base type of node
5555
*/
5656
function getBaseTypeOfLiteralType(type: ts.Type): BaseLiteral {
57-
const constraint = type.getConstraint();
58-
if (
59-
constraint &&
60-
// for generic types with union constraints, it will return itself from getConstraint
61-
// so we have to guard against infinite recursion...
62-
constraint !== type
63-
) {
64-
return getBaseTypeOfLiteralType(constraint);
65-
}
66-
6757
if (type.isNumberLiteral()) {
6858
return 'number';
6959
}
@@ -98,7 +88,7 @@ export default util.createRule<Options, MessageIds>({
9888
*/
9989
function getNodeType(node: TSESTree.Expression): BaseLiteral {
10090
const tsNode = service.esTreeNodeToTSNodeMap.get(node);
101-
const type = typeChecker.getTypeAtLocation(tsNode);
91+
const type = util.getConstrainedTypeAtLocation(typeChecker, tsNode);
10292

10393
return getBaseTypeOfLiteralType(type);
10494
}

packages/eslint-plugin/src/rules/restrict-template-expressions.ts

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -98,21 +98,12 @@ export default util.createRule<Options, MessageId>({
9898
*/
9999
function getNodeType(node: TSESTree.Expression): BaseType[] {
100100
const tsNode = service.esTreeNodeToTSNodeMap.get(node);
101-
const type = typeChecker.getTypeAtLocation(tsNode);
101+
const type = util.getConstrainedTypeAtLocation(typeChecker, tsNode);
102102

103103
return getBaseType(type);
104104
}
105105

106106
function getBaseType(type: ts.Type): BaseType[] {
107-
const constraint = type.getConstraint();
108-
if (
109-
constraint &&
110-
// for generic types with union constraints, it will return itself
111-
constraint !== type
112-
) {
113-
return getBaseType(constraint);
114-
}
115-
116107
if (type.isStringLiteral()) {
117108
return ['string'];
118109
}

packages/eslint-plugin/src/util/types.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,23 @@ import {
1313
} from 'tsutils';
1414
import * as ts from 'typescript';
1515

16+
/**
17+
* Checks if the given type is either an array type,
18+
* or a union made up solely of array types.
19+
*/
20+
export function isTypeArrayTypeOrUnionOfArrayTypes(
21+
type: ts.Type,
22+
checker: ts.TypeChecker,
23+
): boolean {
24+
for (const t of unionTypeParts(type)) {
25+
if (!checker.isArrayType(t)) {
26+
return false;
27+
}
28+
}
29+
30+
return true;
31+
}
32+
1633
/**
1734
* @param type Type being checked by name.
1835
* @param allowedNames Symbol names checking on the type.

packages/eslint-plugin/tests/rules/no-for-in-array.test.ts

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,5 +54,50 @@ for (const x in z) {
5454
},
5555
],
5656
},
57+
{
58+
code: `
59+
const fn = (arr: number[]) => {
60+
for (const x in arr) {
61+
console.log(x);
62+
}
63+
};
64+
`,
65+
errors: [
66+
{
67+
messageId: 'forInViolation',
68+
type: AST_NODE_TYPES.ForInStatement,
69+
},
70+
],
71+
},
72+
{
73+
code: `
74+
const fn = (arr: number[] | string[]) => {
75+
for (const x in arr) {
76+
console.log(x);
77+
}
78+
};
79+
`,
80+
errors: [
81+
{
82+
messageId: 'forInViolation',
83+
type: AST_NODE_TYPES.ForInStatement,
84+
},
85+
],
86+
},
87+
{
88+
code: `
89+
const fn = <T extends any[]>(arr: T) => {
90+
for (const x in arr) {
91+
console.log(x);
92+
}
93+
};
94+
`,
95+
errors: [
96+
{
97+
messageId: 'forInViolation',
98+
type: AST_NODE_TYPES.ForInStatement,
99+
},
100+
],
101+
},
57102
],
58103
});

packages/eslint-plugin/tests/rules/no-unsafe-call.test.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ function foo(x: any) { x() }
4747
function foo(x: any) { x?.() }
4848
function foo(x: any) { x.a.b.c.d.e.f.g() }
4949
function foo(x: any) { x.a.b.c.d.e.f.g?.() }
50+
function foo<T extends any>(x: T) { x() }
5051
`,
5152
errors: [
5253
{
@@ -73,6 +74,12 @@ function foo(x: any) { x.a.b.c.d.e.f.g?.() }
7374
column: 24,
7475
endColumn: 39,
7576
},
77+
{
78+
messageId: 'unsafeCall',
79+
line: 6,
80+
column: 37,
81+
endColumn: 38,
82+
},
7683
],
7784
}),
7885
...batchedSingleLineTests({

packages/eslint-plugin/tests/rules/no-unsafe-return.test.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,24 @@ function foo(): Set<number> {
7373
`,
7474
],
7575
invalid: [
76+
{
77+
code: `
78+
function fn<T extends any>(x: T) {
79+
return x;
80+
}
81+
`,
82+
errors: [
83+
{
84+
messageId: 'unsafeReturnAssignment',
85+
data: {
86+
sender: 'any',
87+
receiver: 'T',
88+
},
89+
line: 3,
90+
column: 3,
91+
},
92+
],
93+
},
7694
...batchedSingleLineTests({
7795
code: noFormat`
7896
function foo() { return (1 as any); }

0 commit comments

Comments
 (0)
0