From f78720d34550336d5476cbe51a51c87a8ef78897 Mon Sep 17 00:00:00 2001 From: Spencer Miskoviak <5247455+skovy@users.noreply.github.com> Date: Fri, 16 Oct 2020 03:52:41 -0700 Subject: [PATCH] fix(prefer-explicit-assert): only enforce assertion when presence/absence matcher (#238) Relates to #218 --- docs/rules/prefer-explicit-assert.md | 6 ++- lib/rules/prefer-explicit-assert.ts | 38 +++++++++++++---- lib/rules/prefer-presence-queries.ts | 4 +- lib/utils.ts | 5 +++ .../lib/rules/prefer-explicit-assert.test.ts | 42 ++++++++++++++++++- 5 files changed, 82 insertions(+), 13 deletions(-) diff --git a/docs/rules/prefer-explicit-assert.md b/docs/rules/prefer-explicit-assert.md index b0a0079f..b52a17e8 100644 --- a/docs/rules/prefer-explicit-assert.md +++ b/docs/rules/prefer-explicit-assert.md @@ -56,7 +56,11 @@ This rule has a few options: with `getBy*` queries. By default, any assertion is valid (`toBeTruthy`, `toBeDefined`, etc.). However, they all assert slightly different things. This option ensures all `getBy*` assertions are consistent and use the same - assertion. + assertion. This rule only allows defining a presence matcher + (`toBeInTheDocument`, `toBeTruthy`, or `toBeDefined`), but checks for both + presence and absence matchers (`not.toBeFalsy` and `not.toBeNull`). This means + other assertions such as `toHaveValue` or `toBeDisabled` will not trigger this + rule since these are valid uses with `getBy*`. ```js "testing-library/prefer-explicit-assert": ["error", {"assertion": "toBeInTheDocument"}], diff --git a/lib/rules/prefer-explicit-assert.ts b/lib/rules/prefer-explicit-assert.ts index 29b0e954..fb634603 100644 --- a/lib/rules/prefer-explicit-assert.ts +++ b/lib/rules/prefer-explicit-assert.ts @@ -1,6 +1,15 @@ import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils'; -import { getDocsUrl, ALL_QUERIES_METHODS } from '../utils'; -import { isMemberExpression } from '../node-utils'; +import { + getDocsUrl, + ALL_QUERIES_METHODS, + PRESENCE_MATCHERS, + ABSENCE_MATCHERS, +} from '../utils'; +import { + findClosestCallNode, + isIdentifier, + isMemberExpression, +} from '../node-utils'; export const RULE_NAME = 'prefer-explicit-assert'; export type MessageIds = @@ -48,6 +57,7 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ properties: { assertion: { type: 'string', + enum: PRESENCE_MATCHERS, }, customQueryNames: { type: 'array', @@ -84,15 +94,29 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ messageId: 'preferExplicitAssert', }); } else if (assertion) { - const expectation = node.parent.parent.parent; + const expectCallNode = findClosestCallNode(node, 'expect'); + + const expectStatement = expectCallNode.parent as TSESTree.MemberExpression; + const property = expectStatement.property as TSESTree.Identifier; + let matcher = property.name; + let isNegatedMatcher = false; if ( - expectation.type === 'MemberExpression' && - expectation.property.type === 'Identifier' && - expectation.property.name !== assertion + matcher === 'not' && + isMemberExpression(expectStatement.parent) && + isIdentifier(expectStatement.parent.property) ) { + isNegatedMatcher = true; + matcher = expectStatement.parent.property.name; + } + + const shouldEnforceAssertion = + (!isNegatedMatcher && PRESENCE_MATCHERS.includes(matcher)) || + (isNegatedMatcher && ABSENCE_MATCHERS.includes(matcher)); + + if (shouldEnforceAssertion && matcher !== assertion) { context.report({ - node: expectation.property, + node: property, messageId: 'preferExplicitAssertAssertion', data: { assertion, diff --git a/lib/rules/prefer-presence-queries.ts b/lib/rules/prefer-presence-queries.ts index 83464d7d..06c87515 100644 --- a/lib/rules/prefer-presence-queries.ts +++ b/lib/rules/prefer-presence-queries.ts @@ -1,5 +1,5 @@ import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils'; -import { getDocsUrl, ALL_QUERIES_METHODS } from '../utils'; +import { getDocsUrl, ALL_QUERIES_METHODS, PRESENCE_MATCHERS, ABSENCE_MATCHERS } from '../utils'; import { findClosestCallNode, isMemberExpression, @@ -13,8 +13,6 @@ type Options = []; const QUERIES_REGEXP = new RegExp( `^(get|query)(All)?(${ALL_QUERIES_METHODS.join('|')})$` ); -const PRESENCE_MATCHERS = ['toBeInTheDocument', 'toBeTruthy', 'toBeDefined']; -const ABSENCE_MATCHERS = ['toBeNull', 'toBeFalsy']; function isThrowingQuery(node: TSESTree.Identifier) { return node.name.startsWith('get'); diff --git a/lib/utils.ts b/lib/utils.ts index 18d3d7f9..8ea7512c 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -65,6 +65,9 @@ const ASYNC_UTILS = [ const TESTING_FRAMEWORK_SETUP_HOOKS = ['beforeEach', 'beforeAll']; +const PRESENCE_MATCHERS = ['toBeInTheDocument', 'toBeTruthy', 'toBeDefined']; +const ABSENCE_MATCHERS = ['toBeNull', 'toBeFalsy']; + export { getDocsUrl, SYNC_QUERIES_VARIANTS, @@ -77,4 +80,6 @@ export { ASYNC_UTILS, TESTING_FRAMEWORK_SETUP_HOOKS, LIBRARY_MODULES, + PRESENCE_MATCHERS, + ABSENCE_MATCHERS }; diff --git a/tests/lib/rules/prefer-explicit-assert.test.ts b/tests/lib/rules/prefer-explicit-assert.test.ts index 5715d134..b838ade3 100644 --- a/tests/lib/rules/prefer-explicit-assert.test.ts +++ b/tests/lib/rules/prefer-explicit-assert.test.ts @@ -74,6 +74,14 @@ ruleTester.run(RULE_NAME, rule, { }, ], }, + { + code: `expect(getByText('foo')).toBeEnabled()`, + options: [ + { + assertion: 'toBeInTheDocument', + }, + ], + }, ], invalid: [ @@ -144,14 +152,44 @@ ruleTester.run(RULE_NAME, rule, { code: `expect(getByText('foo')).toBeDefined()`, options: [ { - assertion: 'toBeInDocument', + assertion: 'toBeInTheDocument', + }, + ], + errors: [ + { + messageId: 'preferExplicitAssertAssertion', + column: 26, + data: { assertion: 'toBeInTheDocument' }, + }, + ], + }, + { + code: `expect(getByText('foo')).not.toBeNull()`, + options: [ + { + assertion: 'toBeInTheDocument', + }, + ], + errors: [ + { + messageId: 'preferExplicitAssertAssertion', + column: 26, + data: { assertion: 'toBeInTheDocument' }, + }, + ], + }, + { + code: `expect(getByText('foo')).not.toBeFalsy()`, + options: [ + { + assertion: 'toBeInTheDocument', }, ], errors: [ { messageId: 'preferExplicitAssertAssertion', column: 26, - data: { assertion: 'toBeInDocument' }, + data: { assertion: 'toBeInTheDocument' }, }, ], },