fix(prefer-to-contain): support square bracket accessors (#1009)

* fix(prefer-to-contain): support square bracket accessors * refactor(prefer-to-contain): simplify fixer * refactor(prefer-to-contain): simplify fixer further * refactor(prefer-to-contain): simplify fixer even further * chore(prefer-to-contain): add comments * refactor(prefer-to-contain): swap order of fixers to read better
jest-community · Dec 30, 2021 · 73984a7 · 73984a7
1 parent 902a70d
commit 73984a7
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 128 deletions.
diff --git a/src/rules/__tests__/prefer-to-contain.test.ts b/src/rules/__tests__/prefer-to-contain.test.ts
@@ -32,12 +32,36 @@ ruleTester.run('prefer-to-contain', rule, {
       output: 'expect(a).toContain(b);',
       errors: [{ messageId: 'useToContain', column: 23, line: 1 }],
     },
-    // todo: support this, as it's counted by isSupportedAccessor
-    // {
-    //   code: "expect(a['includes'](b)).toEqual(true);",
-    //   errors: [{ messageId: 'useToContain', column: 23, line: 1 }],
-    //   output: 'expect(a).toContain(b);',
-    // },
+    {
+      code: "expect(a['includes'](b)).toEqual(true);",
+      output: 'expect(a).toContain(b);',
+      errors: [{ messageId: 'useToContain', column: 26, line: 1 }],
+    },
+    {
+      code: "expect(a['includes'](b))['toEqual'](true);",
+      output: 'expect(a).toContain(b);',
+      errors: [{ messageId: 'useToContain', column: 26, line: 1 }],
+    },
+    {
+      code: "expect(a['includes'](b)).toEqual(false);",
+      output: 'expect(a).not.toContain(b);',
+      errors: [{ messageId: 'useToContain', column: 26, line: 1 }],
+    },
+    {
+      code: "expect(a['includes'](b)).not.toEqual(false);",
+      output: 'expect(a).toContain(b);',
+      errors: [{ messageId: 'useToContain', column: 26, line: 1 }],
+    },
+    {
+      code: "expect(a['includes'](b))['not'].toEqual(false);",
+      output: 'expect(a).toContain(b);',
+      errors: [{ messageId: 'useToContain', column: 26, line: 1 }],
+    },
+    {
+      code: "expect(a['includes'](b))['not']['toEqual'](false);",
+      output: 'expect(a).toContain(b);',
+      errors: [{ messageId: 'useToContain', column: 26, line: 1 }],
+    },
     {
       code: 'expect(a.includes(b)).toEqual(false);',
       output: 'expect(a).not.toContain(b);',

diff --git a/src/rules/prefer-to-contain.ts b/src/rules/prefer-to-contain.ts
@@ -1,14 +1,12 @@
 import {
   AST_NODE_TYPES,
-  TSESLint,
   TSESTree,
 } from '@typescript-eslint/experimental-utils';
 import {
   CallExpressionWithSingleArgument,
   KnownCallExpression,
   MaybeTypeCast,
   ModifierName,
-  NotNegatableParsedModifier,
   ParsedEqualityMatcherCall,
   ParsedExpectMatcher,
   createRule,
@@ -57,103 +55,15 @@ type FixableIncludesCallExpression = KnownCallExpression<'includes'> &
  * @param {CallExpression} node
  *
  * @return {node is FixableIncludesCallExpression}
- *
- * @todo support `['includes']()` syntax (remove last property.type check to begin)
- * @todo break out into `isMethodCall<Name extends string>(node: TSESTree.Node, method: Name)` util-fn
  */
 const isFixableIncludesCallExpression = (
   node: TSESTree.Node,
 ): node is FixableIncludesCallExpression =>
   node.type === AST_NODE_TYPES.CallExpression &&
   node.callee.type === AST_NODE_TYPES.MemberExpression &&
   isSupportedAccessor(node.callee.property, 'includes') &&
-  node.callee.property.type === AST_NODE_TYPES.Identifier &&
   hasOnlyOneArgument(node);
 
-const buildToContainFuncExpectation = (negated: boolean) =>
-  negated ? `${ModifierName.not}.toContain` : 'toContain';
-
-/**
- * Finds the first `.` character token between the `object` & `property` of the given `member` expression.
- *
- * @param {TSESTree.MemberExpression} member
- * @param {SourceCode} sourceCode
- *
- * @return {Token | null}
- */
-const findPropertyDotToken = (
-  member: TSESTree.MemberExpression,
-  sourceCode: TSESLint.SourceCode,
-) =>
-  sourceCode.getFirstTokenBetween(
-    member.object,
-    member.property,
-    token => token.value === '.',
-  );
-
-const getNegationFixes = (
-  node: FixableIncludesCallExpression,
-  modifier: NotNegatableParsedModifier,
-  matcher: ParsedBooleanEqualityMatcherCall,
-  sourceCode: TSESLint.SourceCode,
-  fixer: TSESLint.RuleFixer,
-  fileName: string,
-) => {
-  const [containArg] = node.arguments;
-  const negationPropertyDot = findPropertyDotToken(modifier.node, sourceCode);
-
-  const toContainFunc = buildToContainFuncExpectation(
-    followTypeAssertionChain(matcher.arguments[0]).value,
-  );
-
-  /* istanbul ignore if */
-  if (negationPropertyDot === null) {
-    throw new Error(
-      `Unexpected null when attempting to fix ${fileName} - please file a github issue at https://github.com/jest-community/eslint-plugin-jest`,
-    );
-  }
-
-  return [
-    fixer.remove(negationPropertyDot),
-    fixer.remove(modifier.node.property),
-    fixer.replaceText(matcher.node.property, toContainFunc),
-    fixer.replaceText(matcher.arguments[0], sourceCode.getText(containArg)),
-  ];
-};
-
-const getCommonFixes = (
-  node: FixableIncludesCallExpression,
-  sourceCode: TSESLint.SourceCode,
-  fileName: string,
-): Array<TSESTree.Node | TSESTree.Token> => {
-  const [containArg] = node.arguments;
-  const includesCallee = node.callee;
-
-  const propertyDot = findPropertyDotToken(includesCallee, sourceCode);
-
-  const closingParenthesis = sourceCode.getTokenAfter(containArg);
-  const openParenthesis = sourceCode.getTokenBefore(containArg);
-
-  /* istanbul ignore if */
-  if (
-    propertyDot === null ||
-    closingParenthesis === null ||
-    openParenthesis === null
-  ) {
-    throw new Error(
-      `Unexpected null when attempting to fix ${fileName} - please file a github issue at https://github.com/jest-community/eslint-plugin-jest`,
-    );
-  }
-
-  return [
-    containArg,
-    includesCallee.property,
-    propertyDot,
-    closingParenthesis,
-    openParenthesis,
-  ];
-};
-
 // expect(array.includes(<value>)[not.]{toBe,toEqual}(<boolean>)
 export default createRule({
   name: __filename,
@@ -181,6 +91,7 @@ export default createRule({
         const {
           expect: {
             arguments: [includesCall],
+            range: [, expectCallEnd],
           },
           matcher,
           modifier,
@@ -199,42 +110,32 @@ export default createRule({
         context.report({
           fix(fixer) {
             const sourceCode = context.getSourceCode();
-            const fileName = context.getFilename();
-
-            const fixArr = getCommonFixes(
-              includesCall,
-              sourceCode,
-              fileName,
-            ).map(target => fixer.remove(target));
 
-            if (modifier) {
-              return getNegationFixes(
-                includesCall,
-                modifier,
-                matcher,
-                sourceCode,
-                fixer,
-                fileName,
-              ).concat(fixArr);
-            }
-
-            const toContainFunc = buildToContainFuncExpectation(
-              !followTypeAssertionChain(matcher.arguments[0]).value,
-            );
-
-            const [containArg] = includesCall.arguments;
-
-            fixArr.push(
-              fixer.replaceText(matcher.node.property, toContainFunc),
-            );
-            fixArr.push(
+            // we need to negate the expectation if the current expected
+            // value is itself negated by the "not" modifier
+            const addNotModifier =
+              followTypeAssertionChain(matcher.arguments[0]).value ===
+              !!modifier;
+
+            return [
+              // remove the "includes" call entirely
+              fixer.removeRange([
+                includesCall.callee.property.range[0] - 1,
+                includesCall.range[1],
+              ]),
+              // replace the current matcher with "toContain", adding "not" if needed
+              fixer.replaceTextRange(
+                [expectCallEnd, matcher.node.range[1]],
+                addNotModifier
+                  ? `.${ModifierName.not}.toContain`
+                  : '.toContain',
+              ),
+              // replace the matcher argument with the value from the "includes"
               fixer.replaceText(
                 matcher.arguments[0],
-                sourceCode.getText(containArg),
+                sourceCode.getText(includesCall.arguments[0]),
               ),
-            );
-
-            return fixArr;
+            ];
           },
           messageId: 'useToContain',
           node: (modifier || matcher).node.property,