Fix nullish coalescing parenthesis with mixed logical operators (prettier#6863)

jridgewell · thorn0 · commit 8188876633ad · 2019-11-07T20:56:44.000+02:00
* Fix nullish coalescing with mixed logical operators parenthesis Mixing nullish coalescing (`??`) with the other logical operators (`&&` and `||`) requires parenthesis to disambiguate the inteded short circuiting. Without it, it's a `SyntaxError`. Earlier drafts of the spec allowed mixing, but it was disallowed when we reached Stage 3. See https://v8.dev/features/nullish-coalescing#mixing-and-matching-operators * Update changelog * Fixes and cleanup * Update changelog
diff --git a/CHANGELOG.unreleased.md b/CHANGELOG.unreleased.md
@@ -483,20 +483,23 @@ Previously, Prettier would sometimes ignore whitespace when formatting comments.
 </div>
 ```
 
-#### JavaScript: Update `??` precedence to match stage 3 proposal ([#6404] by [@vjeux])
+#### JavaScript: Update `??` precedence to match stage 3 proposal ([#6404] by [@vjeux], [#6863] by [@jridgewell])
 
-We've updated Prettier's support for the nullish coalescing operator to match a spec update that no longer allows it to immediately contain, or be contained within an `&&` or `||` operation.
+We've updated Prettier's support for the nullish coalescing operator to match a spec update that no longer allows it to immediately contain, or be contained within, an `&&` or `||` operation.
 
 <!-- prettier-ignore -->
 ```js
 // Input
-(foo ?? baz) || baz;
+(foo ?? bar) || baz;
+(foo || bar) ?? baz;
 
 // Output (Prettier stable)
-foo ?? baz || baz;
+foo ?? bar || baz;
+foo || bar ?? baz;
 
 // Output (Prettier master)
-(foo ?? baz) || baz;
+(foo ?? bar) || baz;
+(foo || bar) ?? baz;
 ```
 
 Please note, as we update our parsers with versions that support this spec update, code without the parenthesis will throw a parse error.
@@ -1453,6 +1456,7 @@ async function f() {
 [#6796]: https://github.com/prettier/prettier/pull/6796
 [#6848]: https://github.com/prettier/prettier/pull/6848
 [#6856]: https://github.com/prettier/prettier/pull/6856
+[#6863]: https://github.com/prettier/prettier/pull/6863
 [@brainkim]: https://github.com/brainkim
 [@duailibe]: https://github.com/duailibe
 [@gavinjoyce]: https://github.com/gavinjoyce
@@ -1474,3 +1478,4 @@ async function f() {
 [@andersk]: https://github.com/andersk
 [@lydell]: https://github.com/lydell
 [@aymericbouzy]: https://github.com/aymericbouzy
+[@jridgewell]: https://github.com/jridgewell
diff --git a/src/language-js/needs-parens.js b/src/language-js/needs-parens.js
@@ -354,7 +354,14 @@ function needsParens(path, options) {
             return true;
           }
 
-          if ((po === "||" || po === "??") && no === "&&") {
+          if (
+            (po === "??" && (no === "||" || no === "&&")) ||
+            (no === "??" && (po === "||" || po === "&&"))
+          ) {
+            return true;
+          }
+
+          if (po === "||" && no === "&&") {
             return true;
           }
 
diff --git a/tests/nullish_coalescing/__snapshots__/jsfmt.spec.js.snap b/tests/nullish_coalescing/__snapshots__/jsfmt.spec.js.snap
@@ -13,8 +13,18 @@ const x = (foo, bar = foo ?? bar) => {};
 foo ? bar ?? foo : baz;
 
 foo ?? (bar ?? baz);
+(foo ?? bar) ?? baz;
 
+// Mixing ?? and (&& or ||) requires parens
+// It's a syntax error without it.
 (foo ?? baz) || baz;
+foo ?? (baz || baz);
+
+(foo ?? baz) && baz;
+foo ?? (baz && baz);
+
+(foo || baz) ?? baz;
+foo || (baz ?? baz);
 
 (foo && baz) ?? baz;
 foo && (baz ?? baz);
@@ -27,8 +37,18 @@ const x = (foo, bar = foo ?? bar) => {};
 foo ? bar ?? foo : baz;
 
 foo ?? (bar ?? baz);
+foo ?? bar ?? baz;
 
+// Mixing ?? and (&& or ||) requires parens
+// It's a syntax error without it.
 (foo ?? baz) || baz;
+foo ?? (baz || baz);
+
+(foo ?? baz) && baz;
+foo ?? (baz && baz);
+
+(foo || baz) ?? baz;
+foo || (baz ?? baz);
 
 (foo && baz) ?? baz;
 foo && (baz ?? baz);
diff --git a/tests/nullish_coalescing/nullish_coalesing_operator.js b/tests/nullish_coalescing/nullish_coalesing_operator.js
@@ -5,8 +5,18 @@ const x = (foo, bar = foo ?? bar) => {};
 foo ? bar ?? foo : baz;
 
 foo ?? (bar ?? baz);
+(foo ?? bar) ?? baz;
 
+// Mixing ?? and (&& or ||) requires parens
+// It's a syntax error without it.
 (foo ?? baz) || baz;
+foo ?? (baz || baz);
+
+(foo ?? baz) && baz;
+foo ?? (baz && baz);
+
+(foo || baz) ?? baz;
+foo || (baz ?? baz);
 
 (foo && baz) ?? baz;
 foo && (baz ?? baz);

Original file line number	Diff line number	Diff line change
`@@ -354,7 +354,14 @@ function needsParens(path, options) {`
`354`	`354`	`return true;`
`355`	`355`	`}`
`356`	`356`
`357`		`- if ((po === "\|\|" \|\| po === "??") && no === "&&") {`
	`357`	`+ if (`
	`358`	`+ (po === "??" && (no === "\|\|" \|\| no === "&&")) \|\|`
	`359`	`+ (no === "??" && (po === "\|\|" \|\| po === "&&"))`
	`360`	`+ ) {`
	`361`	`+ return true;`
	`362`	`+ }`
	`363`	`+`
	`364`	`+ if (po === "\|\|" && no === "&&") {`
`358`	`365`	`return true;`
`359`	`366`	`}`
`360`	`367`