fix(eslint-plugin): [prefer-includes] ignore option chaining before i…

…ndexOfs (#3432)
typescript-eslint · Jun 6, 2021 · bf0cddb · bf0cddb
1 parent d596985
commit bf0cddb
Show file tree

Hide file tree

Showing 3 changed files with 200 additions and 168 deletions.
diff --git a/packages/eslint-plugin/docs/rules/prefer-includes.md b/packages/eslint-plugin/docs/rules/prefer-includes.md
@@ -22,6 +22,7 @@ let str: string;
 let array: any[];
 let readonlyArray: ReadonlyArray<any>;
 let typedArray: UInt8Array;
+let maybe: string;
 let userDefined: {
   indexOf(x: any): number;
   includes(x: any): boolean;
@@ -31,6 +32,7 @@ str.indexOf(value) !== -1;
 array.indexOf(value) !== -1;
 readonlyArray.indexOf(value) === -1;
 typedArray.indexOf(value) > -1;
+maybe?.indexOf('') !== -1;
 userDefined.indexOf(value) >= 0;
 
 // simple RegExp test

diff --git a/packages/eslint-plugin/src/rules/prefer-includes.ts b/packages/eslint-plugin/src/rules/prefer-includes.ts
@@ -1,5 +1,6 @@
 import {
   AST_NODE_TYPES,
+  TSESLint,
   TSESTree,
 } from '@typescript-eslint/experimental-utils';
 import { AST as RegExpAST, parseRegExpLiteral } from 'regexpp';
@@ -125,67 +126,81 @@ export default createRule({
       );
     }
 
-    return {
-      [[
-        // a.indexOf(b) !== 1
-        "BinaryExpression > CallExpression.left > MemberExpression.callee[property.name='indexOf'][computed=false]",
-        // a?.indexOf(b) !== 1
-        "BinaryExpression > ChainExpression.left > CallExpression > MemberExpression.callee[property.name='indexOf'][computed=false]",
-      ].join(', ')](node: TSESTree.MemberExpression): void {
-        // Check if the comparison is equivalent to `includes()`.
-        const callNode = node.parent as TSESTree.CallExpression;
-        const compareNode = (
-          callNode.parent?.type === AST_NODE_TYPES.ChainExpression
-            ? callNode.parent.parent
-            : callNode.parent
-        ) as TSESTree.BinaryExpression;
-        const negative = isNegativeCheck(compareNode);
-        if (!negative && !isPositiveCheck(compareNode)) {
-          return;
-        }
+    function checkArrayIndexOf(
+      node: TSESTree.MemberExpression,
+      allowFixing: boolean,
+    ): void {
+      // Check if the comparison is equivalent to `includes()`.
+      const callNode = node.parent as TSESTree.CallExpression;
+      const compareNode = (
+        callNode.parent?.type === AST_NODE_TYPES.ChainExpression
+          ? callNode.parent.parent
+          : callNode.parent
+      ) as TSESTree.BinaryExpression;
+      const negative = isNegativeCheck(compareNode);
+      if (!negative && !isPositiveCheck(compareNode)) {
+        return;
+      }
 
-        // Get the symbol of `indexOf` method.
-        const tsNode = services.esTreeNodeToTSNodeMap.get(node.property);
-        const indexofMethodDeclarations = types
-          .getSymbolAtLocation(tsNode)
+      // Get the symbol of `indexOf` method.
+      const tsNode = services.esTreeNodeToTSNodeMap.get(node.property);
+      const indexofMethodDeclarations = types
+        .getSymbolAtLocation(tsNode)
+        ?.getDeclarations();
+      if (
+        indexofMethodDeclarations == null ||
+        indexofMethodDeclarations.length === 0
+      ) {
+        return;
+      }
+
+      // Check if every declaration of `indexOf` method has `includes` method
+      // and the two methods have the same parameters.
+      for (const instanceofMethodDecl of indexofMethodDeclarations) {
+        const typeDecl = instanceofMethodDecl.parent;
+        const type = types.getTypeAtLocation(typeDecl);
+        const includesMethodDecl = type
+          .getProperty('includes')
           ?.getDeclarations();
         if (
-          indexofMethodDeclarations == null ||
-          indexofMethodDeclarations.length === 0
+          includesMethodDecl == null ||
+          !includesMethodDecl.some(includesMethodDecl =>
+            hasSameParameters(includesMethodDecl, instanceofMethodDecl),
+          )
         ) {
           return;
         }
+      }
 
-        // Check if every declaration of `indexOf` method has `includes` method
-        // and the two methods have the same parameters.
-        for (const instanceofMethodDecl of indexofMethodDeclarations) {
-          const typeDecl = instanceofMethodDecl.parent;
-          const type = types.getTypeAtLocation(typeDecl);
-          const includesMethodDecl = type
-            .getProperty('includes')
-            ?.getDeclarations();
-          if (
-            includesMethodDecl == null ||
-            !includesMethodDecl.some(includesMethodDecl =>
-              hasSameParameters(includesMethodDecl, instanceofMethodDecl),
-            )
-          ) {
-            return;
-          }
-        }
-
-        // Report it.
-        context.report({
-          node: compareNode,
-          messageId: 'preferIncludes',
-          *fix(fixer) {
+      // Report it.
+      context.report({
+        node: compareNode,
+        messageId: 'preferIncludes',
+        ...(allowFixing && {
+          *fix(fixer): Generator<TSESLint.RuleFix> {
             if (negative) {
               yield fixer.insertTextBefore(callNode, '!');
             }
             yield fixer.replaceText(node.property, 'includes');
             yield fixer.removeRange([callNode.range[1], compareNode.range[1]]);
           },
-        });
+        }),
+      });
+    }
+
+    return {
+      // a.indexOf(b) !== 1
+      "BinaryExpression > CallExpression.left > MemberExpression.callee[property.name='indexOf'][computed=false]"(
+        node: TSESTree.MemberExpression,
+      ): void {
+        checkArrayIndexOf(node, /* allowFixing */ true);
+      },
+
+      // a?.indexOf(b) !== 1
+      "BinaryExpression > ChainExpression.left > CallExpression > MemberExpression.callee[property.name='indexOf'][computed=false]"(
+        node: TSESTree.MemberExpression,
+      ): void {
+        checkArrayIndexOf(node, /* allowFixing */ false);
       },
 
       // /bar/.test(foo)
@@ -230,7 +245,7 @@ export default createRule({
             }
             yield fixer.insertTextAfter(
               argNode,
-              `${node.optional ? '?.' : '.'}includes(${JSON.stringify(text)}`,
+              `${node.optional ? '?.' : '.'}includes('${text}'`,
             );
           },
         });