-
Notifications
You must be signed in to change notification settings - Fork 153
feat(prefer-implicit-assert): adding new rule #815
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Belco90
merged 9 commits into
testing-library:main
from
adevick:pr/prefer-implicit-assert
Oct 12, 2023
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
52e5c37
feat(prefer-implicit-assert): adding new rule
adevick 004cf1a
feat(prefer-implicit-assert): increment number of rules
adevick 32ad37d
feat(prefer-implicit-assert): reduce duplication
adevick 766afcd
feat(prefer-implicit-assert): add recommened rule by library, more te…
adevick 37b2318
feat(prefer-implicit-assert): added findBy link
adevick 7feb82d
feat(prefer-implicit-assert): added line and column checks
adevick 0c44edf
feat(prefer-implicit-assert): use existing utils
adevick cfe2b8a
Merge branch 'main' into pr/prefer-implicit-assert
adevick 1a2eb85
feat(prefer-implicit-assert): remove unnecessary checks
adevick File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
# Suggest using implicit assertions for getBy* & findBy* queries (`testing-library/prefer-implicit-assert`) | ||
|
||
<!-- end auto-generated rule header --> | ||
|
||
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(<Component />); | ||
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(<Component />); | ||
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) | ||
Belco90 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
- [findBy query](https://testing-library.com/docs/dom-testing-library/api-queries#findBy) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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<Options, MessageIds>({ | ||
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; | ||
Belco90 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
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*'); | ||
} | ||
} | ||
} | ||
}); | ||
}, | ||
}; | ||
}, | ||
}); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.