diff --git a/README.md b/README.md index ba4547cc..aca4bef3 100644 --- a/README.md +++ b/README.md @@ -228,6 +228,7 @@ module.exports = { | [no-wait-for-snapshot](docs/rules/no-wait-for-snapshot.md) | Ensures no snapshot is generated inside of a `waitFor` call | ![badge-angular][] ![badge-dom][] ![badge-marko][] ![badge-react][] ![badge-vue][] | | | | [prefer-explicit-assert](docs/rules/prefer-explicit-assert.md) | Suggest using explicit assertions rather than standalone queries | | | | | [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-implicit-assert](docs/rules/prefer-implicit-assert.md) | Suggest using implicit assertions for getBy* & findBy* queries | | | | | [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 | | | | diff --git a/docs/rules/prefer-explicit-assert.md b/docs/rules/prefer-explicit-assert.md index a5c6fc93..df021751 100644 --- a/docs/rules/prefer-explicit-assert.md +++ b/docs/rules/prefer-explicit-assert.md @@ -72,7 +72,10 @@ This is how you can use these options in eslint configuration: ## When Not To Use It -If you prefer to use `getBy*` queries implicitly as an assert-like method itself, then this rule is not recommended. +If you prefer to use `getBy*` queries implicitly as an assert-like method itself, then this rule is not recommended. Instead check out this rule [prefer-implicit-assert](https://github.com/testing-library/eslint-plugin-testing-library/blob/main/docs/rules/prefer-implicit-assert.md) + +- Never use both `prefer-explicit-assert` & `prefer-implicit-assert` choose one. +- This library recommends `prefer-explicit-assert` to make it more clear to readers that it is not just a query without an assertion, but that it is checking for existence of an element ## Further Reading diff --git a/docs/rules/prefer-implicit-assert.md b/docs/rules/prefer-implicit-assert.md new file mode 100644 index 00000000..8de4b2a0 --- /dev/null +++ b/docs/rules/prefer-implicit-assert.md @@ -0,0 +1,57 @@ +# Suggest using implicit assertions for getBy* & findBy* queries (`testing-library/prefer-implicit-assert`) + + + +Testing Library `getBy*` & `findBy*` queries throw an error if the element is not +found. Therefore it is not necessary to also assert existance with things like `expect(getBy*.toBeInTheDocument()` or `expect(awaint findBy*).not.toBeNull()` + +## Rule Details + +This rule aims to reuduce uncecessary assertion's for presense of an element, +when using queries that implicitly fail when said element is not found. + +Examples of **incorrect** code for this rule with the default configuration: + +```js +// wrapping the getBy or findBy queries within a `expect` and using existence matchers for +// making the assertion is not necessary +expect(getByText('foo')).toBeInTheDocument(); +expect(await findByText('foo')).toBeInTheDocument(); + +expect(getByText('foo')).toBeDefined(); +expect(await findByText('foo')).toBeDefined(); + +const utils = render(); +expect(utils.getByText('foo')).toBeInTheDocument(); +expect(await utils.findByText('foo')).toBeInTheDocument(); + +expect(await findByText('foo')).not.toBeNull(); +expect(await findByText('foo')).not.toBeUndified(); +``` + +Examples of **correct** code for this rule with the default configuration: + +```js +getByText('foo'); +await findByText('foo'); + +const utils = render(); +utils.getByText('foo'); +await utils.findByText('foo'); + +// When using queryBy* queries thees do not implicitly fial therefore you should explicitly check if your elements eixst or not +expect(queryByText('foo')).toBeInTheDocument(); +expect(queryByText('foo')).not.toBeInTheDocument(); +``` + +## When Not To Use It + +If you prefer to use `getBy*` & `findBy*` queries with explicitly asserting existence of elements, then this rule is not recommended. Instead check out this rule [prefer-explicit-assert](https://github.com/testing-library/eslint-plugin-testing-library/blob/main/docs/rules/prefer-explicit-assert.md) + +- Never use both `prefer-implicit-assert` & `prefer-explicit-assert` choose one. +- This library recommends `prefer-explicit-assert` to make it more clear to readers that it is not just a query without an assertion, but that it is checking for existence of an element + +## Further Reading + +- [getBy query](https://testing-library.com/docs/dom-testing-library/api-queries#getby) +- [findBy query](https://testing-library.com/docs/dom-testing-library/api-queries#findBy) diff --git a/lib/rules/prefer-implicit-assert.ts b/lib/rules/prefer-implicit-assert.ts new file mode 100644 index 00000000..43f7682d --- /dev/null +++ b/lib/rules/prefer-implicit-assert.ts @@ -0,0 +1,136 @@ +import { + TSESTree, + ASTUtils, + AST_NODE_TYPES, + TSESLint, +} from '@typescript-eslint/utils'; + +import { createTestingLibraryRule } from '../create-testing-library-rule'; +import { TestingLibrarySettings } from '../create-testing-library-rule/detect-testing-library-utils'; +import { isCallExpression, isMemberExpression } from '../node-utils'; + +export const RULE_NAME = 'prefer-implicit-assert'; +export type MessageIds = 'preferImplicitAssert'; +type Options = []; + +const isCalledUsingSomeObject = (node: TSESTree.Identifier) => + isMemberExpression(node.parent) && + node.parent.object.type === AST_NODE_TYPES.Identifier; + +const isCalledInExpect = ( + node: TSESTree.Identifier | TSESTree.Node, + isAsyncQuery: boolean +) => { + if (isAsyncQuery) { + return ( + isCallExpression(node.parent) && + ASTUtils.isAwaitExpression(node.parent.parent) && + isCallExpression(node.parent.parent.parent) && + ASTUtils.isIdentifier(node.parent.parent.parent.callee) && + node.parent.parent.parent.callee.name === 'expect' + ); + } + return ( + isCallExpression(node.parent) && + isCallExpression(node.parent.parent) && + ASTUtils.isIdentifier(node.parent.parent.callee) && + node.parent.parent.callee.name === 'expect' + ); +}; + +const reportError = ( + context: Readonly< + TSESLint.RuleContext<'preferImplicitAssert', []> & { + settings: TestingLibrarySettings; + } + >, + node: TSESTree.Identifier | TSESTree.Node | undefined, + queryType: string +) => { + if (node) { + return context.report({ + node, + messageId: 'preferImplicitAssert', + data: { + queryType, + }, + }); + } +}; + +export default createTestingLibraryRule({ + name: RULE_NAME, + meta: { + type: 'suggestion', + docs: { + description: + 'Suggest using implicit assertions for getBy* & findBy* queries', + recommendedConfig: { + dom: false, + angular: false, + react: false, + vue: false, + marko: false, + }, + }, + messages: { + preferImplicitAssert: + "Don't wrap `{{queryType}}` query with `expect` & presence matchers like `toBeInTheDocument` or `not.toBeNull` as `{{queryType}}` queries fail implicitly when element is not found", + }, + schema: [], + }, + defaultOptions: [], + create(context, _, helpers) { + const findQueryCalls: TSESTree.Identifier[] = []; + const getQueryCalls: TSESTree.Identifier[] = []; + + return { + 'CallExpression Identifier'(node: TSESTree.Identifier) { + if (helpers.isFindQueryVariant(node)) { + findQueryCalls.push(node); + } + if (helpers.isGetQueryVariant(node)) { + getQueryCalls.push(node); + } + }, + 'Program:exit'() { + findQueryCalls.forEach((queryCall) => { + const isAsyncQuery = true; + const node: TSESTree.Identifier | TSESTree.Node | undefined = + isCalledUsingSomeObject(queryCall) ? queryCall.parent : queryCall; + + if (node) { + if (isCalledInExpect(node, isAsyncQuery)) { + if ( + isMemberExpression(node.parent?.parent?.parent?.parent) && + node.parent?.parent?.parent?.parent.property.type === + AST_NODE_TYPES.Identifier && + helpers.isPresenceAssert(node.parent.parent.parent.parent) + ) { + return reportError(context, node, 'findBy*'); + } + } + } + }); + + getQueryCalls.forEach((queryCall) => { + const isAsyncQuery = false; + const node: TSESTree.Identifier | TSESTree.Node | undefined = + isCalledUsingSomeObject(queryCall) ? queryCall.parent : queryCall; + if (node) { + if (isCalledInExpect(node, isAsyncQuery)) { + if ( + isMemberExpression(node.parent?.parent?.parent) && + node.parent?.parent?.parent.property.type === + AST_NODE_TYPES.Identifier && + helpers.isPresenceAssert(node.parent.parent.parent) + ) { + return reportError(context, node, 'getBy*'); + } + } + } + }); + }, + }; + }, +}); diff --git a/tests/index.test.ts b/tests/index.test.ts index c69e0c6a..6788a479 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 = 26; +const numberOfRules = 27; const ruleNames = Object.keys(plugin.rules); // eslint-disable-next-line jest/expect-expect diff --git a/tests/lib/rules/prefer-implicit-assert.test.ts b/tests/lib/rules/prefer-implicit-assert.test.ts new file mode 100644 index 00000000..973c45be --- /dev/null +++ b/tests/lib/rules/prefer-implicit-assert.test.ts @@ -0,0 +1,552 @@ +import rule, { RULE_NAME } from '../../../lib/rules/prefer-implicit-assert'; +import { ALL_QUERIES_METHODS } from '../../../lib/utils'; +import { createRuleTester } from '../test-utils'; + +const ruleTester = createRuleTester(); + +const COMBINED_QUERIES_METHODS = [...ALL_QUERIES_METHODS, 'ByIcon']; + +ruleTester.run(RULE_NAME, rule, { + valid: [ + ...COMBINED_QUERIES_METHODS.map((queryMethod) => ({ + code: `await find${queryMethod}('qux');`, + })), + ...COMBINED_QUERIES_METHODS.map((queryMethod) => ({ + code: `screen.find${queryMethod}('foo')`, + })), + ...COMBINED_QUERIES_METHODS.map((queryMethod) => ({ + code: `await screen.find${queryMethod}('foo')`, + })), + ...COMBINED_QUERIES_METHODS.map((queryMethod) => ({ + code: `const utils = render(); + await utils.find${queryMethod}('foo')`, + })), + ...COMBINED_QUERIES_METHODS.map((queryMethod) => ({ + code: `get${queryMethod}('qux');`, + })), + ...COMBINED_QUERIES_METHODS.map((queryMethod) => ({ + code: `screen.get${queryMethod}('foo')`, + })), + ...COMBINED_QUERIES_METHODS.map((queryMethod) => ({ + code: `const utils = render(); + utils.get${queryMethod}('foo')`, + })), + ...COMBINED_QUERIES_METHODS.map((queryMethod) => ({ + code: `expect(query${queryMethod}('qux')).toBeInTheDocument();`, + })), + ...COMBINED_QUERIES_METHODS.map((queryMethod) => ({ + code: `expect(query${queryMethod}('qux')).not.toBeInTheDocument();`, + })), + ...COMBINED_QUERIES_METHODS.map((queryMethod) => ({ + code: `const something = await find${queryMethod}('qux');`, + })), + ...COMBINED_QUERIES_METHODS.map((queryMethod) => ({ + code: `const something = get${queryMethod}('qux');`, + })), + ...COMBINED_QUERIES_METHODS.map((queryMethod) => ({ + code: `const something = query${queryMethod}('qux');`, + })), + ], + invalid: [ + ...COMBINED_QUERIES_METHODS.map( + (queryMethod) => + ({ + code: `expect(await find${queryMethod}('qux')).toBeInTheDocument();`, + errors: [ + { + line: 1, + column: 14, + messageId: 'preferImplicitAssert', + data: { + queryType: 'findBy*', + }, + }, + ], + } as const) + ), + ...COMBINED_QUERIES_METHODS.map( + (queryMethod) => + ({ + code: `expect(await find${queryMethod}('qux')).toBeTruthy();`, + errors: [ + { + line: 1, + column: 14, + messageId: 'preferImplicitAssert', + data: { + queryType: 'findBy*', + }, + }, + ], + } as const) + ), + ...COMBINED_QUERIES_METHODS.map( + (queryMethod) => + ({ + code: `expect(await find${queryMethod}('qux')).toBeDefined();`, + errors: [ + { + line: 1, + column: 14, + messageId: 'preferImplicitAssert', + data: { + queryType: 'findBy*', + }, + }, + ], + } as const) + ), + ...COMBINED_QUERIES_METHODS.map( + (queryMethod) => + ({ + code: `expect(await find${queryMethod}('qux')).not.toBeNull();`, + errors: [ + { + line: 1, + column: 14, + messageId: 'preferImplicitAssert', + data: { + queryType: 'findBy*', + }, + }, + ], + } as const) + ), + ...COMBINED_QUERIES_METHODS.map( + (queryMethod) => + ({ + code: `expect(await find${queryMethod}('qux')).not.toBeFalsy();`, + errors: [ + { + line: 1, + column: 14, + messageId: 'preferImplicitAssert', + data: { + queryType: 'findBy*', + }, + }, + ], + } as const) + ), + ...COMBINED_QUERIES_METHODS.map( + (queryMethod) => + ({ + code: `expect(await screen.find${queryMethod}('qux')).toBeInTheDocument();`, + errors: [ + { + line: 1, + column: 14, + messageId: 'preferImplicitAssert', + data: { + queryType: 'findBy*', + }, + }, + ], + } as const) + ), + ...COMBINED_QUERIES_METHODS.map( + (queryMethod) => + ({ + code: `expect(await screen.find${queryMethod}('qux')).toBeTruthy();`, + errors: [ + { + line: 1, + column: 14, + messageId: 'preferImplicitAssert', + data: { + queryType: 'findBy*', + }, + }, + ], + } as const) + ), + ...COMBINED_QUERIES_METHODS.map( + (queryMethod) => + ({ + code: `expect(await screen.find${queryMethod}('qux')).toBeDefined();`, + errors: [ + { + line: 1, + column: 14, + messageId: 'preferImplicitAssert', + data: { + queryType: 'findBy*', + }, + }, + ], + } as const) + ), + ...COMBINED_QUERIES_METHODS.map( + (queryMethod) => + ({ + code: `expect(await screen.find${queryMethod}('qux')).not.toBeNull();`, + errors: [ + { + line: 1, + column: 14, + messageId: 'preferImplicitAssert', + data: { + queryType: 'findBy*', + }, + }, + ], + } as const) + ), + ...COMBINED_QUERIES_METHODS.map( + (queryMethod) => + ({ + code: `expect(await screen.find${queryMethod}('qux')).not.toBeFalsy();`, + errors: [ + { + line: 1, + column: 14, + messageId: 'preferImplicitAssert', + data: { + queryType: 'findBy*', + }, + }, + ], + } as const) + ), + ...COMBINED_QUERIES_METHODS.map( + (queryMethod) => + ({ + code: ` + const utils = render(); + expect(await utils.find${queryMethod}('foo')).toBeInTheDocument();`, + errors: [ + { + line: 3, + column: 20, + messageId: 'preferImplicitAssert', + data: { + queryType: 'findBy*', + }, + }, + ], + } as const) + ), + ...COMBINED_QUERIES_METHODS.map( + (queryMethod) => + ({ + code: ` + const utils = render(); + expect(await utils.find${queryMethod}('foo')).toBeTruthy();`, + errors: [ + { + line: 3, + column: 20, + messageId: 'preferImplicitAssert', + data: { + queryType: 'findBy*', + }, + }, + ], + } as const) + ), + ...COMBINED_QUERIES_METHODS.map( + (queryMethod) => + ({ + code: ` + const utils = render(); + expect(await utils.find${queryMethod}('foo')).toBeDefined();`, + errors: [ + { + line: 3, + column: 20, + messageId: 'preferImplicitAssert', + data: { + queryType: 'findBy*', + }, + }, + ], + } as const) + ), + ...COMBINED_QUERIES_METHODS.map( + (queryMethod) => + ({ + code: ` + const utils = render(); + expect(await utils.find${queryMethod}('foo')).not.toBeFalsy();`, + errors: [ + { + line: 3, + column: 20, + messageId: 'preferImplicitAssert', + data: { + queryType: 'findBy*', + }, + }, + ], + } as const) + ), + ...COMBINED_QUERIES_METHODS.map( + (queryMethod) => + ({ + code: ` + const utils = render(); + expect(await utils.find${queryMethod}('foo')).not.toBeNull();`, + errors: [ + { + line: 3, + column: 20, + messageId: 'preferImplicitAssert', + data: { + queryType: 'findBy*', + }, + }, + ], + } as const) + ), + ...COMBINED_QUERIES_METHODS.map( + (queryMethod) => + ({ + code: `expect(get${queryMethod}('qux')).toBeInTheDocument();`, + errors: [ + { + line: 1, + column: 8, + messageId: 'preferImplicitAssert', + data: { + queryType: 'getBy*', + }, + }, + ], + } as const) + ), + ...COMBINED_QUERIES_METHODS.map( + (queryMethod) => + ({ + code: `expect(get${queryMethod}('qux')).toBeTruthy();`, + errors: [ + { + line: 1, + column: 8, + messageId: 'preferImplicitAssert', + data: { + queryType: 'getBy*', + }, + }, + ], + } as const) + ), + ...COMBINED_QUERIES_METHODS.map( + (queryMethod) => + ({ + code: `expect(get${queryMethod}('qux')).toBeDefined();`, + errors: [ + { + line: 1, + column: 8, + messageId: 'preferImplicitAssert', + data: { + queryType: 'getBy*', + }, + }, + ], + } as const) + ), + ...COMBINED_QUERIES_METHODS.map( + (queryMethod) => + ({ + code: `expect(get${queryMethod}('qux')).not.toBeNull();`, + errors: [ + { + line: 1, + column: 8, + messageId: 'preferImplicitAssert', + data: { + queryType: 'getBy*', + }, + }, + ], + } as const) + ), + ...COMBINED_QUERIES_METHODS.map( + (queryMethod) => + ({ + code: `expect(get${queryMethod}('qux')).not.toBeFalsy();`, + errors: [ + { + line: 1, + column: 8, + messageId: 'preferImplicitAssert', + data: { + queryType: 'getBy*', + }, + }, + ], + } as const) + ), + ...COMBINED_QUERIES_METHODS.map( + (queryMethod) => + ({ + code: `expect(screen.get${queryMethod}('qux')).toBeInTheDocument();`, + errors: [ + { + line: 1, + column: 8, + messageId: 'preferImplicitAssert', + data: { + queryType: 'getBy*', + }, + }, + ], + } as const) + ), + ...COMBINED_QUERIES_METHODS.map( + (queryMethod) => + ({ + code: `expect(screen.get${queryMethod}('qux')).toBeTruthy();`, + errors: [ + { + line: 1, + column: 8, + messageId: 'preferImplicitAssert', + data: { + queryType: 'getBy*', + }, + }, + ], + } as const) + ), + ...COMBINED_QUERIES_METHODS.map( + (queryMethod) => + ({ + code: `expect(screen.get${queryMethod}('qux')).toBeDefined();`, + errors: [ + { + line: 1, + column: 8, + messageId: 'preferImplicitAssert', + data: { + queryType: 'getBy*', + }, + }, + ], + } as const) + ), + ...COMBINED_QUERIES_METHODS.map( + (queryMethod) => + ({ + code: `expect(screen.get${queryMethod}('qux')).not.toBeNull();`, + errors: [ + { + line: 1, + column: 8, + messageId: 'preferImplicitAssert', + data: { + queryType: 'getBy*', + }, + }, + ], + } as const) + ), + ...COMBINED_QUERIES_METHODS.map( + (queryMethod) => + ({ + code: `expect(screen.get${queryMethod}('qux')).not.toBeFalsy();`, + errors: [ + { + line: 1, + column: 8, + messageId: 'preferImplicitAssert', + data: { + queryType: 'getBy*', + }, + }, + ], + } as const) + ), + ...COMBINED_QUERIES_METHODS.map( + (queryMethod) => + ({ + code: ` + const utils = render(); + expect(utils.get${queryMethod}('foo')).toBeInTheDocument();`, + errors: [ + { + line: 3, + column: 14, + messageId: 'preferImplicitAssert', + data: { + queryType: 'getBy*', + }, + }, + ], + } as const) + ), + ...COMBINED_QUERIES_METHODS.map( + (queryMethod) => + ({ + code: ` + const utils = render(); + expect(utils.get${queryMethod}('foo')).toBeTruthy();`, + errors: [ + { + line: 3, + column: 14, + messageId: 'preferImplicitAssert', + data: { + queryType: 'getBy*', + }, + }, + ], + } as const) + ), + ...COMBINED_QUERIES_METHODS.map( + (queryMethod) => + ({ + code: ` + const utils = render(); + expect(utils.get${queryMethod}('foo')).toBeDefined();`, + errors: [ + { + line: 3, + column: 14, + messageId: 'preferImplicitAssert', + data: { + queryType: 'getBy*', + }, + }, + ], + } as const) + ), + ...COMBINED_QUERIES_METHODS.map( + (queryMethod) => + ({ + code: ` + const utils = render(); + expect(utils.get${queryMethod}('foo')).not.toBeFalsy();`, + errors: [ + { + line: 3, + column: 14, + messageId: 'preferImplicitAssert', + data: { + queryType: 'getBy*', + }, + }, + ], + } as const) + ), + ...COMBINED_QUERIES_METHODS.map( + (queryMethod) => + ({ + code: ` + const utils = render(); + expect(utils.get${queryMethod}('foo')).not.toBeNull();`, + errors: [ + { + line: 3, + column: 14, + messageId: 'preferImplicitAssert', + data: { + queryType: 'getBy*', + }, + }, + ], + } as const) + ), + ], +});