From 32352667f13c0de0f3422475f36d6c5318f9d62c Mon Sep 17 00:00:00 2001 From: Ben Monro Date: Fri, 27 Nov 2020 15:23:27 -0800 Subject: [PATCH 01/11] feat(prefer-in-document): add support for assigments --- src/__tests__/lib/rules/prefer-in-document.js | 38 +++++++++++++- src/rules/prefer-in-document.js | 52 ++++++++++++++++--- 2 files changed, 81 insertions(+), 9 deletions(-) diff --git a/src/__tests__/lib/rules/prefer-in-document.js b/src/__tests__/lib/rules/prefer-in-document.js index 917a619..409e615 100644 --- a/src/__tests__/lib/rules/prefer-in-document.js +++ b/src/__tests__/lib/rules/prefer-in-document.js @@ -32,9 +32,13 @@ const valid = [ `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);`, ]), `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 @@ -51,6 +55,38 @@ 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) => [ diff --git a/src/rules/prefer-in-document.js b/src/rules/prefer-in-document.js index fd06149..839c8eb 100644 --- a/src/rules/prefer-in-document.js +++ b/src/rules/prefer-in-document.js @@ -31,13 +31,13 @@ function check( context, { queryNode, matcherNode, matcherArguments, negatedMatcher } ) { + if (!queryNode.name && !queryNode.property) return; 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, @@ -78,7 +78,7 @@ export const create = (context) => { const alternativeMatchers = /(toHaveLength|toBeDefined|toBeNull)/; return { - // Grabbing expect().not. + // expect().not. [`CallExpression[callee.object.object.callee.name='expect'][callee.object.property.name='not'][callee.property.name=${alternativeMatchers}]`]( node ) { @@ -93,14 +93,33 @@ export const create = (context) => { matcherArguments, }); }, - - // Grabbing expect(). - [`CallExpression[callee.object.callee.name='expect'][callee.property.name=${alternativeMatchers}]`]( + // const foo = expect(foo). + [`MemberExpression[object.callee.name=expect][property.name=${alternativeMatchers}][object.arguments.0.type=Identifier]`]( node ) { - const queryNode = node.callee.object.arguments[0].callee; - const matcherNode = node.callee.property; - const matcherArguments = node.arguments; + const variable = context + .getScope() + .set.get(node.object.arguments[0].name); + const init = variable.defs[0].node.init; + + let queryNode; + if (init) { + queryNode = init.callee.property || init.callee; + } else { + const assignmentRef = variable.references + .reverse() + .find((ref) => !!ref.writeExpr); + if (!assignmentRef) { + return; + } + queryNode = + assignmentRef.writeExpr.type === "CallExpression" + ? assignmentRef.writeExpr.callee + : assignmentRef.writeExpr; + } + const matcherNode = node.property; + + const matcherArguments = node.parent.arguments; check(context, { negatedMatcher: false, @@ -109,5 +128,22 @@ export const create = (context) => { matcherArguments, }); }, + // expect(). + [`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; + + if (queryNode) { + check(context, { + negatedMatcher: false, + queryNode, + matcherNode, + matcherArguments, + }); + } + }, }; }; From 973f37537852e22a615735bc7ce993a24ad94139 Mon Sep 17 00:00:00 2001 From: Ben Monro Date: Fri, 27 Nov 2020 15:27:44 -0800 Subject: [PATCH 02/11] added coverage --- src/__tests__/lib/rules/prefer-in-document.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/__tests__/lib/rules/prefer-in-document.js b/src/__tests__/lib/rules/prefer-in-document.js index 409e615..ca10308 100644 --- a/src/__tests__/lib/rules/prefer-in-document.js +++ b/src/__tests__/lib/rules/prefer-in-document.js @@ -36,6 +36,11 @@ const valid = [ foo = screen.${q}('foo'); foo = somethingElse; expect(foo).toHaveLength(1);`, + `let foo; + foo = "bar"; + expect(foo).toHaveLength(1);`, + `let foo; + expect(foo).toHaveLength(1);`, ]), `expect(screen.notAQuery('foo-bar')).toHaveLength(1)`, `expect(screen.getAllByText('foo-bar')).toHaveLength(2)`, From 3d1b4106369950908d75561b28b3e3040f9b865a Mon Sep 17 00:00:00 2001 From: Ben Monro Date: Fri, 27 Nov 2020 17:51:46 -0800 Subject: [PATCH 03/11] added not use case --- package.json | 3 +- src/__tests__/lib/rules/prefer-in-document.js | 35 ++++++- src/rules/prefer-in-document.js | 92 ++++++++++++------- 3 files changed, 90 insertions(+), 40 deletions(-) diff --git a/package.json b/package.json index d083277..2c319b5 100644 --- a/package.json +++ b/package.json @@ -55,7 +55,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 ca10308..20be7a8 100644 --- a/src/__tests__/lib/rules/prefer-in-document.js +++ b/src/__tests__/lib/rules/prefer-in-document.js @@ -36,12 +36,15 @@ const valid = [ foo = screen.${q}('foo'); foo = somethingElse; expect(foo).toHaveLength(1);`, - `let foo; - foo = "bar"; - expect(foo).toHaveLength(1);`, - `let foo; - 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.getAllByText('foo-bar')).toHaveLength(2)`, ]; @@ -115,6 +118,28 @@ const invalid = [ `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();` + ), ]), ]; diff --git a/src/rules/prefer-in-document.js b/src/rules/prefer-in-document.js index 839c8eb..e4007a2 100644 --- a/src/rules/prefer-in-document.js +++ b/src/rules/prefer-in-document.js @@ -31,13 +31,14 @@ function check( context, { queryNode, matcherNode, matcherArguments, negatedMatcher } ) { - if (!queryNode.name && !queryNode.property) return; - const query = queryNode.name || queryNode.property.name; - + 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, @@ -64,7 +65,7 @@ function check( operations.push(fixer.replaceText(matcherNode, "toBeInTheDocument")); // Remove any arguments in the matcher - for (const argument of matcherArguments) { + for (const argument of Array.from(matcherArguments)) { operations.push(fixer.remove(argument)); } @@ -76,7 +77,30 @@ function check( 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.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 === "CallExpression" + ? assignmentRef.writeExpr.callee + : assignmentRef.writeExpr; + } + return queryNode; + } return { // expect().not. [`CallExpression[callee.object.object.callee.name='expect'][callee.object.property.name='not'][callee.property.name=${alternativeMatchers}]`]( @@ -84,7 +108,26 @@ export const create = (context) => { ) { const queryNode = node.callee.object.object.arguments[0].callee; const matcherNode = node.callee.property; - const matcherArguments = node.arguments; + const matcherArguments = node; + + check(context, { + negatedMatcher: true, + queryNode, + matcherNode, + matcherArguments, + }); + }, + + // // 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; check(context, { negatedMatcher: true, @@ -97,26 +140,9 @@ export const create = (context) => { [`MemberExpression[object.callee.name=expect][property.name=${alternativeMatchers}][object.arguments.0.type=Identifier]`]( node ) { - const variable = context - .getScope() - .set.get(node.object.arguments[0].name); - const init = variable.defs[0].node.init; - - let queryNode; - if (init) { - queryNode = init.callee.property || init.callee; - } else { - const assignmentRef = variable.references - .reverse() - .find((ref) => !!ref.writeExpr); - if (!assignmentRef) { - return; - } - queryNode = - assignmentRef.writeExpr.type === "CallExpression" - ? assignmentRef.writeExpr.callee - : assignmentRef.writeExpr; - } + const queryNode = getQueryNodeFromAssignment( + node.object.arguments[0].name + ); const matcherNode = node.property; const matcherArguments = node.parent.arguments; @@ -136,14 +162,12 @@ export const create = (context) => { const matcherNode = node.callee.property; const matcherArguments = node.arguments; - if (queryNode) { - check(context, { - negatedMatcher: false, - queryNode, - matcherNode, - matcherArguments, - }); - } + check(context, { + negatedMatcher: false, + queryNode, + matcherNode, + matcherArguments, + }); }, }; }; From 8eae96d1bac96a6a4f8de4b41ddc097846413126 Mon Sep 17 00:00:00 2001 From: Ben Monro Date: Fri, 27 Nov 2020 17:54:37 -0800 Subject: [PATCH 04/11] spacing --- src/__tests__/lib/rules/prefer-in-document.js | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/__tests__/lib/rules/prefer-in-document.js b/src/__tests__/lib/rules/prefer-in-document.js index 20be7a8..4e76a21 100644 --- a/src/__tests__/lib/rules/prefer-in-document.js +++ b/src/__tests__/lib/rules/prefer-in-document.js @@ -64,14 +64,10 @@ const invalid = [ `expect(wrapper.${q}('foo')).toBeInTheDocument()` ), invalidCase( - ` - const foo = screen.${q}('foo'); - expect(foo).toHaveLength(1); - `, - ` - const foo = screen.${q}('foo'); - expect(foo).toBeInTheDocument(); - ` + `const foo = screen.${q}('foo'); + expect(foo).toHaveLength(1);`, + `const foo = screen.${q}('foo'); + expect(foo).toBeInTheDocument();` ), invalidCase( `const foo = ${q}('foo'); From 24894c140ce901ef476f8219c12be936ca2b606f Mon Sep 17 00:00:00 2001 From: Ben Monro Date: Fri, 27 Nov 2020 17:58:29 -0800 Subject: [PATCH 05/11] updated docs --- docs/rules/prefer-in-document.md | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/docs/rules/prefer-in-document.md b/docs/rules/prefer-in-document.md index 394a390..e4559c4 100644 --- a/docs/rules/prefer-in-document.md +++ b/docs/rules/prefer-in-document.md @@ -17,6 +17,12 @@ 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: @@ -24,10 +30,16 @@ 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(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.toBeInTheDocument(); ``` ## When Not To Use It From 5a3f7c187d6d000bd5a7879b82708c96ee50dcae Mon Sep 17 00:00:00 2001 From: Ben Monro Date: Sat, 28 Nov 2020 09:32:22 -0800 Subject: [PATCH 06/11] fixed spelling mistake --- src/rules/prefer-in-document.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rules/prefer-in-document.js b/src/rules/prefer-in-document.js index e4007a2..1c9f970 100644 --- a/src/rules/prefer-in-document.js +++ b/src/rules/prefer-in-document.js @@ -47,7 +47,7 @@ function check( fix(fixer) { const operations = []; - // Flip the .not if neccessary + // Flip the .not if necessary if (isAntonymMatcher(matcherNode, matcherArguments)) { if (negatedMatcher) { operations.push( From 0249708cfb0f232f56eb12ff09fc7bf8b82729c7 Mon Sep 17 00:00:00 2001 From: Ben Monro Date: Sat, 28 Nov 2020 11:13:32 -0800 Subject: [PATCH 07/11] fixed bug with not handling --- src/__tests__/lib/rules/prefer-in-document.js | 10 +++++-- src/rules/prefer-in-document.js | 28 +++++++++++-------- 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/src/__tests__/lib/rules/prefer-in-document.js b/src/__tests__/lib/rules/prefer-in-document.js index 4e76a21..376dfda 100644 --- a/src/__tests__/lib/rules/prefer-in-document.js +++ b/src/__tests__/lib/rules/prefer-in-document.js @@ -106,13 +106,17 @@ 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; @@ -125,7 +129,7 @@ const invalid = [ invalidCase( `let foo; foo = screen.${q}('foo'); - expect(foo).not.toBeNull();`, + expect(foo) .not.toBeNull();`, `let foo; foo = screen.${q}('foo'); expect(foo).toBeInTheDocument();` diff --git a/src/rules/prefer-in-document.js b/src/rules/prefer-in-document.js index 1c9f970..a9630af 100644 --- a/src/rules/prefer-in-document.js +++ b/src/rules/prefer-in-document.js @@ -29,7 +29,7 @@ function isAntonymMatcher(matcherNode, matcherArguments) { function check( context, - { queryNode, matcherNode, matcherArguments, negatedMatcher } + { queryNode, matcherNode, matcherArguments, negatedMatcher, expect } ) { if (!queryNode || (!queryNode.name && !queryNode.property)) return; // toHaveLength() is only invalid with 0 or 1 @@ -47,15 +47,21 @@ function check( 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.removeRange([ - matcherNode.range[0] - 5, - matcherNode.range[0] - 1, - ]) + fixer.replaceTextRange( + [expect.range[1], matcherNode.range[1]], + ".toBeInTheDocument" + ) ); + + return operations; } else { operations.push(fixer.insertTextBefore(matcherNode, "not.")); } @@ -64,11 +70,6 @@ function check( // Replace the actual matcher operations.push(fixer.replaceText(matcherNode, "toBeInTheDocument")); - // Remove any arguments in the matcher - for (const argument of Array.from(matcherArguments)) { - operations.push(fixer.remove(argument)); - } - return operations; }, }); @@ -108,13 +109,15 @@ export const create = (context) => { ) { const queryNode = node.callee.object.object.arguments[0].callee; const matcherNode = node.callee.property; - const matcherArguments = node; + const matcherArguments = node.arguments; + const expect = node.callee.object.object; check(context, { negatedMatcher: true, queryNode, matcherNode, matcherArguments, + expect, }); }, @@ -129,11 +132,14 @@ export const create = (context) => { const matcherArguments = node.parent.arguments; + const expect = node.object.object; + check(context, { negatedMatcher: true, queryNode, matcherNode, matcherArguments, + expect, }); }, // const foo = expect(foo). From b2330fcbb3f0da4283fc2afefc2b5c176ecd83be Mon Sep 17 00:00:00 2001 From: Ben Monro Date: Sun, 29 Nov 2020 09:46:28 -0800 Subject: [PATCH 08/11] support awaits --- .github/ISSUE_TEMPLATE.md | 1 + README.md | 24 +++++----- other/MAINTAINING.md | 3 +- src/__tests__/lib/rules/prefer-in-document.js | 47 +++++++++++++++++-- .../lib/rules/prefer-to-have-style.js | 2 +- src/queries.js | 6 --- src/rules/prefer-in-document.js | 16 +++++-- 7 files changed, 70 insertions(+), 29 deletions(-) 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 78e3803..6072522 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/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/src/__tests__/lib/rules/prefer-in-document.js b/src/__tests__/lib/rules/prefer-in-document.js index 376dfda..0b0e71b 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,7 +27,7 @@ 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()`, @@ -50,7 +49,7 @@ const valid = [ ]; const invalid = [ // Invalid cases that applies to all variants - ...queries.map((q) => [ + ...["getByText", "getByRole"].map((q) => [ invalidCase( `expect(screen.${q}('foo')).toHaveLength(1)`, `expect(screen.${q}('foo')).toBeInTheDocument()` @@ -93,7 +92,7 @@ const invalid = [ ), ]), // Invalid cases that applies to queryBy* and queryAllBy* - ...queriesByVariant.query.map((q) => [ + ...["queryByText"].map((q) => [ invalidCase( `expect(${q}('foo')).toHaveLength(0)`, `expect(${q}('foo')).not.toBeInTheDocument()` @@ -141,9 +140,47 @@ const invalid = [ 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 screen.findByText(/Compressing video/)).toBeDefined(); + })`, + `it("foo", async () => { + expect(await screen.findByText(/Compressing video/)).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 () => { + 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(); + });` + ), ]; -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 a9630af..812a6fa 100644 --- a/src/rules/prefer-in-document.js +++ b/src/rules/prefer-in-document.js @@ -85,7 +85,10 @@ export const create = (context) => { let queryNode; if (init) { // let foo = screen.(); - queryNode = init.callee.property || init.callee; + queryNode = + init.type === "AwaitExpression" + ? init.argument.callee.property + : init.callee.property || init.callee; } else { // let foo; // foo = screen.(); @@ -96,7 +99,9 @@ export const create = (context) => { return; } queryNode = - assignmentRef.writeExpr.type === "CallExpression" + assignmentRef.writeExpr.type === "AwaitExpression" + ? assignmentRef.writeExpr.argument.callee + : assignmentRef.writeExpr.type === "CallExpression" ? assignmentRef.writeExpr.callee : assignmentRef.writeExpr; } @@ -160,11 +165,14 @@ export const create = (context) => { matcherArguments, }); }, + // expect(await ). // expect(). - [`CallExpression[callee.object.callee.name='expect'][callee.property.name=${alternativeMatchers}]`]( + [`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; From ae23678d165798a8bfc09840d4b5a19afc8de797 Mon Sep 17 00:00:00 2001 From: Ben Monro Date: Sun, 29 Nov 2020 10:45:48 -0800 Subject: [PATCH 09/11] more async support --- src/__tests__/lib/rules/prefer-in-document.js | 42 ++++++++++++++++++- src/rules/prefer-in-document.js | 6 ++- 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/src/__tests__/lib/rules/prefer-in-document.js b/src/__tests__/lib/rules/prefer-in-document.js index 0b0e71b..6b659f6 100644 --- a/src/__tests__/lib/rules/prefer-in-document.js +++ b/src/__tests__/lib/rules/prefer-in-document.js @@ -49,7 +49,7 @@ const valid = [ ]; const invalid = [ // Invalid cases that applies to all variants - ...["getByText", "getByRole"].map((q) => [ + ...["getByText", "getAllByRole"].map((q) => [ invalidCase( `expect(screen.${q}('foo')).toHaveLength(1)`, `expect(screen.${q}('foo')).toBeInTheDocument()` @@ -92,7 +92,7 @@ const invalid = [ ), ]), // Invalid cases that applies to queryBy* and queryAllBy* - ...["queryByText"].map((q) => [ + ...["queryByText", "queryAllByText"].map((q) => [ invalidCase( `expect(${q}('foo')).toHaveLength(0)`, `expect(${q}('foo')).not.toBeInTheDocument()` @@ -148,6 +148,14 @@ const invalid = [ 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(); @@ -156,6 +164,14 @@ const invalid = [ 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/); @@ -166,6 +182,16 @@ const invalid = [ 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; @@ -178,6 +204,18 @@ const invalid = [ 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: 2017 } }); diff --git a/src/rules/prefer-in-document.js b/src/rules/prefer-in-document.js index 812a6fa..b807e9a 100644 --- a/src/rules/prefer-in-document.js +++ b/src/rules/prefer-in-document.js @@ -109,10 +109,12 @@ export const create = (context) => { } return { // expect().not. - [`CallExpression[callee.object.object.callee.name='expect'][callee.object.property.name='not'][callee.property.name=${alternativeMatchers}]`]( + [`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; From 9c845086b1350ecbca2182ba9f0feea180736521 Mon Sep 17 00:00:00 2001 From: Ben Monro Date: Sun, 29 Nov 2020 10:49:29 -0800 Subject: [PATCH 10/11] move check into create --- src/rules/prefer-in-document.js | 101 ++++++++++++++++---------------- 1 file changed, 52 insertions(+), 49 deletions(-) diff --git a/src/rules/prefer-in-document.js b/src/rules/prefer-in-document.js index b807e9a..3c62cbc 100644 --- a/src/rules/prefer-in-document.js +++ b/src/rules/prefer-in-document.js @@ -27,57 +27,60 @@ function isAntonymMatcher(matcherNode, matcherArguments) { ); } -function check( - context, - { 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; - } +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 = []; - 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 = []; - - // 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" - ) - ); - - return operations; - } else { - operations.push(fixer.insertTextBefore(matcherNode, "not.")); + // 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" + ) + ); + + return operations; + } else { + operations.push(fixer.insertTextBefore(matcherNode, "not.")); + } } - } - // Replace the actual matcher - operations.push(fixer.replaceText(matcherNode, "toBeInTheDocument")); + // 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; @@ -119,7 +122,7 @@ export const create = (context) => { const matcherArguments = node.arguments; const expect = node.callee.object.object; - check(context, { + check({ negatedMatcher: true, queryNode, matcherNode, @@ -141,7 +144,7 @@ export const create = (context) => { const expect = node.object.object; - check(context, { + check({ negatedMatcher: true, queryNode, matcherNode, @@ -160,7 +163,7 @@ export const create = (context) => { const matcherArguments = node.parent.arguments; - check(context, { + check({ negatedMatcher: false, queryNode, matcherNode, @@ -178,7 +181,7 @@ export const create = (context) => { const matcherNode = node.callee.property; const matcherArguments = node.arguments; - check(context, { + check({ negatedMatcher: false, queryNode, matcherNode, From e55deabd2dfbbae95907d416dff8947a0e3f0d34 Mon Sep 17 00:00:00 2001 From: Ben Monro Date: Mon, 30 Nov 2020 13:34:45 -0800 Subject: [PATCH 11/11] Apply suggestions from code review --- docs/rules/prefer-in-document.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/docs/rules/prefer-in-document.md b/docs/rules/prefer-in-document.md index e4559c4..7043fc3 100644 --- a/docs/rules/prefer-in-document.md +++ b/docs/rules/prefer-in-document.md @@ -29,7 +29,7 @@ Examples of **correct** code for this rule: ```js expect(screen.queryByText("foo")).toBeInTheDocument(); -expect(screen.queryByText("foo")).toBeInTheDocument(); +expect(await screen.findByText("foo")).toBeInTheDocument(); expect(queryByText("foo")).toBeInTheDocument(); expect(wrapper.queryAllByTestId("foo")).toBeInTheDocument(); expect(screen.getAllByLabel("foo-bar")).toHaveLength(2); @@ -39,7 +39,10 @@ const foo = screen.getAllByText("foo"); expect(foo).toHaveLength(3); const bar = screen.queryByText("bar"); -expect(bar).not.toBeInTheDocument(); +expect(bar).not.toBeDefined(); + +const baz = await screen.findByText("baz"); +expect(baz).toBeDefined() ``` ## When Not To Use It