feat(rules): added new rule for prefer-to-have-style (#76)

* feat(rules): added new rule prefer-to-have-style * updated readme * updated readme * updated readme * updated readme fix(rules): use .range instead of start/end
testing-library · Aug 31, 2020 · 8a55022 · 8a55022
1 parent e3a12dc
commit 8a55022
Show file tree

Hide file tree

Showing 14 changed files with 377 additions and 117 deletions.
diff --git a/README.md b/README.md
@@ -95,21 +95,22 @@ module.exports = {
 
 ## Supported Rules
 
-✔️ indicates that a rule is recommended for all users.
+👍 indicates that a rule is recommended for all users.
 
-🛠 indicates that a rule is fixable.
+🔧 indicates that a rule is fixable.
 
 <!-- __BEGIN AUTOGENERATED TABLE__ -->
 
-| Name                                                                                                                                           | ✔️  | 🛠   | Description                                                    |
+| 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-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-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     |
+| [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-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     |
 
 <!-- __END AUTOGENERATED TABLE__ -->
 
@@ -156,6 +157,7 @@ Thanks goes to these people ([emoji key][emojis]):
 
 <!-- markdownlint-enable -->
 <!-- prettier-ignore-end -->
+
 <!-- ALL-CONTRIBUTORS-LIST:END -->
 
 This project follows the [all-contributors][all-contributors] specification.

diff --git a/build/generate-readme-table.js b/build/generate-readme-table.js
@@ -15,15 +15,15 @@ const expectedTableLines = Object.keys(rules)
       lines.push(
         [
           `[${ruleId}](https://github.com/testing-library/eslint-plugin-jest-dom/blob/master/docs/rules/${ruleId}.md)`,
-          rule.meta.docs.recommended ? "✔️" : "",
-          rule.meta.fixable ? "🛠" : "",
+          rule.meta.docs.recommended ? "👍" : "",
+          rule.meta.fixable ? "🔧" : "",
           rule.meta.docs.description,
         ].join(" | ")
       );
 
       return lines;
     },
-    ["Name | ✔️ | 🛠 | Description", "----- | ----- | ----- | -----"]
+    ["Name | 👍 | 🔧 | Description", "----- | ----- | ----- | -----"]
   )
   .join("\n");
 

diff --git a/docs/rules/prefer-to-have-style.md b/docs/rules/prefer-to-have-style.md
@@ -0,0 +1,39 @@
+# prefer toHaveProperty over checking element.style (prefer-to-have-style)
+
+This rule is an autofixable rule that reports usages of checking element.style in expect statements in preference of using the jest-dom
+`toHaveStyle` matcher.
+
+## Rule Details
+
+Examples of **incorrect** code for this rule:
+
+```js
+expect(el.style.foo).toBe("bar");
+expect(el.style.foo).not.toBe("bar");
+expect(el.style).toHaveProperty("background-color", "green");
+expect(screen.getByTestId("foo").style["scroll-snap-type"]).toBe("x mandatory");
+expect(el.style).toContain("background-color");
+expect(el.style).not.toContain("background-color");
+expect(el).toHaveAttribute(
+  "style",
+  "background-color: green; border-width: 10px; color: blue;"
+);
+```
+
+Examples of **correct** code for this rule:
+
+```js
+expect(el).toHaveStyle({ foo: "bar" });
+expect(el.style).toMatchSnapshot();
+expect(el.style).toEqual(foo);
+```
+
+## When Not To Use It
+
+If you don't care about using built in matchers for checking style on dom
+elements.
+
+## Further Reading
+
+- [jest-dom toHaveStyle](https://github.com/testing-library/jest-dom#tohavestyle)
+- [ElementCSSInlineStyle.style](https://developer.mozilla.org/en-US/docs/Web/API/ElementCSSInlineStyle/style)
diff --git a/package.json b/package.json
@@ -35,6 +35,7 @@
     "requireindex": "^1.2.0"
   },
   "devDependencies": {
+    "jest-extended": "^0.11.5",
     "kcd-scripts": "^6.0.0"
   },
   "peerDependencies": {

diff --git a/src/__tests__/__snapshots__/index.test.js.snap b/src/__tests__/__snapshots__/index.test.js.snap
diff --git a/src/__tests__/index.test.js b/src/__tests__/index.test.js
@@ -1,7 +1,16 @@
 import { rules, generateRecommendedConfig } from "../index";
+import "jest-extended";
 
 it("should have all the rules", () => {
-  expect(rules).toMatchSnapshot();
+  expect(Object.keys(rules).length).toBeGreaterThan(0);
+});
+
+it.each(Object.keys(rules))("%s should export required fields", (ruleName) => {
+  const rule = rules[ruleName];
+  expect(rule).toHaveProperty("create", expect.any(Function));
+  expect(rule.meta.docs.url).not.toBeEmpty();
+  expect(rule.meta.docs.category).toBe("jest-dom");
+  expect(rule.meta.docs.description).not.toBeEmpty();
 });
 
 it("should have a recommended config with recommended rules", () => {

diff --git a/src/__tests__/lib/rules/.eslintrc b/src/__tests__/lib/rules/.eslintrc
@@ -0,0 +1,3 @@
+{"rules":{
+    "no-template-curly-in-string":"off"
+}}
diff --git a/src/__tests__/lib/rules/prefer-to-have-style.js b/src/__tests__/lib/rules/prefer-to-have-style.js
@@ -0,0 +1,62 @@
+import { RuleTester } from "eslint";
+import * as rule from "../../../rules/prefer-to-have-style";
+
+const errors = [
+  { message: "Use toHaveStyle instead of asserting on element style" },
+];
+const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 2015 } });
+ruleTester.run("prefer-to-have-attribute", rule, {
+  valid: [
+    `expect(el).toHaveStyle({foo:"bar"})`,
+    `expect(el.style).toMatchSnapshot()`,
+    `expect(el.style).toEqual(foo)`,
+    `expect(el).toHaveAttribute("style")`,
+  ],
+  invalid: [
+    {
+      code: `expect(el.style.foo).toBe("bar")`,
+      errors,
+      output: `expect(el).toHaveStyle({foo:"bar"})`,
+    },
+    {
+      code: `expect(el.style.foo).not.toBe("bar")`,
+      errors,
+      output: `expect(el).not.toHaveStyle({foo:"bar"})`,
+    },
+    {
+      code: `expect(el.style).toHaveProperty("background-color", "green")`,
+      errors,
+      output: `expect(el).toHaveStyle({backgroundColor: "green"})`,
+    },
+    {
+      code: `expect(el.style).not.toHaveProperty("background-color", "green")`,
+      errors,
+      output: `expect(el).not.toHaveStyle({backgroundColor: "green"})`,
+    },
+    {
+      code: `expect(screen.getByTestId("foo").style["scroll-snap-type"]).toBe("x mandatory")`,
+      errors,
+      output: `expect(screen.getByTestId("foo")).toHaveStyle({scrollSnapType: "x mandatory"})`,
+    },
+    {
+      code: `expect(screen.getByTestId("foo").style["scroll-snap-type"]).not.toBe("x mandatory")`,
+      errors,
+      output: `expect(screen.getByTestId("foo")).not.toHaveStyle({scrollSnapType: "x mandatory"})`,
+    },
+    {
+      code: `expect(el.style).toContain("background-color")`,
+      errors,
+      output: `expect(el).toHaveStyle({backgroundColor: expect.anything()})`,
+    },
+    {
+      code: `expect(el.style).not.toContain("background-color")`,
+      errors,
+      output: `expect(el).not.toHaveStyle({backgroundColor: expect.anything()})`,
+    },
+    {
+      code: `expect(el).toHaveAttribute("style", "background-color: green; border-width: 10px; color: blue;")`,
+      errors,
+      output: `expect(el).toHaveStyle("background-color: green; border-width: 10px; color: blue;")`,
+    },
+  ],
+});
diff --git a/src/createBannedAttributeRule.js b/src/createBannedAttributeRule.js
@@ -39,7 +39,7 @@ export default ({ preferred, negatedPreferred, attributes }) => (context) => {
       node
     ) {
       const {
-        arguments: [{ property, property: { name } = {} }],
+        arguments: [{ object, property, property: { name } = {} }],
       } = node.callee.object;
       const matcher = node.callee.property.name;
       const matcherArg = node.arguments.length && node.arguments[0].value;
@@ -58,7 +58,7 @@ export default ({ preferred, negatedPreferred, attributes }) => (context) => {
         node,
         message: `Use ${correctFunction}() instead of checking .${name} directly`,
         fix: (fixer) => [
-          fixer.removeRange([property.range[0] - 1, property.range[1]]),
+          fixer.removeRange([object.range[1], property.range[1]]),
           fixer.replaceTextRange(
             [node.callee.property.range[0], node.range[1]],
             `${correctFunction}()`

diff --git a/src/rules/prefer-empty.js b/src/rules/prefer-empty.js
@@ -21,7 +21,7 @@ export const create = (context) => ({
       node,
       message: "Use toBeEmptyDOMElement instead of checking inner html.",
       fix: (fixer) => [
-        fixer.removeRange([node.left.property.range[0] - 1, node.range[1]]),
+        fixer.removeRange([node.left.object.range[1], node.range[1]]),
         fixer.replaceText(
           node.parent.parent.property,
           Boolean(node.parent.parent.parent.arguments[0].value) ===
@@ -40,7 +40,7 @@ export const create = (context) => ({
       node,
       message: "Use toBeEmptyDOMElement instead of checking inner html.",
       fix: (fixer) => [
-        fixer.removeRange([node.left.property.range[0] - 1, node.range[1]]),
+        fixer.removeRange([node.left.object.range[1], node.range[1]]),
         fixer.replaceText(
           node.parent.parent.property,
           Boolean(node.parent.parent.parent.arguments[0].value) ===
@@ -64,7 +64,7 @@ export const create = (context) => ({
       node,
       message: "Use toBeEmptyDOMElement instead of checking inner html.",
       fix: (fixer) => [
-        fixer.removeRange([node.property.range[0] - 1, node.property.range[1]]),
+        fixer.removeRange([node.object.range[1], node.property.range[1]]),
         fixer.replaceText(node.parent.parent.property, "toBeEmptyDOMElement"),
         fixer.remove(node.parent.parent.parent.arguments[0]),
       ],
@@ -83,7 +83,7 @@ export const create = (context) => ({
       node,
       message: "Use toBeEmptyDOMElement instead of checking inner html.",
       fix: (fixer) => [
-        fixer.removeRange([node.property.range[0] - 1, node.property.range[1]]),
+        fixer.removeRange([node.object.range[1], node.property.range[1]]),
         fixer.replaceText(
           node.parent.parent.parent.property,
           "toBeEmptyDOMElement"
@@ -99,7 +99,7 @@ export const create = (context) => ({
       node,
       message: "Use toBeEmptyDOMElement instead of checking inner html.",
       fix: (fixer) => [
-        fixer.removeRange([node.property.range[0] - 1, node.property.range[1]]),
+        fixer.removeRange([node.object.range[1], node.property.range[1]]),
         fixer.replaceText(node.parent.parent.property, "toBeEmptyDOMElement"),
       ],
     });
@@ -115,7 +115,7 @@ export const create = (context) => ({
       node,
       message: "Use toBeEmptyDOMElement instead of checking inner html.",
       fix: (fixer) => [
-        fixer.removeRange([node.property.range[0] - 1, node.property.range[1]]),
+        fixer.removeRange([node.object.range[1], node.property.range[1]]),
         fixer.replaceText(
           node.parent.parent.parent.property,
           "toBeEmptyDOMElement"
@@ -131,7 +131,7 @@ export const create = (context) => ({
       node,
       message: "Use toBeEmptyDOMElement instead of checking inner html.",
       fix: (fixer) => [
-        fixer.removeRange([node.property.range[0] - 1, node.property.range[1]]),
+        fixer.removeRange([node.object.range[1], node.property.range[1]]),
         fixer.replaceText(
           node.parent.parent.parent.property,
           "toBeEmptyDOMElement"
@@ -150,7 +150,7 @@ export const create = (context) => ({
       node,
       message: "Use toBeEmptyDOMElement instead of checking inner html.",
       fix: (fixer) => [
-        fixer.removeRange([node.property.range[0] - 1, node.property.range[1]]),
+        fixer.removeRange([node.object.range[1], node.property.range[1]]),
         fixer.replaceText(node.parent.parent.property, "toBeEmptyDOMElement"),
         fixer.remove(node.parent.parent.parent.arguments[0]),
       ],

diff --git a/src/rules/prefer-focus.js b/src/rules/prefer-focus.js
@@ -14,6 +14,7 @@ const variantsOfDoc = [
 
 export const meta = {
   docs: {
+    url: "prefer-focus",
     description: "prefer toHaveFocus over checking document.activeElement",
     category: "jest-dom",
     recommended: true,

diff --git a/src/rules/prefer-to-have-attribute.js b/src/rules/prefer-to-have-attribute.js
@@ -9,8 +9,10 @@
 
 export const meta = {
   docs: {
+    category: "jest-dom",
     description:
       "prefer toHaveAttribute over checking  getAttribute/hasAttribute ",
+    url: "prefer-to-have-attribute",
     recommended: true,
   },
   fixable: "code",
@@ -107,7 +109,7 @@ export const create = (context) => ({
       message: "Invalid matcher for getAttribute",
     });
   },
-  [`CallExpression[callee.property.name='hasAttribute'][parent.callee.name='expect'][parent.parent.property.name=/toBe$|to(Striclty)?Equal/]`](
+  [`CallExpression[callee.property.name='hasAttribute'][parent.callee.name='expect'][parent.parent.property.name=/toBe$|to(Strict)?Equal/]`](
     node
   ) {
     if (typeof node.parent.parent.parent.arguments[0].value === "boolean") {