Skip to content

Commit 7959b12

Browse files
jridgewelllydell
jridgewell
authored and
lydell
committed
Don't require parens for same-operator logical expressions (prettier#6864)
Multiple same-operator logical expressions do not require parentheses to disambiguate.
1 parent 3618361 commit 7959b12

8 files changed

Lines changed: 173 additions & 14 deletions

File tree

‎CHANGELOG.unreleased.md‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1406,6 +1406,26 @@ async function f() {
14061406
}
14071407
```
14081408

1409+
#### JavaScript: Don't require parens for same-operator logical expressions ([#6864] by [@jridgewell])
1410+
1411+
<!-- prettier-ignore -->
1412+
```js
1413+
// Input
1414+
foo && (bar && baz);
1415+
foo || (bar || baz);
1416+
foo ?? (bar ?? baz);
1417+
1418+
// Output (Prettier stable)
1419+
foo && (bar && baz);
1420+
foo || (bar || baz);
1421+
foo ?? (bar ?? baz);
1422+
1423+
// Output (Prettier master)
1424+
foo && bar && baz;
1425+
foo || bar || baz;
1426+
foo ?? bar ?? baz;
1427+
```
1428+
14091429
#### CLI: Display invalid config filename in error message ([#6865] by [@fisker])
14101430

14111431
<!-- prettier-ignore -->
@@ -1515,6 +1535,7 @@ color: #F00
15151535
[#6865]: https://github.com/prettier/prettier/pull/6865
15161536
[#6875]: https://github.com/prettier/prettier/pull/6875
15171537
[#6863]: https://github.com/prettier/prettier/pull/6863
1538+
[#6864]: https://github.com/prettier/prettier/pull/6864
15181539
[@brainkim]: https://github.com/brainkim
15191540
[@duailibe]: https://github.com/duailibe
15201541
[@gavinjoyce]: https://github.com/gavinjoyce

‎cspell.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,7 @@
288288
"React's",
289289
"readline",
290290
"readlines",
291+
"rebalance",
291292
"rebeccapurple",
292293
"recurse",
293294
"recurses",

‎src/language-js/clean.js‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,11 @@ function clean(ast, newObj, parent) {
3737
return null;
3838
}
3939

40+
// We remove unneeded parens around same-operator LogicalExpressions
41+
if (isUnbalancedLogicalTree(newObj)) {
42+
return rebalanceLogicalTree(newObj);
43+
}
44+
4045
// (TypeScript) Ignore `static` in `constructor(static p) {}`
4146
// and `export` in `constructor(export p) {}`
4247
if (
@@ -194,4 +199,32 @@ function clean(ast, newObj, parent) {
194199
}
195200
}
196201

202+
function isUnbalancedLogicalTree(newObj) {
203+
return (
204+
newObj.type === "LogicalExpression" &&
205+
newObj.right.type === "LogicalExpression" &&
206+
newObj.operator === newObj.right.operator
207+
);
208+
}
209+
210+
function rebalanceLogicalTree(newObj) {
211+
if (isUnbalancedLogicalTree(newObj)) {
212+
return rebalanceLogicalTree({
213+
type: "LogicalExpression",
214+
operator: newObj.operator,
215+
left: rebalanceLogicalTree({
216+
type: "LogicalExpression",
217+
operator: newObj.operator,
218+
left: newObj.left,
219+
right: newObj.right.left,
220+
loc: {}
221+
}),
222+
right: newObj.right.right,
223+
loc: {}
224+
});
225+
}
226+
227+
return newObj;
228+
}
229+
197230
module.exports = clean;

‎src/language-js/needs-parens.js‎

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -339,8 +339,13 @@ function needsParens(path, options) {
339339
(node.type === "TSTypeAssertion" || node.type === "TSAsExpression")
340340
);
341341

342-
case "BinaryExpression":
343-
case "LogicalExpression": {
342+
case "LogicalExpression":
343+
if (node.type === "LogicalExpression") {
344+
return parent.operator !== node.operator;
345+
}
346+
// else fallthrough
347+
348+
case "BinaryExpression": {
344349
if (!node.operator && node.type !== "TSTypeAssertion") {
345350
return true;
346351
}
@@ -354,17 +359,6 @@ function needsParens(path, options) {
354359
return true;
355360
}
356361

357-
if (
358-
(po === "??" && (no === "||" || no === "&&")) ||
359-
(no === "??" && (po === "||" || po === "&&"))
360-
) {
361-
return true;
362-
}
363-
364-
if (po === "||" && no === "&&") {
365-
return true;
366-
}
367-
368362
if (pp === np && name === "right") {
369363
assert.strictEqual(parent.right, node);
370364
return true;

‎tests/logical_expressions/__snapshots__/jsfmt.spec.js.snap‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
// Jest Snapshot v1, https://goo.gl/fbAQLP
2+
3+
exports[`logical_expression_operators.js 1`] = `
4+
====================================options=====================================
5+
parsers: ["babel", "flow", "typescript"]
6+
printWidth: 80
7+
| printWidth
8+
=====================================input======================================
9+
// Same operators do not require parens
10+
(foo && bar) && baz;
11+
foo && (bar && baz);
12+
foo && ((bar && baz) && qux);
13+
foo && (bar && (baz && qux));
14+
foo && (bar && ((baz && qux) && xyz));
15+
foo && (bar && (baz && (qux && xyz)));
16+
17+
(foo || bar) || baz;
18+
foo || (bar || baz);
19+
foo || ((bar || baz) || qux);
20+
foo || (bar || (baz || qux));
21+
foo || (bar || ((baz || qux) || xyz));
22+
foo || (bar || (baz || (qux || xyz)));
23+
24+
(foo ?? bar) ?? baz;
25+
foo ?? (bar ?? baz);
26+
foo ?? ((bar ?? baz) ?? qux);
27+
foo ?? (bar ?? (baz ?? qux));
28+
foo ?? (bar ?? ((baz ?? qux) ?? xyz));
29+
foo ?? (bar ?? (baz ?? (qux ?? xyz)));
30+
31+
// Explicitly parenthesized && and || requires parens
32+
(foo && bar) || baz;
33+
(foo || bar) && baz;
34+
35+
foo && (bar || baz);
36+
foo || (bar && baz);
37+
38+
// Implicitly parenthesized && and || requires parens
39+
foo && bar || baz;
40+
foo || bar && baz;
41+
42+
=====================================output=====================================
43+
// Same operators do not require parens
44+
foo && bar && baz;
45+
foo && bar && baz;
46+
foo && bar && baz && qux;
47+
foo && bar && baz && qux;
48+
foo && bar && baz && qux && xyz;
49+
foo && bar && baz && qux && xyz;
50+
51+
foo || bar || baz;
52+
foo || bar || baz;
53+
foo || bar || baz || qux;
54+
foo || bar || baz || qux;
55+
foo || bar || baz || qux || xyz;
56+
foo || bar || baz || qux || xyz;
57+
58+
foo ?? bar ?? baz;
59+
foo ?? bar ?? baz;
60+
foo ?? bar ?? baz ?? qux;
61+
foo ?? bar ?? baz ?? qux;
62+
foo ?? bar ?? baz ?? qux ?? xyz;
63+
foo ?? bar ?? baz ?? qux ?? xyz;
64+
65+
// Explicitly parenthesized && and || requires parens
66+
(foo && bar) || baz;
67+
(foo || bar) && baz;
68+
69+
foo && (bar || baz);
70+
foo || (bar && baz);
71+
72+
// Implicitly parenthesized && and || requires parens
73+
(foo && bar) || baz;
74+
foo || (bar && baz);
75+
76+
================================================================================
77+
`;

‎tests/logical_expressions/jsfmt.spec.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
run_spec(__dirname, ["babel", "flow", "typescript"]);

‎tests/logical_expressions/logical_expression_operators.js‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
// Same operators do not require parens
2+
(foo && bar) && baz;
3+
foo && (bar && baz);
4+
foo && ((bar && baz) && qux);
5+
foo && (bar && (baz && qux));
6+
foo && (bar && ((baz && qux) && xyz));
7+
foo && (bar && (baz && (qux && xyz)));
8+
9+
(foo || bar) || baz;
10+
foo || (bar || baz);
11+
foo || ((bar || baz) || qux);
12+
foo || (bar || (baz || qux));
13+
foo || (bar || ((baz || qux) || xyz));
14+
foo || (bar || (baz || (qux || xyz)));
15+
16+
(foo ?? bar) ?? baz;
17+
foo ?? (bar ?? baz);
18+
foo ?? ((bar ?? baz) ?? qux);
19+
foo ?? (bar ?? (baz ?? qux));
20+
foo ?? (bar ?? ((baz ?? qux) ?? xyz));
21+
foo ?? (bar ?? (baz ?? (qux ?? xyz)));
22+
23+
// Explicitly parenthesized && and || requires parens
24+
(foo && bar) || baz;
25+
(foo || bar) && baz;
26+
27+
foo && (bar || baz);
28+
foo || (bar && baz);
29+
30+
// Implicitly parenthesized && and || requires parens
31+
foo && bar || baz;
32+
foo || bar && baz;

‎tests/nullish_coalescing/__snapshots__/jsfmt.spec.js.snap‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ const x = (foo, bar = foo ?? bar) => {};
3636
3737
foo ? bar ?? foo : baz;
3838
39-
foo ?? (bar ?? baz);
39+
foo ?? bar ?? baz;
4040
foo ?? bar ?? baz;
4141
4242
// Mixing ?? and (&& or ||) requires parens

0 commit comments

Comments
 (0)