Skip to content

Commit

Permalink
feat(prefer-implicit-assert): adding new rule
Browse files Browse the repository at this point in the history
  • Loading branch information
adevick committed Sep 11, 2023
1 parent 8b0b9cc commit 52e5c37
Show file tree
Hide file tree
Showing 5 changed files with 727 additions and 1 deletion.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 | | | |
Expand Down
2 changes: 1 addition & 1 deletion docs/rules/prefer-explicit-assert.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ 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)

## Further Reading

Expand Down
53 changes: 53 additions & 0 deletions docs/rules/prefer-implicit-assert.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
# 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 existance 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)

## Further Reading

- [getBy query](https://testing-library.com/docs/dom-testing-library/api-queries#getby)
189 changes: 189 additions & 0 deletions lib/rules/prefer-implicit-assert.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,189 @@
import { TSESTree, ASTUtils, AST_NODE_TYPES } from '@typescript-eslint/utils';

import { createTestingLibraryRule } from '../create-testing-library-rule';
import { isCallExpression, isMemberExpression } from '../node-utils';
import { PRESENCE_MATCHERS, ABSENCE_MATCHERS } from '../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 usesPresenceAssertion = (
node: TSESTree.Identifier | TSESTree.Node,
isAsyncQuery: boolean
) => {
if (isAsyncQuery) {
return (
isMemberExpression(node.parent?.parent?.parent?.parent) &&
node.parent?.parent?.parent?.parent.property.type ===
AST_NODE_TYPES.Identifier &&
PRESENCE_MATCHERS.includes(node.parent.parent.parent.parent.property.name)
);
}
return (
isMemberExpression(node.parent?.parent?.parent) &&
node.parent?.parent?.parent.property.type === AST_NODE_TYPES.Identifier &&
PRESENCE_MATCHERS.includes(node.parent.parent.parent.property.name)
);
};

const usesNotPresenceAssertion = (
node: TSESTree.Identifier | TSESTree.Node,
isAsyncQuery: boolean
) => {
if (isAsyncQuery) {
return (
isMemberExpression(node.parent?.parent?.parent?.parent) &&
node.parent?.parent?.parent?.parent.property.type ===
AST_NODE_TYPES.Identifier &&
node.parent.parent.parent.parent.property.name === 'not' &&
isMemberExpression(node.parent.parent.parent.parent.parent) &&
node.parent.parent.parent.parent.parent.property.type ===
AST_NODE_TYPES.Identifier &&
ABSENCE_MATCHERS.includes(
node.parent.parent.parent.parent.parent.property.name
)
);
}
return (
isMemberExpression(node.parent?.parent?.parent) &&
node.parent?.parent?.parent.property.type === AST_NODE_TYPES.Identifier &&
node.parent.parent.parent.property.name === 'not' &&
isMemberExpression(node.parent.parent.parent.parent) &&
node.parent.parent.parent.parent.property.type ===
AST_NODE_TYPES.Identifier &&
ABSENCE_MATCHERS.includes(node.parent.parent.parent.parent.property.name)
);
};

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'() {
let isAsyncQuery = true;
findQueryCalls.forEach((queryCall) => {
const node: TSESTree.Identifier | TSESTree.Node | undefined =
isCalledUsingSomeObject(queryCall) ? queryCall.parent : queryCall;

if (node) {
if (
isCalledInExpect(node, isAsyncQuery) &&
usesPresenceAssertion(node, isAsyncQuery)
) {
return context.report({
node: queryCall,
messageId: 'preferImplicitAssert',
data: {
queryType: 'findBy*',
},
});
}

if (
isCalledInExpect(node, isAsyncQuery) &&
usesNotPresenceAssertion(node, isAsyncQuery)
) {
return context.report({
node: queryCall,
messageId: 'preferImplicitAssert',
data: {
queryType: 'findBy*',
},
});
}
}
});

getQueryCalls.forEach((queryCall) => {
isAsyncQuery = false;
const node: TSESTree.Identifier | TSESTree.Node | undefined =
isCalledUsingSomeObject(queryCall) ? queryCall.parent : queryCall;
if (node) {
if (
isCalledInExpect(node, isAsyncQuery) &&
usesPresenceAssertion(node, isAsyncQuery)
) {
return context.report({
node: queryCall,
messageId: 'preferImplicitAssert',
data: {
queryType: 'getBy*',
},
});
}

if (
isCalledInExpect(node, isAsyncQuery) &&
usesNotPresenceAssertion(node, isAsyncQuery)
) {
return context.report({
node: queryCall,
messageId: 'preferImplicitAssert',
data: {
queryType: 'getBy*',
},
});
}
}
});
},
};
},
});
Loading

0 comments on commit 52e5c37

Please sign in to comment.