From 73984a79f790986a17116589a587506bcc10efc0 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Fri, 31 Dec 2021 10:18:08 +1300 Subject: [PATCH] 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 --- src/rules/__tests__/prefer-to-contain.test.ts | 36 ++++- src/rules/prefer-to-contain.ts | 145 +++--------------- 2 files changed, 53 insertions(+), 128 deletions(-) diff --git a/src/rules/__tests__/prefer-to-contain.test.ts b/src/rules/__tests__/prefer-to-contain.test.ts index 387d2c2f1..646c783d7 100644 --- 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 index b13439f81..17a8dfddc 100644 --- a/src/rules/prefer-to-contain.ts +++ b/src/rules/prefer-to-contain.ts @@ -1,6 +1,5 @@ import { AST_NODE_TYPES, - TSESLint, TSESTree, } from '@typescript-eslint/experimental-utils'; import { @@ -8,7 +7,6 @@ import { KnownCallExpression, MaybeTypeCast, ModifierName, - NotNegatableParsedModifier, ParsedEqualityMatcherCall, ParsedExpectMatcher, createRule, @@ -57,9 +55,6 @@ 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(node: TSESTree.Node, method: Name)` util-fn */ const isFixableIncludesCallExpression = ( node: TSESTree.Node, @@ -67,93 +62,8 @@ const isFixableIncludesCallExpression = ( 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 => { - 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()[not.]{toBe,toEqual}() 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,