Forbid rename path in callbacks (#18038)

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

.github/workflows/lint.yml

@@ -70,7 +70,7 @@ jobs:
         uses: actions/dependency-review-action@56339e523c0409420f6c2c9a2f4292bbb3c07dd3 # v4.8.0
 
       - name: Validate renovate config
-        run: npx --package renovate@latest --call renovate-config-validator
+        run: yarn dlx --package renovate@latest renovate-config-validator
 
       - name: Run yarn (/)
         run: |

.github/workflows/prod-test.yml

@@ -188,6 +188,6 @@ jobs:
       - name: Release
         # Use `--compact` after we publish `@prettier/plugin-oxc`
         # run: |
-        #   npx pkg-pr-new publish "./dist/*" --compact --packageManager=yarn
+        #   yarn dlx pkg-pr-new publish "./dist/*" --compact --packageManager=yarn
         run: |
-          npx pkg-pr-new publish "./dist/*" --packageManager=yarn
+          yarn dlx pkg-pr-new publish "./dist/*" --packageManager=yarn

eslint.config.js

@@ -142,6 +142,7 @@ const configs = [
       "prettier-internal-rules/massage-ast-parameter-names": "error",
       "prettier-internal-rules/no-identifier-n": "error",
       "prettier-internal-rules/prefer-fs-promises-submodule": "error",
+      "prettier-internal-rules/no-useless-ast-path-callback-parameter": "error",
 
       /* @stylistic/eslint-plugin */
       "@stylistic/quotes": [

knip.config.js

@@ -30,11 +30,7 @@ const config = {
         "buffer",
         "deno-path-from-file-url",
       ],
-      ignoreBinaries: [
+      ignoreBinaries: ["test-coverage"],
-        "test-coverage",
-        "renovate-config-validator",
-        "pkg-pr-new",
-      ],
     },
     website: {
       entry: [

scripts/tools/eslint-plugin-prettier-internal-rules/no-useless-ast-path-callback-parameter.js

@@ -0,0 +1,104 @@
+const MESSAGE_ID_PATH = "error/path";
+
+function checkPathParameter(context, callExpression) {
+  const [callback] = callExpression.arguments;
+  if (callback.params.length === 0) {
+    return;
+  }
+
+  const pathParameter = callback.params[0];
+  if (pathParameter.type !== "Identifier") {
+    return;
+  }
+
+  const objectName = callExpression.callee.object.name;
+
+  const problem = {
+    node: pathParameter,
+    messageId: MESSAGE_ID_PATH,
+    data: { name: objectName },
+  };
+
+  if (callback.params.length === 1) {
+    const { sourceCode } = context;
+    const variable =
+      pathParameter.name === objectName
+        ? undefined
+        : sourceCode
+            .getDeclaredVariables(callback)
+            .find((variable) => variable.defs[0]?.name === pathParameter);
+
+    if (variable || pathParameter.name === objectName) {
+      problem.fix = function* (fixer) {
+        yield fixer.remove(pathParameter);
+
+        const tokenAfter = sourceCode.getTokenAfter(pathParameter);
+
+        if (tokenAfter.value === ",") {
+          yield fixer.remove(tokenAfter);
+        }
+
+        if (pathParameter.name !== objectName) {
+          for (const reference of variable.references) {
+            yield fixer.replaceText(reference.identifier, objectName);
+          }
+        }
+      };
+    }
+  }
+
+  context.report(problem);
+}
+
+export default {
+  meta: {
+    type: "suggestion",
+    messages: {
+      [MESSAGE_ID_PATH]:
+        "Use `{{name}}` directly instead of the first parameter.",
+    },
+    fixable: "code",
+  },
+  create(context) {
+    return {
+      CallExpression(callExpression) {
+        const { callee } = callExpression;
+
+        if (
+          !(
+            !callExpression.optional &&
+            callExpression.arguments.length > 0 &&
+            callee.type === "MemberExpression" &&
+            !callee.optional &&
+            !callee.computed &&
+            callee.object.type === "Identifier" &&
+            callee.property.type === "Identifier" &&
+            (callee.property.name === "call" ||
+              callee.property.name === "callParent" ||
+              callee.property.name === "each" ||
+              callee.property.name === "map")
+          )
+        ) {
+          return;
+        }
+
+        const objectName = callee.object.name;
+        if (!(objectName === "path" || objectName.endsWith("Path"))) {
+          return;
+        }
+
+        const [callback] = callExpression.arguments;
+        if (
+          !(
+            callback.type === "FunctionExpression" ||
+            callback.type === "ArrowFunctionExpression"
+          )
+        ) {
+          return;
+        }
+
+        checkPathParameter(context, callExpression);
+      },
+    };
+  },
+};

scripts/tools/eslint-plugin-prettier-internal-rules/package.json

@@ -9,7 +9,7 @@
   "license": "MIT",
   "scripts": {
     "test": "node test.js",
-    "test-coverage": "npx nyc node test.js"
+    "test-coverage": "yarn dlx c8 node test.js"
   },
   "packageManager": "yarn@4.10.3"
 }

scripts/tools/eslint-plugin-prettier-internal-rules/test.js

@@ -949,3 +949,48 @@ test("massage-ast-parameter-names", {
     },
   ],
 });
+
+test("no-useless-ast-path-callback-parameter", {
+  valid: [
+    "path.call?.((childPath) => childPath)",
+    "path?.call((childPath) => childPath)",
+    "path[call]((childPath) => childPath)",
+    "path.notCall((childPath) => childPath)",
+    "not_ast_path.call((childPath) => childPath)",
+    "path.call(({first},) => first)",
+    "path.call((...a) => a)",
+    "path.call(notFunctionExpression)",
+  ],
+  invalid: [
+    ...["call", "callParent", "each", "map"].map((method) => ({
+      code: `path.${method}((childPath) => childPath)`,
+      output: `path.${method}(() => path)`,
+      errors: 1,
+    })),
+    {
+      code: "path.call(function(childPath){childPath})",
+      output: "path.call(function(){path})",
+      errors: 1,
+    },
+    {
+      code: "path.call((childPath,) => childPath)",
+      output: "path.call(() => path)",
+      errors: 1,
+    },
+    {
+      code: "path.each((childPath,index, foo) => {})",
+      output: null,
+      errors: 1,
+    },
+    {
+      code: "fooPath.call((childPath) => childPath)",
+      output: "fooPath.call(() => fooPath)",
+      errors: 1,
+    },
+    {
+      code: "path.call((path) => path)",
+      output: "path.call(() => path)",
+      errors: 1,
+    },
+  ],
+});

src/language-js/print/binaryish.js

@@ -222,9 +222,9 @@ function printBinaryishExpressions(
   if (shouldFlatten(node.operator, node.left.operator)) {
     // Flatten them out by recursively calling this function.
     parts = path.call(
-      (left) =>
+      () =>
         printBinaryishExpressions(
-          left,
+          path,
           options,
           print,
           /* isNested */ true,
@@ -282,9 +282,9 @@ function printBinaryishExpressions(
       operator === "|>" && path.root.extra?.__isUsingHackPipeline;
     const rightContent = isHackPipeline
       ? path.call(
-          (left) =>
+          () =>
             printBinaryishExpressions(
-              left,
+              path,
               options,
               print,
               /* isNested */ true,

src/language-js/print/call-expression.js

@@ -80,7 +80,7 @@ function printCallExpression(path, options, print) {
     !isNewExpression &&
     isMemberish(node.callee) &&
     !path.call(
-      (path) => pathNeedsParens(path, options),
+      () => pathNeedsParens(path, options),
       "callee",
       ...(node.callee.type === "ChainExpression" ? ["expression"] : []),
     )

src/language-js/print/class.js

@@ -85,7 +85,7 @@ function printClass(path, options, print) {
       ),
     ];
     const printedWithComments = path.call(
-      (superClass) => ["extends ", printComments(superClass, printed, options)],
+      () => ["extends ", printComments(path, printed, options)],
       "superClass",
     );
     if (groupMode) {

src/language-js/print/match.js

@@ -219,12 +219,12 @@ function printMatchOrPattern(path, options, print) {
   // | child1
   // // comment
   // | child2
-  const printed = path.map((patternPath) => {
+  const printed = path.map(() => {
     let printedPattern = print();
     if (!shouldHug) {
       printedPattern = align(2, printedPattern);
     }
-    return printComments(patternPath, printedPattern, options);
+    return printComments(path, printedPattern, options);
   }, "patterns");
 
   if (shouldHug) {

src/language-js/print/property.js

@@ -154,7 +154,7 @@ function printPropertyKey(path, options, print) {
       ),
       options,
     );
-    return path.call((keyPath) => printComments(keyPath, prop, options), "key");
+    return path.call(() => printComments(path, prop, options), "key");
   }
 
   if (
@@ -166,9 +166,9 @@ function printPropertyKey(path, options, print) {
     // '1' -> 1
     // '1.5' -> 1.5
     return path.call(
-      (keyPath) =>
+      () =>
         printComments(
-          keyPath,
+          path,
           /^\d/u.test(key.value) ? printNumber(key.value) : key.value,
           options,
         ),

src/language-js/print/template-literal.js

@@ -258,10 +258,7 @@ function printTemplateExpression(path, print) {
 }
 
 function printTemplateExpressions(path, print) {
-  return path.map(
+  return path.map(() => printTemplateExpression(path, print), "expressions");
-    (path) => printTemplateExpression(path, print),
-    "expressions",
-  );
 }
 
 function escapeTemplateCharacters(doc, raw) {

src/language-js/print/ternary.js

@@ -282,25 +282,22 @@ function printTernary(path, options, print, args) {
     !isConsequentTernary &&
     hasComment(consequentNode, CommentCheckFlags.Dangling)
   ) {
-    path.call((childPath) => {
+    path.call(() => {
-      consequentComments.push(
+      consequentComments.push(printDanglingComments(path, options), hardline);
-        printDanglingComments(childPath, options),
-        hardline,
-      );
     }, "consequent");
   }
   const alternateComments = [];
   if (hasComment(node.test, CommentCheckFlags.Dangling)) {
-    path.call((childPath) => {
+    path.call(() => {
-      alternateComments.push(printDanglingComments(childPath, options));
+      alternateComments.push(printDanglingComments(path, options));
     }, "test");
   }
   if (
     !isAlternateTernary &&
     hasComment(alternateNode, CommentCheckFlags.Dangling)
   ) {
-    path.call((childPath) => {
+    path.call(() => {
-      alternateComments.push(printDanglingComments(childPath, options));
+      alternateComments.push(printDanglingComments(path, options));
     }, "alternate");
   }
   if (hasComment(node, CommentCheckFlags.Dangling)) {

src/language-js/print/type-annotation.js

@@ -224,12 +224,12 @@ function printUnionType(path, options, print) {
   // | child1
   // // comment
   // | child2
-  const printed = path.map((typePath) => {
+  const printed = path.map(() => {
     let printedType = print();
     if (!shouldHug) {
       printedType = align(2, printedType);
     }
-    return printComments(typePath, printedType, options);
+    return printComments(path, printedType, options);
   }, "types");
 
   if (shouldHug) {

tests/unit/ast-path.js

@@ -1,3 +1,4 @@
+/* eslint-disable prettier-internal-rules/no-useless-ast-path-callback-parameter */
 import AstPath from "../../src/common/ast-path.js";
 
 const error = new Error("A dummy error");