From f37f6161508f75f61db3dd0ad76c6382e9bd87a3 Mon Sep 17 00:00:00 2001 From: Matthew Wills Date: Wed, 9 Jun 2021 17:57:57 +1000 Subject: [PATCH] fix(prefer-to-have-style): Handle case where matcher toBe contains empty string value This change purposefully treats an empty string value for certain matchers as value instead of as null. Prior to this change, the fixer was incorrectly inferring a .not. Matchers affected are: - toBe - toStrictEqual - toEqual --- .../lib/rules/prefer-to-have-attribute.js | 54 +++++++++++++++++++ src/rules/prefer-to-have-attribute.js | 7 ++- 2 files changed, 57 insertions(+), 4 deletions(-) diff --git a/src/__tests__/lib/rules/prefer-to-have-attribute.js b/src/__tests__/lib/rules/prefer-to-have-attribute.js index cf0cfe3..cf02195 100644 --- a/src/__tests__/lib/rules/prefer-to-have-attribute.js +++ b/src/__tests__/lib/rules/prefer-to-have-attribute.js @@ -72,6 +72,24 @@ ruleTester.run("prefer-to-have-attribute", rule, { ], output: `expect(getByText("yes")).toHaveAttribute("data-blah", expect.stringMatching(/foo/))`, }, + { + code: `expect(getByText("yes").getAttribute("data-blah")).toBe("")`, + errors: [ + { + message: "Use toHaveAttribute instead of asserting on getAttribute", + }, + ], + output: `expect(getByText("yes")).toHaveAttribute("data-blah", "")`, + }, + { + code: `expect(getByText("yes").getAttribute("data-blah")).toBe('')`, + errors: [ + { + message: "Use toHaveAttribute instead of asserting on getAttribute", + }, + ], + output: `expect(getByText("yes")).toHaveAttribute("data-blah", '')`, + }, { code: `expect(getByText('foo').hasAttribute('foo')).toBe(null)`, errors: [ @@ -171,6 +189,24 @@ ruleTester.run("prefer-to-have-attribute", rule, { ], output: 'expect(element).toHaveAttribute("foo", "bar")', }, + { + code: `expect(getByText("yes").getAttribute("data-blah")).toEqual("")`, + errors: [ + { + message: "Use toHaveAttribute instead of asserting on getAttribute", + }, + ], + output: `expect(getByText("yes")).toHaveAttribute("data-blah", "")`, + }, + { + code: `expect(getByText("yes").getAttribute("data-blah")).toEqual('')`, + errors: [ + { + message: "Use toHaveAttribute instead of asserting on getAttribute", + }, + ], + output: `expect(getByText("yes")).toHaveAttribute("data-blah", '')`, + }, { code: 'expect(element.getAttribute("foo")).toStrictEqual("bar")', errors: [ @@ -180,6 +216,24 @@ ruleTester.run("prefer-to-have-attribute", rule, { ], output: 'expect(element).toHaveAttribute("foo", "bar")', }, + { + code: `expect(getByText("yes").getAttribute("data-blah")).toStrictEqual("")`, + errors: [ + { + message: "Use toHaveAttribute instead of asserting on getAttribute", + }, + ], + output: `expect(getByText("yes")).toHaveAttribute("data-blah", "")`, + }, + { + code: `expect(getByText("yes").getAttribute("data-blah")).toStrictEqual('')`, + errors: [ + { + message: "Use toHaveAttribute instead of asserting on getAttribute", + }, + ], + output: `expect(getByText("yes")).toHaveAttribute("data-blah", '')`, + }, { code: 'expect(element.getAttribute("foo")).toBe(null)', errors: [ diff --git a/src/rules/prefer-to-have-attribute.js b/src/rules/prefer-to-have-attribute.js index 89dfcd9..5c0b16d 100644 --- a/src/rules/prefer-to-have-attribute.js +++ b/src/rules/prefer-to-have-attribute.js @@ -64,15 +64,14 @@ export const create = (context) => ({ node ) { const arg = node.parent.parent.parent.arguments; - const isNullOrEmpty = - arg.length > 0 && (arg[0].value === null || arg[0].value === ""); + const isNull = arg.length > 0 && arg[0].value === null; const sourceCode = context.getSourceCode(); context.report({ node: node.parent, message: `Use toHaveAttribute instead of asserting on getAttribute`, fix: (fixer) => { - const lastFixer = isNullOrEmpty + const lastFixer = isNull ? fixer.replaceText( node.parent.parent.parent.arguments[0], sourceCode.getText(node.arguments[0]) @@ -86,7 +85,7 @@ export const create = (context) => ({ fixer.removeRange([node.callee.object.range[1], node.range[1]]), fixer.replaceText( node.parent.parent.property, - `${isNullOrEmpty ? "not." : ""}toHaveAttribute` + `${isNull ? "not." : ""}toHaveAttribute` ), lastFixer, ];