diff --git a/README.md b/README.md index de21469a..19ac8bcd 100644 --- a/README.md +++ b/README.md @@ -228,6 +228,7 @@ module.exports = { | [prefer-find-by](docs/rules/prefer-find-by.md) | Suggest using `find(All)By*` query instead of `waitFor` + `get(All)By*` to wait for elements | ![badge-angular][] ![badge-dom][] ![badge-marko][] ![badge-react][] ![badge-vue][] | 🔧 | | [prefer-presence-queries](docs/rules/prefer-presence-queries.md) | Ensure appropriate `get*`/`query*` queries are used with their respective matchers | ![badge-angular][] ![badge-dom][] ![badge-marko][] ![badge-react][] ![badge-vue][] | | | [prefer-query-by-disappearance](docs/rules/prefer-query-by-disappearance.md) | Suggest using `queryBy*` queries when waiting for disappearance | ![badge-angular][] ![badge-dom][] ![badge-marko][] ![badge-react][] ![badge-vue][] | | +| [prefer-query-matchers](docs/rules/prefer-query-matchers.md) | Ensure the configured `get*`/`query*` query is used with the corresponding matchers | | | | [prefer-screen-queries](docs/rules/prefer-screen-queries.md) | Suggest using `screen` while querying | ![badge-angular][] ![badge-dom][] ![badge-marko][] ![badge-react][] ![badge-vue][] | | | [prefer-user-event](docs/rules/prefer-user-event.md) | Suggest using `userEvent` over `fireEvent` for simulating user interactions | | | | [prefer-wait-for](docs/rules/prefer-wait-for.md) | Use `waitFor` instead of deprecated wait methods | | 🔧 | diff --git a/docs/rules/prefer-presence-queries.md b/docs/rules/prefer-presence-queries.md index af1df93c..6b773089 100644 --- a/docs/rules/prefer-presence-queries.md +++ b/docs/rules/prefer-presence-queries.md @@ -43,6 +43,7 @@ Examples of **correct** code for this rule: ```js test('some test', async () => { render(); + // check element is present with `getBy*` expect(screen.getByText('button')).toBeInTheDocument(); expect(screen.getAllByText('button')[9]).toBeTruthy(); diff --git a/docs/rules/prefer-query-matchers.md b/docs/rules/prefer-query-matchers.md new file mode 100644 index 00000000..eec3fa2f --- /dev/null +++ b/docs/rules/prefer-query-matchers.md @@ -0,0 +1,85 @@ +# Ensure the configured `get*`/`query*` query is used with the corresponding matchers (`testing-library/prefer-query-matchers`) + + + +The (DOM) Testing Library allows to query DOM elements using different types of queries such as `get*` and `query*`. Using `get*` throws an error in case the element is not found, while `query*` returns null instead of throwing (or empty array for `queryAllBy*` ones). + +It may be helpful to ensure that either `get*` or `query*` are always used for a given matcher. For example, `.toBeVisible()` and the negation `.not.toBeVisible()` both assume that an element exists in the DOM and will error if not. Using `get*` with `.toBeVisible()` ensures that if the element is not found the error thrown will offer better info than with `query*`. + +## Rule details + +This rule must be configured with a list of `validEntries`: for a given matcher, is `get*` or `query*` required. + +Assuming the following configuration: + +```json +{ + "testing-library/prefer-query-matchers": [ + 2, + { + "validEntries": [{ "matcher": "toBeVisible", "query": "get" }] + } + ] +} +``` + +Examples of **incorrect** code for this rule with the above configuration: + +```js +test('some test', () => { + render(); + + // use configured matcher with the disallowed `query*` + expect(screen.queryByText('button')).toBeVisible(); + expect(screen.queryByText('button')).not.toBeVisible(); + expect(screen.queryAllByText('button')[0]).toBeVisible(); + expect(screen.queryAllByText('button')[0]).not.toBeVisible(); +}); +``` + +Examples of **correct** code for this rule: + +```js +test('some test', async () => { + render(); + // use configured matcher with the allowed `get*` + expect(screen.getByText('button')).toBeVisible(); + expect(screen.getByText('button')).not.toBeVisible(); + expect(screen.getAllByText('button')[0]).toBeVisible(); + expect(screen.getAllByText('button')[0]).not.toBeVisible(); + + // use an unconfigured matcher with either `get* or `query* + expect(screen.getByText('button')).toBeEnabled(); + expect(screen.getAllByText('checkbox')[0]).not.toBeChecked(); + expect(screen.queryByText('button')).toHaveFocus(); + expect(screen.queryAllByText('button')[0]).not.toMatchMyCustomMatcher(); + + // `findBy*` queries are out of the scope for this rule + const button = await screen.findByText('submit'); + expect(button).toBeVisible(); +}); +``` + +## Options + +| Option | Required | Default | Details | +| -------------- | -------- | ------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| `validEntries` | No | `[]` | A list of objects with a `matcher` property (the name of any matcher, such as "toBeVisible") and a `query` property (either "get" or "query"). Indicates whether `get*` or `query*` are allowed with this matcher. | + +## Example + +```json +{ + "testing-library/prefer-query-matchers": [ + 2, + { + "validEntries": [{ "matcher": "toBeVisible", "query": "get" }] + } + ] +} +``` + +## Further Reading + +- [Testing Library queries cheatsheet](https://testing-library.com/docs/dom-testing-library/cheatsheet#queries) +- [jest-dom note about using `getBy` within assertions](https://testing-library.com/docs/ecosystem-jest-dom) diff --git a/lib/create-testing-library-rule/detect-testing-library-utils.ts b/lib/create-testing-library-rule/detect-testing-library-utils.ts index cf65a0e4..9393a88b 100644 --- a/lib/create-testing-library-rule/detect-testing-library-utils.ts +++ b/lib/create-testing-library-rule/detect-testing-library-utils.ts @@ -85,6 +85,10 @@ type IsDebugUtilFn = ( validNames?: ReadonlyArray<(typeof DEBUG_UTILS)[number]> ) => boolean; type IsPresenceAssertFn = (node: TSESTree.MemberExpression) => boolean; +type IsMatchingAssertFn = ( + node: TSESTree.MemberExpression, + matcherName: string +) => boolean; type IsAbsenceAssertFn = (node: TSESTree.MemberExpression) => boolean; type CanReportErrorsFn = () => boolean; type FindImportedTestingLibraryUtilSpecifierFn = ( @@ -122,6 +126,7 @@ export interface DetectionHelpers { isActUtil: (node: TSESTree.Identifier) => boolean; isPresenceAssert: IsPresenceAssertFn; isAbsenceAssert: IsAbsenceAssertFn; + isMatchingAssert: IsMatchingAssertFn; canReportErrors: CanReportErrorsFn; findImportedTestingLibraryUtilSpecifier: FindImportedTestingLibraryUtilSpecifierFn; isNodeComingFromTestingLibrary: IsNodeComingFromTestingLibraryFn; @@ -819,6 +824,16 @@ export function detectTestingLibraryUtils< : ABSENCE_MATCHERS.includes(matcher); }; + const isMatchingAssert: IsMatchingAssertFn = (node, matcherName) => { + const { matcher } = getAssertNodeInfo(node); + + if (!matcher) { + return false; + } + + return matcher === matcherName; + }; + /** * Finds the import util specifier related to Testing Library for a given name. */ @@ -977,6 +992,7 @@ export function detectTestingLibraryUtils< isDebugUtil, isActUtil, isPresenceAssert, + isMatchingAssert, isAbsenceAssert, canReportErrors, findImportedTestingLibraryUtilSpecifier, diff --git a/lib/rules/prefer-query-matchers.ts b/lib/rules/prefer-query-matchers.ts new file mode 100644 index 00000000..3f94a8e9 --- /dev/null +++ b/lib/rules/prefer-query-matchers.ts @@ -0,0 +1,105 @@ +import { TSESTree } from '@typescript-eslint/utils'; + +import { createTestingLibraryRule } from '../create-testing-library-rule'; +import { findClosestCallNode, isMemberExpression } from '../node-utils'; + +export const RULE_NAME = 'prefer-query-matchers'; +export type MessageIds = 'wrongQueryForMatcher'; +export type Options = [ + { + validEntries: { + query: 'get' | 'query'; + matcher: string; + }[]; + } +]; + +export default createTestingLibraryRule({ + name: RULE_NAME, + meta: { + docs: { + description: + 'Ensure the configured `get*`/`query*` query is used with the corresponding matchers', + recommendedConfig: { + dom: false, + angular: false, + react: false, + vue: false, + marko: false, + }, + }, + messages: { + wrongQueryForMatcher: 'Use `{{ query }}By*` queries for {{ matcher }}', + }, + schema: [ + { + type: 'object', + additionalProperties: false, + properties: { + validEntries: { + type: 'array', + items: { + type: 'object', + properties: { + query: { + type: 'string', + enum: ['get', 'query'], + }, + matcher: { + type: 'string', + }, + }, + }, + }, + }, + }, + ], + type: 'suggestion', + }, + defaultOptions: [ + { + validEntries: [], + }, + ], + + create(context, [{ validEntries }], helpers) { + return { + 'CallExpression Identifier'(node: TSESTree.Identifier) { + const expectCallNode = findClosestCallNode(node, 'expect'); + + if (!expectCallNode || !isMemberExpression(expectCallNode.parent)) { + return; + } + + // Sync queries (getBy and queryBy) and corresponding ones + // are supported. If none found, stop the rule. + if (!helpers.isSyncQuery(node)) { + return; + } + + const isGetBy = helpers.isGetQueryVariant(node); + const expectStatement = expectCallNode.parent; + for (const entry of validEntries) { + const { query, matcher } = entry; + const isMatchingAssertForThisEntry = helpers.isMatchingAssert( + expectStatement, + matcher + ); + + if (!isMatchingAssertForThisEntry) { + continue; + } + + const actualQuery = isGetBy ? 'get' : 'query'; + if (query !== actualQuery) { + context.report({ + node, + messageId: 'wrongQueryForMatcher', + data: { query, matcher }, + }); + } + } + }, + }; + }, +}); diff --git a/tests/index.test.ts b/tests/index.test.ts index 6788a479..b3b535b2 100644 --- a/tests/index.test.ts +++ b/tests/index.test.ts @@ -3,7 +3,7 @@ import { resolve } from 'path'; import plugin from '../lib'; -const numberOfRules = 27; +const numberOfRules = 28; const ruleNames = Object.keys(plugin.rules); // eslint-disable-next-line jest/expect-expect diff --git a/tests/lib/rules/prefer-query-matchers.test.ts b/tests/lib/rules/prefer-query-matchers.test.ts new file mode 100644 index 00000000..e16cf983 --- /dev/null +++ b/tests/lib/rules/prefer-query-matchers.test.ts @@ -0,0 +1,400 @@ +import { TSESLint } from '@typescript-eslint/utils'; + +import rule, { + RULE_NAME, + MessageIds, + Options, +} from '../../../lib/rules/prefer-query-matchers'; +import { ALL_QUERIES_METHODS } from '../../../lib/utils'; +import { createRuleTester } from '../test-utils'; + +const ruleTester = createRuleTester(); + +const getByQueries = ALL_QUERIES_METHODS.map((method) => `get${method}`); +const getAllByQueries = ALL_QUERIES_METHODS.map((method) => `getAll${method}`); +const queryByQueries = ALL_QUERIES_METHODS.map((method) => `query${method}`); +const queryAllByQueries = ALL_QUERIES_METHODS.map( + (method) => `queryAll${method}` +); + +type RuleValidTestCase = TSESLint.ValidTestCase; +type RuleInvalidTestCase = TSESLint.InvalidTestCase; + +type AssertionFnParams = { + query: string; + matcher: string; + options: Options; +}; + +const wrapExpectInTest = (expectStatement: string) => ` +import { render, screen } from '@testing-library/react' + +test('a fake test', () => { + render() + + ${expectStatement} +})`; + +const getValidAssertions = ({ + query, + matcher, + options, +}: AssertionFnParams): RuleValidTestCase[] => { + const expectStatement = `expect(${query}('Hello'))${matcher}`; + const expectScreenStatement = `expect(screen.${query}('Hello'))${matcher}`; + return [ + { + // name: `${expectStatement} with default options of empty validEntries`, + code: wrapExpectInTest(expectStatement), + }, + { + // name: `${expectStatement} with provided options`, + code: wrapExpectInTest(expectStatement), + options, + }, + { + // name: `${expectScreenStatement} with default options of empty validEntries`, + code: wrapExpectInTest(expectScreenStatement), + }, + { + // name: `${expectScreenStatement} with provided options`, + code: wrapExpectInTest(expectScreenStatement), + options, + }, + ]; +}; + +const getInvalidAssertions = ({ + query, + matcher, + options, +}: AssertionFnParams): RuleInvalidTestCase[] => { + const expectStatement = `expect(${query}('Hello'))${matcher}`; + const expectScreenStatement = `expect(screen.${query}('Hello'))${matcher}`; + const messageId: MessageIds = 'wrongQueryForMatcher'; + const [ + { + validEntries: [validEntry], + }, + ] = options; + return [ + { + // name: `${expectStatement} with provided options`, + code: wrapExpectInTest(expectStatement), + options, + errors: [ + { + messageId, + line: 7, + column: 10, + data: { query: validEntry.query, matcher: validEntry.matcher }, + }, + ], + }, + { + // name: `${expectScreenStatement} with provided options`, + code: wrapExpectInTest(expectScreenStatement), + options, + errors: [ + { + messageId, + line: 7, + column: 17, + data: { query: validEntry.query, matcher: validEntry.matcher }, + }, + ], + }, + ]; +}; + +ruleTester.run(RULE_NAME, rule, { + valid: [ + // cases: methods not matching Testing Library queries pattern + `expect(queryElement('foo')).toBeVisible()`, + `expect(getElement('foo')).not.toBeVisible()`, + // cases: asserting with a configured allowed `[screen.]getBy*` query + ...getByQueries.reduce( + (validRules, queryName) => [ + ...validRules, + ...getValidAssertions({ + query: queryName, + matcher: '.toBeVisible()', + options: [ + { validEntries: [{ query: 'get', matcher: 'toBeVisible' }] }, + ], + }), + ...getValidAssertions({ + query: queryName, + matcher: '.toBeHelloWorld()', + options: [ + { validEntries: [{ query: 'get', matcher: 'toBeVisible' }] }, + ], + }), + ...getValidAssertions({ + query: queryName, + matcher: '.not.toBeVisible()', + options: [ + { validEntries: [{ query: 'get', matcher: 'toBeVisible' }] }, + ], + }), + ...getValidAssertions({ + query: queryName, + matcher: '.not.toBeHelloWorld()', + options: [ + { validEntries: [{ query: 'get', matcher: 'toBeVisible' }] }, + ], + }), + ...getValidAssertions({ + query: queryName, + matcher: '.toBeHelloWorld()', + options: [ + { validEntries: [{ query: 'query', matcher: 'toBeVisible' }] }, + ], + }), + ], + [] + ), + // cases: asserting with a configured allowed `[screen.]getAllBy*` query + ...getAllByQueries.reduce( + (validRules, queryName) => [ + ...validRules, + ...getValidAssertions({ + query: queryName, + matcher: '.toBeVisible()', + options: [ + { validEntries: [{ query: 'get', matcher: 'toBeVisible' }] }, + ], + }), + ...getValidAssertions({ + query: queryName, + matcher: '.toBeHelloWorld()', + options: [ + { validEntries: [{ query: 'get', matcher: 'toBeVisible' }] }, + ], + }), + ...getValidAssertions({ + query: queryName, + matcher: '.not.toBeVisible()', + options: [ + { validEntries: [{ query: 'get', matcher: 'toBeVisible' }] }, + ], + }), + ...getValidAssertions({ + query: queryName, + matcher: '.not.toBeHelloWorld()', + options: [ + { validEntries: [{ query: 'get', matcher: 'toBeVisible' }] }, + ], + }), + ...getValidAssertions({ + query: queryName, + matcher: '.toBeHelloWorld()', + options: [ + { validEntries: [{ query: 'query', matcher: 'toBeVisible' }] }, + ], + }), + ], + [] + ), + // cases: asserting with a configured allowed `[screen.]queryBy*` query + ...queryByQueries.reduce( + (validRules, queryName) => [ + ...validRules, + ...getValidAssertions({ + query: queryName, + matcher: '.toBeVisible()', + options: [ + { validEntries: [{ query: 'query', matcher: 'toBeVisible' }] }, + ], + }), + ...getValidAssertions({ + query: queryName, + matcher: '.toBeHelloWorld()', + options: [ + { validEntries: [{ query: 'query', matcher: 'toBeVisible' }] }, + ], + }), + ...getValidAssertions({ + query: queryName, + matcher: '.not.toBeVisible()', + options: [ + { validEntries: [{ query: 'query', matcher: 'toBeVisible' }] }, + ], + }), + ...getValidAssertions({ + query: queryName, + matcher: '.not.toBeHelloWorld()', + options: [ + { validEntries: [{ query: 'query', matcher: 'toBeVisible' }] }, + ], + }), + ...getValidAssertions({ + query: queryName, + matcher: '.toBeHelloWorld()', + options: [ + { validEntries: [{ query: 'get', matcher: 'toBeVisible' }] }, + ], + }), + ], + [] + ), + // cases: asserting with a configured allowed `[screen.]queryAllBy*` query + ...queryAllByQueries.reduce( + (validRules, queryName) => [ + ...validRules, + ...getValidAssertions({ + query: queryName, + matcher: '.toBeVisible()', + options: [ + { validEntries: [{ query: 'query', matcher: 'toBeVisible' }] }, + ], + }), + ...getValidAssertions({ + query: queryName, + matcher: '.toBeHelloWorld()', + options: [ + { validEntries: [{ query: 'query', matcher: 'toBeVisible' }] }, + ], + }), + ...getValidAssertions({ + query: queryName, + matcher: '.not.toBeVisible()', + options: [ + { validEntries: [{ query: 'query', matcher: 'toBeVisible' }] }, + ], + }), + ...getValidAssertions({ + query: queryName, + matcher: '.not.toBeHelloWorld()', + options: [ + { validEntries: [{ query: 'query', matcher: 'toBeVisible' }] }, + ], + }), + ...getValidAssertions({ + query: queryName, + matcher: '.toBeHelloWorld()', + options: [ + { validEntries: [{ query: 'get', matcher: 'toBeVisible' }] }, + ], + }), + ], + [] + ), + // case: getting outside an expectation + { + code: 'const el = getByText("button")', + }, + // case: querying outside an expectation + { + code: 'const el = queryByText("button")', + }, + // case: finding outside an expectation + { + code: `async () => { + const el = await findByText('button') + expect(el).toBeVisible() + }`, + }, + { + code: `// case: query an element with getBy but then check its absence after doing + // some action which makes it disappear. + + // submit button exists + const submitButton = screen.getByRole('button') + fireEvent.click(submitButton) + + // right after clicking submit button it disappears + expect(submitButton).toBeHelloWorld() + `, + options: [ + { validEntries: [{ query: 'query', matcher: 'toBeHelloWorld' }] }, + ], + }, + ], + invalid: [ + // cases: asserting with a disallowed `[screen.]getBy*` query + ...getByQueries.reduce( + (validRules, queryName) => [ + ...validRules, + ...getInvalidAssertions({ + query: queryName, + matcher: '.toBeHelloWorld()', + options: [ + { validEntries: [{ query: 'query', matcher: 'toBeHelloWorld' }] }, + ], + }), + ], + [] + ), + // cases: asserting with a disallowed `[screen.]getAllBy*` query + ...getAllByQueries.reduce( + (validRules, queryName) => [ + ...validRules, + ...getInvalidAssertions({ + query: queryName, + matcher: '.toBeHelloWorld()', + options: [ + { validEntries: [{ query: 'query', matcher: 'toBeHelloWorld' }] }, + ], + }), + ], + [] + ), + // cases: asserting with a disallowed `[screen.]getBy*` query + ...queryByQueries.reduce( + (invalidRules, queryName) => [ + ...invalidRules, + ...getInvalidAssertions({ + query: queryName, + matcher: '.toBeVisible()', + options: [ + { validEntries: [{ query: 'get', matcher: 'toBeVisible' }] }, + ], + }), + ], + [] + ), + // cases: asserting with a disallowed `[screen.]queryAllBy*` query + ...queryAllByQueries.reduce( + (invalidRules, queryName) => [ + ...invalidRules, + ...getInvalidAssertions({ + query: queryName, + matcher: '.toBeVisible()', + options: [ + { validEntries: [{ query: 'get', matcher: 'toBeVisible' }] }, + ], + }), + ], + [] + ), + // cases: indexing into an `AllBy` result within the expectation + { + code: 'expect(screen.queryAllByText("button")[1]).toBeVisible()', + options: [{ validEntries: [{ query: 'get', matcher: 'toBeVisible' }] }], + errors: [ + { + messageId: 'wrongQueryForMatcher', + line: 1, + column: 15, + data: { query: 'get', matcher: 'toBeVisible' }, + }, + ], + }, + { + code: ` + // case: asserting presence incorrectly with custom queryBy* query + expect(queryByCustomQuery("button")).toBeVisible() + `, + options: [{ validEntries: [{ query: 'get', matcher: 'toBeVisible' }] }], + errors: [ + { + messageId: 'wrongQueryForMatcher', + line: 3, + column: 12, + data: { query: 'get', matcher: 'toBeVisible' }, + }, + ], + }, + ], +});