From 83a4c48176628c5c2ab9a773667359b234982dc5 Mon Sep 17 00:00:00 2001 From: Catalin Pirvu Date: Sun, 14 Oct 2018 20:53:44 +0300 Subject: [PATCH] feat: add prefer-to-contain rule (#174) Fixes #100 --- README.md | 2 + docs/rules/prefer-to-contain.md | 47 +++++ index.js | 2 + rules/__tests__/prefer-to-contain.test.js | 207 ++++++++++++++++++++++ rules/prefer-to-contain.js | 127 +++++++++++++ 5 files changed, 385 insertions(+) create mode 100644 docs/rules/prefer-to-contain.md create mode 100644 rules/__tests__/prefer-to-contain.test.js create mode 100644 rules/prefer-to-contain.js diff --git a/README.md b/README.md index 3f1195d6d..e7388e049 100644 --- a/README.md +++ b/README.md @@ -97,6 +97,7 @@ for more information about extending configuration files. | [prefer-strict-equal][] | Suggest using `toStrictEqual()` | | ![fixable-green][] | | [prefer-to-be-null][] | Suggest using `toBeNull()` | | ![fixable-green][] | | [prefer-to-be-undefined][] | Suggest using `toBeUndefined()` | | ![fixable-green][] | +| [prefer-to-contain][] | Suggest using `toContain()` | | ![fixable-green][] | | [prefer-to-have-length][] | Suggest using `toHaveLength()` | ![recommended][] | ![fixable-green][] | | [prefer-inline-snapshots][] | Suggest using `toMatchInlineSnapshot()` | | ![fixable-green][] | | [require-tothrow-message][] | Require that `toThrow()` and `toThrowError` includes a message | | | @@ -126,6 +127,7 @@ for more information about extending configuration files. [prefer-strict-equal]: docs/rules/prefer-strict-equal.md [prefer-to-be-null]: docs/rules/prefer-to-be-null.md [prefer-to-be-undefined]: docs/rules/prefer-to-be-undefined.md +[prefer-to-contain]: docs/rules/prefer-to-contain.md [prefer-to-have-length]: docs/rules/prefer-to-have-length.md [prefer-inline-snapshots]: docs/rules/prefer-inline-snapshots.md [require-tothrow-message]: docs/rules/require-tothrow-message.md diff --git a/docs/rules/prefer-to-contain.md b/docs/rules/prefer-to-contain.md new file mode 100644 index 000000000..3014967dc --- /dev/null +++ b/docs/rules/prefer-to-contain.md @@ -0,0 +1,47 @@ +# Suggest using `toContain()` (prefer-to-contain) + +In order to have a better failure message, `toContain()` should be used upon +asserting expectations on an array containing an object. + +## Rule details + +This rule triggers a warning if `toBe()` or `isEqual()` is used to assert object +inclusion in an array + +```js +expect(a.includes(b)).toBe(true); +``` + +```js +expect(a.includes(b)).not.toBe(true); +``` + +```js +expect(a.includes(b)).toBe(false); +``` + +### Default configuration + +The following patterns are considered a warning: + +```js +expect(a.includes(b)).toBe(true); +``` + +```js +expect(a.includes(b)).not.toBe(true); +``` + +```js +expect(a.includes(b)).toBe(false); +``` + +The following patterns are not a warning: + +```js +expect(a).toContain(b); +``` + +```js +expect(a).not.toContain(b); +``` diff --git a/index.js b/index.js index 8c90da4d7..56aa28705 100644 --- a/index.js +++ b/index.js @@ -14,6 +14,7 @@ const noTestPrefixes = require('./rules/no-test-prefixes'); const noTestReturnStatement = require('./rules/no-test-return-statement'); const preferToBeNull = require('./rules/prefer-to-be-null'); const preferToBeUndefined = require('./rules/prefer-to-be-undefined'); +const preferToContain = require('./rules/prefer-to-contain'); const preferToHaveLength = require('./rules/prefer-to-have-length'); const validDescribe = require('./rules/valid-describe'); const validExpect = require('./rules/valid-expect'); @@ -84,6 +85,7 @@ module.exports = { 'no-test-return-statement': noTestReturnStatement, 'prefer-to-be-null': preferToBeNull, 'prefer-to-be-undefined': preferToBeUndefined, + 'prefer-to-contain': preferToContain, 'prefer-to-have-length': preferToHaveLength, 'valid-describe': validDescribe, 'valid-expect': validExpect, diff --git a/rules/__tests__/prefer-to-contain.test.js b/rules/__tests__/prefer-to-contain.test.js new file mode 100644 index 000000000..6e94f2030 --- /dev/null +++ b/rules/__tests__/prefer-to-contain.test.js @@ -0,0 +1,207 @@ +'use strict'; + +const RuleTester = require('eslint').RuleTester; +const rule = require('../prefer-to-contain'); + +const ruleTester = new RuleTester(); + +ruleTester.run('prefer-to-contain', rule, { + valid: [ + 'expect(a).toContain(b);', + "expect(a.name).toBe('b');", + 'expect(a).toBe(true);', + `expect(a).toEqual(b)`, + `expect(a.test(c)).toEqual(b)`, + `expect(a.includes(b)).toEqual()`, + `expect(a.includes(b)).toEqual("test")`, + `expect(a.includes(b)).toBe("test")`, + `expect(a.includes()).toEqual()`, + `expect(a.includes()).toEqual(true)`, + `expect(a.includes(b,c)).toBe(true)`, + `expect([{a:1}]).toContain({a:1})`, + `expect([1].includes(1)).toEqual`, + `expect([1].includes).toEqual`, + `expect([1].includes).not`, + `expect(a.test(b)).resolves.toEqual(true)`, + `expect(a.test(b)).resolves.not.toEqual(true)`, + `expect(a).not.toContain(b)`, + ], + invalid: [ + { + code: 'expect(a.includes(b)).toEqual(true);', + errors: [ + { + message: 'Use toContain() instead', + column: 23, + line: 1, + }, + ], + output: 'expect(a).toContain(b);', + }, + { + code: 'expect(a.includes(b)).toEqual(false);', + errors: [ + { + message: 'Use toContain() instead', + column: 23, + line: 1, + }, + ], + output: 'expect(a).not.toContain(b);', + }, + { + code: 'expect(a.includes(b)).not.toEqual(false);', + errors: [ + { + message: 'Use toContain() instead', + column: 23, + line: 1, + }, + ], + output: 'expect(a).toContain(b);', + }, + { + code: 'expect(a.includes(b)).not.toEqual(true);', + errors: [ + { + message: 'Use toContain() instead', + column: 23, + line: 1, + }, + ], + output: 'expect(a).not.toContain(b);', + }, + { + code: 'expect(a.includes(b)).toBe(true);', + errors: [ + { + message: 'Use toContain() instead', + column: 23, + line: 1, + }, + ], + output: 'expect(a).toContain(b);', + }, + { + code: 'expect(a.includes(b)).toBe(false);', + errors: [ + { + message: 'Use toContain() instead', + column: 23, + line: 1, + }, + ], + output: 'expect(a).not.toContain(b);', + }, + { + code: 'expect(a.includes(b)).not.toBe(false);', + errors: [ + { + message: 'Use toContain() instead', + column: 23, + line: 1, + }, + ], + output: 'expect(a).toContain(b);', + }, + { + code: 'expect(a.includes(b)).not.toBe(true);', + errors: [ + { + message: 'Use toContain() instead', + column: 23, + line: 1, + }, + ], + output: 'expect(a).not.toContain(b);', + }, + { + code: 'expect(a.test(t).includes(b.test(p))).toEqual(true);', + errors: [ + { + message: 'Use toContain() instead', + column: 39, + line: 1, + }, + ], + output: 'expect(a.test(t)).toContain(b.test(p));', + }, + { + code: 'expect(a.test(t).includes(b.test(p))).toEqual(false);', + errors: [ + { + message: 'Use toContain() instead', + column: 39, + line: 1, + }, + ], + output: 'expect(a.test(t)).not.toContain(b.test(p));', + }, + { + code: 'expect(a.test(t).includes(b.test(p))).not.toEqual(true);', + errors: [ + { + message: 'Use toContain() instead', + column: 39, + line: 1, + }, + ], + output: 'expect(a.test(t)).not.toContain(b.test(p));', + }, + { + code: 'expect(a.test(t).includes(b.test(p))).not.toEqual(false);', + errors: [ + { + message: 'Use toContain() instead', + column: 39, + line: 1, + }, + ], + output: 'expect(a.test(t)).toContain(b.test(p));', + }, + { + code: 'expect([{a:1}].includes({a:1})).toBe(true);', + errors: [ + { + message: 'Use toContain() instead', + column: 33, + line: 1, + }, + ], + output: 'expect([{a:1}]).toContain({a:1});', + }, + { + code: 'expect([{a:1}].includes({a:1})).toBe(false);', + errors: [ + { + message: 'Use toContain() instead', + column: 33, + line: 1, + }, + ], + output: 'expect([{a:1}]).not.toContain({a:1});', + }, + { + code: 'expect([{a:1}].includes({a:1})).not.toBe(true);', + errors: [ + { + message: 'Use toContain() instead', + column: 33, + line: 1, + }, + ], + output: 'expect([{a:1}]).not.toContain({a:1});', + }, + { + code: 'expect([{a:1}].includes({a:1})).not.toBe(false);', + errors: [ + { + message: 'Use toContain() instead', + column: 33, + line: 1, + }, + ], + output: 'expect([{a:1}]).toContain({a:1});', + }, + ], +}); diff --git a/rules/prefer-to-contain.js b/rules/prefer-to-contain.js new file mode 100644 index 000000000..210e03c08 --- /dev/null +++ b/rules/prefer-to-contain.js @@ -0,0 +1,127 @@ +'use strict'; + +const getDocsUrl = require('./util').getDocsUrl; +const expectCase = require('./util').expectCase; +const expectResolveCase = require('./util').expectResolveCase; +const expectRejectCase = require('./util').expectRejectCase; +const method = require('./util').method; +const argument = require('./util').argument; + +const isEqualityCheck = node => + method(node) && + (method(node).name === 'toBe' || method(node).name === 'toEqual'); + +const isArgumentValid = node => + argument(node).value === true || argument(node).value === false; + +const hasOneArgument = node => node.arguments && node.arguments.length === 1; + +const isValidEqualityCheck = node => + isEqualityCheck(node) && + hasOneArgument(node.parent.parent) && + isArgumentValid(node); + +const isEqualityNegation = node => + method(node).name === 'not' && isValidEqualityCheck(node.parent); + +const hasIncludesMethod = node => + node.arguments[0] && + node.arguments[0].callee && + node.arguments[0].callee.property && + node.arguments[0].callee.property.name === 'includes'; + +const isValidIncludesMethod = node => + hasIncludesMethod(node) && hasOneArgument(node.arguments[0]); + +const getNegationFixes = (node, sourceCode, fixer) => { + const negationPropertyDot = sourceCode.getFirstTokenBetween( + node.parent.object, + node.parent.property, + token => token.value === '.' + ); + const toContainFunc = + isEqualityNegation(node) && argument(node.parent).value + ? 'not.toContain' + : 'toContain'; + + //.includes function argument + const containArg = node.arguments[0].arguments[0]; + return [ + fixer.remove(negationPropertyDot), + fixer.remove(method(node)), + fixer.replaceText(method(node.parent), toContainFunc), + fixer.replaceText(argument(node.parent), sourceCode.getText(containArg)), + ]; +}; + +const getCommonFixes = (node, sourceCode, fixer) => { + const containArg = node.arguments[0].arguments[0]; + const includesCaller = node.arguments[0].callee; + + const propertyDot = sourceCode.getFirstTokenBetween( + includesCaller.object, + includesCaller.property, + token => token.value === '.' + ); + + const closingParenthesis = sourceCode.getTokenAfter(containArg); + const openParenthesis = sourceCode.getTokenBefore(containArg); + + return [ + fixer.remove(containArg), + fixer.remove(includesCaller.property), + fixer.remove(propertyDot), + fixer.remove(closingParenthesis), + fixer.remove(openParenthesis), + ]; +}; + +module.exports = { + meta: { + docs: { + url: getDocsUrl(__filename), + }, + fixable: 'code', + }, + create(context) { + return { + CallExpression(node) { + if ( + !(expectResolveCase(node) || expectRejectCase(node)) && + expectCase(node) && + (isEqualityNegation(node) || isValidEqualityCheck(node)) && + isValidIncludesMethod(node) + ) { + context.report({ + fix(fixer) { + const sourceCode = context.getSourceCode(); + + let fixArr = getCommonFixes(node, sourceCode, fixer); + if (isEqualityNegation(node)) { + return getNegationFixes(node, sourceCode, fixer).concat(fixArr); + } + + const toContainFunc = argument(node).value + ? 'toContain' + : 'not.toContain'; + + //.includes function argument + const containArg = node.arguments[0].arguments[0]; + + fixArr.push(fixer.replaceText(method(node), toContainFunc)); + fixArr.push( + fixer.replaceText( + argument(node), + sourceCode.getText(containArg) + ) + ); + return fixArr; + }, + message: 'Use toContain() instead', + node: method(node), + }); + } + }, + }; + }, +};