fix(eslint-plugin): use isTypeArrayTypeOrUnionOfArrayTypes util for…

… checking if type is array (#1728)
typescript-eslint · Apr 13, 2020 · 05030f8 · 05030f8
1 parent c92d240
commit 05030f8
Show file tree

Hide file tree

Showing 10 changed files with 106 additions and 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -54,16 +54,6 @@ export default util.createRule<Options, MessageIds>({
      * 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<Options, MessageIds>({
      */
     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
@@ -98,21 +98,12 @@ export default util.createRule<Options, MessageId>({
      */
     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
@@ -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
@@ -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 = <T extends any[]>(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
@@ -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<T extends any>(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
@@ -73,6 +73,24 @@ function foo(): Set<number> {
     `,
   ],
   invalid: [
+    {
+      code: `
+function fn<T extends any>(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); }