diff --git a/.github/ISSUE_TEMPLATE.md b/.github/ISSUE_TEMPLATE.md index 3a4cd1c..7771a5b 100644 --- a/.github/ISSUE_TEMPLATE.md +++ b/.github/ISSUE_TEMPLATE.md @@ -18,6 +18,7 @@ learn how: http://kcd.im/pull-request Relevant code or config ```javascript + ``` What you did: diff --git a/README.md b/README.md index 818d40c..44c91f7 100644 --- a/README.md +++ b/README.md @@ -100,17 +100,19 @@ module.exports = { 🔧 indicates that a rule is fixable. -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 + +| 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 | + ## Issues diff --git a/docs/rules/prefer-in-document.md b/docs/rules/prefer-in-document.md index 394a390..7043fc3 100644 --- a/docs/rules/prefer-in-document.md +++ b/docs/rules/prefer-in-document.md @@ -17,17 +17,32 @@ expect(queryByText("foo")).toBeNull(); expect(queryByText("foo")).not.toBeNull(); expect(queryByText("foo")).toBeDefined(); expect(queryByText("foo")).not.toBeDefined(); + +const foo = screen.getByText("foo"); +expect(foo).toHaveLength(1); + +const bar = screen.queryByText("bar"); +expect(bar).toHaveLength(0); ``` 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)`; +expect(await screen.findByText("foo")).toBeInTheDocument(); +expect(queryByText("foo")).toBeInTheDocument(); +expect(wrapper.queryAllByTestId("foo")).toBeInTheDocument(); +expect(screen.getAllByLabel("foo-bar")).toHaveLength(2); +expect(notAQuery("foo-bar")).toHaveLength(1); + +const foo = screen.getAllByText("foo"); +expect(foo).toHaveLength(3); + +const bar = screen.queryByText("bar"); +expect(bar).not.toBeDefined(); + +const baz = await screen.findByText("baz"); +expect(baz).toBeDefined() ``` ## When Not To Use It diff --git a/other/MAINTAINING.md b/other/MAINTAINING.md index 703126d..c78fc0b 100644 --- a/other/MAINTAINING.md +++ b/other/MAINTAINING.md @@ -66,5 +66,4 @@ necessary by the git commit messages. With this in mind, **please brush up on Thank you so much for helping to maintain this project! -[commit]: - https://github.com/conventional-changelog-archived-repos/conventional-changelog-angular/blob/ed32559941719a130bb0327f886d6a32a8cbc2ba/convention.md +[commit]: https://github.com/conventional-changelog-archived-repos/conventional-changelog-angular/blob/ed32559941719a130bb0327f886d6a32a8cbc2ba/convention.md diff --git a/package.json b/package.json index 7b198a3..c7665da 100644 --- a/package.json +++ b/package.json @@ -59,7 +59,8 @@ "rules": { "babel/quotes": "off", "max-lines-per-function": "off", - "testing-library/no-dom-import": "off" + "testing-library/no-dom-import": "off", + "consistent-return": "off" } }, "eslintIgnore": [ diff --git a/src/__tests__/lib/rules/prefer-in-document.js b/src/__tests__/lib/rules/prefer-in-document.js index 917a619..6b659f6 100644 --- a/src/__tests__/lib/rules/prefer-in-document.js +++ b/src/__tests__/lib/rules/prefer-in-document.js @@ -8,7 +8,6 @@ //------------------------------------------------------------------------------ import { RuleTester } from "eslint"; -import { queries, queriesByVariant } from "../../../queries"; import * as rule from "../../../rules/prefer-in-document"; //------------------------------------------------------------------------------ @@ -28,17 +27,29 @@ function invalidCase(code, output) { } const valid = [ - ...queries.map((q) => [ + ...["getByText", "getByRole"].map((q) => [ `expect(screen.${q}('foo')).toBeInTheDocument()`, `expect(${q}('foo')).toBeInTheDocument()`, `expect(wrapper.${q}('foo')).toBeInTheDocument()`, + `let foo; + foo = screen.${q}('foo'); + foo = somethingElse; + expect(foo).toHaveLength(1);`, ]), + `let foo; + foo = "bar"; + expect(foo).toHaveLength(1);`, + `let foo; + foo = "bar"; + expect(foo).toHaveLength(0);`, + `let foo; + expect(foo).toHaveLength(1);`, `expect(screen.notAQuery('foo-bar')).toHaveLength(1)`, - `expect(screen.getByText('foo-bar')).toHaveLength(2)`, + `expect(screen.getAllByText('foo-bar')).toHaveLength(2)`, ]; const invalid = [ // Invalid cases that applies to all variants - ...queries.map((q) => [ + ...["getByText", "getAllByRole"].map((q) => [ invalidCase( `expect(screen.${q}('foo')).toHaveLength(1)`, `expect(screen.${q}('foo')).toBeInTheDocument()` @@ -51,9 +62,37 @@ const invalid = [ `expect(wrapper.${q}('foo')).toHaveLength(1)`, `expect(wrapper.${q}('foo')).toBeInTheDocument()` ), + invalidCase( + `const foo = screen.${q}('foo'); + expect(foo).toHaveLength(1);`, + `const foo = screen.${q}('foo'); + expect(foo).toBeInTheDocument();` + ), + invalidCase( + `const foo = ${q}('foo'); + expect(foo).toHaveLength(1);`, + `const foo = ${q}('foo'); + expect(foo).toBeInTheDocument();` + ), + invalidCase( + `let foo; + foo = ${q}('foo'); + expect(foo).toHaveLength(1);`, + `let foo; + foo = ${q}('foo'); + expect(foo).toBeInTheDocument();` + ), + invalidCase( + `let foo; + foo = screen.${q}('foo'); + expect(foo).toHaveLength(1);`, + `let foo; + foo = screen.${q}('foo'); + expect(foo).toBeInTheDocument();` + ), ]), // Invalid cases that applies to queryBy* and queryAllBy* - ...queriesByVariant.query.map((q) => [ + ...["queryByText", "queryAllByText"].map((q) => [ invalidCase( `expect(${q}('foo')).toHaveLength(0)`, `expect(${q}('foo')).not.toBeInTheDocument()` @@ -66,18 +105,120 @@ const invalid = [ `expect(${q}('foo')).not.toBeNull()`, `expect(${q}('foo')).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()` + `expect(${q}('foo')) .not .toBeDefined()`, + `expect(${q}('foo')) .not .toBeInTheDocument()` + ), + invalidCase( + `let foo; + foo = screen.${q}('foo'); + expect(foo).toHaveLength(0);`, + `let foo; + foo = screen.${q}('foo'); + expect(foo).not.toBeInTheDocument();` + ), + invalidCase( + `let foo; + foo = screen.${q}('foo'); + expect(foo) .not.toBeNull();`, + `let foo; + foo = screen.${q}('foo'); + expect(foo).toBeInTheDocument();` + ), + invalidCase( + `let foo = screen.${q}('foo'); + expect(foo).not.toBeNull();`, + `let foo = screen.${q}('foo'); + expect(foo).toBeInTheDocument();` ), ]), + invalidCase( + `it("foo", async () => { + expect(await findByRole("button")).toBeDefined(); + })`, + `it("foo", async () => { + expect(await findByRole("button")).toBeInTheDocument(); + })` + ), + invalidCase( + `it("foo", async () => { + expect(await findByRole("button")).not.toBeNull(); + })`, + `it("foo", async () => { + expect(await findByRole("button")).toBeInTheDocument(); + })` + ), + invalidCase( + `it("foo", async () => { + expect(await screen.findByText(/Compressing video/)).toBeDefined(); + })`, + `it("foo", async () => { + expect(await screen.findByText(/Compressing video/)).toBeInTheDocument(); + })` + ), + invalidCase( + `it("foo", async () => { + expect(await screen.findByText(/Compressing video/)).not.toBeDefined(); + })`, + `it("foo", async () => { + expect(await screen.findByText(/Compressing video/)).not.toBeInTheDocument(); + })` + ), + invalidCase( + `it("foo", async () => { + const compressingFeedback = await screen.findByText(/Compressing video/); + expect(compressingFeedback).toBeDefined(); + });`, + `it("foo", async () => { + const compressingFeedback = await screen.findByText(/Compressing video/); + expect(compressingFeedback).toBeInTheDocument(); + });` + ), + invalidCase( + `it("foo", async () => { + const compressingFeedback = await screen.findByText(/Compressing video/); + expect(compressingFeedback).not.toBeNull(); + });`, + `it("foo", async () => { + const compressingFeedback = await screen.findByText(/Compressing video/); + expect(compressingFeedback).toBeInTheDocument(); + });` + ), + invalidCase( + `it("foo", async () => { + let compressingFeedback; + compressingFeedback = await screen.findByText(/Compressing video/); + expect(compressingFeedback).toBeDefined(); + });`, + `it("foo", async () => { + let compressingFeedback; + compressingFeedback = await screen.findByText(/Compressing video/); + expect(compressingFeedback).toBeInTheDocument(); + });` + ), + invalidCase( + `it("foo", async () => { + let compressingFeedback; + compressingFeedback = await screen.findByText(/Compressing video/); + expect(compressingFeedback).not.toBeDefined(); + });`, + `it("foo", async () => { + let compressingFeedback; + compressingFeedback = await screen.findByText(/Compressing video/); + expect(compressingFeedback).not.toBeInTheDocument(); + });` + ), ]; -const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 2015 } }); +const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 2017 } }); ruleTester.run("prefer-in-document", rule, { valid: [].concat(...valid), invalid: [].concat(...invalid), diff --git a/src/__tests__/lib/rules/prefer-to-have-style.js b/src/__tests__/lib/rules/prefer-to-have-style.js index b19562a..af5a90d 100644 --- a/src/__tests__/lib/rules/prefer-to-have-style.js +++ b/src/__tests__/lib/rules/prefer-to-have-style.js @@ -20,7 +20,7 @@ ruleTester.run("prefer-to-have-style", rule, { invalid: [ { code: `expect(a.style).toHaveProperty('transform')`, - errors + errors, }, { code: `expect(el.style.foo).toBe("bar")`, diff --git a/src/queries.js b/src/queries.js index fecca5a..cd8a64f 100644 --- a/src/queries.js +++ b/src/queries.js @@ -1,9 +1,3 @@ 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")), -}; diff --git a/src/rules/prefer-in-document.js b/src/rules/prefer-in-document.js index fd06149..3c62cbc 100644 --- a/src/rules/prefer-in-document.js +++ b/src/rules/prefer-in-document.js @@ -27,82 +27,161 @@ function isAntonymMatcher(matcherNode, matcherArguments) { ); } -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; - } +export const create = (context) => { + const alternativeMatchers = /(toHaveLength|toBeDefined|toBeNull)/; + function check({ + queryNode, + matcherNode, + matcherArguments, + negatedMatcher, + expect, + }) { + if (!queryNode || (!queryNode.name && !queryNode.property)) return; + // toHaveLength() is only invalid with 0 or 1 + if (matcherNode.name === "toHaveLength" && matcherArguments[0].value > 1) { + return; + } + + const query = queryNode.name || queryNode.property.name; - 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.")); + if (queries.includes(query)) { + context.report({ + node: matcherNode, + messageId: "use-document", + loc: matcherNode.loc, + fix(fixer) { + const operations = []; + + // Remove any arguments in the matcher + for (const argument of Array.from(matcherArguments)) { + operations.push(fixer.remove(argument)); } - } + // Flip the .not if necessary + if (isAntonymMatcher(matcherNode, matcherArguments)) { + if (negatedMatcher) { + operations.push( + fixer.replaceTextRange( + [expect.range[1], matcherNode.range[1]], + ".toBeInTheDocument" + ) + ); - // Replace the actual matcher - operations.push(fixer.replaceText(matcherNode, "toBeInTheDocument")); + return operations; + } else { + operations.push(fixer.insertTextBefore(matcherNode, "not.")); + } + } - // Remove any arguments in the matcher - for (const argument of matcherArguments) { - operations.push(fixer.remove(argument)); - } + // Replace the actual matcher + operations.push(fixer.replaceText(matcherNode, "toBeInTheDocument")); - return operations; - }, - }); + return operations; + }, + }); + } } -} -export const create = (context) => { - const alternativeMatchers = /(toHaveLength|toBeDefined|toBeNull)/; + function getQueryNodeFromAssignment(identifierName) { + const variable = context.getScope().set.get(identifierName); + const init = variable.defs[0].node.init; + let queryNode; + if (init) { + // let foo = screen.(); + queryNode = + init.type === "AwaitExpression" + ? init.argument.callee.property + : init.callee.property || init.callee; + } else { + // let foo; + // foo = screen.(); + const assignmentRef = variable.references + .reverse() + .find((ref) => !!ref.writeExpr); + if (!assignmentRef) { + return; + } + queryNode = + assignmentRef.writeExpr.type === "AwaitExpression" + ? assignmentRef.writeExpr.argument.callee + : assignmentRef.writeExpr.type === "CallExpression" + ? assignmentRef.writeExpr.callee + : assignmentRef.writeExpr; + } + return queryNode; + } return { - // Grabbing expect().not. - [`CallExpression[callee.object.object.callee.name='expect'][callee.object.property.name='not'][callee.property.name=${alternativeMatchers}]`]( + // expect().not. + [`CallExpression[callee.object.object.callee.name='expect'][callee.object.property.name='not'][callee.property.name=${alternativeMatchers}], CallExpression[callee.object.callee.name='expect'][callee.object.property.name='not'][callee.object.arguments.0.argument.callee.name=${alternativeMatchers}]`]( node ) { - const queryNode = node.callee.object.object.arguments[0].callee; + const arg = node.callee.object.object.arguments[0]; + const queryNode = + arg.type === "AwaitExpression" ? arg.argument.callee : arg.callee; const matcherNode = node.callee.property; const matcherArguments = node.arguments; - check(context, { + const expect = node.callee.object.object; + check({ + negatedMatcher: true, + queryNode, + matcherNode, + matcherArguments, + expect, + }); + }, + + // // const foo = expect(foo).not. + [`MemberExpression[object.object.callee.name=expect][object.property.name=not][property.name=${alternativeMatchers}][object.object.arguments.0.type=Identifier]`]( + node + ) { + const queryNode = getQueryNodeFromAssignment( + node.object.object.arguments[0].name + ); + const matcherNode = node.property; + + const matcherArguments = node.parent.arguments; + + const expect = node.object.object; + + check({ negatedMatcher: true, queryNode, matcherNode, matcherArguments, + expect, }); }, + // const foo = expect(foo). + [`MemberExpression[object.callee.name=expect][property.name=${alternativeMatchers}][object.arguments.0.type=Identifier]`]( + node + ) { + const queryNode = getQueryNodeFromAssignment( + node.object.arguments[0].name + ); + const matcherNode = node.property; + + const matcherArguments = node.parent.arguments; - // Grabbing expect(). - [`CallExpression[callee.object.callee.name='expect'][callee.property.name=${alternativeMatchers}]`]( + check({ + negatedMatcher: false, + queryNode, + matcherNode, + matcherArguments, + }); + }, + // expect(await ). + // expect(). + [`CallExpression[callee.object.callee.name='expect'][callee.property.name=${alternativeMatchers}], CallExpression[callee.object.callee.name='expect'][callee.object.arguments.0.argument.callee.name=${alternativeMatchers}]`]( node ) { - const queryNode = node.callee.object.arguments[0].callee; + const arg = node.callee.object.arguments[0]; + const queryNode = + arg.type === "AwaitExpression" ? arg.argument.callee : arg.callee; const matcherNode = node.callee.property; const matcherArguments = node.arguments; - check(context, { + check({ negatedMatcher: false, queryNode, matcherNode,