Skip to content

Commit 56a1900

Browse files
adevickadevick
authored
feat(prefer-implicit-assert): adding new rule (#815)
Closes #743 * feat(prefer-implicit-assert): adding new rule * feat(prefer-implicit-assert): increment number of rules * feat(prefer-implicit-assert): reduce duplication * feat(prefer-implicit-assert): add recommened rule by library, more test cases, update docs * feat(prefer-implicit-assert): added findBy link * feat(prefer-implicit-assert): added line and column checks * feat(prefer-implicit-assert): use existing utils * feat(prefer-implicit-assert): remove unnecessary checks
1 parent 7c44703 commit 56a1900

File tree

6 files changed

+751
-2
lines changed

6 files changed

+751
-2
lines changed

‎README.md

+1
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,7 @@ module.exports = {
228228
| [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][] | | |
229229
| [prefer-explicit-assert](docs/rules/prefer-explicit-assert.md) | Suggest using explicit assertions rather than standalone queries | | | |
230230
| [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][] | | 🔧 |
231+
| [prefer-implicit-assert](docs/rules/prefer-implicit-assert.md) | Suggest using implicit assertions for getBy* & findBy* queries | | | |
231232
| [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][] | | |
232233
| [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][] | | |
233234
| [prefer-query-matchers](docs/rules/prefer-query-matchers.md) | Ensure the configured `get*`/`query*` query is used with the corresponding matchers | | | |

‎docs/rules/prefer-explicit-assert.md

+4-1
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,10 @@ This is how you can use these options in eslint configuration:
7272

7373
## When Not To Use It
7474

75-
If you prefer to use `getBy*` queries implicitly as an assert-like method itself, then this rule is not recommended.
75+
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)
76+
77+
- Never use both `prefer-explicit-assert` & `prefer-implicit-assert` choose one.
78+
- 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
7679

7780
## Further Reading
7881

‎docs/rules/prefer-implicit-assert.md

+57
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
# Suggest using implicit assertions for getBy* & findBy* queries (`testing-library/prefer-implicit-assert`)
2+
3+
<!-- end auto-generated rule header -->
4+
5+
Testing Library `getBy*` & `findBy*` queries throw an error if the element is not
6+
found. Therefore it is not necessary to also assert existance with things like `expect(getBy*.toBeInTheDocument()` or `expect(awaint findBy*).not.toBeNull()`
7+
8+
## Rule Details
9+
10+
This rule aims to reuduce uncecessary assertion's for presense of an element,
11+
when using queries that implicitly fail when said element is not found.
12+
13+
Examples of **incorrect** code for this rule with the default configuration:
14+
15+
```js
16+
// wrapping the getBy or findBy queries within a `expect` and using existence matchers for
17+
// making the assertion is not necessary
18+
expect(getByText('foo')).toBeInTheDocument();
19+
expect(await findByText('foo')).toBeInTheDocument();
20+
21+
expect(getByText('foo')).toBeDefined();
22+
expect(await findByText('foo')).toBeDefined();
23+
24+
const utils = render(<Component />);
25+
expect(utils.getByText('foo')).toBeInTheDocument();
26+
expect(await utils.findByText('foo')).toBeInTheDocument();
27+
28+
expect(await findByText('foo')).not.toBeNull();
29+
expect(await findByText('foo')).not.toBeUndified();
30+
```
31+
32+
Examples of **correct** code for this rule with the default configuration:
33+
34+
```js
35+
getByText('foo');
36+
await findByText('foo');
37+
38+
const utils = render(<Component />);
39+
utils.getByText('foo');
40+
await utils.findByText('foo');
41+
42+
// When using queryBy* queries thees do not implicitly fial therefore you should explicitly check if your elements eixst or not
43+
expect(queryByText('foo')).toBeInTheDocument();
44+
expect(queryByText('foo')).not.toBeInTheDocument();
45+
```
46+
47+
## When Not To Use It
48+
49+
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)
50+
51+
- Never use both `prefer-implicit-assert` & `prefer-explicit-assert` choose one.
52+
- 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
53+
54+
## Further Reading
55+
56+
- [getBy query](https://testing-library.com/docs/dom-testing-library/api-queries#getby)
57+
- [findBy query](https://testing-library.com/docs/dom-testing-library/api-queries#findBy)

‎lib/rules/prefer-implicit-assert.ts

+136
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
import {
2+
TSESTree,
3+
ASTUtils,
4+
AST_NODE_TYPES,
5+
TSESLint,
6+
} from '@typescript-eslint/utils';
7+
8+
import { createTestingLibraryRule } from '../create-testing-library-rule';
9+
import { TestingLibrarySettings } from '../create-testing-library-rule/detect-testing-library-utils';
10+
import { isCallExpression, isMemberExpression } from '../node-utils';
11+
12+
export const RULE_NAME = 'prefer-implicit-assert';
13+
export type MessageIds = 'preferImplicitAssert';
14+
type Options = [];
15+
16+
const isCalledUsingSomeObject = (node: TSESTree.Identifier) =>
17+
isMemberExpression(node.parent) &&
18+
node.parent.object.type === AST_NODE_TYPES.Identifier;
19+
20+
const isCalledInExpect = (
21+
node: TSESTree.Identifier | TSESTree.Node,
22+
isAsyncQuery: boolean
23+
) => {
24+
if (isAsyncQuery) {
25+
return (
26+
isCallExpression(node.parent) &&
27+
ASTUtils.isAwaitExpression(node.parent.parent) &&
28+
isCallExpression(node.parent.parent.parent) &&
29+
ASTUtils.isIdentifier(node.parent.parent.parent.callee) &&
30+
node.parent.parent.parent.callee.name === 'expect'
31+
);
32+
}
33+
return (
34+
isCallExpression(node.parent) &&
35+
isCallExpression(node.parent.parent) &&
36+
ASTUtils.isIdentifier(node.parent.parent.callee) &&
37+
node.parent.parent.callee.name === 'expect'
38+
);
39+
};
40+
41+
const reportError = (
42+
context: Readonly<
43+
TSESLint.RuleContext<'preferImplicitAssert', []> & {
44+
settings: TestingLibrarySettings;
45+
}
46+
>,
47+
node: TSESTree.Identifier | TSESTree.Node | undefined,
48+
queryType: string
49+
) => {
50+
if (node) {
51+
return context.report({
52+
node,
53+
messageId: 'preferImplicitAssert',
54+
data: {
55+
queryType,
56+
},
57+
});
58+
}
59+
};
60+
61+
export default createTestingLibraryRule<Options, MessageIds>({
62+
name: RULE_NAME,
63+
meta: {
64+
type: 'suggestion',
65+
docs: {
66+
description:
67+
'Suggest using implicit assertions for getBy* & findBy* queries',
68+
recommendedConfig: {
69+
dom: false,
70+
angular: false,
71+
react: false,
72+
vue: false,
73+
marko: false,
74+
},
75+
},
76+
messages: {
77+
preferImplicitAssert:
78+
"Don't wrap `{{queryType}}` query with `expect` & presence matchers like `toBeInTheDocument` or `not.toBeNull` as `{{queryType}}` queries fail implicitly when element is not found",
79+
},
80+
schema: [],
81+
},
82+
defaultOptions: [],
83+
create(context, _, helpers) {
84+
const findQueryCalls: TSESTree.Identifier[] = [];
85+
const getQueryCalls: TSESTree.Identifier[] = [];
86+
87+
return {
88+
'CallExpression Identifier'(node: TSESTree.Identifier) {
89+
if (helpers.isFindQueryVariant(node)) {
90+
findQueryCalls.push(node);
91+
}
92+
if (helpers.isGetQueryVariant(node)) {
93+
getQueryCalls.push(node);
94+
}
95+
},
96+
'Program:exit'() {
97+
findQueryCalls.forEach((queryCall) => {
98+
const isAsyncQuery = true;
99+
const node: TSESTree.Identifier | TSESTree.Node | undefined =
100+
isCalledUsingSomeObject(queryCall) ? queryCall.parent : queryCall;
101+
102+
if (node) {
103+
if (isCalledInExpect(node, isAsyncQuery)) {
104+
if (
105+
isMemberExpression(node.parent?.parent?.parent?.parent) &&
106+
node.parent?.parent?.parent?.parent.property.type ===
107+
AST_NODE_TYPES.Identifier &&
108+
helpers.isPresenceAssert(node.parent.parent.parent.parent)
109+
) {
110+
return reportError(context, node, 'findBy*');
111+
}
112+
}
113+
}
114+
});
115+
116+
getQueryCalls.forEach((queryCall) => {
117+
const isAsyncQuery = false;
118+
const node: TSESTree.Identifier | TSESTree.Node | undefined =
119+
isCalledUsingSomeObject(queryCall) ? queryCall.parent : queryCall;
120+
if (node) {
121+
if (isCalledInExpect(node, isAsyncQuery)) {
122+
if (
123+
isMemberExpression(node.parent?.parent?.parent) &&
124+
node.parent?.parent?.parent.property.type ===
125+
AST_NODE_TYPES.Identifier &&
126+
helpers.isPresenceAssert(node.parent.parent.parent)
127+
) {
128+
return reportError(context, node, 'getBy*');
129+
}
130+
}
131+
}
132+
});
133+
},
134+
};
135+
},
136+
});

‎tests/index.test.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { resolve } from 'path';
33

44
import plugin from '../lib';
55

6-
const numberOfRules = 26;
6+
const numberOfRules = 27;
77
const ruleNames = Object.keys(plugin.rules);
88

99
// eslint-disable-next-line jest/expect-expect

0 commit comments

Comments
 (0)