Remove prefer-ast-path-node and add prefer-ast-path-getters (#13648)

* Support `getParent` * Fix * Fix * Refactor * Rename rule and add tests * Fix by rule * Fix error message * Add error message tests * Run Prettier! * Address review * Improve error message * Refactor * Stricter check & refactor * Minor tweak Co-authored-by: fisker Cheung <lionkay@gmail.com>
prettier · Oct 18, 2022 · 92a6af2 · 92a6af2
1 parent 4a8de7f
commit 92a6af2
Show file tree

Hide file tree

Showing 32 changed files with 349 additions and 132 deletions.
diff --git a/.eslintrc.cjs b/.eslintrc.cjs
@@ -344,7 +344,7 @@ module.exports = {
         "prettier-internal-rules/prefer-indent-if-break": "error",
         "prettier-internal-rules/prefer-is-non-empty-array": "error",
         "prettier-internal-rules/prefer-fs-promises-submodule": "error",
-        "prettier-internal-rules/prefer-ast-path-node": "error",
+        "prettier-internal-rules/prefer-ast-path-getters": "error",
       },
     },
     {

diff --git a/scripts/tools/eslint-plugin-prettier-internal-rules/index.js b/scripts/tools/eslint-plugin-prettier-internal-rules/index.js
@@ -18,6 +18,6 @@ module.exports = {
     "prefer-indent-if-break": require("./prefer-indent-if-break.js"),
     "prefer-is-non-empty-array": require("./prefer-is-non-empty-array.js"),
     "prefer-fs-promises-submodule": require("./prefer-fs-promises-submodule.js"),
-    "prefer-ast-path-node": require("./prefer-ast-path-node.js"),
+    "prefer-ast-path-getters": require("./prefer-ast-path-getters.js"),
   },
 };
diff --git a/scripts/tools/eslint-plugin-prettier-internal-rules/prefer-ast-path-getters.js b/scripts/tools/eslint-plugin-prettier-internal-rules/prefer-ast-path-getters.js
@@ -0,0 +1,92 @@
+"use strict";
+
+const selector = [
+  "CallExpression",
+  "[optional=false]",
+  "[arguments.length<2]",
+  '[callee.type="MemberExpression"]',
+  "[callee.computed=false]",
+  "[callee.optional=false]",
+  '[callee.object.type="Identifier"]',
+  "[callee.object.name=/[pP]ath$/]",
+  '[callee.property.type="Identifier"]',
+].join("");
+
+const messageId = "prefer-ast-path-getters";
+
+function getReplacement(callExpression) {
+  const method = callExpression.callee.property.name;
+  const description = `${method}()`;
+  switch (method) {
+    case "getValue":
+    case "getNode":
+      if (callExpression.arguments.length === 0) {
+        return { getter: "node", description };
+      }
+      break;
+
+    case "getParentNode": {
+      // `path.getParentNode()`
+      if (callExpression.arguments.length === 0) {
+        return { getter: "parent", description };
+      }
+
+      // `path.getParentNode(count)`
+      const [countNode] = callExpression.arguments;
+      if (countNode.type !== "Literal") {
+        return;
+      }
+
+      const count = countNode.value;
+      if (count === 0) {
+        return { getter: "parent", description: `${method}(0)` };
+      }
+
+      if (count === 1) {
+        return { getter: "grandparent", description: `${method}(1)` };
+      }
+      break;
+    }
+  }
+}
+
+module.exports = {
+  meta: {
+    type: "suggestion",
+    docs: {
+      url: "https://github.com/prettier/prettier/blob/main/scripts/tools/eslint-plugin-prettier-internal-rules/prefer-ast-path-getters.js",
+    },
+    fixable: "code",
+    messages: {
+      [messageId]:
+        "Prefer `AstPath#{{ getter }}` over `AstPath#{{ description }}`.",
+    },
+  },
+  create(context) {
+    return {
+      [selector](callExpression) {
+        const replacement = getReplacement(callExpression);
+        if (!replacement) {
+          return;
+        }
+
+        context.report({
+          node: callExpression.callee,
+          messageId,
+          data: {
+            getter: replacement.getter,
+            description: replacement.description,
+          },
+          fix: (fixer) =>
+            fixer.replaceTextRange(
+              [
+                callExpression.callee.property.range[0],
+                callExpression.range[1],
+              ],
+              replacement.getter
+            ),
+        });
+      },
+    };
+  },
+};
diff --git a/scripts/tools/eslint-plugin-prettier-internal-rules/prefer-ast-path-node.js b/scripts/tools/eslint-plugin-prettier-internal-rules/prefer-ast-path-node.js
diff --git a/scripts/tools/eslint-plugin-prettier-internal-rules/test.js b/scripts/tools/eslint-plugin-prettier-internal-rules/test.js
@@ -497,44 +497,223 @@ test("prefer-fs-promises-submodule", {
   })),
 });
 
