From 519cdff7f1de57c1a62d85d0e30d91c8da0de706 Mon Sep 17 00:00:00 2001 From: Justin Ridgewell Date: Fri, 13 Mar 2020 04:18:40 -0400 Subject: [PATCH] Fix printing edge cases in Nullish Coalescing and Optional Chaining This is a mix of changes, most importantly: - Always print parens when mixing nullish coalescing and another logical - Fixes assignment in the callee of an optional chain, which now blocks #11248 Also, cleans up a bit of code, and removes an unnecessary parens around `class A extends new B {}` --- packages/babel-generator/src/node/index.js | 22 +--------------- .../babel-generator/src/node/parentheses.js | 25 +++++++++++++------ .../assignment-expression/input.js | 3 +++ .../assignment-expression/output.js | 4 ++- .../parentheses/class-extends/output.js | 4 +-- .../parentheses/nullish-coalescing/input.js | 17 ++++++++++++- .../parentheses/nullish-coalescing/output.js | 12 ++++++++- .../fixtures/types/LogicalExpression/input.js | 12 ++++++++- .../types/LogicalExpression/output.js | 9 ++++++- 9 files changed, 73 insertions(+), 35 deletions(-) diff --git a/packages/babel-generator/src/node/index.js b/packages/babel-generator/src/node/index.js index faf700dc681b..43bddb3e1993 100644 --- a/packages/babel-generator/src/node/index.js +++ b/packages/babel-generator/src/node/index.js @@ -46,14 +46,7 @@ function isOrHasCallExpression(node) { return true; } - if (t.isMemberExpression(node)) { - return ( - isOrHasCallExpression(node.object) || - (!node.computed && isOrHasCallExpression(node.property)) - ); - } else { - return false; - } + return t.isMemberExpression(node) && isOrHasCallExpression(node.object); } export function needsWhitespace(node, parent, type) { @@ -97,18 +90,5 @@ export function needsParens(node, parent, printStack) { if (isOrHasCallExpression(node)) return true; } - /* this check is for NullishCoalescing being used with LogicalOperators like && and || - * For example when someone creates an ast programmaticaly like this - * t.logicalExpression( - * "??", - * t.logicalExpression("||", t.identifier("a"), t.identifier("b")), - * t.identifier("c"), - * ); - * In the example above the AST is equivalent to writing a || b ?? c - * This is incorrect because NullishCoalescing when used with LogicalExpressions should have parenthesis - * The correct syntax is (a || b) ?? c, that is why we need parenthesis in this case - */ - if (t.isLogicalExpression(node) && parent.operator === "??") return true; - return find(expandedParens, node, parent, printStack); } diff --git a/packages/babel-generator/src/node/parentheses.js b/packages/babel-generator/src/node/parentheses.js index 79b97902b8c6..95c32050f783 100644 --- a/packages/babel-generator/src/node/parentheses.js +++ b/packages/babel-generator/src/node/parentheses.js @@ -122,8 +122,6 @@ export function Binary(node: Object, parent: Object): boolean { return true; } } - - return false; } export function UnionTypeAnnotation(node: Object, parent: Object): boolean { @@ -244,7 +242,8 @@ export function ConditionalExpression(node: Object, parent: Object): boolean { t.isBinary(parent) || t.isConditionalExpression(parent, { test: node }) || t.isAwaitExpression(parent) || - t.isOptionalMemberExpression(parent) || + t.isOptionalMemberExpression(parent, { object: node }) || + t.isOptionalCallExpression(parent, { callee: node }) || t.isTaggedTemplateExpression(parent) || t.isTSTypeAssertion(parent) || t.isTSAsExpression(parent) @@ -272,16 +271,28 @@ export function OptionalCallExpression(node: Object, parent: Object): boolean { ); } -export function AssignmentExpression(node: Object): boolean { +export function AssignmentExpression( + node: Object, + parent: Object, + printStack: Array, +): boolean { if (t.isObjectPattern(node.left)) { return true; } else { - return ConditionalExpression(...arguments); + return ConditionalExpression(node, parent, printStack); } } -export function NewExpression(node: Object, parent: Object): boolean { - return isClassExtendsClause(node, parent); +export function LogicalExpression(node: Object, parent: Object): boolean { + switch (node.operator) { + case "||": + if (!t.isLogicalExpression(parent)) return false; + return parent.operator === "??" || parent.operator === "&&"; + case "&&": + return t.isLogicalExpression(parent, { operator: "??" }); + case "??": + return t.isLogicalExpression(parent) && parent.operator !== "??"; + } } // Walk up the print stack to determine if our node can come first diff --git a/packages/babel-generator/test/fixtures/parentheses/assignment-expression/input.js b/packages/babel-generator/test/fixtures/parentheses/assignment-expression/input.js index f1b168c7207f..0144ab8ea036 100644 --- a/packages/babel-generator/test/fixtures/parentheses/assignment-expression/input.js +++ b/packages/babel-generator/test/fixtures/parentheses/assignment-expression/input.js @@ -1,3 +1,6 @@ 1 + (a = 2); 1 + (a += 2); a = a || (a = {}); + +(a = b)(); +(a = b)?.(); diff --git a/packages/babel-generator/test/fixtures/parentheses/assignment-expression/output.js b/packages/babel-generator/test/fixtures/parentheses/assignment-expression/output.js index fbc4744e9e4d..85217e309303 100644 --- a/packages/babel-generator/test/fixtures/parentheses/assignment-expression/output.js +++ b/packages/babel-generator/test/fixtures/parentheses/assignment-expression/output.js @@ -1,3 +1,5 @@ 1 + (a = 2); 1 + (a += 2); -a = a || (a = {}); \ No newline at end of file +a = a || (a = {}); +(a = b)(); +(a = b)?.(); \ No newline at end of file diff --git a/packages/babel-generator/test/fixtures/parentheses/class-extends/output.js b/packages/babel-generator/test/fixtures/parentheses/class-extends/output.js index a898c15b5a13..11b2ee978828 100644 --- a/packages/babel-generator/test/fixtures/parentheses/class-extends/output.js +++ b/packages/babel-generator/test/fixtures/parentheses/class-extends/output.js @@ -12,7 +12,7 @@ class A6 extends class {} {} class A7 extends (B ? C : D) {} -class A8 extends (new B()) {} +class A8 extends new B() {} class A9 extends (B, C) {} @@ -32,4 +32,4 @@ async function f1() { function* f2() { class A16 extends (yield 1) {} -} \ No newline at end of file +} diff --git a/packages/babel-generator/test/fixtures/parentheses/nullish-coalescing/input.js b/packages/babel-generator/test/fixtures/parentheses/nullish-coalescing/input.js index 2e5054151af1..9c0ee73d545b 100644 --- a/packages/babel-generator/test/fixtures/parentheses/nullish-coalescing/input.js +++ b/packages/babel-generator/test/fixtures/parentheses/nullish-coalescing/input.js @@ -1,2 +1,17 @@ const foo = 'test' -console.log((foo ?? '') == '') \ No newline at end of file +console.log((foo ?? '') == '') + + +a ?? (b ?? c); +(a ?? b) ?? c; + +a ?? (b || c); +a || (b ?? c); +(a ?? b) || c; +(a || b) ?? c; + +a ?? (b && c); +a && (b ?? c); +(a ?? b) && c; +(a && b) ?? c; + diff --git a/packages/babel-generator/test/fixtures/parentheses/nullish-coalescing/output.js b/packages/babel-generator/test/fixtures/parentheses/nullish-coalescing/output.js index 767c8f1b02d4..38de5e1b5d2b 100644 --- a/packages/babel-generator/test/fixtures/parentheses/nullish-coalescing/output.js +++ b/packages/babel-generator/test/fixtures/parentheses/nullish-coalescing/output.js @@ -1,2 +1,12 @@ const foo = 'test'; -console.log((foo ?? '') == ''); \ No newline at end of file +console.log((foo ?? '') == ''); +a ?? b ?? c; +a ?? b ?? c; +a ?? (b || c); +a || (b ?? c); +(a ?? b) || c; +(a || b) ?? c; +a ?? (b && c); +a && (b ?? c); +(a ?? b) && c; +(a && b) ?? c; \ No newline at end of file diff --git a/packages/babel-generator/test/fixtures/types/LogicalExpression/input.js b/packages/babel-generator/test/fixtures/types/LogicalExpression/input.js index 3ff8c4d4040e..1f1ae59c50a7 100644 --- a/packages/babel-generator/test/fixtures/types/LogicalExpression/input.js +++ b/packages/babel-generator/test/fixtures/types/LogicalExpression/input.js @@ -2,4 +2,14 @@ foo ||bar; (x => x)|| bar; (function a(x){return x;})|| 2; 0||(function(){return alpha;}); -a ?? (b || c); + +a && (b && c); +(a && b) && c; + +a || (b || c); +(a || b) || c; + +a || (b && c); +a && (b || c); +(a || b) && c; +(a && b) || c; diff --git a/packages/babel-generator/test/fixtures/types/LogicalExpression/output.js b/packages/babel-generator/test/fixtures/types/LogicalExpression/output.js index 994dc9ec1b59..254b73ae3e8c 100644 --- a/packages/babel-generator/test/fixtures/types/LogicalExpression/output.js +++ b/packages/babel-generator/test/fixtures/types/LogicalExpression/output.js @@ -9,4 +9,11 @@ foo || bar; return alpha; }; -a ?? (b || c); +a && b && c; +a && b && c; +a || b || c; +a || b || c; +a || b && c; +a && (b || c); +(a || b) && c; +a && b || c; \ No newline at end of file