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: added new rule prefer-in-document #95

Merged
merged 18 commits into from
Nov 24, 2020
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 12 additions & 12 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -100,18 +100,17 @@ module.exports = {
🔧 indicates that a rule is fixable.

<!-- __BEGIN AUTOGENERATED TABLE__ -->

| Name | 👍 | 🔧 | Description |
| ---------------------------------------------------------------------------------------------------------------------------------------------- | --- | --- | -------------------------------------------------------------- |
| [prefer-checked](https://github.com/testing-library/eslint-plugin-jest-dom/blob/master/docs/rules/prefer-checked.md) | 👍 | 🔧 | prefer toBeChecked over checking attributes |
| [prefer-empty](https://github.com/testing-library/eslint-plugin-jest-dom/blob/master/docs/rules/prefer-empty.md) | 👍 | 🔧 | Prefer toBeEmpty over checking innerHTML |
| [prefer-enabled-disabled](https://github.com/testing-library/eslint-plugin-jest-dom/blob/master/docs/rules/prefer-enabled-disabled.md) | 👍 | 🔧 | prefer toBeDisabled or toBeEnabled over checking attributes |
| [prefer-focus](https://github.com/testing-library/eslint-plugin-jest-dom/blob/master/docs/rules/prefer-focus.md) | 👍 | 🔧 | prefer toHaveFocus over checking document.activeElement |
| [prefer-required](https://github.com/testing-library/eslint-plugin-jest-dom/blob/master/docs/rules/prefer-required.md) | 👍 | 🔧 | prefer toBeRequired over checking properties |
| [prefer-to-have-attribute](https://github.com/testing-library/eslint-plugin-jest-dom/blob/master/docs/rules/prefer-to-have-attribute.md) | 👍 | 🔧 | prefer toHaveAttribute over checking getAttribute/hasAttribute |
| [prefer-to-have-style](https://github.com/testing-library/eslint-plugin-jest-dom/blob/master/docs/rules/prefer-to-have-style.md) | 👍 | 🔧 | prefer toHaveStyle over checking element style |
| [prefer-to-have-text-content](https://github.com/testing-library/eslint-plugin-jest-dom/blob/master/docs/rules/prefer-to-have-text-content.md) | 👍 | 🔧 | Prefer toHaveTextContent over checking element.textContent |

Name | 👍 | 🔧 | Description
----- | ----- | ----- | -----
[prefer-checked](https://github.com/testing-library/eslint-plugin-jest-dom/blob/master/docs/rules/prefer-checked.md) | 👍 | 🔧 | prefer toBeChecked over checking attributes
[prefer-empty](https://github.com/testing-library/eslint-plugin-jest-dom/blob/master/docs/rules/prefer-empty.md) | 👍 | 🔧 | Prefer toBeEmpty over checking innerHTML
[prefer-enabled-disabled](https://github.com/testing-library/eslint-plugin-jest-dom/blob/master/docs/rules/prefer-enabled-disabled.md) | 👍 | 🔧 | prefer toBeDisabled or toBeEnabled over checking attributes
[prefer-focus](https://github.com/testing-library/eslint-plugin-jest-dom/blob/master/docs/rules/prefer-focus.md) | 👍 | 🔧 | prefer toHaveFocus over checking document.activeElement
[prefer-in-document](https://github.com/testing-library/eslint-plugin-jest-dom/blob/master/docs/rules/prefer-in-document.md) | | 🔧 | Prefer .toBeInTheDocument() in favor of checking the length of the result using .toHaveLength(1)
[prefer-required](https://github.com/testing-library/eslint-plugin-jest-dom/blob/master/docs/rules/prefer-required.md) | 👍 | 🔧 | prefer toBeRequired over checking properties
[prefer-to-have-attribute](https://github.com/testing-library/eslint-plugin-jest-dom/blob/master/docs/rules/prefer-to-have-attribute.md) | 👍 | 🔧 | prefer toHaveAttribute over checking getAttribute/hasAttribute
[prefer-to-have-style](https://github.com/testing-library/eslint-plugin-jest-dom/blob/master/docs/rules/prefer-to-have-style.md) | 👍 | 🔧 | prefer toHaveStyle over checking element style
[prefer-to-have-text-content](https://github.com/testing-library/eslint-plugin-jest-dom/blob/master/docs/rules/prefer-to-have-text-content.md) | 👍 | 🔧 | Prefer toHaveTextContent over checking element.textContent
<!-- __END AUTOGENERATED TABLE__ -->

## Issues
Expand Down Expand Up @@ -159,6 +158,7 @@ Thanks goes to these people ([emoji key][emojis]):

<!-- markdownlint-enable -->
<!-- prettier-ignore-end -->

<!-- ALL-CONTRIBUTORS-LIST:END -->

This project follows the [all-contributors][all-contributors] specification.
Expand Down
40 changes: 40 additions & 0 deletions docs/rules/prefer-in-document.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
# Prefer .toBeInTheDocument in favor of .toHaveLength(1) (prefer-in-document)

## Rule Details

This rule enforces checking existance of DOM nodes using `.toBeInTheDocument()`.
The rule prefers that matcher over various existance checks such as `.toHaveLength(1)`, `.not.toBeNull()` and
similar.

Examples of **incorrect** code for this rule:

```js
expect(screen.queryByText("foo")).toHaveLength(1);
expect(queryByText("foo")).toHaveLength(1);
expect(wrapper.queryByText("foo")).toHaveLength(1);
expect(queryByText("foo")).toHaveLength(0);
expect(queryByText("foo")).toBeNull();
expect(queryByText("foo")).not.toBeNull();
expect(queryByText("foo")).toBeDefined();
expect(queryByText("foo")).not.toBeDefined();
```

Examples of **correct** code for this rule:

```js
expect(screen.queryByText("foo")).toBeInTheDocument();
expect(screen.queryByText("foo")).toBeInTheDocument();
expect(queryByText("foo")).toBeInTheDocument()`;
expect(wrapper.queryAllByTestId('foo')).toBeInTheDocument()`;
expect(screen.getAllByLabel("foo-bar")).toHaveLength(2)`;
expect(notAQuery('foo-bar')).toHaveLength(1)`;
```
AntonNiklasson marked this conversation as resolved.
Show resolved Hide resolved

## When Not To Use It

Don't use this rule if you don't care about the added readability and
improvements that `toBeInTheDocument` offers to your expects.

## Further Reading

- [Docs on toBeInTheDocument](https://github.com/testing-library/jest-dom#tobeinthedocument)
4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
"requireindex": "^1.2.0"
},
"devDependencies": {
"@testing-library/dom": "^7.27.1",
benmonro marked this conversation as resolved.
Show resolved Hide resolved
"eslint": "7.14",
"jest-extended": "^0.11.5",
"kcd-scripts": "6.5.1"
Expand All @@ -53,7 +54,8 @@
"extends": "./node_modules/kcd-scripts/eslint.js",
"rules": {
"babel/quotes": "off",
"max-lines-per-function": "off"
"max-lines-per-function": "off",
"testing-library/no-dom-import": "off"
}
},
"eslintIgnore": [
Expand Down
84 changes: 84 additions & 0 deletions src/__tests__/lib/rules/prefer-in-document.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
/**
* @fileoverview Prefer toBeInTheDocument over querying and asserting length.
* @author Anton Niklasson
*/

//------------------------------------------------------------------------------
// Requirements
//------------------------------------------------------------------------------

import { RuleTester } from "eslint";
import { queries, queriesByVariant } from "../../../queries";
import * as rule from "../../../rules/prefer-in-document";

//------------------------------------------------------------------------------
// Tests
//------------------------------------------------------------------------------

function invalidCase(code, output) {
return {
code,
output,
errors: [
{
messageId: "use-document",
},
],
};
}

const valid = [
...queries.map((q) => [
`expect(screen.${q}('foo')).toBeInTheDocument()`,
`expect(${q}('foo')).toBeInTheDocument()`,
`expect(wrapper.${q}('foo')).toBeInTheDocument()`,
]),
`expect(screen.notAQuery('foo-bar')).toHaveLength(1)`,
`expect(screen.getByText('foo-bar')).toHaveLength(2)`,
];
const invalid = [
// Invalid cases that applies to all variants
...queries.map((q) => [
invalidCase(
`expect(screen.${q}('foo')).toHaveLength(1)`,
`expect(screen.${q}('foo')).toBeInTheDocument()`
),
invalidCase(
`expect(${q}('foo')).toHaveLength(1)`,
`expect(${q}('foo')).toBeInTheDocument()`
),
invalidCase(
`expect(wrapper.${q}('foo')).toHaveLength(1)`,
`expect(wrapper.${q}('foo')).toBeInTheDocument()`
),
]),
// Invalid cases that applies to queryBy* and queryAllBy*
...queriesByVariant.query.map((q) => [
invalidCase(
`expect(${q}('foo')).toHaveLength(0)`,
`expect(${q}('foo')).not.toBeInTheDocument()`
),
invalidCase(
`expect(${q}('foo')).toBeNull()`,
`expect(${q}('foo')).not.toBeInTheDocument()`
),
invalidCase(
`expect(${q}('foo')).not.toBeNull()`,
`expect(${q}('foo')).toBeInTheDocument()`
),
invalidCase(
`expect(${q}('foo')).toBeDefined()`,
`expect(${q}('foo')).toBeInTheDocument()`
),
invalidCase(
`expect(${q}('foo')).not.toBeDefined()`,
`expect(${q}('foo')).not.toBeInTheDocument()`
),
]),
];

const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 2015 } });
ruleTester.run("prefer-in-document", rule, {
valid: [].concat(...valid),
invalid: [].concat(...invalid),
});
9 changes: 9 additions & 0 deletions src/queries.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { queries as allQueries } from "@testing-library/dom";

export const queries = Object.keys(allQueries);

export const queriesByVariant = {
query: queries.filter((q) => q.startsWith("query")),
get: queries.filter((q) => q.startsWith("get")),
find: queries.filter((q) => q.startsWith("find")),
};
113 changes: 113 additions & 0 deletions src/rules/prefer-in-document.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
/**
* @fileoverview prefer toBeInTheDocument over checking getAttribute/hasAttribute
* @author Anton Niklasson
*/

import { queries } from "../queries";

export const meta = {
type: "suggestion",
docs: {
category: "jest-dom",
description:
"Prefer .toBeInTheDocument() for asserting the existence of a DOM node",
url: "prefer-in-document",
recommended: false,
},
fixable: "code",
messages: {
"use-document": `Prefer .toBeInTheDocument() for asserting DOM node existence`,
},
};

function isAntonymMatcher(matcherNode, matcherArguments) {
return (
matcherNode.name === "toBeNull" ||
(matcherNode.name === "toHaveLength" && matcherArguments[0].value === 0)
);
}

function check(
context,
{ queryNode, matcherNode, matcherArguments, negatedMatcher }
) {
const query = queryNode.name || queryNode.property.name;

// toHaveLength() is only invalid with 0 or 1
if (matcherNode.name === "toHaveLength" && matcherArguments[0].value > 1) {
return;
}

if (queries.includes(query)) {
context.report({
node: matcherNode,
messageId: "use-document",
loc: matcherNode.loc,
fix(fixer) {
const operations = [];

// Flip the .not if neccessary
if (isAntonymMatcher(matcherNode, matcherArguments)) {
if (negatedMatcher) {
operations.push(
fixer.removeRange([
matcherNode.range[0] - 5,
matcherNode.range[0] - 1,
])
);
} else {
operations.push(fixer.insertTextBefore(matcherNode, "not."));
}
}

// Replace the actual matcher
operations.push(fixer.replaceText(matcherNode, "toBeInTheDocument"));

// Remove any arguments in the matcher
for (const argument of matcherArguments) {
operations.push(fixer.remove(argument));
}

return operations;
},
});
}
}

export const create = (context) => {
const alternativeMatchers = /(toHaveLength|toBeDefined|toBeNull)/;

return {
// Grabbing expect(<query>).not.<matcher>
[`CallExpression[callee.object.object.callee.name='expect'][callee.object.property.name='not'][callee.property.name=${alternativeMatchers}]`](
node
) {
const queryNode = node.callee.object.object.arguments[0].callee;
const matcherNode = node.callee.property;
const matcherArguments = node.arguments;

check(context, {
negatedMatcher: true,
queryNode,
matcherNode,
matcherArguments,
});
},

// Grabbing expect(<query>).<matcher>
[`CallExpression[callee.object.callee.name='expect'][callee.property.name=${alternativeMatchers}]`](
node
) {
const queryNode = node.callee.object.arguments[0].callee;
const matcherNode = node.callee.property;
const matcherArguments = node.arguments;

check(context, {
negatedMatcher: false,
queryNode,
matcherNode,
matcherArguments,
});
},
};
};