From ac926e779958240506ee506047c9a5364bb70aea Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Wed, 13 May 2020 08:03:36 +1200 Subject: [PATCH] feat: create `no-restricted-matchers` rule (#575) --- README.md | 1 + docs/rules/no-restricted-matchers.md | 47 +++++ .../__snapshots__/rules.test.ts.snap | 1 + src/__tests__/rules.test.ts | 2 +- .../__tests__/no-restricted-matchers.test.ts | 185 ++++++++++++++++++ src/rules/no-restricted-matchers.ts | 97 +++++++++ 6 files changed, 332 insertions(+), 1 deletion(-) create mode 100644 docs/rules/no-restricted-matchers.md create mode 100644 src/rules/__tests__/no-restricted-matchers.test.ts create mode 100644 src/rules/no-restricted-matchers.ts diff --git a/README.md b/README.md index aad622061..68a8192ef 100644 --- a/README.md +++ b/README.md @@ -147,6 +147,7 @@ installations requiring long-term consistency. | [no-jest-import](docs/rules/no-jest-import.md) | Disallow importing Jest | ![recommended][] | | | [no-large-snapshots](docs/rules/no-large-snapshots.md) | disallow large snapshots | | | | [no-mocks-import](docs/rules/no-mocks-import.md) | Disallow manually importing from **mocks** | ![recommended][] | | +| [no-restricted-matchers](docs/rules/no-restricted-matchers.md) | Disallow specific matchers & modifiers | | | | [no-standalone-expect](docs/rules/no-standalone-expect.md) | Prevents expects that are outside of an it or test block. | ![recommended][] | | | [no-test-callback](docs/rules/no-test-callback.md) | Avoid using a callback in asynchronous tests | ![recommended][] | ![fixable][] | | [no-test-prefixes](docs/rules/no-test-prefixes.md) | Use `.only` and `.skip` over `f` and `x` | ![recommended][] | ![fixable][] | diff --git a/docs/rules/no-restricted-matchers.md b/docs/rules/no-restricted-matchers.md new file mode 100644 index 000000000..ad01e3ca0 --- /dev/null +++ b/docs/rules/no-restricted-matchers.md @@ -0,0 +1,47 @@ +# Disallow specific matchers & modifiers (`no-restricted-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/no-restricted-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..23d76c1c8 100644 --- a/src/__tests__/__snapshots__/rules.test.ts.snap +++ b/src/__tests__/__snapshots__/rules.test.ts.snap @@ -28,6 +28,7 @@ Object { "jest/no-jest-import": "error", "jest/no-large-snapshots": "error", "jest/no-mocks-import": "error", + "jest/no-restricted-matchers": "error", "jest/no-standalone-expect": "error", "jest/no-test-callback": "error", "jest/no-test-prefixes": "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__/no-restricted-matchers.test.ts b/src/rules/__tests__/no-restricted-matchers.test.ts new file mode 100644 index 000000000..60424233d --- /dev/null +++ b/src/rules/__tests__/no-restricted-matchers.test.ts @@ -0,0 +1,185 @@ +import { TSESLint } from '@typescript-eslint/experimental-utils'; +import resolveFrom from 'resolve-from'; +import rule from '../no-restricted-matchers'; + +const ruleTester = new TSESLint.RuleTester({ + parser: resolveFrom(require.resolve('eslint'), 'espree'), + parserOptions: { + ecmaVersion: 2017, + }, +}); + +ruleTester.run('no-restricted-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: 'restrictedChain', + data: { + message: null, + chain: 'toBe', + }, + column: 11, + line: 1, + }, + ], + }, + { + code: 'expect(a)["toBe"](b)', + options: [{ toBe: null }], + errors: [ + { + messageId: 'restrictedChain', + data: { + message: null, + chain: 'toBe', + }, + column: 11, + line: 1, + }, + ], + }, + { + code: 'expect(a).not', + options: [{ not: null }], + errors: [ + { + messageId: 'restrictedChain', + data: { + message: null, + chain: 'not', + }, + column: 11, + line: 1, + }, + ], + }, + { + code: 'expect(a).not.toBe(b)', + options: [{ not: null }], + errors: [ + { + messageId: 'restrictedChain', + data: { + message: null, + chain: 'not', + }, + column: 11, + line: 1, + }, + ], + }, + { + code: 'expect(a).not.toBe(b)', + options: [{ 'not.toBe': null }], + errors: [ + { + messageId: 'restrictedChain', + data: { + message: null, + chain: 'not.toBe', + }, + endColumn: 19, + column: 11, + line: 1, + }, + ], + }, + { + code: 'expect(a).toBe(b)', + options: [{ toBe: 'Prefer `toStrictEqual` instead' }], + errors: [ + { + messageId: 'restrictedChainWithMessage', + 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: 'restrictedChainWithMessage', + data: { + message: 'Use `expect(await promise)` instead.', + chain: 'resolves', + }, + endColumn: 52, + column: 44, + }, + ], + }, + { + code: 'expect(Promise.resolve({})).rejects.toBeFalsy()', + options: [{ toBeFalsy: null }], + errors: [ + { + messageId: 'restrictedChain', + 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: 'restrictedChainWithMessage', + data: { + message: 'Use not.toHaveBeenCalled instead', + chain: 'not.toHaveBeenCalledWith', + }, + endColumn: 48, + column: 24, + }, + ], + }, + ], +}); diff --git a/src/rules/no-restricted-matchers.ts b/src/rules/no-restricted-matchers.ts new file mode 100644 index 000000000..d439ecd5b --- /dev/null +++ b/src/rules/no-restricted-matchers.ts @@ -0,0 +1,97 @@ +import { createRule, isExpectCall, parseExpectCall } from './utils'; + +export default createRule< + [Record], + 'restrictedChain' | 'restrictedChainWithMessage' +>({ + name: __filename, + meta: { + docs: { + category: 'Best Practices', + description: 'Disallow specific matchers & modifiers', + recommended: false, + }, + type: 'suggestion', + schema: [ + { + type: 'object', + additionalProperties: { + type: ['string', 'null'], + }, + }, + ], + messages: { + restrictedChain: 'Use of `{{ chain }}` is disallowed', + restrictedChainWithMessage: '{{ message }}', + }, + }, + defaultOptions: [{}], + create(context, [restrictedChains]) { + return { + CallExpression(node) { + if (!isExpectCall(node)) { + return; + } + + const { matcher, modifier } = parseExpectCall(node); + + if (matcher) { + const chain = matcher.name; + + if (chain in restrictedChains) { + const message = restrictedChains[chain]; + + context.report({ + messageId: message + ? 'restrictedChainWithMessage' + : 'restrictedChain', + data: { message, chain }, + node: matcher.node.property, + }); + + return; + } + } + + if (modifier) { + const chain = modifier.name; + + if (chain in restrictedChains) { + const message = restrictedChains[chain]; + + context.report({ + messageId: message + ? 'restrictedChainWithMessage' + : 'restrictedChain', + data: { message, chain }, + node: modifier.node.property, + }); + + return; + } + } + + if (matcher && modifier) { + const chain = `${modifier.name}.${matcher.name}`; + + if (chain in restrictedChains) { + const message = restrictedChains[chain]; + + context.report({ + messageId: message + ? 'restrictedChainWithMessage' + : 'restrictedChain', + data: { message, chain }, + loc: { + start: modifier.node.property.loc.start, + end: matcher.node.property.loc.end, + }, + }); + + return; + } + } + }, + }; + }, +});