Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix printing edge cases in Nullish Coalescing and Optional Chaining #11255

Merged
merged 1 commit into from Mar 13, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 1 addition & 21 deletions packages/babel-generator/src/node/index.js
Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
}
25 changes: 18 additions & 7 deletions packages/babel-generator/src/node/parentheses.js
Expand Up @@ -122,8 +122,6 @@ export function Binary(node: Object, parent: Object): boolean {
return true;
}
}

return false;
}

export function UnionTypeAnnotation(node: Object, parent: Object): boolean {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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<Object>,
): 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
Expand Down
@@ -1,3 +1,6 @@
1 + (a = 2);
1 + (a += 2);
a = a || (a = {});

(a = b)();
(a = b)?.();
@@ -1,3 +1,5 @@
1 + (a = 2);
1 + (a += 2);
a = a || (a = {});
a = a || (a = {});
(a = b)();
(a = b)?.();
Expand Up @@ -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) {}

Expand All @@ -32,4 +32,4 @@ async function f1() {

function* f2() {
class A16 extends (yield 1) {}
}
}
@@ -1,2 +1,17 @@
const foo = 'test'
console.log((foo ?? '') == '')
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;

@@ -1,2 +1,12 @@
const foo = 'test';
console.log((foo ?? '') == '');
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;
Expand Up @@ -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;
Expand Up @@ -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;