-test("prefer-ast-path-node", {
+test("prefer-ast-path-getters", {
   valid: [
     "path.getNode(2)",
     "path.getNode",
     "getNode",
     "this.getNode()",
     "path.node",
+    "path.getParentNode(2)",
+    "path.getParentNode",
+    "getParentNode",
+    "this.getParentNode()",
+    "path.parent",
+    "path.grandparent",
   ],
   invalid: [
+    // path.getNode
     {
       code: "path.getNode()",
       output: "path.node",
-      errors: 1,
+      errors: [
+        {
+          message: "Prefer `AstPath#node` over `AstPath#getNode()`.",
+        },
+      ],
     },
     {
       code: "const node = path.getNode()",
       output: "const node = path.node",
-      errors: 1,
+      errors: [
+        {
+          message: "Prefer `AstPath#node` over `AstPath#getNode()`.",
+        },
+      ],
+    },
+    {
+      code: "fooPath.getNode()",
+      output: "fooPath.node",
+      errors: [
+        {
+          message: "Prefer `AstPath#node` over `AstPath#getNode()`.",
+        },
+      ],
     },
+
+    // path.getValue()
     {
       code: "path.getValue()",
       output: "path.node",
-      errors: 1,
+      errors: [
+        {
+          message: "Prefer `AstPath#node` over `AstPath#getValue()`.",
+        },
+      ],
     },
     {
       code: "const node = path.getValue()",
       output: "const node = path.node",
-      errors: 1,
+      errors: [
+        {
+          message: "Prefer `AstPath#node` over `AstPath#getValue()`.",
+        },
+      ],
     },
     {
       code: "fooPath.getValue()",
       output: "fooPath.node",
-      errors: 1,
+      errors: [
+        {
+          message: "Prefer `AstPath#node` over `AstPath#getValue()`.",
+        },
+      ],
     },
+
+    // path.getParentNode()
     {
-      code: "fooPath.getNode()",
-      output: "fooPath.node",
-      errors: 1,
+      code: "path.getParentNode()",
+      output: "path.parent",
+      errors: [
+        {
+          message: "Prefer `AstPath#parent` over `AstPath#getParentNode()`.",
+        },
+      ],
+    },
+    {
+      code: "const node = path.getParentNode()",
+      output: "const node = path.parent",
+      errors: [
+        {
+          message: "Prefer `AstPath#parent` over `AstPath#getParentNode()`.",
+        },
+      ],
+    },
+    {
+      code: "path.getParentNode()",
+      output: "path.parent",
+      errors: [
+        {
+          message: "Prefer `AstPath#parent` over `AstPath#getParentNode()`.",
+        },
+      ],
+    },
+    {
+      code: "const node = path.getParentNode()",
+      output: "const node = path.parent",
+      errors: [
+        {
+          message: "Prefer `AstPath#parent` over `AstPath#getParentNode()`.",
+        },
+      ],
+    },
+    {
+      code: "fooPath.getParentNode()",
+      output: "fooPath.parent",
+      errors: [
+        {
+          message: "Prefer `AstPath#parent` over `AstPath#getParentNode()`.",
+        },
+      ],
+    },
+
+    // path.getParentNode(0)
+    {
+      code: "path.getParentNode(0)",
+      output: "path.parent",
+      errors: [
+        {
+          message: "Prefer `AstPath#parent` over `AstPath#getParentNode(0)`.",
+        },
+      ],
+    },
+    {
+      code: "const node = path.getParentNode(0)",
+      output: "const node = path.parent",
+      errors: [
+        {
+          message: "Prefer `AstPath#parent` over `AstPath#getParentNode(0)`.",
+        },
+      ],
+    },
+    {
+      code: "path.getParentNode(0)",
+      output: "path.parent",
+      errors: [
+        {
+          message: "Prefer `AstPath#parent` over `AstPath#getParentNode(0)`.",
+        },
+      ],
+    },
+    {
+      code: "const node = path.getParentNode(0)",
+      output: "const node = path.parent",
+      errors: [
+        {
+          message: "Prefer `AstPath#parent` over `AstPath#getParentNode(0)`.",
+        },
+      ],
+    },
+    {
+      code: "fooPath.getParentNode(0)",
+      output: "fooPath.parent",
+      errors: [
+        {
+          message: "Prefer `AstPath#parent` over `AstPath#getParentNode(0)`.",
+        },
+      ],
+    },
+
+    // path.getParentNode(1)
+    {
+      code: "path.getParentNode(1)",
+      output: "path.grandparent",
+      errors: [
+        {
+          message:
+            "Prefer `AstPath#grandparent` over `AstPath#getParentNode(1)`.",
+        },
+      ],
+    },
+    {
+      code: "const node = path.getParentNode(1)",
+      output: "const node = path.grandparent",
+      errors: [
+        {
+          message:
+            "Prefer `AstPath#grandparent` over `AstPath#getParentNode(1)`.",
+        },
+      ],
+    },
+    {
+      code: "path.getParentNode(1)",
+      output: "path.grandparent",
+      errors: [
+        {
+          message:
+            "Prefer `AstPath#grandparent` over `AstPath#getParentNode(1)`.",
+        },
+      ],
+    },
+    {
+      code: "const node = path.getParentNode(1)",
+      output: "const node = path.grandparent",
+      errors: [
+        {
+          message:
+            "Prefer `AstPath#grandparent` over `AstPath#getParentNode(1)`.",
+        },
+      ],
+    },
+    {
+      code: "fooPath.getParentNode(1)",
+      output: "fooPath.grandparent",
+      errors: [
+        {
+          message:
+            "Prefer `AstPath#grandparent` over `AstPath#getParentNode(1)`.",
+        },
+      ],
     },
   ],
 });