From 9b8f6e86acfa02500a2171abcebe745a3c8ded87 Mon Sep 17 00:00:00 2001 From: fisker Cheung Date: Tue, 2 Dec 2025 06:00:26 +0800 Subject: [PATCH] Avoid double parentheses around logical expression (#18397) --- changelog_unreleased/javascript/18397.md | 13 ++ .../parentheses/needs-parentheses.js | 9 +- src/language-js/print/binaryish.js | 4 +- src/language-js/print/estree.js | 6 +- .../__snapshots__/format.test.js.snap | 36 ++-- .../__snapshots__/format.test.js.snap | 178 ++++++++++++++++++ .../in-unary-expression.js | 87 +++++++++ .../__snapshots__/format.test.js.snap | 20 +- 8 files changed, 317 insertions(+), 36 deletions(-) create mode 100644 changelog_unreleased/javascript/18397.md create mode 100644 tests/format/js/logical-expressions/in-unary-expression.js diff --git a/changelog_unreleased/javascript/18397.md b/changelog_unreleased/javascript/18397.md new file mode 100644 index 000000000..0d8eb39e7 --- /dev/null +++ b/changelog_unreleased/javascript/18397.md @@ -0,0 +1,13 @@ +#### Avoid double parentheses around logical expression (#18397 by @fisker) + + +```jsx +// Input +const a = !(x || y /* foo */); + +// Prettier stable +const a = !((x || y) /* foo */); + +// Prettier main +const a = !(x || y /* foo */); +``` diff --git a/src/language-js/parentheses/needs-parentheses.js b/src/language-js/parentheses/needs-parentheses.js index 9de0107c4..e442fdf8b 100644 --- a/src/language-js/parentheses/needs-parentheses.js +++ b/src/language-js/parentheses/needs-parentheses.js @@ -4,6 +4,7 @@ import { getFunctionParameters, getLeftSidePathName, getPrecedence, + hasComment, hasNakedLeftSide, hasNode, isArrayExpression, @@ -380,7 +381,6 @@ function needsParentheses(path, options) { case "TSTypeAssertion": case "TaggedTemplateExpression": - case "UnaryExpression": case "JSXSpreadAttribute": case "SpreadElement": case "BindExpression": @@ -388,6 +388,12 @@ function needsParentheses(path, options) { case "TSNonNullExpression": case "UpdateExpression": return true; + case "UnaryExpression": + // `UnaryExpression` adds parentheses and indention when argument has comment + if (!hasComment(node)) { + return true; + } + break; case "MemberExpression": case "OptionalMemberExpression": @@ -451,6 +457,7 @@ function needsParentheses(path, options) { default: return false; } + break; case "SequenceExpression": // Although parentheses wouldn't hurt around sequence diff --git a/src/language-js/print/binaryish.js b/src/language-js/print/binaryish.js index 33d6022bf..bd99a1fc5 100644 --- a/src/language-js/print/binaryish.js +++ b/src/language-js/print/binaryish.js @@ -85,7 +85,8 @@ function printBinaryishExpression(path, options, print) { if ( (key === "callee" && (isCallExpression(parent) || parent.type === "NewExpression")) || - parent.type === "UnaryExpression" || + // `UnaryExpression` adds parentheses and indention when argument has comment + (parent.type === "UnaryExpression" && !hasComment(node)) || (isMemberExpression(parent) && !parent.computed) ) { return group([indent([softline, ...parts]), softline]); @@ -112,6 +113,7 @@ function printBinaryishExpression(path, options, print) { !isCallExpression(grandparent) && grandparent.type !== "NewExpression") || parent.type === "TemplateLiteral" || + (key === "argument" && parent.type === "UnaryExpression") || isBooleanTypeCoercion(path); const shouldIndentIfInlining = diff --git a/src/language-js/print/estree.js b/src/language-js/print/estree.js index fbf9a1ef0..ddf9bf1c5 100644 --- a/src/language-js/print/estree.js +++ b/src/language-js/print/estree.js @@ -292,12 +292,14 @@ function printEstree(path, options, print, args) { parts.push(" "); } + const argumentDoc = print("argument"); + if (hasComment(node.argument)) { parts.push( - group(["(", indent([softline, print("argument")]), softline, ")"]), + group(["(", indent([softline, argumentDoc]), softline, ")"]), ); } else { - parts.push(print("argument")); + parts.push(argumentDoc); } return parts; diff --git a/tests/format/js/comments/__snapshots__/format.test.js.snap b/tests/format/js/comments/__snapshots__/format.test.js.snap index 9bc3d111f..5a8cb858c 100644 --- a/tests/format/js/comments/__snapshots__/format.test.js.snap +++ b/tests/format/js/comments/__snapshots__/format.test.js.snap @@ -81,11 +81,9 @@ void( =====================================output===================================== !( - ( - x || // foo - y || // bar - z - ) /* + x || // foo + y || // bar + z /* * comment */ ) @@ -115,12 +113,10 @@ void( ) void ( - ( - (p && // first - q) || // second - (r && // third - s) - ) // fourth + (p && // first + q) || // second + (r && // third + s) // fourth ) void ( @@ -241,11 +237,9 @@ void( =====================================output===================================== !( - ( - x || // foo - y || // bar - z - ) /* + x || // foo + y || // bar + z /* * comment */ ); @@ -275,12 +269,10 @@ void( ); void ( - ( - (p && // first - q) || // second - (r && // third - s) - ) // fourth + (p && // first + q) || // second + (r && // third + s) // fourth ); void ( diff --git a/tests/format/js/logical-expressions/__snapshots__/format.test.js.snap b/tests/format/js/logical-expressions/__snapshots__/format.test.js.snap index 76bd2e589..b7b2dad53 100644 --- a/tests/format/js/logical-expressions/__snapshots__/format.test.js.snap +++ b/tests/format/js/logical-expressions/__snapshots__/format.test.js.snap @@ -1,5 +1,183 @@ // Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing +exports[`in-unary-expression.js format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 + | printWidth +=====================================input====================================== + + +// https://github.com/estree/estree/blob/96fee942ecc2b3b9d3c34163ec142b75daf4cca1/es5.md#unaryoperator +[ +!( + // + octahedralAnteaterGenerator || + // + stellatedFossilProcessor +), ++( + // + octahedralAnteaterGenerator && + // + stellatedFossilProcessor +), +typeof ( + // + octahedralAnteaterGenerator ?? + // + stellatedFossilProcessor +), +]; + +// Realworld case from Prettier repo +if ( + !( + // \`import("foo")\` + ( + node.type === "ImportExpression" || + // \`type foo = import("foo")\` + node.type === "TSImportType" || + // \`import type A = require("foo")\` + node.type === "TSExternalModuleReference" || + // \`require("foo")\` + // \`require.resolve("foo")\` + // \`require.resolve.paths("foo")\` + // \`import.meta.resolve("foo")\` + (node.type === "CallExpression" && + !node.optional && + isNodeMatches(node.callee, moduleImportCallees)) + ) + ) +) { +} + +// Realworld case from Babel repo +const argsOptEligible = + !state.deopted && + !( + // ex: \`args[0] = "whatever"\` + ( + (grandparentPath.isAssignmentExpression() && + parentPath.node === grandparentPath.node.left) || + // ex: \`[args[0]] = ["whatever"]\` + grandparentPath.isLVal() || + // ex: \`for (rest[0] in this)\` + // ex: \`for (rest[0] of this)\` + grandparentPath.isForXStatement() || + // ex: \`++args[0]\` + // ex: \`args[0]--\` + grandparentPath.isUpdateExpression() || + // ex: \`delete args[0]\` + grandparentPath.isUnaryExpression({ operator: "delete" }) || + // ex: \`args[0]()\` + // ex: \`new args[0]()\` + // ex: \`new args[0]\` + ((grandparentPath.isCallExpression() || + grandparentPath.isNewExpression()) && + parentPath.node === grandparentPath.node.callee) + ) + ); + +// Realworld case from excalidraw repo +const foo = () => + !!( + // versions are required integers + ( + Number.isInteger(deleted.version) && + Number.isInteger(inserted.version) && + // versions should be positive, zero included + deleted.version >= 0 && + inserted.version >= 0 && + // versions should never be the same + deleted.version !== inserted.version + ) + ); + +=====================================output===================================== +// https://github.com/estree/estree/blob/96fee942ecc2b3b9d3c34163ec142b75daf4cca1/es5.md#unaryoperator +[ + !( + // + octahedralAnteaterGenerator || + // + stellatedFossilProcessor + ), + +( + // + octahedralAnteaterGenerator && + // + stellatedFossilProcessor + ), + typeof ( + // + octahedralAnteaterGenerator ?? + // + stellatedFossilProcessor + ), +]; + +// Realworld case from Prettier repo +if ( + !( + // \`import("foo")\` + node.type === "ImportExpression" || + // \`type foo = import("foo")\` + node.type === "TSImportType" || + // \`import type A = require("foo")\` + node.type === "TSExternalModuleReference" || + // \`require("foo")\` + // \`require.resolve("foo")\` + // \`require.resolve.paths("foo")\` + // \`import.meta.resolve("foo")\` + (node.type === "CallExpression" && + !node.optional && + isNodeMatches(node.callee, moduleImportCallees)) + ) +) { +} + +// Realworld case from Babel repo +const argsOptEligible = + !state.deopted && + !( + // ex: \`args[0] = "whatever"\` + (grandparentPath.isAssignmentExpression() && + parentPath.node === grandparentPath.node.left) || + // ex: \`[args[0]] = ["whatever"]\` + grandparentPath.isLVal() || + // ex: \`for (rest[0] in this)\` + // ex: \`for (rest[0] of this)\` + grandparentPath.isForXStatement() || + // ex: \`++args[0]\` + // ex: \`args[0]--\` + grandparentPath.isUpdateExpression() || + // ex: \`delete args[0]\` + grandparentPath.isUnaryExpression({ operator: "delete" }) || + // ex: \`args[0]()\` + // ex: \`new args[0]()\` + // ex: \`new args[0]\` + ((grandparentPath.isCallExpression() || + grandparentPath.isNewExpression()) && + parentPath.node === grandparentPath.node.callee) + ); + +// Realworld case from excalidraw repo +const foo = () => + !!( + // versions are required integers + Number.isInteger(deleted.version) && + Number.isInteger(inserted.version) && + // versions should be positive, zero included + deleted.version >= 0 && + inserted.version >= 0 && + // versions should never be the same + deleted.version !== inserted.version + ); + +================================================================================ +`; + exports[`issue-7024.js format 1`] = ` ====================================options===================================== parsers: ["babel", "flow", "typescript"] diff --git a/tests/format/js/logical-expressions/in-unary-expression.js b/tests/format/js/logical-expressions/in-unary-expression.js new file mode 100644 index 000000000..07ea40e6e --- /dev/null +++ b/tests/format/js/logical-expressions/in-unary-expression.js @@ -0,0 +1,87 @@ + + +// https://github.com/estree/estree/blob/96fee942ecc2b3b9d3c34163ec142b75daf4cca1/es5.md#unaryoperator +[ +!( + // + octahedralAnteaterGenerator || + // + stellatedFossilProcessor +), ++( + // + octahedralAnteaterGenerator && + // + stellatedFossilProcessor +), +typeof ( + // + octahedralAnteaterGenerator ?? + // + stellatedFossilProcessor +), +]; + +// Realworld case from Prettier repo +if ( + !( + // `import("foo")` + ( + node.type === "ImportExpression" || + // `type foo = import("foo")` + node.type === "TSImportType" || + // `import type A = require("foo")` + node.type === "TSExternalModuleReference" || + // `require("foo")` + // `require.resolve("foo")` + // `require.resolve.paths("foo")` + // `import.meta.resolve("foo")` + (node.type === "CallExpression" && + !node.optional && + isNodeMatches(node.callee, moduleImportCallees)) + ) + ) +) { +} + +// Realworld case from Babel repo +const argsOptEligible = + !state.deopted && + !( + // ex: `args[0] = "whatever"` + ( + (grandparentPath.isAssignmentExpression() && + parentPath.node === grandparentPath.node.left) || + // ex: `[args[0]] = ["whatever"]` + grandparentPath.isLVal() || + // ex: `for (rest[0] in this)` + // ex: `for (rest[0] of this)` + grandparentPath.isForXStatement() || + // ex: `++args[0]` + // ex: `args[0]--` + grandparentPath.isUpdateExpression() || + // ex: `delete args[0]` + grandparentPath.isUnaryExpression({ operator: "delete" }) || + // ex: `args[0]()` + // ex: `new args[0]()` + // ex: `new args[0]` + ((grandparentPath.isCallExpression() || + grandparentPath.isNewExpression()) && + parentPath.node === grandparentPath.node.callee) + ) + ); + +// Realworld case from excalidraw repo +const foo = () => + !!( + // versions are required integers + ( + Number.isInteger(deleted.version) && + Number.isInteger(inserted.version) && + // versions should be positive, zero included + deleted.version >= 0 && + inserted.version >= 0 && + // versions should never be the same + deleted.version !== inserted.version + ) + ); diff --git a/tests/format/js/unary-expression/__snapshots__/format.test.js.snap b/tests/format/js/unary-expression/__snapshots__/format.test.js.snap index 5240a25a7..1c791f8a3 100644 --- a/tests/format/js/unary-expression/__snapshots__/format.test.js.snap +++ b/tests/format/js/unary-expression/__snapshots__/format.test.js.snap @@ -309,33 +309,33 @@ async function bar2() { ); !(x + y); -!((x + y) /* foo */); -!(/* foo */ (x + y)); +!(x + y /* foo */); +!(/* foo */ x + y); !( /* foo */ - (x + y) + x + y ); !( - (x + y) + x + y /* foo */ ); !( - (x + y) // foo + x + y // foo ); !(x || y); -!(/* foo */ (x || y)); -!((x || y) /* foo */); +!(/* foo */ x || y); +!(x || y /* foo */); !( /* foo */ - (x || y) + x || y ); !( - (x || y) + x || y /* foo */ ); !( - (x || y) // foo + x || y // foo ); ![1, 2, 3];