diff --git a/README.md b/README.md index a6cb2a3..11b528b 100644 --- a/README.md +++ b/README.md @@ -60,13 +60,12 @@ To enable this configuration use the `extends` property in your 🛠 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-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-required](https://github.com/testing-library/eslint-plugin-jest-dom/blob/master/docs/rules/prefer-required.md) | ✔️ | 🛠 | prefer toBeRequired over checking properties | - +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-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-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 ## Contributors ✨ diff --git a/docs/rules/prefer-to-have-attribute.md b/docs/rules/prefer-to-have-attribute.md new file mode 100644 index 0000000..1acc7e6 --- /dev/null +++ b/docs/rules/prefer-to-have-attribute.md @@ -0,0 +1,42 @@ +# prefer toHaveAttribute over checking getAttribute/hasAttribute (prefer-to-have-attribute) + +This rule is an autofixable rule that reports usages of `getAttribute` or `hasAttribute` in expect statements in preference of using the jest-dom `toHaveAttribute` matcher. + + +## Rule Details + +This checks the various built in jest-dom matchers when used in conjunction with get/hasAttribute. The only valid use case if when using greater/less than matchers since there isn't any equivalent use with `toHaveAttribute()` + +Examples of **incorrect** code for this rule: + +```js + + expect(element.getAttribute('foo')).toMatch(/bar/) + expect(element.getAttribute('foo')).toContain('bar') + expect(getByText('thing').getAttribute("foo")).toBe("bar") + expect(getByText("yes").getAttribute("data-blah")).toBe(expect.stringMatching(/foo/)) + expect(element.hasAttribute("foo")).toBeTruthy() + + +``` + +Examples of **correct** code for this rule: + +```js + + expect(element.foo).toBeTruthy() + expect(element.getAttributeNode()).toBeNull() + expect(element.getAttribute('foo')).toBeGreaterThan(2) + expect(element.getAttribute('foo')).toBeLessThan(2) + +``` + +## When Not To Use It + +If you don't care about using built in matchers for checking attributes on dom elements. + +## Further Reading + +* [jest-dom toHaveAttribute](https://github.com/testing-library/jest-dom#tohaveattribute) +* [getAttribute](https://developer.mozilla.org/en-US/docs/Web/API/Element/getAttribute) +* [hasAttribute](https://developer.mozilla.org/en-US/docs/Web/API/Element/hasAttribute) diff --git a/lib/rules/prefer-to-have-attribute.js b/lib/rules/prefer-to-have-attribute.js new file mode 100644 index 0000000..4cdada0 --- /dev/null +++ b/lib/rules/prefer-to-have-attribute.js @@ -0,0 +1,171 @@ +/** + * @fileoverview prefer toHaveAttribute over checking getAttribute/hasAttribute + * @author Ben Monro + */ +'use strict'; + +//------------------------------------------------------------------------------ +// Rule Definition +//------------------------------------------------------------------------------ + +module.exports = { + meta: { + docs: { + description: + 'prefer toHaveAttribute over checking getAttribute/hasAttribute ', + recommended: false, + }, + fixable: 'code', + }, + + create: function(context) { + return { + [`CallExpression[callee.property.name='getAttribute'][parent.callee.name='expect'][parent.parent.property.name=/toBeNull/]`]( + node + ) { + context.report({ + node: node.parent, + message: `Use toHaveAttribute instead of asserting on getAttribute`, + fix(fixer) { + return [ + fixer.removeRange([node.callee.object.end, node.end]), + fixer.replaceTextRange( + [ + node.parent.parent.property.start, + node.parent.parent.parent.end, + ], + `not.toHaveAttribute(${node.arguments[0].raw})` + ), + ]; + }, + }); + }, + [`CallExpression[callee.property.name='getAttribute'][parent.callee.name='expect'][parent.parent.property.name=/toContain$|toMatch$/]`]( + node + ) { + context.report({ + node: node.parent, + message: `Use toHaveAttribute instead of asserting on getAttribute`, + fix(fixer) { + return [ + fixer.removeRange([node.callee.object.end, node.end]), + fixer.replaceText(node.parent.parent.property, 'toHaveAttribute'), + fixer.replaceText( + node.parent.parent.parent.arguments[0], + `${ + node.arguments[0].raw + }, expect.string${node.parent.parent.property.name.slice( + 2 + )}ing(${node.parent.parent.parent.arguments[0].raw})` + ), + ]; + }, + }); + }, + [`CallExpression[callee.property.name='getAttribute'][parent.callee.name='expect'][parent.parent.property.name=/toBe$|to(Strict)?Equal/]`]( + node + ) { + const arg = node.parent.parent.parent.arguments; + const isNullOrEmpty = + arg.length > 0 && (arg[0].value === null || arg[0].value === ''); + + context.report({ + node: node.parent, + message: `Use toHaveAttribute instead of asserting on getAttribute`, + fix(fixer) { + let lastFixer; + if (isNullOrEmpty) { + lastFixer = fixer.replaceText( + node.parent.parent.parent.arguments[0], + node.arguments[0].raw + ); + } else { + lastFixer = fixer.insertTextBefore( + node.parent.parent.parent.arguments[0], + `${node.arguments[0].raw}, ` + ); + } + return [ + fixer.removeRange([node.callee.object.end, node.end]), + fixer.replaceText( + node.parent.parent.property, + `${isNullOrEmpty ? 'not.' : ''}toHaveAttribute` + ), + lastFixer, + ]; + }, + }); + }, + [`CallExpression[callee.property.name='hasAttribute'][parent.callee.name='expect'][parent.parent.property.name=/toBeNull|toBeUndefined|toBeDefined/]`]( + node + ) { + context.report({ + node: node.parent.parent.property, + message: 'Invalid matcher for hasAttribute', + }); + }, + [`CallExpression[callee.property.name='getAttribute'][parent.callee.name='expect'][parent.parent.property.name=/toBeUndefined|toBeDefined/]`]( + node + ) { + context.report({ + node: node.parent.parent.property, + message: 'Invalid matcher for getAttribute', + }); + }, + [`CallExpression[callee.property.name='hasAttribute'][parent.callee.name='expect'][parent.parent.property.name=/toBe$|to(Striclty)?Equal/]`]( + node + ) { + if (typeof node.parent.parent.parent.arguments[0].value === 'boolean') { + context.report({ + node: node.parent, + message: `Use toHaveAttribute instead of asserting on hasAttribute`, + fix(fixer) { + return [ + fixer.removeRange([node.callee.object.end, node.end]), + fixer.replaceText( + node.parent.parent.property, + `${ + node.parent.parent.parent.arguments[0].value === false + ? 'not.' + : '' + }toHaveAttribute` + ), + fixer.replaceText( + node.parent.parent.parent.arguments[0], + node.arguments[0].raw + ), + ]; + }, + }); + } else { + context.report({ + node: node.parent.parent.property, + message: 'Invalid matcher for hasAttribute', + }); + } + }, + [`CallExpression[callee.property.name='hasAttribute'][parent.callee.name='expect'][parent.parent.property.name=/toBeTruthy|toBeFalsy/]`]( + node + ) { + context.report({ + node: node.parent, + message: `Use toHaveAttribute instead of asserting on hasAttribute`, + fix(fixer) { + return [ + fixer.removeRange([node.callee.object.end, node.end]), + fixer.replaceTextRange( + [ + node.parent.parent.property.start, + node.parent.parent.parent.end, + ], + `${ + node.parent.parent.property.name === 'toBeFalsy' ? 'not.' : '' + }toHaveAttribute(${node.arguments[0].raw})` + ), + ]; + }, + }); + }, + }; + }, +}; diff --git a/tests/__snapshots__/index.test.js.snap b/tests/__snapshots__/index.test.js.snap index a10e7eb..19f39ac 100644 --- a/tests/__snapshots__/index.test.js.snap +++ b/tests/__snapshots__/index.test.js.snap @@ -38,5 +38,15 @@ Object { "fixable": "code", }, }, + "prefer-to-have-attribute": Object { + "create": [Function], + "meta": Object { + "docs": Object { + "description": "prefer toHaveAttribute over checking getAttribute/hasAttribute ", + "recommended": false, + }, + "fixable": "code", + }, + }, } `; diff --git a/tests/lib/rules/prefer-to-have-attribute.js b/tests/lib/rules/prefer-to-have-attribute.js new file mode 100644 index 0000000..47219e9 --- /dev/null +++ b/tests/lib/rules/prefer-to-have-attribute.js @@ -0,0 +1,191 @@ +/** + * @fileoverview prefer toHaveAttribute over checking getAttribute/hasAttribute + * @author Ben Monro + */ +'use strict'; + +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +let rule = require('../../../lib/rules/prefer-to-have-attribute'), + RuleTester = require('eslint').RuleTester; + +//------------------------------------------------------------------------------ +// Tests +//------------------------------------------------------------------------------ + +let ruleTester = new RuleTester(); +ruleTester.run('prefer-to-have-attribute', rule, { + valid: [ + 'expect(element.foo).toBeTruthy()', + 'expect(element.getAttributeNode()).toBeNull()', + `expect(element.getAttribute('foo')).toBeGreaterThan(2)`, + `expect(element.getAttribute('foo')).toBeLessThan(2)`, + ], + + invalid: [ + { + code: `expect(element.getAttribute('foo')).toMatch(/bar/);`, + errors: [ + { + message: 'Use toHaveAttribute instead of asserting on getAttribute', + }, + ], + output: `expect(element).toHaveAttribute('foo', expect.stringMatching(/bar/));`, + }, + { + code: `expect(element.getAttribute('foo')).toContain('bar');`, + errors: [ + { + message: 'Use toHaveAttribute instead of asserting on getAttribute', + }, + ], + output: `expect(element).toHaveAttribute('foo', expect.stringContaining('bar'));`, + }, + { + code: 'expect(element.getAttribute("foo")).toBe("bar")', + errors: [ + { + message: 'Use toHaveAttribute instead of asserting on getAttribute', + }, + ], + output: 'expect(element).toHaveAttribute("foo", "bar")', + }, + { + code: `expect(getByText("yes").getAttribute("data-blah")).toBe(expect.stringMatching(/foo/))`, + errors: [ + { + message: 'Use toHaveAttribute instead of asserting on getAttribute', + }, + ], + output: `expect(getByText("yes")).toHaveAttribute("data-blah", expect.stringMatching(/foo/))`, + }, + { + code: `expect(getByText('foo').hasAttribute('foo')).toBe(null)`, + errors: [ + { + message: 'Invalid matcher for hasAttribute', + }, + ], + output: null, + }, + { + code: `expect(getByText('foo').hasAttribute('foo')).toBeNull()`, + errors: [ + { + message: 'Invalid matcher for hasAttribute', + }, + ], + output: null, + }, + { + code: `expect(getByText('foo').getAttribute('foo')).toBeDefined()`, + errors: [ + { + message: 'Invalid matcher for getAttribute', + }, + ], + output: null, + }, + { + code: `expect(getByText('foo').getAttribute('foo')).toBeUndefined()`, + errors: [ + { + message: 'Invalid matcher for getAttribute', + }, + ], + output: null, + }, + { + code: `expect(getByText('foo').hasAttribute('foo')).toBeUndefined()`, + errors: [ + { + message: 'Invalid matcher for hasAttribute', + }, + ], + output: null, + }, + { + code: 'expect(element.hasAttribute("foo")).toBeTruthy()', + errors: [ + { + message: 'Use toHaveAttribute instead of asserting on hasAttribute', + }, + ], + output: 'expect(element).toHaveAttribute("foo")', + }, + { + code: 'expect(element.hasAttribute("foo")).toBeFalsy()', + errors: [ + { + message: 'Use toHaveAttribute instead of asserting on hasAttribute', + }, + ], + output: 'expect(element).not.toHaveAttribute("foo")', + }, + { + code: 'expect(element.hasAttribute("foo")).toBe(true)', + errors: [ + { + message: 'Use toHaveAttribute instead of asserting on hasAttribute', + }, + ], + output: 'expect(element).toHaveAttribute("foo")', + }, + { + code: 'expect(element.hasAttribute("foo")).toBe(false)', + errors: [ + { + message: 'Use toHaveAttribute instead of asserting on hasAttribute', + }, + ], + output: 'expect(element).not.toHaveAttribute("foo")', + }, + { + code: 'expect(element.hasAttribute("foo")).toEqual(false)', + errors: [ + { + message: 'Use toHaveAttribute instead of asserting on hasAttribute', + }, + ], + output: 'expect(element).not.toHaveAttribute("foo")', + }, + { + code: 'expect(element.getAttribute("foo")).toEqual("bar")', + errors: [ + { + message: 'Use toHaveAttribute instead of asserting on getAttribute', + }, + ], + output: 'expect(element).toHaveAttribute("foo", "bar")', + }, + { + code: 'expect(element.getAttribute("foo")).toStrictEqual("bar")', + errors: [ + { + message: 'Use toHaveAttribute instead of asserting on getAttribute', + }, + ], + output: 'expect(element).toHaveAttribute("foo", "bar")', + }, + { + code: 'expect(element.getAttribute("foo")).toBe(null)', + errors: [ + { + message: 'Use toHaveAttribute instead of asserting on getAttribute', + }, + ], + output: 'expect(element).not.toHaveAttribute("foo")', + }, + { + code: 'expect(element.getAttribute("foo")).toBeNull()', + errors: [ + { + message: 'Use toHaveAttribute instead of asserting on getAttribute', + }, + ], + output: 'expect(element).not.toHaveAttribute("foo")', + }, + ], +});