feat(enforce-ref-last-prop): add fixer (#8671)

**Related Issue:** #8080 ## Summary Adds a fixer to place the `ref` prop last along with a corresponding comment to disable `react/jsx-sort-props`. --------- Co-authored-by: JC Franco <[email protected]>
Esri · Apr 6, 2024 · 688bc51 · 688bc51
1 parent 322842f
commit 688bc51
Show file tree

Hide file tree

Showing 6 changed files with 182 additions and 18 deletions.
diff --git a/packages/eslint-plugin-calcite-components/docs/enforce-ref-last-prop.md b/packages/eslint-plugin-calcite-components/docs/enforce-ref-last-prop.md
@@ -13,3 +13,5 @@ No config is needed
 ```json
 { "@esri/calcite-components/enforce-ref-last-prop": "error" }
 ```
+
+> Fix included
diff --git a/packages/eslint-plugin-calcite-components/src/rules/enforce-ref-last-prop.ts b/packages/eslint-plugin-calcite-components/src/rules/enforce-ref-last-prop.ts
@@ -1,5 +1,5 @@
 import { Rule } from "eslint";
-import type { JSXAttribute, JSXSpreadAttribute, JSXOpeningElement } from "@babel/types";
+import type { JSXAttribute, JSXOpeningElement, JSXSpreadAttribute } from "@babel/types";
 
 const rule: Rule.RuleModule = {
   meta: {
@@ -17,21 +17,46 @@ const rule: Rule.RuleModule = {
       JSXIdentifier(node) {
         const openingElement = node.parent as JSXOpeningElement;
         if (openingElement.type === "JSXOpeningElement") {
-          const attributes: string[] = [];
+          const attributes = openingElement.attributes
+            .map((attr) => {
+              if (attr.type === "JSXAttribute" && attr.name?.type === "JSXIdentifier") {
+                return attr;
+              }
+            })
+            .filter(Boolean);
 
-          openingElement.attributes.forEach((attr: JSXAttribute | JSXSpreadAttribute) => {
-            if (attr.type === "JSXAttribute" && attr.name?.type === "JSXIdentifier") {
-              attributes.push(attr.name.name);
-            }
-          });
+          const refAttribute = attributes.find(
+            (attr: JSXAttribute | JSXSpreadAttribute) =>
+              attr.type === "JSXAttribute" && attr.name?.type === "JSXIdentifier" && attr.name.name === "ref",
+          );
 
-          const refAttribute = attributes.find((attr: string) => attr === "ref");
+          if (refAttribute) {
+            const { sourceCode } = context;
+            const refAttrText = sourceCode.getText(refAttribute as typeof node);
+            const otherAttrs = attributes.filter((attr) => attr !== refAttribute);
+            const indentation = new Array(refAttribute.loc.start.column).fill(" ").join("");
+            const tokenBeforeRefAttr = sourceCode.getTokenBefore(refAttribute as typeof node);
+            const eslintDisableComments = sourceCode
+              .getCommentsBefore(refAttribute as typeof node)
+              .filter((comment) => comment.value.includes("eslint-disable-next-line"));
+            const refIsLastWithSortDisablingComment =
+              attributes.indexOf(refAttribute) === attributes.length - 1 && eslintDisableComments.length !== 0;
 
-          if (refAttribute && attributes.indexOf(refAttribute) !== attributes.length - 1) {
-            context.report({
-              node,
-              message: `"ref" prop should be placed last in JSX to ensure the node attrs/props are in sync.`,
-            });
+            if (!refIsLastWithSortDisablingComment) {
+              context.report({
+                node,
+                message: `"ref" prop should be placed last in JSX to ensure the node attrs/props are in sync.`,
+                fix(fixer) {
+                  return [
+                    fixer.removeRange([tokenBeforeRefAttr.range[1], refAttribute.range[1]]),
+                    fixer.insertTextAfterRange(
+                      otherAttrs[otherAttrs.length - 1].range,
+                      `\n${indentation}// eslint-disable-next-line react/jsx-sort-props -- auto-generated by @esri/calcite-components/enforce-ref-last-prop\n${indentation}${refAttrText}`,
+                    ),
+                  ];
+                },
+              });
+            }
           }
         }
       },

diff --git a/...n-calcite-components/tests/lib/rules/enforce-ref-last-prop/enforce-ref-last-prop.good.tsx b/...n-calcite-components/tests/lib/rules/enforce-ref-last-prop/enforce-ref-last-prop.good.tsx
@@ -6,16 +6,19 @@ export class SampleTag {
       <Host>
         <div
           class="some-class"
-          id={`${guid}-element`}
+          id="element"
           onClick={() => {
             /* click! */
           }}
           tabIndex={0}
+          // eslint-disable-next-line enforce-ref-last-prop -- auto-generated by @esri/calcite-components/enforce-ref-last-prop
           ref={(el: HTMLDivElement): void => {
             /* refEl */
           }}
         >
-          test
+          test Note: we are intentionally specifying `enforce-ref-last-prop` in the disable line as
+          RuleTester can't be configured with multiple rules and this does not disable the custom
+          rule, please see the output file for the correctly generated disable line
         </div>
       </Host>
     );

diff --git a/...calcite-components/tests/lib/rules/enforce-ref-last-prop/enforce-ref-last-prop.output.tsx b/...calcite-components/tests/lib/rules/enforce-ref-last-prop/enforce-ref-last-prop.output.tsx
@@ -0,0 +1,70 @@
+// @ts-nocheck
+@Component({ tag: "sample-tag" })
+export class SampleTag {
+  render() {
+    return (
+      <Host>
+        <div
+          class="some-class"
+          id="use-case-1"
+          onClick={() => {
+            /* click! */
+          }}
+          tabIndex={0}
+          // eslint-disable-next-line react/jsx-sort-props -- auto-generated by @esri/calcite-components/enforce-ref-last-prop
+          ref={(el: HTMLDivElement): void => {
+            /* refEl */
+          }}
+        >
+          case where ref is not last prop
+        </div>
+        <div
+          class="some-class"
+          id="use-case-2"
+          onClick={() => {
+            /* click! */
+          }}
+          tabIndex={0}
+          // eslint-disable-next-line react/jsx-sort-props -- auto-generated by @esri/calcite-components/enforce-ref-last-prop
+          ref={(el: HTMLDivElement): void => {
+            /* refEl */
+          }}
+        >
+          case where ref last prop, but not commented
+        </div>
+        <div
+          class="some-class"
+          id="use-case-3"
+          onClick={() => {
+            /* click! */
+          }}
+          tabIndex={0}
+          // eslint-disable-next-line react/jsx-sort-props -- auto-generated by @esri/calcite-components/enforce-ref-last-prop
+          ref={(el: HTMLDivElement): void => {
+            /* refEl */
+          }}
+        >
+          case where ref last prop, and already commented Note: this is marked as wrong because
+          RuleTester can't configure multiple rules, so we ignore the ESLint error from not finding
+          the disabled rule
+        </div>
+        <div
+          class="some-class"
+          id="use-case-4"
+          onClick={() => {
+            /* click! */
+          }}
+          tabIndex={0}
+          // eslint-disable-next-line react/jsx-sort-props -- auto-generated by @esri/calcite-components/enforce-ref-last-prop
+          ref={(el: HTMLDivElement): void => {
+            /* refEl */
+          }}
+        >
+          case where ref is not last prop and already commented Note: this is marked as wrong
+          because RuleTester can't configure multiple rules, so we ignore the ESLint error from not
+          finding the disabled rule
+        </div>
+      </Host>
+    );
+  }
+}
diff --git a/...in-calcite-components/tests/lib/rules/enforce-ref-last-prop/enforce-ref-last-prop.spec.ts b/...in-calcite-components/tests/lib/rules/enforce-ref-last-prop/enforce-ref-last-prop.spec.ts
@@ -9,6 +9,7 @@ describe("enforce-ref-last-prop rule", () => {
   const files = {
     good: path.resolve(__dirname, "enforce-ref-last-prop.good.tsx"),
     wrong: path.resolve(__dirname, "enforce-ref-last-prop.wrong.tsx"),
+    output: path.resolve(__dirname, "enforce-ref-last-prop.output.tsx"),
   };
   ruleTester(projectPath).run("enforce-ref-last-prop", rule, {
     valid: [
@@ -22,7 +23,25 @@ describe("enforce-ref-last-prop rule", () => {
       {
         code: fs.readFileSync(files.wrong, "utf8"),
         filename: files.wrong,
-        errors: 1,
+        errors: [
+          // we include the disabled rule not found error because RuleTester doesn't support multiple rules
+          {
+            message: `"ref" prop should be placed last in JSX to ensure the node attrs/props are in sync.`,
+          },
+          {
+            message: `"ref" prop should be placed last in JSX to ensure the node attrs/props are in sync.`,
+          },
+          {
+            message: `Definition for rule 'react/jsx-sort-props' was not found.`,
+          },
+          {
+            message: `"ref" prop should be placed last in JSX to ensure the node attrs/props are in sync.`,
+          },
+          {
+            message: `Definition for rule 'react/jsx-sort-props' was not found.`,
+          },
+        ],
+        output: fs.readFileSync(files.output, "utf8"),
       },
     ],
   });

diff --git a/...-calcite-components/tests/lib/rules/enforce-ref-last-prop/enforce-ref-last-prop.wrong.tsx b/...-calcite-components/tests/lib/rules/enforce-ref-last-prop/enforce-ref-last-prop.wrong.tsx
@@ -9,13 +9,58 @@ export class SampleTag {
             /* refEl */
           }}
           class="some-class"
-          id={`${guid}-element`}
+          id="use-case-1"
           onClick={() => {
             /* click! */
           }}
           tabIndex={0}
         >
-          test
+          case where ref is not last prop
+        </div>
+        <div
+          class="some-class"
+          id="use-case-2"
+          onClick={() => {
+            /* click! */
+          }}
+          tabIndex={0}
+          ref={(el: HTMLDivElement): void => {
+            /* refEl */
+          }}
+        >
+          case where ref last prop, but not commented
+        </div>
+        <div
+          class="some-class"
+          id="use-case-3"
+          onClick={() => {
+            /* click! */
+          }}
+          tabIndex={0}
+          // eslint-disable-next-line react/jsx-sort-props -- auto-generated by @esri/calcite-components/enforce-ref-last-prop
+          ref={(el: HTMLDivElement): void => {
+            /* refEl */
+          }}
+        >
+          case where ref last prop, and already commented Note: this is marked as wrong because
+          RuleTester can't configure multiple rules, so we ignore the ESLint error from not finding
+          the disabled rule
+        </div>
+        <div
+          // eslint-disable-next-line react/jsx-sort-props -- auto-generated by @esri/calcite-components/enforce-ref-last-prop
+          ref={(el: HTMLDivElement): void => {
+            /* refEl */
+          }}
+          class="some-class"
+          id="use-case-4"
+          onClick={() => {
+            /* click! */
+          }}
+          tabIndex={0}
+        >
+          case where ref is not last prop and already commented Note: this is marked as wrong
+          because RuleTester can't configure multiple rules, so we ignore the ESLint error from not
+          finding the disabled rule
         </div>
       </Host>
     );
-Original file line number
+Diff line change
@@ Expand Up / @@ -13,3 +13,5 @@ No config is needed @@
     ```json
     { "@esri/calcite-components/enforce-ref-last-prop": "error" }
     ```
+    > Fix included