Skip to content
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

feat(prefer-implicit-assert): adding new rule #815

Merged
merged 9 commits into from
Oct 12, 2023
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
5 changes: 4 additions & 1 deletion docs/rules/prefer-explicit-assert.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

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

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

if (node) {
if (isCalledInExpect(node, isAsyncQuery)) {
if (usesPresenceAssertion(node, isAsyncQuery))
return reportError(context, node, 'findBy*');
if (usesNotPresenceAssertion(node, isAsyncQuery))
return reportError(context, node, 'findBy*');
}
}
});

getQueryCalls.forEach((queryCall) => {
isAsyncQuery = false;
const node: TSESTree.Identifier | TSESTree.Node | undefined =
isCalledUsingSomeObject(queryCall) ? queryCall.parent : queryCall;
if (node) {
if (isCalledInExpect(node, isAsyncQuery)) {
if (usesPresenceAssertion(node, isAsyncQuery))
return reportError(context, node, 'getBy*');
if (usesNotPresenceAssertion(node, isAsyncQuery))
return reportError(context, node, 'getBy*');
}
}
});
},
};
},
});
2 changes: 1 addition & 1 deletion tests/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading