From f4dd97a7ec3b985d0f7e42a5a6331bc0c65a7d56 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Sun, 28 Aug 2022 21:47:36 +1200 Subject: [PATCH] feat(no-restricted-matchers): match based on start of chain, requiring each permutation to be set (#1218) BREAKING CHANGE: `no-restricted-matchers` now checks against the start of the expect chain, meaning you have to explicitly list each possible matcher & modifier permutations that you want to restrict --- docs/rules/no-restricted-matchers.md | 15 +++- .../__tests__/no-restricted-matchers.test.ts | 83 ++++++++----------- src/rules/no-restricted-matchers.ts | 29 ++----- 3 files changed, 57 insertions(+), 70 deletions(-) diff --git a/docs/rules/no-restricted-matchers.md b/docs/rules/no-restricted-matchers.md index ad01e3ca0..77138b558 100644 --- a/docs/rules/no-restricted-matchers.md +++ b/docs/rules/no-restricted-matchers.md @@ -8,8 +8,9 @@ alternatives. 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. +Bans are checked against the start of the `expect` chain - this means that to +ban a specific matcher entirely you must specify all six permutations, but +allows you to ban modifiers as well. By default, this map is empty, meaning no matchers or modifiers are banned. @@ -22,7 +23,12 @@ For example: { "toBeFalsy": null, "resolves": "Use `expect(await promise)` instead.", - "not.toHaveBeenCalledWith": null + "toHaveBeenCalledWith": null, + "not.toHaveBeenCalledWith": null, + "resolves.toHaveBeenCalledWith": null, + "rejects.toHaveBeenCalledWith": null, + "resolves.not.toHaveBeenCalledWith": null, + "rejects.not.toHaveBeenCalledWith": null } ] } @@ -32,15 +38,18 @@ Examples of **incorrect** code for this rule with the above configuration ```js it('is false', () => { + // if this has a modifer (i.e. `not.toBeFalsy`), it would be considered fine expect(a).toBeFalsy(); }); it('resolves', async () => { + // all uses of this modifier are disallowed, regardless of matcher await expect(myPromise()).resolves.toBe(true); }); describe('when an error happens', () => { it('does not upload the file', async () => { + // all uses of this matcher are disallowed expect(uploadFileMock).not.toHaveBeenCalledWith('file.name'); }); }); diff --git a/src/rules/__tests__/no-restricted-matchers.test.ts b/src/rules/__tests__/no-restricted-matchers.test.ts index 38018f55f..1d134ec38 100644 --- a/src/rules/__tests__/no-restricted-matchers.test.ts +++ b/src/rules/__tests__/no-restricted-matchers.test.ts @@ -37,6 +37,26 @@ ruleTester.run('no-restricted-matchers', rule, { code: 'expect(a)["toBe"](b)', options: [{ 'not.toBe': null }], }, + { + code: 'expect(a).resolves.not.toBe(b)', + options: [{ not: null }], + }, + { + code: 'expect(a).resolves.not.toBe(b)', + options: [{ 'not.toBe': null }], + }, + { + code: "expect(uploadFileMock).resolves.toHaveBeenCalledWith('file.name')", + options: [ + { 'not.toHaveBeenCalledWith': 'Use not.toHaveBeenCalled instead' }, + ], + }, + { + code: "expect(uploadFileMock).resolves.not.toHaveBeenCalledWith('file.name')", + options: [ + { 'not.toHaveBeenCalledWith': 'Use not.toHaveBeenCalled instead' }, + ], + }, ], invalid: [ { @@ -47,7 +67,7 @@ ruleTester.run('no-restricted-matchers', rule, { messageId: 'restrictedChain', data: { message: null, - chain: 'toBe', + restriction: 'toBe', }, column: 11, line: 1, @@ -62,7 +82,7 @@ ruleTester.run('no-restricted-matchers', rule, { messageId: 'restrictedChain', data: { message: null, - chain: 'toBe', + restriction: 'toBe', }, column: 11, line: 1, @@ -77,7 +97,7 @@ ruleTester.run('no-restricted-matchers', rule, { messageId: 'restrictedChain', data: { message: null, - chain: 'not', + restriction: 'not', }, column: 11, line: 1, @@ -92,7 +112,7 @@ ruleTester.run('no-restricted-matchers', rule, { messageId: 'restrictedChain', data: { message: null, - chain: 'not', + restriction: 'not', }, column: 11, line: 1, @@ -107,28 +127,13 @@ ruleTester.run('no-restricted-matchers', rule, { messageId: 'restrictedChain', data: { message: null, - chain: 'resolves', + restriction: 'resolves', }, column: 11, line: 1, }, ], }, - { - code: 'expect(a).resolves.not.toBe(b)', - options: [{ not: null }], - errors: [ - { - messageId: 'restrictedChain', - data: { - message: null, - chain: 'not', - }, - column: 20, - line: 1, - }, - ], - }, { code: 'expect(a).resolves.not.toBe(b)', options: [{ resolves: null }], @@ -137,7 +142,7 @@ ruleTester.run('no-restricted-matchers', rule, { messageId: 'restrictedChain', data: { message: null, - chain: 'resolves', + restriction: 'resolves', }, column: 11, line: 1, @@ -152,29 +157,13 @@ ruleTester.run('no-restricted-matchers', rule, { messageId: 'restrictedChain', data: { message: null, - chain: 'resolves.not', + restriction: 'resolves.not', }, column: 11, line: 1, }, ], }, - { - code: 'expect(a).resolves.not.toBe(b)', - options: [{ 'not.toBe': null }], - errors: [ - { - messageId: 'restrictedChain', - data: { - message: null, - chain: 'not.toBe', - }, - endColumn: 28, - column: 20, - line: 1, - }, - ], - }, { code: 'expect(a).not.toBe(b)', options: [{ 'not.toBe': null }], @@ -183,7 +172,7 @@ ruleTester.run('no-restricted-matchers', rule, { messageId: 'restrictedChain', data: { message: null, - chain: 'not.toBe', + restriction: 'not.toBe', }, endColumn: 19, column: 11, @@ -199,7 +188,7 @@ ruleTester.run('no-restricted-matchers', rule, { messageId: 'restrictedChain', data: { message: null, - chain: 'resolves.not.toBe', + restriction: 'resolves.not.toBe', }, endColumn: 28, column: 11, @@ -215,7 +204,7 @@ ruleTester.run('no-restricted-matchers', rule, { messageId: 'restrictedChainWithMessage', data: { message: 'Prefer `toStrictEqual` instead', - chain: 'toBe', + restriction: 'toBe', }, column: 11, line: 1, @@ -234,25 +223,25 @@ ruleTester.run('no-restricted-matchers', rule, { messageId: 'restrictedChainWithMessage', data: { message: 'Use `expect(await promise)` instead.', - chain: 'resolves', + restriction: 'resolves', }, - endColumn: 52, + endColumn: 57, column: 44, }, ], }, { code: 'expect(Promise.resolve({})).rejects.toBeFalsy()', - options: [{ toBeFalsy: null }], + options: [{ 'rejects.toBeFalsy': null }], errors: [ { messageId: 'restrictedChain', data: { message: null, - chain: 'toBeFalsy', + restriction: 'rejects.toBeFalsy', }, endColumn: 46, - column: 37, + column: 29, }, ], }, @@ -266,7 +255,7 @@ ruleTester.run('no-restricted-matchers', rule, { messageId: 'restrictedChainWithMessage', data: { message: 'Use not.toHaveBeenCalled instead', - chain: 'not.toHaveBeenCalledWith', + restriction: 'not.toHaveBeenCalledWith', }, endColumn: 48, column: 24, diff --git a/src/rules/no-restricted-matchers.ts b/src/rules/no-restricted-matchers.ts index e918db706..ebe2f52fd 100644 --- a/src/rules/no-restricted-matchers.ts +++ b/src/rules/no-restricted-matchers.ts @@ -21,7 +21,7 @@ export default createRule< }, ], messages: { - restrictedChain: 'Use of `{{ chain }}` is disallowed', + restrictedChain: 'Use of `{{ restriction }}` is disallowed', restrictedChainWithMessage: '{{ message }}', }, }, @@ -35,31 +35,20 @@ export default createRule< return; } - const permutations = [jestFnCall.members]; - - if (jestFnCall.members.length > 2) { - permutations.push([jestFnCall.members[0], jestFnCall.members[1]]); - permutations.push([jestFnCall.members[1], jestFnCall.members[2]]); - } - - if (jestFnCall.members.length > 1) { - permutations.push(...jestFnCall.members.map(nod => [nod])); - } - - for (const permutation of permutations) { - const chain = permutation.map(nod => getAccessorValue(nod)).join('.'); - - if (chain in restrictedChains) { - const message = restrictedChains[chain]; + const chain = jestFnCall.members + .map(nod => getAccessorValue(nod)) + .join('.'); + for (const [restriction, message] of Object.entries(restrictedChains)) { + if (chain.startsWith(restriction)) { context.report({ messageId: message ? 'restrictedChainWithMessage' : 'restrictedChain', - data: { message, chain }, + data: { message, restriction }, loc: { - start: permutation[0].loc.start, - end: permutation[permutation.length - 1].loc.end, + start: jestFnCall.members[0].loc.start, + end: jestFnCall.members[jestFnCall.members.length - 1].loc.end, }, });