From 05030f8d2bd5a50e95053bc61380891da71cc567 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Mon, 13 Apr 2020 12:54:59 +1200 Subject: [PATCH] fix(eslint-plugin): use `isTypeArrayTypeOrUnionOfArrayTypes` util for checking if type is array (#1728) --- .../src/rules/no-for-in-array.ts | 8 ++-- .../eslint-plugin/src/rules/no-unsafe-call.ts | 3 +- .../src/rules/no-unsafe-return.ts | 3 +- .../src/rules/require-array-sort-compare.ts | 27 ++++------- .../src/rules/restrict-plus-operands.ts | 12 +---- .../rules/restrict-template-expressions.ts | 11 +---- packages/eslint-plugin/src/util/types.ts | 17 +++++++ .../tests/rules/no-for-in-array.test.ts | 45 +++++++++++++++++++ .../tests/rules/no-unsafe-call.test.ts | 7 +++ .../tests/rules/no-unsafe-return.test.ts | 18 ++++++++ 10 files changed, 106 insertions(+), 45 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-for-in-array.ts b/packages/eslint-plugin/src/rules/no-for-in-array.ts index 511b744ee4c..f89d2ffd08d 100644 --- a/packages/eslint-plugin/src/rules/no-for-in-array.ts +++ b/packages/eslint-plugin/src/rules/no-for-in-array.ts @@ -25,11 +25,13 @@ export default util.createRule({ const checker = parserServices.program.getTypeChecker(); const originalNode = parserServices.esTreeNodeToTSNodeMap.get(node); - const type = checker.getTypeAtLocation(originalNode.expression); + const type = util.getConstrainedTypeAtLocation( + checker, + originalNode.expression, + ); if ( - (typeof type.symbol !== 'undefined' && - type.symbol.name === 'Array') || + util.isTypeArrayTypeOrUnionOfArrayTypes(type, checker) || (type.flags & ts.TypeFlags.StringLike) !== 0 ) { context.report({ diff --git a/packages/eslint-plugin/src/rules/no-unsafe-call.ts b/packages/eslint-plugin/src/rules/no-unsafe-call.ts index 66f66448c28..3661bd44f16 100644 --- a/packages/eslint-plugin/src/rules/no-unsafe-call.ts +++ b/packages/eslint-plugin/src/rules/no-unsafe-call.ts @@ -31,7 +31,8 @@ export default util.createRule<[], MessageIds>({ messageId: MessageIds, ): void { const tsNode = esTreeNodeToTSNodeMap.get(node); - const type = checker.getTypeAtLocation(tsNode); + const type = util.getConstrainedTypeAtLocation(checker, tsNode); + if (util.isTypeAnyType(type)) { context.report({ node: reportingNode, diff --git a/packages/eslint-plugin/src/rules/no-unsafe-return.ts b/packages/eslint-plugin/src/rules/no-unsafe-return.ts index fef04cd6365..5fbbfac9b90 100644 --- a/packages/eslint-plugin/src/rules/no-unsafe-return.ts +++ b/packages/eslint-plugin/src/rules/no-unsafe-return.ts @@ -74,7 +74,8 @@ export default util.createRule({ } // function has an explicit return type, so ensure it's a safe return - const returnNodeType = checker.getTypeAtLocation( + const returnNodeType = util.getConstrainedTypeAtLocation( + checker, esTreeNodeToTSNodeMap.get(returnNode), ); const functionTSNode = esTreeNodeToTSNodeMap.get(functionNode); diff --git a/packages/eslint-plugin/src/rules/require-array-sort-compare.ts b/packages/eslint-plugin/src/rules/require-array-sort-compare.ts index 25c4c2e5747..2f388d364dd 100644 --- a/packages/eslint-plugin/src/rules/require-array-sort-compare.ts +++ b/packages/eslint-plugin/src/rules/require-array-sort-compare.ts @@ -1,5 +1,4 @@ import { TSESTree } from '@typescript-eslint/experimental-utils'; -import * as ts from 'typescript'; import * as util from '../util'; export default util.createRule({ @@ -27,26 +26,16 @@ export default util.createRule({ return { ":matches(CallExpression, OptionalCallExpression)[arguments.length=0] > :matches(MemberExpression, OptionalMemberExpression)[property.name='sort'][computed=false]"( - node: TSESTree.MemberExpression | TSESTree.OptionalMemberExpression, + callee: TSESTree.MemberExpression | TSESTree.OptionalMemberExpression, ): void { - // Get the symbol of the `sort` method. - const tsNode = service.esTreeNodeToTSNodeMap.get(node); - const sortSymbol = checker.getSymbolAtLocation(tsNode); - if (sortSymbol == null) { - return; - } + const tsNode = service.esTreeNodeToTSNodeMap.get(callee.object); + const calleeObjType = util.getConstrainedTypeAtLocation( + checker, + tsNode, + ); - // Check the owner type of the `sort` method. - for (const methodDecl of sortSymbol.declarations) { - const typeDecl = methodDecl.parent; - if ( - ts.isInterfaceDeclaration(typeDecl) && - ts.isSourceFile(typeDecl.parent) && - typeDecl.name.escapedText === 'Array' - ) { - context.report({ node: node.parent!, messageId: 'requireCompare' }); - return; - } + if (util.isTypeArrayTypeOrUnionOfArrayTypes(calleeObjType, checker)) { + context.report({ node: callee.parent!, messageId: 'requireCompare' }); } }, }; diff --git a/packages/eslint-plugin/src/rules/restrict-plus-operands.ts b/packages/eslint-plugin/src/rules/restrict-plus-operands.ts index a833825a207..2702009046f 100644 --- a/packages/eslint-plugin/src/rules/restrict-plus-operands.ts +++ b/packages/eslint-plugin/src/rules/restrict-plus-operands.ts @@ -54,16 +54,6 @@ export default util.createRule({ * Helper function to get base type of node */ function getBaseTypeOfLiteralType(type: ts.Type): BaseLiteral { - const constraint = type.getConstraint(); - if ( - constraint && - // for generic types with union constraints, it will return itself from getConstraint - // so we have to guard against infinite recursion... - constraint !== type - ) { - return getBaseTypeOfLiteralType(constraint); - } - if (type.isNumberLiteral()) { return 'number'; } @@ -98,7 +88,7 @@ export default util.createRule({ */ function getNodeType(node: TSESTree.Expression): BaseLiteral { const tsNode = service.esTreeNodeToTSNodeMap.get(node); - const type = typeChecker.getTypeAtLocation(tsNode); + const type = util.getConstrainedTypeAtLocation(typeChecker, tsNode); return getBaseTypeOfLiteralType(type); } diff --git a/packages/eslint-plugin/src/rules/restrict-template-expressions.ts b/packages/eslint-plugin/src/rules/restrict-template-expressions.ts index 63b257ffe7f..d73d8d98a25 100644 --- a/packages/eslint-plugin/src/rules/restrict-template-expressions.ts +++ b/packages/eslint-plugin/src/rules/restrict-template-expressions.ts @@ -98,21 +98,12 @@ export default util.createRule({ */ function getNodeType(node: TSESTree.Expression): BaseType[] { const tsNode = service.esTreeNodeToTSNodeMap.get(node); - const type = typeChecker.getTypeAtLocation(tsNode); + const type = util.getConstrainedTypeAtLocation(typeChecker, tsNode); return getBaseType(type); } function getBaseType(type: ts.Type): BaseType[] { - const constraint = type.getConstraint(); - if ( - constraint && - // for generic types with union constraints, it will return itself - constraint !== type - ) { - return getBaseType(constraint); - } - if (type.isStringLiteral()) { return ['string']; } diff --git a/packages/eslint-plugin/src/util/types.ts b/packages/eslint-plugin/src/util/types.ts index ae6ee3843aa..261c154361b 100644 --- a/packages/eslint-plugin/src/util/types.ts +++ b/packages/eslint-plugin/src/util/types.ts @@ -13,6 +13,23 @@ import { } from 'tsutils'; import * as ts from 'typescript'; +/** + * Checks if the given type is either an array type, + * or a union made up solely of array types. + */ +export function isTypeArrayTypeOrUnionOfArrayTypes( + type: ts.Type, + checker: ts.TypeChecker, +): boolean { + for (const t of unionTypeParts(type)) { + if (!checker.isArrayType(t)) { + return false; + } + } + + return true; +} + /** * @param type Type being checked by name. * @param allowedNames Symbol names checking on the type. diff --git a/packages/eslint-plugin/tests/rules/no-for-in-array.test.ts b/packages/eslint-plugin/tests/rules/no-for-in-array.test.ts index 134fd513e87..7c3ec309185 100644 --- a/packages/eslint-plugin/tests/rules/no-for-in-array.test.ts +++ b/packages/eslint-plugin/tests/rules/no-for-in-array.test.ts @@ -54,5 +54,50 @@ for (const x in z) { }, ], }, + { + code: ` +const fn = (arr: number[]) => { + for (const x in arr) { + console.log(x); + } +}; + `, + errors: [ + { + messageId: 'forInViolation', + type: AST_NODE_TYPES.ForInStatement, + }, + ], + }, + { + code: ` +const fn = (arr: number[] | string[]) => { + for (const x in arr) { + console.log(x); + } +}; + `, + errors: [ + { + messageId: 'forInViolation', + type: AST_NODE_TYPES.ForInStatement, + }, + ], + }, + { + code: ` +const fn = (arr: T) => { + for (const x in arr) { + console.log(x); + } +}; + `, + errors: [ + { + messageId: 'forInViolation', + type: AST_NODE_TYPES.ForInStatement, + }, + ], + }, ], }); diff --git a/packages/eslint-plugin/tests/rules/no-unsafe-call.test.ts b/packages/eslint-plugin/tests/rules/no-unsafe-call.test.ts index 8adb44b9960..f860410da5c 100644 --- a/packages/eslint-plugin/tests/rules/no-unsafe-call.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unsafe-call.test.ts @@ -47,6 +47,7 @@ function foo(x: any) { x() } function foo(x: any) { x?.() } function foo(x: any) { x.a.b.c.d.e.f.g() } function foo(x: any) { x.a.b.c.d.e.f.g?.() } +function foo(x: T) { x() } `, errors: [ { @@ -73,6 +74,12 @@ function foo(x: any) { x.a.b.c.d.e.f.g?.() } column: 24, endColumn: 39, }, + { + messageId: 'unsafeCall', + line: 6, + column: 37, + endColumn: 38, + }, ], }), ...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 b0c1ead9007..42a58734a47 100644 --- a/packages/eslint-plugin/tests/rules/no-unsafe-return.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unsafe-return.test.ts @@ -73,6 +73,24 @@ function foo(): Set { `, ], invalid: [ + { + code: ` +function fn(x: T) { + return x; +} + `, + errors: [ + { + messageId: 'unsafeReturnAssignment', + data: { + sender: 'any', + receiver: 'T', + }, + line: 3, + column: 3, + }, + ], + }, ...batchedSingleLineTests({ code: noFormat` function foo() { return (1 as any); }