diff --git a/README.md b/README.md index ae8aa58b9..515e1e0cd 100644 --- a/README.md +++ b/README.md @@ -127,6 +127,7 @@ installations requiring long-term consistency. | Rule | Description | Configurations | Fixable | | ------------------------------ | ----------------------------------------------------------------- | ---------------- | ------------------- | +| [ban-matchers][] | Bans specific matchers & modifiers from being used | | | | [consistent-test-it][] | Enforce consistent test or it keyword | | ![fixable-green][] | | [expect-expect][] | Enforce assertion to be made in a test body | ![recommended][] | | | [lowercase-name][] | Disallow capitalized test names | | ![fixable-green][] | @@ -183,6 +184,7 @@ ensure consistency and readability in jest test suites. https://github.com/dangreenisrael/eslint-plugin-jest-formatting +[ban-matchers]: docs/rules/ban-matchers.md [consistent-test-it]: docs/rules/consistent-test-it.md [expect-expect]: docs/rules/expect-expect.md [lowercase-name]: docs/rules/lowercase-name.md diff --git a/docs/rules/ban-matchers.md b/docs/rules/ban-matchers.md new file mode 100644 index 000000000..abdc52548 --- /dev/null +++ b/docs/rules/ban-matchers.md @@ -0,0 +1,47 @@ +# Bans specific matchers & modifiers from being used (`ban-matchers`) + +This rule bans specific matchers & modifiers from being used, and can suggest +alternatives. + +## Rule Details + +Bans are expressed in the form of a map, with the value being either a string +message to be shown, or `null` if the default rule message should be used. + +Both matchers, modifiers, and chains of the two are checked, allowing for +specific variations of a matcher to be banned if desired. + +By default, this map is empty, meaning no matchers or modifiers are banned. + +For example: + +```json +{ + "jest/ban-matchers": [ + "error", + { + "toBeFalsy": null, + "resolves": "Use `expect(await promise)` instead.", + "not.toHaveBeenCalledWith": null + } + ] +} +``` + +Examples of **incorrect** code for this rule with the above configuration + +```js +it('is false', () => { + expect(a).toBeFalsy(); +}); + +it('resolves', async () => { + await expect(myPromise()).resolves.toBe(true); +}); + +describe('when an error happens', () => { + it('does not upload the file', async () => { + expect(uploadFileMock).not.toHaveBeenCalledWith('file.name'); + }); +}); +``` diff --git a/src/__tests__/__snapshots__/rules.test.ts.snap b/src/__tests__/__snapshots__/rules.test.ts.snap index ec79e907d..207d076a2 100644 --- a/src/__tests__/__snapshots__/rules.test.ts.snap +++ b/src/__tests__/__snapshots__/rules.test.ts.snap @@ -10,6 +10,7 @@ Object { "jest", ], "rules": Object { + "jest/ban-matchers": "error", "jest/consistent-test-it": "error", "jest/expect-expect": "error", "jest/lowercase-name": "error", diff --git a/src/__tests__/rules.test.ts b/src/__tests__/rules.test.ts index eee9d9c32..ec4461e4a 100644 --- a/src/__tests__/rules.test.ts +++ b/src/__tests__/rules.test.ts @@ -3,7 +3,7 @@ import { resolve } from 'path'; import plugin from '../'; const ruleNames = Object.keys(plugin.rules); -const numberOfRules = 41; +const numberOfRules = 42; describe('rules', () => { it('should have a corresponding doc for each rule', () => { diff --git a/src/rules/__tests__/ban-matchers.test.ts b/src/rules/__tests__/ban-matchers.test.ts new file mode 100644 index 000000000..81c432da3 --- /dev/null +++ b/src/rules/__tests__/ban-matchers.test.ts @@ -0,0 +1,185 @@ +import { TSESLint } from '@typescript-eslint/experimental-utils'; +import resolveFrom from 'resolve-from'; +import rule from '../ban-matchers'; + +const ruleTester = new TSESLint.RuleTester({ + parser: resolveFrom(require.resolve('eslint'), 'espree'), + parserOptions: { + ecmaVersion: 2017, + }, +}); + +ruleTester.run('ban-matchers', rule, { + valid: [ + 'expect(a).toHaveBeenCalled()', + 'expect(a).not.toHaveBeenCalled()', + 'expect(a).toHaveBeenCalledTimes()', + 'expect(a).toHaveBeenCalledWith()', + 'expect(a).toHaveBeenLastCalledWith()', + 'expect(a).toHaveBeenNthCalledWith()', + 'expect(a).toHaveReturned()', + 'expect(a).toHaveReturnedTimes()', + 'expect(a).toHaveReturnedWith()', + 'expect(a).toHaveLastReturnedWith()', + 'expect(a).toHaveNthReturnedWith()', + 'expect(a).toThrow()', + 'expect(a).rejects;', + 'expect(a);', + { + code: 'expect(a).resolves', + options: [{ not: null }], + }, + { + code: 'expect(a).toBe(b)', + options: [{ 'not.toBe': null }], + }, + { + code: 'expect(a)["toBe"](b)', + options: [{ 'not.toBe': null }], + }, + ], + invalid: [ + { + code: 'expect(a).toBe(b)', + options: [{ toBe: null }], + errors: [ + { + messageId: 'bannedChain', + data: { + message: null, + chain: 'toBe', + }, + column: 11, + line: 1, + }, + ], + }, + { + code: 'expect(a)["toBe"](b)', + options: [{ toBe: null }], + errors: [ + { + messageId: 'bannedChain', + data: { + message: null, + chain: 'toBe', + }, + column: 11, + line: 1, + }, + ], + }, + { + code: 'expect(a).not', + options: [{ not: null }], + errors: [ + { + messageId: 'bannedChain', + data: { + message: null, + chain: 'not', + }, + column: 11, + line: 1, + }, + ], + }, + { + code: 'expect(a).not.toBe(b)', + options: [{ not: null }], + errors: [ + { + messageId: 'bannedChain', + data: { + message: null, + chain: 'not', + }, + column: 11, + line: 1, + }, + ], + }, + { + code: 'expect(a).not.toBe(b)', + options: [{ 'not.toBe': null }], + errors: [ + { + messageId: 'bannedChain', + data: { + message: null, + chain: 'not.toBe', + }, + endColumn: 19, + column: 11, + line: 1, + }, + ], + }, + { + code: 'expect(a).toBe(b)', + options: [{ toBe: 'Prefer `toStrictEqual` instead' }], + errors: [ + { + messageId: 'bannedChainWithMessage', + data: { + message: 'Prefer `toStrictEqual` instead', + chain: 'toBe', + }, + column: 11, + line: 1, + }, + ], + }, + { + code: ` + test('some test', async () => { + await expect(Promise.resolve(1)).resolves.toBe(1); + }); + `, + options: [{ resolves: 'Use `expect(await promise)` instead.' }], + errors: [ + { + messageId: 'bannedChainWithMessage', + data: { + message: 'Use `expect(await promise)` instead.', + chain: 'resolves', + }, + endColumn: 52, + column: 44, + }, + ], + }, + { + code: 'expect(Promise.resolve({})).rejects.toBeFalsy()', + options: [{ toBeFalsy: null }], + errors: [ + { + messageId: 'bannedChain', + data: { + message: null, + chain: 'toBeFalsy', + }, + endColumn: 46, + column: 37, + }, + ], + }, + { + code: "expect(uploadFileMock).not.toHaveBeenCalledWith('file.name')", + options: [ + { 'not.toHaveBeenCalledWith': 'Use not.toHaveBeenCalled instead' }, + ], + errors: [ + { + messageId: 'bannedChainWithMessage', + data: { + message: 'Use not.toHaveBeenCalled instead', + chain: 'not.toHaveBeenCalledWith', + }, + endColumn: 48, + column: 24, + }, + ], + }, + ], +}); diff --git a/src/rules/ban-matchers.ts b/src/rules/ban-matchers.ts new file mode 100644 index 000000000..57585f8d6 --- /dev/null +++ b/src/rules/ban-matchers.ts @@ -0,0 +1,91 @@ +import { createRule, isExpectCall, parseExpectCall } from './utils'; + +export default createRule< + [Record], + 'bannedChain' | 'bannedChainWithMessage' +>({ + name: __filename, + meta: { + docs: { + category: 'Best Practices', + description: 'Bans specific matchers & modifiers from being used', + recommended: false, + }, + type: 'suggestion', + schema: [ + { + type: 'object', + additionalProperties: { + type: ['string', 'null'], + }, + }, + ], + messages: { + bannedChain: '{{ chain }} is banned, and so should not be used', + bannedChainWithMessage: '{{ message }}', + }, + }, + defaultOptions: [{}], + create(context, [bannedChains]) { + return { + CallExpression(node) { + if (!isExpectCall(node)) { + return; + } + + const { matcher, modifier } = parseExpectCall(node); + + if (matcher) { + const chain = matcher.name; + + if (chain in bannedChains) { + const message = bannedChains[chain]; + + context.report({ + messageId: message ? 'bannedChainWithMessage' : 'bannedChain', + data: { message, chain }, + node: matcher.node.property, + }); + + return; + } + } + + if (modifier) { + const chain = modifier.name; + + if (chain in bannedChains) { + const message = bannedChains[chain]; + + context.report({ + messageId: message ? 'bannedChainWithMessage' : 'bannedChain', + data: { message, chain }, + node: modifier.node.property, + }); + + return; + } + } + + if (matcher && modifier) { + const chain = `${modifier.name}.${matcher.name}`; + + if (chain in bannedChains) { + const message = bannedChains[chain]; + + context.report({ + messageId: message ? 'bannedChainWithMessage' : 'bannedChain', + data: { message, chain }, + loc: { + start: modifier.node.property.loc.start, + end: matcher.node.property.loc.end, + }, + }); + + return; + } + } + }, + }; + }, +});