From 160426ea7127000ff3e642350ecb8e2e46e2091f Mon Sep 17 00:00:00 2001 From: Doma Date: Sun, 9 Oct 2022 04:46:09 +0000 Subject: [PATCH 1/9] fix(prefer-in-document): make toHaveLength(1) with AllBy queries valid --- src/__tests__/lib/rules/prefer-in-document.js | 68 +++++-------------- src/rules/prefer-in-document.js | 51 ++++++++++++-- 2 files changed, 63 insertions(+), 56 deletions(-) diff --git a/src/__tests__/lib/rules/prefer-in-document.js b/src/__tests__/lib/rules/prefer-in-document.js index adc3a8b..de3e047 100644 --- a/src/__tests__/lib/rules/prefer-in-document.js +++ b/src/__tests__/lib/rules/prefer-in-document.js @@ -101,6 +101,23 @@ const valid = [ ` const element = getByText('value') expect(element).toBeInTheDocument`, + + // *AllBy queries with `.toHaveLength(1)` is valid + // see conclusion at https://github.com/testing-library/eslint-plugin-jest-dom/issues/171#issuecomment-895074086 + `expect(screen.getAllByRole('foo')).toHaveLength(1)`, + `expect(await screen.findAllByRole('foo')).toHaveLength(1)`, + `expect(getAllByRole('foo')).toHaveLength(1)`, + `expect(wrapper.getAllByRole('foo')).toHaveLength(1)`, + `const foo = screen.getAllByRole('foo'); + expect(foo).toHaveLength(1);`, + `const foo = getAllByRole('foo'); + expect(foo).toHaveLength(1);`, + `let foo; + foo = getAllByRole('foo'); + expect(foo).toHaveLength(1);`, + `let foo; + foo = screen.getAllByRole('foo'); + expect(foo).toHaveLength(1);`, ]; const invalid = [ invalidCase( @@ -230,51 +247,6 @@ const invalid = [ foo = screen.getByText('foo'); expect(foo).toBeInTheDocument();` ), - invalidCase( - `expect(screen.getAllByRole('foo')).toHaveLength(1)`, - `expect(screen.getByRole('foo')).toBeInTheDocument()` - ), - invalidCase( - `expect(await screen.findAllByRole('foo')).toHaveLength(1)`, - `expect(await screen.findByRole('foo')).toBeInTheDocument()` - ), - invalidCase( - `expect(getAllByRole('foo')).toHaveLength(1)`, - `expect(getByRole('foo')).toBeInTheDocument()` - ), - invalidCase( - `expect(wrapper.getAllByRole('foo')).toHaveLength(1)`, - `expect(wrapper.getByRole('foo')).toBeInTheDocument()` - ), - invalidCase( - `const foo = screen.getAllByRole('foo'); - expect(foo).toHaveLength(1);`, - `const foo = screen.getByRole('foo'); - expect(foo).toBeInTheDocument();` - ), - invalidCase( - `const foo = getAllByRole('foo'); - expect(foo).toHaveLength(1);`, - `const foo = getByRole('foo'); - expect(foo).toBeInTheDocument();` - ), - invalidCase( - `let foo; - foo = getAllByRole('foo'); - expect(foo).toHaveLength(1);`, - `let foo; - foo = getByRole('foo'); - expect(foo).toBeInTheDocument();` - ), - invalidCase( - `let foo; - foo = screen.getAllByRole('foo'); - expect(foo).toHaveLength(1);`, - `let foo; - foo = screen.getByRole('foo'); - expect(foo).toBeInTheDocument();` - ), - invalidCase( `expect(screen.getByText('foo')).toHaveLength(1)`, `expect(screen.getByText('foo')).toBeInTheDocument()` @@ -541,12 +513,6 @@ const invalid = [ span = getByText('foo') as HTMLSpanElement expect(span).toBeInTheDocument()` ), - invalidCase( - `const things = screen.getAllByText("foo"); - expect(things).toHaveLength(1);`, - `const things = screen.getByText("foo"); - expect(things).toBeInTheDocument();` - ), ]; const ruleTester = new RuleTester({ diff --git a/src/rules/prefer-in-document.js b/src/rules/prefer-in-document.js index d9a5618..1afae25 100644 --- a/src/rules/prefer-in-document.js +++ b/src/rules/prefer-in-document.js @@ -48,6 +48,24 @@ function usesToHaveLengthZero(matcherNode, matcherArguments) { ); } +/** + * Extract the DTL query identifier from a call expression + * + * () -> + * screen.() -> + */ +function getDTLQueryIdentifierNode(callExpressionNode) { + if (!callExpressionNode || callExpressionNode.type !== "CallExpression") { + return; + } + + if (callExpressionNode.callee.type === "Identifier") { + return callExpressionNode.callee; + } + + return callExpressionNode.callee.property; +} + export const create = (context) => { const alternativeMatchers = /^(toHaveLength|toBeDefined|toBeNull|toBe|toEqual|toBeTruthy|toBeFalsy)$/; @@ -89,7 +107,12 @@ export const create = (context) => { // isNotToHaveLengthZero represents .not.toHaveLength(0) which is a valid use of toHaveLength const isNotToHaveLengthZero = usesToHaveLengthZero(matcherNode, matcherArguments) && negatedMatcher; + const isValidUseOfToHaveLength = + // .toHaveLength(1) is valid when used with *AllBy* queries + // meaning checking for exactly one match + // see discussion https://github.com/testing-library/eslint-plugin-jest-dom/issues/171 + (lengthValue === 1 && /AllBy/.test(queryNode.name)) || isNotToHaveLengthZero || !["Literal", "Identifier"].includes(matcherArguments[0].type) || lengthValue === undefined || @@ -195,6 +218,12 @@ export const create = (context) => { context, node.object.object.arguments[0].name ); + + // Not an RTL query + if (!queryNode || queryNode.type !== "CallExpression") { + return; + } + const matcherNode = node.property; const matcherArguments = node.parent.arguments; @@ -212,16 +241,25 @@ export const create = (context) => { [`MemberExpression[object.callee.name=expect][property.name=${alternativeMatchers}][object.arguments.0.type=Identifier]`]( node ) { - const queryNode = getAssignmentForIdentifier( + // Value expression being assigned to the left-hand value + const rightValueNode = getAssignmentForIdentifier( context, node.object.arguments[0].name ); + + // Not a DTL query + if (!rightValueNode || rightValueNode.type !== "CallExpression") { + return; + } + + const queryIdentifierNode = getDTLQueryIdentifierNode(rightValueNode); + const matcherNode = node.property; const matcherArguments = node.parent.arguments; check({ negatedMatcher: false, - queryNode: (queryNode && queryNode.callee) || queryNode, + queryNode: queryIdentifierNode, matcherNode, matcherArguments, }); @@ -237,14 +275,17 @@ export const create = (context) => { return; } - const queryNode = - arg.type === "AwaitExpression" ? arg.argument.callee : arg.callee; + const queryIdentifierNode = + arg.type === "AwaitExpression" + ? getDTLQueryIdentifierNode(arg.argument) + : getDTLQueryIdentifierNode(arg); + const matcherNode = node.callee.property; const matcherArguments = node.arguments; check({ negatedMatcher: false, - queryNode, + queryNode: queryIdentifierNode, matcherNode, matcherArguments, }); From bc2aa4d8785fbedcc1ead905f7a24b0b6f82d870 Mon Sep 17 00:00:00 2001 From: Doma Date: Sun, 9 Oct 2022 07:01:33 +0000 Subject: [PATCH 2/9] fix(prefer-in-document): make toHaveLength(0) with AllBy queries valid --- src/__tests__/lib/rules/prefer-in-document.js | 26 +++++++------------ src/rules/prefer-in-document.js | 11 +++++--- 2 files changed, 16 insertions(+), 21 deletions(-) diff --git a/src/__tests__/lib/rules/prefer-in-document.js b/src/__tests__/lib/rules/prefer-in-document.js index de3e047..bdcf2a4 100644 --- a/src/__tests__/lib/rules/prefer-in-document.js +++ b/src/__tests__/lib/rules/prefer-in-document.js @@ -102,7 +102,7 @@ const valid = [ const element = getByText('value') expect(element).toBeInTheDocument`, - // *AllBy queries with `.toHaveLength(1)` is valid + // *AllBy* queries with `.toHaveLength(1)` is valid // see conclusion at https://github.com/testing-library/eslint-plugin-jest-dom/issues/171#issuecomment-895074086 `expect(screen.getAllByRole('foo')).toHaveLength(1)`, `expect(await screen.findAllByRole('foo')).toHaveLength(1)`, @@ -118,6 +118,14 @@ const valid = [ `let foo; foo = screen.getAllByRole('foo'); expect(foo).toHaveLength(1);`, + + // *AllBy* queries with `.toHaveLength(0)` is valid + // see conclusion at https://github.com/testing-library/eslint-plugin-jest-dom/issues/171#issuecomment-895074086 + `expect(screen.queryAllByTestId("foo")).toHaveLength(0)`, + `expect(queryAllByText('foo')).toHaveLength(0)`, + `let foo; + foo = screen.queryAllByText('foo'); + expect(foo).toHaveLength(0);`, ]; const invalid = [ invalidCase( @@ -207,10 +215,6 @@ const invalid = [ expect(screen.getByText('foo')).toBeInTheDocument()` ), - invalidCase( - `expect(screen.queryAllByTestId("foo")).toHaveLength(0)`, - `expect(screen.queryByTestId("foo")).not.toBeInTheDocument()` - ), invalidCase( `expect(getByText('foo')).toHaveLength(1)`, `expect(getByText('foo')).toBeInTheDocument()` @@ -369,10 +373,6 @@ const invalid = [ expect(foo).toBeInTheDocument();` ), - invalidCase( - `expect(queryAllByText('foo')).toHaveLength(0)`, - `expect(queryByText('foo')).not.toBeInTheDocument()` - ), invalidCase( `expect(queryAllByText('foo')).toBeNull()`, `expect(queryByText('foo')).not.toBeInTheDocument()` @@ -393,14 +393,6 @@ const invalid = [ `expect(queryAllByText('foo')) .not .toBeDefined()`, `expect(queryByText('foo')) .not .toBeInTheDocument()` ), - invalidCase( - `let foo; - foo = screen.queryAllByText('foo'); - expect(foo).toHaveLength(0);`, - `let foo; - foo = screen.queryByText('foo'); - expect(foo).not.toBeInTheDocument();` - ), invalidCase( `let foo; foo = screen.queryAllByText('foo'); diff --git a/src/rules/prefer-in-document.js b/src/rules/prefer-in-document.js index 1afae25..d7f18ec 100644 --- a/src/rules/prefer-in-document.js +++ b/src/rules/prefer-in-document.js @@ -108,11 +108,14 @@ export const create = (context) => { const isNotToHaveLengthZero = usesToHaveLengthZero(matcherNode, matcherArguments) && negatedMatcher; + // .toHaveLength(1)/.toHaveLength(0) is valid when used with *AllBy* queries + // meaning checking for exactly one/zero match + // see discussion https://github.com/testing-library/eslint-plugin-jest-dom/issues/171 + const isValidUsageWithAllByQueries = + /AllBy/.test(queryNode.name) && [1, 0].includes(lengthValue); + const isValidUseOfToHaveLength = - // .toHaveLength(1) is valid when used with *AllBy* queries - // meaning checking for exactly one match - // see discussion https://github.com/testing-library/eslint-plugin-jest-dom/issues/171 - (lengthValue === 1 && /AllBy/.test(queryNode.name)) || + isValidUsageWithAllByQueries || isNotToHaveLengthZero || !["Literal", "Identifier"].includes(matcherArguments[0].type) || lengthValue === undefined || From c3562fbbe6e8aa45a3f3155306b9789e3db021a0 Mon Sep 17 00:00:00 2001 From: Doma Date: Sun, 9 Oct 2022 08:19:40 +0000 Subject: [PATCH 3/9] feat(prefer-in-document): give suggestions when reporting invalid combination of *By* query with .toHaveLength(1) assertion --- src/__tests__/lib/rules/prefer-in-document.js | 134 +++++++++++------- src/rules/prefer-in-document.js | 75 +++++++--- 2 files changed, 142 insertions(+), 67 deletions(-) diff --git a/src/__tests__/lib/rules/prefer-in-document.js b/src/__tests__/lib/rules/prefer-in-document.js index bdcf2a4..1b2ecda 100644 --- a/src/__tests__/lib/rules/prefer-in-document.js +++ b/src/__tests__/lib/rules/prefer-in-document.js @@ -26,6 +26,33 @@ function invalidCase(code, output) { }; } +function invalidCaseWithSuggestions( + code, + messageData, + replaceQueryOutput, + replaceAssertionOutput +) { + return { + code, + errors: [ + { + messageId: "invalid-combination-length-1", + data: messageData, + suggestions: [ + { + desc: `Replace ${messageData.query} with ${messageData.allQuery}`, + output: replaceQueryOutput, + }, + { + desc: "Replace .toHaveLength(1) with .toBeInTheDocument()", + output: replaceAssertionOutput, + }, + ], + }, + ], + }; +} + const valid = [ "expect().toBe(true)", ...["getByText", "getByRole"].map((q) => [ @@ -204,93 +231,104 @@ const invalid = [ `expect(screen.getAllByRole('foo')).toHaveLength(1,2,/*comment*/3,)`, `expect(screen.getByRole('foo')).toBeInTheDocument(/*comment*/)` ), - invalidCase( + // Report invalid combination of *By* query with .toHaveLength(1) assertion + // and suggest fixes by: + // - Replacing *By* with *AllBy* query + // - Replacing .toHaveLength(1) with .toBeInTheDocument() assertion + invalidCaseWithSuggestions( `expect(screen.getByText('foo')).toHaveLength(1)`, + { + query: "getByText", + allQuery: "getAllByText", + }, + `expect(screen.getAllByText('foo')).toHaveLength(1)`, `expect(screen.getByText('foo')).toBeInTheDocument()` ), - invalidCase( + invalidCaseWithSuggestions( `const NUM_BUTTONS=1; expect(screen.getByText('foo')).toHaveLength(NUM_BUTTONS)`, + { + query: "getByText", + allQuery: "getAllByText", + }, + `const NUM_BUTTONS=1; + expect(screen.getAllByText('foo')).toHaveLength(NUM_BUTTONS)`, `const NUM_BUTTONS=1; expect(screen.getByText('foo')).toBeInTheDocument()` ), - invalidCase( + invalidCaseWithSuggestions( `expect(getByText('foo')).toHaveLength(1)`, + { + query: "getByText", + allQuery: "getAllByText", + }, + `expect(getAllByText('foo')).toHaveLength(1)`, `expect(getByText('foo')).toBeInTheDocument()` ), - invalidCase( + invalidCaseWithSuggestions( `expect(wrapper.getByText('foo')).toHaveLength(1)`, + { + query: "getByText", + allQuery: "getAllByText", + }, + `expect(wrapper.getAllByText('foo')).toHaveLength(1)`, `expect(wrapper.getByText('foo')).toBeInTheDocument()` ), - invalidCase( + invalidCaseWithSuggestions( `const foo = screen.getByText('foo'); expect(foo).toHaveLength(1);`, + { + query: "getByText", + allQuery: "getAllByText", + }, + `const foo = screen.getAllByText('foo'); + expect(foo).toHaveLength(1);`, `const foo = screen.getByText('foo'); expect(foo).toBeInTheDocument();` ), - invalidCase( + invalidCaseWithSuggestions( `const foo = getByText('foo'); expect(foo).toHaveLength(1);`, + { + query: "getByText", + allQuery: "getAllByText", + }, + `const foo = getAllByText('foo'); + expect(foo).toHaveLength(1);`, `const foo = getByText('foo'); expect(foo).toBeInTheDocument();` ), - invalidCase( + invalidCaseWithSuggestions( `let foo; foo = getByText('foo'); expect(foo).toHaveLength(1);`, + { + query: "getByText", + allQuery: "getAllByText", + }, + `let foo; + foo = getAllByText('foo'); + expect(foo).toHaveLength(1);`, `let foo; foo = getByText('foo'); expect(foo).toBeInTheDocument();` ), - invalidCase( + invalidCaseWithSuggestions( `let foo; foo = screen.getByText('foo'); expect(foo).toHaveLength(1);`, + { + query: "getByText", + allQuery: "getAllByText", + }, + `let foo; + foo = screen.getAllByText('foo'); + expect(foo).toHaveLength(1);`, `let foo; foo = screen.getByText('foo'); expect(foo).toBeInTheDocument();` ), - invalidCase( - `expect(screen.getByText('foo')).toHaveLength(1)`, - `expect(screen.getByText('foo')).toBeInTheDocument()` - ), - invalidCase( - `expect(getByText('foo')).toHaveLength(1)`, - `expect(getByText('foo')).toBeInTheDocument()` - ), - invalidCase( - `expect(wrapper.getByText('foo')).toHaveLength(1)`, - `expect(wrapper.getByText('foo')).toBeInTheDocument()` - ), - invalidCase( - `const foo = screen.getByText('foo'); - expect(foo).toHaveLength(1);`, - `const foo = screen.getByText('foo'); - expect(foo).toBeInTheDocument();` - ), - invalidCase( - `const foo = getByText('foo'); - expect(foo).toHaveLength(1);`, - `const foo = getByText('foo'); - expect(foo).toBeInTheDocument();` - ), - invalidCase( - `let foo; - foo = getByText('foo'); - expect(foo).toHaveLength(1);`, - `let foo; - foo = getByText('foo'); - expect(foo).toBeInTheDocument();` - ), - invalidCase( - `let foo; - foo = screen.getByText('foo'); - expect(foo).toHaveLength(1);`, - `let foo; - foo = screen.getByText('foo'); - expect(foo).toBeInTheDocument();` - ), // Invalid cases that applies to queryBy* and queryAllBy* diff --git a/src/rules/prefer-in-document.js b/src/rules/prefer-in-document.js index d7f18ec..68fc817 100644 --- a/src/rules/prefer-in-document.js +++ b/src/rules/prefer-in-document.js @@ -20,7 +20,9 @@ export const meta = { fixable: "code", messages: { "use-document": `Prefer .toBeInTheDocument() for asserting DOM node existence`, + "invalid-combination-length-1": `Invalid combination of {{query}} and .toHaveLength(1). Did you mean to use {{allQuery}}?`, }, + hasSuggestions: true, }; function isAntonymMatcher(matcherNode, matcherArguments) { @@ -101,29 +103,64 @@ export const create = (context) => { // only report on dom nodes which we can resolve to RTL queries. if (!queryNode || (!queryNode.name && !queryNode.property)) return; - // toHaveLength() is only invalid with 0 or 1 + // *By* query with .toHaveLength(0/1) assertions are considered violations + // + // | Selector type | .toHaveLength(1) | .toHaveLength(0) | + // | ============= | =========================== | ===================================== | + // | *By* query | Did you mean to use *AllBy* | Replace with .not.toBeInTheDocument() | + // | *AllBy* query | Correct | Correct + // + // @see https://github.com/testing-library/eslint-plugin-jest-dom/issues/171 + // if (matcherNode.name === "toHaveLength" && matcherArguments.length) { const lengthValue = getLengthValue(matcherArguments); - // isNotToHaveLengthZero represents .not.toHaveLength(0) which is a valid use of toHaveLength - const isNotToHaveLengthZero = - usesToHaveLengthZero(matcherNode, matcherArguments) && negatedMatcher; - - // .toHaveLength(1)/.toHaveLength(0) is valid when used with *AllBy* queries - // meaning checking for exactly one/zero match - // see discussion https://github.com/testing-library/eslint-plugin-jest-dom/issues/171 - const isValidUsageWithAllByQueries = - /AllBy/.test(queryNode.name) && [1, 0].includes(lengthValue); - - const isValidUseOfToHaveLength = - isValidUsageWithAllByQueries || - isNotToHaveLengthZero || - !["Literal", "Identifier"].includes(matcherArguments[0].type) || - lengthValue === undefined || - lengthValue > 1; - - if (isValidUseOfToHaveLength) { + const queryName = queryNode.name || queryNode.property.name; + + const isSingleQuery = + queries.includes(queryName) && !/AllBy/.test(queryName); + const hasViolation = isSingleQuery && [1, 0].includes(lengthValue); + + if (!hasViolation) { return; } + + // If length === 1, report violation with suggestions + // Otherwise fallback to default report + if (lengthValue === 1) { + const allQuery = queryName.replace("By", "AllBy"); + return context.report({ + node: matcherNode, + messageId: "invalid-combination-length-1", + data: { + query: queryName, + allQuery, + }, + loc: matcherNode.loc, + suggest: [ + { + desc: `Replace ${queryName} with ${allQuery}`, + fix(fixer) { + return fixer.replaceText( + queryNode.property || queryNode, + allQuery + ); + }, + }, + { + desc: "Replace .toHaveLength(1) with .toBeInTheDocument()", + fix(fixer) { + // Remove any arguments in the matcher + return [ + ...Array.from(matcherArguments).map((argument) => + fixer.remove(argument) + ), + fixer.replaceText(matcherNode, "toBeInTheDocument"), + ]; + }, + }, + ], + }); + } } // toBe() or toEqual() are only invalid with null From 6b626ae81bf83de662538006e19d3a35911671bf Mon Sep 17 00:00:00 2001 From: Doma Date: Sun, 9 Oct 2022 08:29:01 +0000 Subject: [PATCH 4/9] docs(prefer-in-document): add explanation for using .toHaveLength matcher with *AllBy* query --- docs/rules/prefer-in-document.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/rules/prefer-in-document.md b/docs/rules/prefer-in-document.md index a66848d..8e1924b 100644 --- a/docs/rules/prefer-in-document.md +++ b/docs/rules/prefer-in-document.md @@ -11,6 +11,7 @@ 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. +However it's considered OK to use `.toHaveLength(value)` matcher with `*AllBy*` queries. Examples of **incorrect** code for this rule: @@ -46,7 +47,7 @@ expect(screen.getByText("foo").length).toBe(1); expect(screen.queryByText("foo")).toBeInTheDocument(); expect(await screen.findByText("foo")).toBeInTheDocument(); expect(queryByText("foo")).toBeInTheDocument(); -expect(wrapper.queryAllByTestId("foo")).toBeInTheDocument(); +expect(wrapper.queryAllByTestId("foo")).toHaveLength(1); expect(screen.getAllByLabel("foo-bar")).toHaveLength(2); expect(notAQuery("foo-bar")).toHaveLength(1); From 155d0dc8892539dd0d498f4ccdc7dd3bde7e1b4e Mon Sep 17 00:00:00 2001 From: Doma Date: Sun, 9 Oct 2022 08:30:41 +0000 Subject: [PATCH 5/9] refactor(prefer-in-document): edit wording assertion -> matcher --- src/__tests__/lib/rules/prefer-in-document.js | 4 ++-- src/rules/prefer-in-document.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/__tests__/lib/rules/prefer-in-document.js b/src/__tests__/lib/rules/prefer-in-document.js index 1b2ecda..11a7365 100644 --- a/src/__tests__/lib/rules/prefer-in-document.js +++ b/src/__tests__/lib/rules/prefer-in-document.js @@ -30,7 +30,7 @@ function invalidCaseWithSuggestions( code, messageData, replaceQueryOutput, - replaceAssertionOutput + replaceMatcherOutput ) { return { code, @@ -45,7 +45,7 @@ function invalidCaseWithSuggestions( }, { desc: "Replace .toHaveLength(1) with .toBeInTheDocument()", - output: replaceAssertionOutput, + output: replaceMatcherOutput, }, ], }, diff --git a/src/rules/prefer-in-document.js b/src/rules/prefer-in-document.js index 68fc817..11cbd25 100644 --- a/src/rules/prefer-in-document.js +++ b/src/rules/prefer-in-document.js @@ -103,7 +103,7 @@ export const create = (context) => { // only report on dom nodes which we can resolve to RTL queries. if (!queryNode || (!queryNode.name && !queryNode.property)) return; - // *By* query with .toHaveLength(0/1) assertions are considered violations + // *By* query with .toHaveLength(0/1) matcher are considered violations // // | Selector type | .toHaveLength(1) | .toHaveLength(0) | // | ============= | =========================== | ===================================== | From 72ec55e55b76ad2249709e0a53ce07f29d900eb7 Mon Sep 17 00:00:00 2001 From: Doma Date: Tue, 15 Nov 2022 14:27:08 +0800 Subject: [PATCH 6/9] refactor(prefer-in-document): use messageId in suggestions --- src/rules/prefer-in-document.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/rules/prefer-in-document.js b/src/rules/prefer-in-document.js index 11cbd25..d8a4674 100644 --- a/src/rules/prefer-in-document.js +++ b/src/rules/prefer-in-document.js @@ -21,6 +21,7 @@ export const meta = { messages: { "use-document": `Prefer .toBeInTheDocument() for asserting DOM node existence`, "invalid-combination-length-1": `Invalid combination of {{query}} and .toHaveLength(1). Did you mean to use {{allQuery}}?`, + "replace-query-with-all": `Replace {{query}} with {{allQuery}}`, }, hasSuggestions: true, }; @@ -138,7 +139,8 @@ export const create = (context) => { loc: matcherNode.loc, suggest: [ { - desc: `Replace ${queryName} with ${allQuery}`, + messageId: "replace-query-with-all", + data: { query: queryName, allQuery }, fix(fixer) { return fixer.replaceText( queryNode.property || queryNode, From cca813753d1b5a624eff6da4858398169d39eba7 Mon Sep 17 00:00:00 2001 From: EricKim987 Date: Wed, 5 Jul 2023 20:18:19 +0800 Subject: [PATCH 7/9] fix test errors --- src/__tests__/lib/rules/prefer-in-document.js | 12 ------------ src/rules/prefer-in-document.js | 3 +-- 2 files changed, 1 insertion(+), 14 deletions(-) diff --git a/src/__tests__/lib/rules/prefer-in-document.js b/src/__tests__/lib/rules/prefer-in-document.js index 11a7365..504e4cb 100644 --- a/src/__tests__/lib/rules/prefer-in-document.js +++ b/src/__tests__/lib/rules/prefer-in-document.js @@ -187,18 +187,6 @@ const invalid = [ `expect(screen.getAllByRole('foo')).toHaveLength(1,2,3,)`, `expect(screen.getByRole('foo')).toBeInTheDocument()` ), - invalidCase( - `expect(screen.getAllByRole('foo')).toHaveLength(0//comment -)`, - `expect(screen.getByRole('foo')).not.toBeInTheDocument(//comment -)` - ), - invalidCase( - `expect(screen.getAllByRole('foo')).toHaveLength(1,//comment -)`, - `expect(screen.getByRole('foo')).toBeInTheDocument(//comment -)` - ), invalidCase( `expect(screen.getAllByRole('foo')).toHaveLength(0,2,3//comment )`, diff --git a/src/rules/prefer-in-document.js b/src/rules/prefer-in-document.js index d8a4674..5715c72 100644 --- a/src/rules/prefer-in-document.js +++ b/src/rules/prefer-in-document.js @@ -113,7 +113,7 @@ export const create = (context) => { // // @see https://github.com/testing-library/eslint-plugin-jest-dom/issues/171 // - if (matcherNode.name === "toHaveLength" && matcherArguments.length) { + if (matcherNode.name === "toHaveLength" && matcherArguments.length === 1) { const lengthValue = getLengthValue(matcherArguments); const queryName = queryNode.name || queryNode.property.name; @@ -124,7 +124,6 @@ export const create = (context) => { if (!hasViolation) { return; } - // If length === 1, report violation with suggestions // Otherwise fallback to default report if (lengthValue === 1) { From e0e18ab178a6de844c769193ee05ae7e14f01f02 Mon Sep 17 00:00:00 2001 From: EricKim987 Date: Fri, 7 Jul 2023 14:11:54 +0800 Subject: [PATCH 8/9] add some test cases --- src/__tests__/lib/rules/prefer-in-document.js | 17 +++++++++++++++++ src/rules/prefer-in-document.js | 8 ++++---- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/src/__tests__/lib/rules/prefer-in-document.js b/src/__tests__/lib/rules/prefer-in-document.js index 504e4cb..74b330e 100644 --- a/src/__tests__/lib/rules/prefer-in-document.js +++ b/src/__tests__/lib/rules/prefer-in-document.js @@ -146,6 +146,16 @@ const valid = [ foo = screen.getAllByRole('foo'); expect(foo).toHaveLength(1);`, + `expect(screen.getAllByRole('foo')).toHaveLength(1,//comment + )`, + `const foo = screen.getAllByRole('foo'); + expect(foo).toHaveLength(1,//comment + );`, + `let foo; + foo = screen.getAllByRole('foo'); + expect(foo).toHaveLength(1,//comment + );`, + // *AllBy* queries with `.toHaveLength(0)` is valid // see conclusion at https://github.com/testing-library/eslint-plugin-jest-dom/issues/171#issuecomment-895074086 `expect(screen.queryAllByTestId("foo")).toHaveLength(0)`, @@ -153,6 +163,13 @@ const valid = [ `let foo; foo = screen.queryAllByText('foo'); expect(foo).toHaveLength(0);`, + + `expect(screen.getAllByRole('foo')).toHaveLength(0//comment + )`, + `let foo; + foo = screen.getAllByRole('foo'); + expect(foo).toHaveLength(0//comment + );`, ]; const invalid = [ invalidCase( diff --git a/src/rules/prefer-in-document.js b/src/rules/prefer-in-document.js index 5715c72..d9b255b 100644 --- a/src/rules/prefer-in-document.js +++ b/src/rules/prefer-in-document.js @@ -20,8 +20,8 @@ export const meta = { fixable: "code", messages: { "use-document": `Prefer .toBeInTheDocument() for asserting DOM node existence`, - "invalid-combination-length-1": `Invalid combination of {{query}} and .toHaveLength(1). Did you mean to use {{allQuery}}?`, - "replace-query-with-all": `Replace {{query}} with {{allQuery}}`, + "invalid-combination-length-1": `Invalid combination of {{ query }} and .toHaveLength(1). Did you mean to use {{ allQuery }}?`, + "replace-query-with-all": `Replace {{ query }} with {{ allQuery }}`, }, hasSuggestions: true, }; @@ -59,7 +59,7 @@ function usesToHaveLengthZero(matcherNode, matcherArguments) { */ function getDTLQueryIdentifierNode(callExpressionNode) { if (!callExpressionNode || callExpressionNode.type !== "CallExpression") { - return; + return null; } if (callExpressionNode.callee.type === "Identifier") { @@ -272,7 +272,7 @@ export const create = (context) => { const expect = node.object.object; check({ negatedMatcher: true, - queryNode: (queryNode && queryNode.callee) || queryNode, + queryNode: queryNode.callee, matcherNode, matcherArguments, expect, From 275f469675fc7cd5f5d9299a1a00667cca9e277b Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Sat, 5 Aug 2023 13:39:05 +1200 Subject: [PATCH 9/9] docs: regenerate documentation --- README.md | 31 ++++++++++++++++--------------- docs/rules/prefer-in-document.md | 2 +- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index 92c953a..e52e6ef 100644 --- a/README.md +++ b/README.md @@ -96,21 +96,22 @@ module.exports = { πŸ’Ό Configurations enabled in.\ βœ… Set in the `recommended` configuration.\ -πŸ”§ Automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/user-guide/command-line-interface#--fix). - -| NameΒ Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β  | Description | πŸ’Ό | πŸ”§ | -| :----------------------------------------------------------------------- | :-------------------------------------------------------------------- | :- | :- | -| [prefer-checked](docs/rules/prefer-checked.md) | prefer toBeChecked over checking attributes | βœ… | πŸ”§ | -| [prefer-empty](docs/rules/prefer-empty.md) | Prefer toBeEmpty over checking innerHTML | βœ… | πŸ”§ | -| [prefer-enabled-disabled](docs/rules/prefer-enabled-disabled.md) | prefer toBeDisabled or toBeEnabled over checking attributes | βœ… | πŸ”§ | -| [prefer-focus](docs/rules/prefer-focus.md) | prefer toHaveFocus over checking document.activeElement | βœ… | πŸ”§ | -| [prefer-in-document](docs/rules/prefer-in-document.md) | Prefer .toBeInTheDocument() for asserting the existence of a DOM node | βœ… | πŸ”§ | -| [prefer-required](docs/rules/prefer-required.md) | prefer toBeRequired over checking properties | βœ… | πŸ”§ | -| [prefer-to-have-attribute](docs/rules/prefer-to-have-attribute.md) | prefer toHaveAttribute over checking getAttribute/hasAttribute | βœ… | πŸ”§ | -| [prefer-to-have-class](docs/rules/prefer-to-have-class.md) | prefer toHaveClass over checking element className | βœ… | πŸ”§ | -| [prefer-to-have-style](docs/rules/prefer-to-have-style.md) | prefer toHaveStyle over checking element style | βœ… | πŸ”§ | -| [prefer-to-have-text-content](docs/rules/prefer-to-have-text-content.md) | Prefer toHaveTextContent over checking element.textContent | βœ… | πŸ”§ | -| [prefer-to-have-value](docs/rules/prefer-to-have-value.md) | prefer toHaveValue over checking element.value | βœ… | πŸ”§ | +πŸ”§ Automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/user-guide/command-line-interface#--fix).\ +πŸ’‘ Manually fixable by [editor suggestions](https://eslint.org/docs/developer-guide/working-with-rules#providing-suggestions). + +| NameΒ Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β  | Description | πŸ’Ό | πŸ”§ | πŸ’‘ | +| :----------------------------------------------------------------------- | :-------------------------------------------------------------------- | :- | :- | :- | +| [prefer-checked](docs/rules/prefer-checked.md) | prefer toBeChecked over checking attributes | βœ… | πŸ”§ | | +| [prefer-empty](docs/rules/prefer-empty.md) | Prefer toBeEmpty over checking innerHTML | βœ… | πŸ”§ | | +| [prefer-enabled-disabled](docs/rules/prefer-enabled-disabled.md) | prefer toBeDisabled or toBeEnabled over checking attributes | βœ… | πŸ”§ | | +| [prefer-focus](docs/rules/prefer-focus.md) | prefer toHaveFocus over checking document.activeElement | βœ… | πŸ”§ | | +| [prefer-in-document](docs/rules/prefer-in-document.md) | Prefer .toBeInTheDocument() for asserting the existence of a DOM node | βœ… | πŸ”§ | πŸ’‘ | +| [prefer-required](docs/rules/prefer-required.md) | prefer toBeRequired over checking properties | βœ… | πŸ”§ | | +| [prefer-to-have-attribute](docs/rules/prefer-to-have-attribute.md) | prefer toHaveAttribute over checking getAttribute/hasAttribute | βœ… | πŸ”§ | | +| [prefer-to-have-class](docs/rules/prefer-to-have-class.md) | prefer toHaveClass over checking element className | βœ… | πŸ”§ | | +| [prefer-to-have-style](docs/rules/prefer-to-have-style.md) | prefer toHaveStyle over checking element style | βœ… | πŸ”§ | | +| [prefer-to-have-text-content](docs/rules/prefer-to-have-text-content.md) | Prefer toHaveTextContent over checking element.textContent | βœ… | πŸ”§ | | +| [prefer-to-have-value](docs/rules/prefer-to-have-value.md) | prefer toHaveValue over checking element.value | βœ… | πŸ”§ | | diff --git a/docs/rules/prefer-in-document.md b/docs/rules/prefer-in-document.md index 8e1924b..3e22abc 100644 --- a/docs/rules/prefer-in-document.md +++ b/docs/rules/prefer-in-document.md @@ -2,7 +2,7 @@ πŸ’Ό This rule is enabled in the βœ… `recommended` config. -πŸ”§ This rule is automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix). +πŸ”§πŸ’‘ This rule is automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix) and manually fixable by [editor suggestions](https://eslint.org/docs/developer-guide/working-with-rules#providing-suggestions).