Support backtick, logical &&, and ternary (#57)

AhmedBaset · Aug 5, 2024 · 1351eee · 1351eee
2 parents af6c2ef + 7dbeb3a
commit 1351eee
Show file tree

Hide file tree

Showing 7 changed files with 216 additions and 21 deletions.
diff --git a/.changeset/shiny-kangaroos-hunt.md b/.changeset/shiny-kangaroos-hunt.md
@@ -0,0 +1,16 @@
+---
+"eslint-plugin-rtl-friendly": minor
+---
+
+# Add support for extra className cases
+
+Previously the plugin only supported literal values like `className="..."`, which lead to a big miss, Now it supports more cases:
+
+- `className={"..."}` (Auto Fixable)
+- Template Literal: `className={``}` (Auto Fixable)
+- Terenary Operator: `className={condition ? "..." : "..."}`:
+  - It reports errors on both consequent and alternate values
+  - But it only fixes the consequent value
+
+
+Currently, Auto Fixing works with only literal values but the goal is to get it to fix all expressions 
diff --git a/e2e/no-physical-properties.tsx b/e2e/no-physical-properties.tsx
@@ -476,4 +476,9 @@ declare const React;
     {/* eslint-disable-next-line rtl-friendly/no-physical-properties */}
     <div className="*:!-scroll-pr-8" />
   </>
+
+  <>
+    {/* eslint-disable-next-line rtl-friendly/no-physical-properties */}
+    <div className={Math.random() > 0.5 ? "pl-1 text-right mr-2" : "pl-1 text-right mr-2"} />
+  </>
 </>;
diff --git a/package.json b/package.json
@@ -61,6 +61,7 @@
     "@types/estree": "^1.0.5",
     "@types/estree-jsx": "^1.0.5",
     "@types/jest": "^29.5.12",
+    "@types/node": "20",
     "eslint": "^9.8.0",
     "eslint-plugin-eslint-plugin": "^6.2.0",
     "globals": "^15.9.0",

diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml
diff --git a/src/rules/no-phyisical-properties/rule.ts b/src/rules/no-phyisical-properties/rule.ts
@@ -1,11 +1,14 @@
-import { Rule } from "eslint";
+import { Rule, type AST } from "eslint";
 
 import * as ESTree from "estree";
 import type { JSXAttribute } from "estree-jsx";
-import { extractFromNode } from "../../utils/ast.js";
+import { extractTokenFromNode } from "../../utils/ast.js";
 import { parseForPhysicalClasses } from "../../utils/tailwind.js";
 
-const cache = new Map</** invalid */ string, /** valid */ string>();
+// const cache = new Map<
+//   /** invalid */ string,
+//   /** valid */ string
+// >();
 
 export const NO_PHYSICAL_CLASSESS = "NO_PHYSICAL_CLASSESS";
 
@@ -33,19 +36,23 @@ export const noPhysicalProperties: Rule.RuleModule = {
         const isClassAttribute = ["className", "class"].includes(attr);
         if (!isClassAttribute) return;
 
-        let result = extractFromNode(node);
-        if (!result) return;
+        // let result = extractFromNode(node);
+        // if (!result) return;
 
-        result = result.filter((c) => typeof c === "string");
-        if (!result.length) return;
+        // result = result.filter((c) => typeof c === "string");
+        // if (!result.length) return;
 
-        const classesAsString = result.join(" ");
-        const cachedValid = cache.get(classesAsString);
-        if (cachedValid) {
-          report({ ctx, node, invalid: classesAsString, valid: cachedValid });
-          return;
-        }
+        // const classesAsString = result.join(" ");
+        // const cachedValid = cache.get(classesAsString);
+        // if (cachedValid) {
+        //   console.log("cachedValid", cachedValid);
+        //   report({ ctx, node, invalid: classesAsString, valid: cachedValid });
+        //   return;
+        // }
 
+        const classesAsString = extractTokenFromNode(node, "checker")?.value;
+        if (!classesAsString) return;
+
         const classes = classesAsString.split(" ");
 
         const parsed = parseForPhysicalClasses(classes);
@@ -56,7 +63,7 @@ export const noPhysicalProperties: Rule.RuleModule = {
         const invalid = parsed.map((p) => p.original).join(" ");
         const valid = parsed.map((p) => p.valid).join(" ");
 
-        cache.set(classesAsString, valid);
+        // cache.set(classesAsString, valid);
         report({ ctx, node, invalid, valid });
       },
     };
@@ -86,10 +93,11 @@ function report({
       end: node.loc!.end,
     },
     fix: (fixer) => {
-      if (node.value?.type === "Literal") {
+      const token = extractTokenFromNode(node, "fixer");
+      if (token?.raw) {
         return fixer.replaceText(
-          node.value,
-          node.value.raw?.replace(invalid, valid) ?? ""
+          token as AST.Token,
+          token.raw?.replace(invalid, valid)
         );
       }
 

diff --git a/src/rules/no-phyisical-properties/test.ts b/src/rules/no-phyisical-properties/test.ts
@@ -47,6 +47,22 @@ tester.run("no-physical-properties", noPhysicalProperties, {
       name: "empty class is ok",
       code: "<div className=\"\" class=''></div>",
     },
+    {
+      name: '{"..."}',
+      code: "<div className={'ps-2'} class={'md:text-start'}></div>",
+    },
+    {
+      name: "{`...`}",
+      code: "<div className={`ps-2`} class={`md:text-start`} />",
+    },
+    {
+      name: '{isCondition && "..."}',
+      code: `<div className={isCondition && "ps-2"} />`,
+    },
+    {
+      name: '{isCondition && "..."}',
+      code: `<div className={isCondition ? "ps-1 text-end me-2" : "pe-1 text-start ms-2"} />`,
+    },
   ],
   invalid: [
     {
@@ -67,6 +83,33 @@ tester.run("no-physical-properties", noPhysicalProperties, {
         { messageId: NO_PHYSICAL_CLASSESS },
       ],
     },
+    {
+      name: `{"..."}`,
+      code: `<div className={"pl-1 extra-class mr-2"}><span class={"pl-2 extra-class pr-2"}>text</span></div>`,
+      output: `<div className={"ps-1 extra-class me-2"}><span class={"ps-2 extra-class pe-2"}>text</span></div>`,
+      errors: [
+        { messageId: NO_PHYSICAL_CLASSESS },
+        { messageId: NO_PHYSICAL_CLASSESS },
+      ],
+    },
+    {
+      name: "{`...`}",
+      code: "<div className={`pl-1 extra-class mr-2`} />",
+      output: "<div className={`ps-1 extra-class me-2`} />",
+      errors: [{ messageId: NO_PHYSICAL_CLASSESS }],
+    },
+    {
+      name: '{isCondition && "..."}',
+      code: `<div className={isCondition && "pl-1 text-right mr-2"} />`,
+      output: `<div className={isCondition && "ps-1 text-end me-2"} />`,
+      errors: [{ messageId: NO_PHYSICAL_CLASSESS }],
+    },
+    {
+      name: '{isCondition ? "..." : "..."}',
+      code: `<div className={isCondition ? "pl-1 text-right mr-2" : "pl-1 text-right mr-2"} />`,
+      // output: `<div className={isCondition ? "ps-1 text-end me-2" : "pl-1 text-right mr-2"} />`, // TODO: Fix this
+      errors: [{ messageId: NO_PHYSICAL_CLASSESS }],
+    },
     {
       name: "should report if physical margin properties are used and fix them",
       code: `<div className="ml-1 mr-2">text</div>`,
@@ -112,7 +155,6 @@ tester.run("no-physical-properties", noPhysicalProperties, {
       output: `<div className="scroll-ms-1 scroll-me-2 scroll-ps-1 scroll-pe-1">text</div>`,
       errors: [{ messageId: NO_PHYSICAL_CLASSESS }],
     },
-
     {
       name: "should report if physical properties are used with the important flag and fix it",
       code: `<div className="!pl-0">text</div>`,

diff --git a/src/utils/ast.ts b/src/utils/ast.ts
@@ -3,6 +3,11 @@ import type { Expression, JSXAttribute } from "estree-jsx";
 type Val = string | number | bigint | boolean | RegExp | null | undefined;
 
 export function extractFromNode(node: JSXAttribute) {
+  // const token = extractTokenFromNode(node)
+  // if (token) {
+  //   return token.raw;
+  // }
+
   // value: Literal | JSXExpressionContainer | JSXElement | JSXFragment | null
   const valueType = node.value?.type;
   if (!valueType) return;
@@ -17,6 +22,10 @@ export function extractFromNode(node: JSXAttribute) {
 
     if (expression?.type === "JSXEmptyExpression" || !expression) return;
 
+    if (expression.type === "Literal") return [expression.value];
+    if (expression.type === "TemplateLiteral")
+      result.push(expression.quasis[0].value.raw);
+
     result.push(...extractFromExpression(expression));
   }
 
@@ -46,9 +55,10 @@ function extractFromExpression(expression: Expression) {
   if (expression.type === "Literal") result.push(expression.value);
   if (expression.type === "TemplateLiteral")
     result.push(expression.quasis[0].value.raw);
-  if (expression.type === "BinaryExpression") {
-    result.push(...extractFromExpression(expression.left));
-    result.push(...extractFromExpression(expression.right));
+  if (expression.type === "ConditionalExpression") {
+    console.log(expression);
+    // result.push(extractFromExpression(expression.left));
+    // result.push(extractFromExpression(expression.right));
   }
   if (expression.type === "CallExpression") {
     expression.arguments.forEach((arg) => {
@@ -69,3 +79,113 @@ function extractFromExpression(expression: Expression) {
 
   return result;
 }
+
+export function extractTokenFromNode(
+  node: JSXAttribute,
+  runner: "checker" | "fixer"
+): { type: string; value?: string; raw?: string } | undefined {
+  // value: Literal | JSXExpressionContainer | JSXElement | JSXFragment | null
+  const type = node.value?.type;
+  if (!type) return;
+
+  if (type === "Literal") return validate(node.value);
+
+  if (type === "JSXExpressionContainer") {
+    const expression = node.value?.expression;
+
+    if (expression?.type === "JSXEmptyExpression" || !expression) return;
+
+    return extractTokenFromExpression(expression, runner);
+  }
+
+  return;
+}
+
+function extractTokenFromExpression(
+  expression: Expression,
+  runner: "checker" | "fixer"
+): { type: string; value: string; raw: string } | undefined {
+  // We care about:
+  // -> Literal;
+  // -> TemplateLiteral;
+  // -> BinaryExpression
+  // -> CallExpression;
+  // -> ConditionalExpression;
+  // -> LogicalExpression;
+
+  const rerun = (expression: Expression) => {
+    return extractTokenFromExpression(expression, runner);
+  };
+
+  const isFixer = runner === "fixer";
+  const type = expression.type;
+
+  if (type === "Literal")
+    return validate({
+      ...expression,
+      value: expression.value || expression.raw,
+    });
+  if (type === "TemplateLiteral") {
+    return validate({
+      ...expression.quasis[0],
+      value: expression.quasis[0].value.cooked,
+      raw: "`" + expression.quasis[0].value.raw + "`",
+    });
+  }
+
+  if (type === "LogicalExpression") {
+    return rerun(expression.right);
+  }
+
+  if (type === "ConditionalExpression") {
+    // TODO: Currently, Auto Fixer works on the consequent only
+    if (isFixer) {
+      return rerun(expression.consequent);
+    } else {
+      const consequent = rerun(expression.consequent);
+      const alternate = rerun(expression.alternate);
+      consequent!.value = `${consequent!.value}" : "${alternate!.value}`;
+      return consequent;
+    }
+  }
+
+  console.log("UNIMPLEMENTED: ", type);
+
+  // if (expression.type === "BinaryExpression") {
+  //   result.push(...extractFromExpression(expression.left));
+  //   result.push(...extractFromExpression(expression.right));
+  // }
+  // if (expression.type === "CallExpression") {
+  //   expression.arguments.forEach((arg) => {
+  //     if (arg.type === "SpreadElement") {
+  //       result.push(...extractFromExpression(arg.argument));
+  //     } else {
+  //       result.push(...extractFromExpression(arg));
+  //     }
+  //   });
+  // }
+  // if (expression.type === "ConditionalExpression") {
+  //   result.push(...extractFromExpression(expression.consequent));
+  //   result.push(...extractFromExpression(expression.alternate));
+  // }
+  // if (expression.type === "LogicalExpression") {
+  //   result.push(...extractFromExpression(expression.right));
+  // }
+
+  return;
+}
+
+// eslint-disable-next-line @typescript-eslint/no-explicit-any
+function validate(result: null | { type: string; value?: any; raw?: string }):
+  | undefined
+  | {
+      type: string;
+      value: string;
+      raw: string;
+    } {
+  if (!result) return;
+  if (typeof result.value !== "string") return;
+  if (typeof result.raw !== "string") return;
+
+  return result as { type: string; value: string; raw: string };
+}