From 858ace6ffa145262e0cf33df8e3aa88c758e73e1 Mon Sep 17 00:00:00 2001 From: fisker Cheung Date: Sat, 11 Oct 2025 01:46:03 +0800 Subject: [PATCH] Forbid rename `path` in callbacks (#18038) Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> --- .github/workflows/lint.yml | 2 +- .github/workflows/prod-test.yml | 4 +- eslint.config.js | 1 + knip.config.js | 6 +- .../no-useless-ast-path-callback-parameter.js | 104 ++++++++++++++++++ .../package.json | 2 +- .../test.js | 45 ++++++++ src/language-js/print/binaryish.js | 8 +- src/language-js/print/call-expression.js | 2 +- src/language-js/print/class.js | 2 +- src/language-js/print/match.js | 4 +- src/language-js/print/property.js | 6 +- src/language-js/print/template-literal.js | 5 +- src/language-js/print/ternary.js | 15 +-- src/language-js/print/type-annotation.js | 4 +- tests/unit/ast-path.js | 1 + 16 files changed, 176 insertions(+), 35 deletions(-) create mode 100644 scripts/tools/eslint-plugin-prettier-internal-rules/no-useless-ast-path-callback-parameter.js diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 4d0759bd2..dec98b0b7 100644 --- a/.github/workflows/lint.yml +++ b/.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: | diff --git a/.github/workflows/prod-test.yml b/.github/workflows/prod-test.yml index b486424f9..fb3457a44 100644 --- a/.github/workflows/prod-test.yml +++ b/.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 diff --git a/eslint.config.js b/eslint.config.js index 4d64f8b74..dd28a30bd 100644 --- a/eslint.config.js +++ b/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": [ diff --git a/knip.config.js b/knip.config.js index 07a0371e4..ee96b4c8b 100644 --- a/knip.config.js +++ b/knip.config.js @@ -30,11 +30,7 @@ const config = { "buffer", "deno-path-from-file-url", ], - ignoreBinaries: [ - "test-coverage", - "renovate-config-validator", - "pkg-pr-new", - ], + ignoreBinaries: ["test-coverage"], }, website: { entry: [ diff --git a/scripts/tools/eslint-plugin-prettier-internal-rules/no-useless-ast-path-callback-parameter.js b/scripts/tools/eslint-plugin-prettier-internal-rules/no-useless-ast-path-callback-parameter.js new file mode 100644 index 000000000..45e5dbe9b --- /dev/null +++ b/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); + }, + }; + }, +}; diff --git a/scripts/tools/eslint-plugin-prettier-internal-rules/package.json b/scripts/tools/eslint-plugin-prettier-internal-rules/package.json index a5194bddf..be9ed5da8 100644 --- a/scripts/tools/eslint-plugin-prettier-internal-rules/package.json +++ b/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" } diff --git a/scripts/tools/eslint-plugin-prettier-internal-rules/test.js b/scripts/tools/eslint-plugin-prettier-internal-rules/test.js index 5878aa801..7b3952d2f 100644 --- a/scripts/tools/eslint-plugin-prettier-internal-rules/test.js +++ b/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, + }, + ], +}); diff --git a/src/language-js/print/binaryish.js b/src/language-js/print/binaryish.js index df8e78b50..04562ca9a 100644 --- a/src/language-js/print/binaryish.js +++ b/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, diff --git a/src/language-js/print/call-expression.js b/src/language-js/print/call-expression.js index dcc2ad541..8794855f2 100644 --- a/src/language-js/print/call-expression.js +++ b/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"] : []), ) diff --git a/src/language-js/print/class.js b/src/language-js/print/class.js index 9c297ab97..9cc83a161 100644 --- a/src/language-js/print/class.js +++ b/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) { diff --git a/src/language-js/print/match.js b/src/language-js/print/match.js index 8a06a1851..931ab651a 100644 --- a/src/language-js/print/match.js +++ b/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) { diff --git a/src/language-js/print/property.js b/src/language-js/print/property.js index c6aabcaf3..1d1898330 100644 --- a/src/language-js/print/property.js +++ b/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, ), diff --git a/src/language-js/print/template-literal.js b/src/language-js/print/template-literal.js index f56ab18f3..b91d262da 100644 --- a/src/language-js/print/template-literal.js +++ b/src/language-js/print/template-literal.js @@ -258,10 +258,7 @@ function printTemplateExpression(path, print) { } function printTemplateExpressions(path, print) { - return path.map( - (path) => printTemplateExpression(path, print), - "expressions", - ); + return path.map(() => printTemplateExpression(path, print), "expressions"); } function escapeTemplateCharacters(doc, raw) { diff --git a/src/language-js/print/ternary.js b/src/language-js/print/ternary.js index 7dadade51..b8dd427d9 100644 --- a/src/language-js/print/ternary.js +++ b/src/language-js/print/ternary.js @@ -282,25 +282,22 @@ function printTernary(path, options, print, args) { !isConsequentTernary && hasComment(consequentNode, CommentCheckFlags.Dangling) ) { - path.call((childPath) => { - consequentComments.push( - printDanglingComments(childPath, options), - hardline, - ); + path.call(() => { + consequentComments.push(printDanglingComments(path, options), hardline); }, "consequent"); } const alternateComments = []; if (hasComment(node.test, CommentCheckFlags.Dangling)) { - path.call((childPath) => { - alternateComments.push(printDanglingComments(childPath, options)); + path.call(() => { + alternateComments.push(printDanglingComments(path, options)); }, "test"); } if ( !isAlternateTernary && hasComment(alternateNode, CommentCheckFlags.Dangling) ) { - path.call((childPath) => { - alternateComments.push(printDanglingComments(childPath, options)); + path.call(() => { + alternateComments.push(printDanglingComments(path, options)); }, "alternate"); } if (hasComment(node, CommentCheckFlags.Dangling)) { diff --git a/src/language-js/print/type-annotation.js b/src/language-js/print/type-annotation.js index acf43974e..956a9bd08 100644 --- a/src/language-js/print/type-annotation.js +++ b/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) { diff --git a/tests/unit/ast-path.js b/tests/unit/ast-path.js index a7916b289..481f25fdf 100644 --- a/tests/unit/ast-path.js +++ b/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");