Parenthesize function expressions in expression position (#941)

* refactor needsParens for function expressions

* snapshots

* comment

src/fast-path.js

@@ -246,9 +246,8 @@ FPp.needsParens = function(assumeExpressionContext) {
   }
 
   if (
-    ((parent.type === "ArrowFunctionExpression" && parent.body === node) ||
-    parent.type === "ExpressionStatement") &&
-    startsWithOpenCurlyBrace(node)
+    parent.type === "ArrowFunctionExpression" && parent.body === node && startsWithNoLookaheadToken(node, /* forbidFunctionAndClass */ false)
+    || parent.type === "ExpressionStatement" && startsWithNoLookaheadToken(node, /* forbidFunctionAndClass */ true)
   ) {
     return true;
   }
@@ -454,66 +453,47 @@ FPp.needsParens = function(assumeExpressionContext) {
       }
 
     case "FunctionExpression":
-    case "ArrowFunctionExpression":
-      if (parent.type === "CallExpression" && name === "callee") {
-        return true;
+      switch (parent.type) {
+        case "CallExpression":
+          return name === "callee"; // Not strictly necessary, but it's clearer to the reader if IIFEs are wrapped in parentheses.
+        case "TaggedTemplateExpression":
+          return true; // This is basically a kind of IIFE.
+        case "ExportDefaultDeclaration":
+          return true;
+        default:
+          return false;
       }
 
+    case "ArrowFunctionExpression":
       switch (parent.type) {
-        case "ConditionalExpression":
-          if (parent.test === node) {
-            return true;
-          }
-          return false;
-
-        case "ExportDefaultDeclaration":
-          if (
-            node.type === "ArrowFunctionExpression" ||
-            node.type === "FunctionDeclaration"
-          ) {
-            return false;
-          }
-          return true;
-
-        case "ExpressionStatement":
-        case "MemberExpression":
-        case "TaggedTemplateExpression":
-        case "UnaryExpression":
-          return true;
+        case "CallExpression":
+          return name === "callee";
 
         case "NewExpression":
           return name === "callee";
 
-        case "LogicalExpression":
-          return node.type === "ArrowFunctionExpression";
-
-        default:
-          return isBinary(parent);
-      }
-
-    case "ClassExpression":
-      switch (parent.type) {
-        case "TaggedTemplateExpression":
-        case "BinaryExpression":
-        case "ExportDefaultDeclaration":
-        case "ExpressionStatement":
-          return true;
-        case "CallExpression":
-          if (parent.callee === node) {
-            return true;
-          }
         case "MemberExpression":
-          return name === "object" && parent.object === node;
+          return name === "object";
+
+        case "BindExpression":
+        case "TaggedTemplateExpression":
+        case "UnaryExpression":  
+        case "LogicalExpression":
+        case "BinaryExpression":
+          return true;
+
         case "ConditionalExpression":
-          if (parent.test === node) {
-            return true;
-          }
+          return name === "test";
+
         default:
           return false;
       }
 
+    case "ClassExpression":
+      return parent.type === "ExportDefaultDeclaration";
+
     case "StringLiteral":
-      return parent.type === "ExpressionStatement"; // to avoid becoming a directive
+      return parent.type === "ExpressionStatement"; // To avoid becoming a directive
   }
 
   if (
@@ -524,14 +504,6 @@ FPp.needsParens = function(assumeExpressionContext) {
     return containsCallExpression(node);
   }
 
-  if (
-    assumeExpressionContext !== true &&
-    !this.canBeFirstInStatement() &&
-    this.firstInStatement()
-  ) {
-    return true;
-  }
-
   return false;
 };
 
@@ -557,25 +529,38 @@ function containsCallExpression(node) {
   return false;
 }
 
-function startsWithOpenCurlyBrace(node) {
+// Tests if an expression starts with `{`, or (if forbidFunctionAndClass holds) `function` or `class`.
+// Will be overzealous if there's already necessary grouping parentheses.
+function startsWithNoLookaheadToken(node, forbidFunctionAndClass) {
   node = getLeftMost(node);
   switch (node.type) {
+    case "FunctionExpression":
+    case "ClassExpression":
+      return forbidFunctionAndClass;
     case "ObjectExpression":
       return true;
     case "MemberExpression":
-      return startsWithOpenCurlyBrace(node.object);
+      return startsWithNoLookaheadToken(node.object, forbidFunctionAndClass);
     case "TaggedTemplateExpression":
-      return startsWithOpenCurlyBrace(node.tag);
+      if (node.tag.type === "FunctionExpression") {
+        // IIFEs are always already parenthesized
+        return false;
+      }
+      return startsWithNoLookaheadToken(node.tag, forbidFunctionAndClass);
     case "CallExpression":
-      return startsWithOpenCurlyBrace(node.callee);
+      if (node.callee.type === "FunctionExpression") {
+        // IIFEs are always already parenthesized
+        return false;
+      }
+      return startsWithNoLookaheadToken(node.callee, forbidFunctionAndClass);
     case "ConditionalExpression":
-      return startsWithOpenCurlyBrace(node.test);
+      return startsWithNoLookaheadToken(node.test, forbidFunctionAndClass);
     case "UpdateExpression":
-      return !node.prefix && startsWithOpenCurlyBrace(node.argument);
+      return !node.prefix && startsWithNoLookaheadToken(node.argument, forbidFunctionAndClass);
     case "BindExpression":
-      return node.object && startsWithOpenCurlyBrace(node.object);
+      return node.object && startsWithNoLookaheadToken(node.object, forbidFunctionAndClass);
     case "SequenceExpression":
-      return startsWithOpenCurlyBrace(node.expressions[0])
+      return startsWithNoLookaheadToken(node.expressions[0], forbidFunctionAndClass)
     default:
       return false;
   }
@@ -589,82 +574,4 @@ function getLeftMost(node) {
   }
 }
 
-FPp.canBeFirstInStatement = function() {
-  const node = this.getNode();
-  return !n.FunctionExpression.check(node) && !n.ClassExpression.check(node);
-};
-
-FPp.firstInStatement = function() {
-  var s = this.stack;
-  var parentName, parent;
-  var childName, child;
-
-  for (var i = s.length - 1; i >= 0; i -= 2) {
-    if (n.Node.check(s[i])) {
-      childName = parentName;
-      child = parent;
-      parentName = s[i - 1];
-      parent = s[i];
-    }
-
-    if (!parent || !child) {
-      continue;
-    }
-
-    if (
-      n.BlockStatement.check(parent) && parentName === "body" && childName === 0
-    ) {
-      assert.strictEqual(parent.body[0], child);
-      return true;
-    }
-
-    if (n.ExpressionStatement.check(parent) && childName === "expression") {
-      assert.strictEqual(parent.expression, child);
-      return true;
-    }
-
-    if (
-      n.SequenceExpression.check(parent) &&
-      parentName === "expressions" &&
-      childName === 0
-    ) {
-      assert.strictEqual(parent.expressions[0], child);
-      continue;
-    }
-
-    if (n.CallExpression.check(parent) && childName === "callee") {
-      assert.strictEqual(parent.callee, child);
-      continue;
-    }
-
-    if (n.MemberExpression.check(parent) && childName === "object") {
-      assert.strictEqual(parent.object, child);
-      continue;
-    }
-
-    if (n.ConditionalExpression.check(parent) && childName === "test") {
-      assert.strictEqual(parent.test, child);
-      continue;
-    }
-
-    if (isBinary(parent) && childName === "left") {
-      assert.strictEqual(parent.left, child);
-      continue;
-    }
-
-    if (
-      n.UnaryExpression.check(parent) &&
-      !parent.prefix &&
-      childName === "argument"
-    ) {
-      assert.strictEqual(parent.argument, child);
-      continue;
-    }
-
-    return false;
-  }
-
-  return true;
-};
-
 module.exports = FastPath;

tests/arrows/__snapshots__/jsfmt.spec.js.snap

@@ -26,7 +26,10 @@ a => ({}::b()\`\`[''].c++ && 0 ? 0 : 0)
 a => ({}().c = 0)
 x => ({}()())
 x => ({}()\`\`)
-x => ({}().b)
+x => ({}().b);
+(a => b)::c;
+a::(b => c);
+a = b => c;
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 (a => {}).length;
 typeof (() => {});
@@ -48,13 +51,16 @@ a = () => ({}[b] && a);
 a = () => ({}\`\` && a);
 a = () => ({} = 0);
 a = () => ({}, a);
-(a => a instanceof {});
-(a => ({}().b && 0));
-(a => ({}::b()\`\`[\\"\\"].c++ && 0 ? 0 : 0));
-(a => ({}().c = 0));
-(x => ({}()()));
-(x => ({}()\`\`));
-(x => ({}().b));
+a => a instanceof {};
+a => ({}().b && 0);
+a => ({}::b()\`\`[\\"\\"].c++ && 0 ? 0 : 0);
+a => ({}().c = 0);
+x => ({}()());
+x => ({}()\`\`);
+x => ({}().b);
+(a => b)::c;
+a::(b => c);
+a = b => c;
 "
 `;
 

tests/arrows/arrow_function_expression.js

@@ -23,4 +23,7 @@ a => ({}::b()``[''].c++ && 0 ? 0 : 0)
 a => ({}().c = 0)
 x => ({}()())
 x => ({}()``)
-x => ({}().b)
+x => ({}().b);
+(a => b)::c;
+a::(b => c);
+a = b => c;

tests/classes/__snapshots__/jsfmt.spec.js.snap

@@ -6,10 +6,10 @@ exports[`binary.js 1`] = `
 (class extends b {}) + 1;
 (class a extends b {}) + 1;
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-(class {}) + 1;
-(class a {}) + 1;
-(class extends b {}) + 1;
-(class a extends b {}) + 1;
+(class {} + 1);
+(class a {} + 1);
+(class extends b {} + 1);
+(class a extends b {} + 1);
 "
 `;
 
@@ -29,7 +29,7 @@ class MyContractSelectionWidget
 exports[`call.js 1`] = `
 "(class {})(class {});
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-(class {})(class {});
+(class {}(class {}));
 "
 `;
 
@@ -68,14 +68,14 @@ exports[`member.js 1`] = `
 "(class {})[1];
 (class {}).a;
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-(class {})[1];
-(class {}).a;
+(class {}[1]);
+(class {}.a);
 "
 `;
 
 exports[`ternary.js 1`] = `
 "if (1) (class {}) ? 1 : 2;
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-if (1) (class {}) ? 1 : 2;
+if (1) (class {} ? 1 : 2);
 "
 `;

tests/export_default/__snapshots__/jsfmt.spec.js.snap

@@ -3,6 +3,6 @@
 exports[`body.js 1`] = `
 "export default (class {}[1] = 1);
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-export default ((class {})[1] = 1);
+export default (class {}[1] = 1);
 "
 `;

tests/flow/async/__snapshots__/jsfmt.spec.js.snap

@@ -189,7 +189,7 @@ class C {
 var e = async function() {};
 var et = async function<T>(a: T) {};
 
-var n = new (async function() {})();
+var n = new async function() {}();
 
 var o = { async m() {} };
 var ot = { async m<T>(a: T) {} };
@@ -497,9 +497,9 @@ var et = async function<T>(a: T) {
   await 1;
 };
 
-var n = new (async function() {
+var n = new async function() {
   await 1;
-})();
+}();
 
 var o = {
   async m() {

tests/flow/async_iteration/__snapshots__/jsfmt.spec.js.snap

@@ -42,11 +42,11 @@ async function* delegate_yield() {
   }
   yield* inner();
 }
-(async () => {
+async () => {
   for await (const x of delegate_yield()) {
     (x: void); // error: number ~> void
   }
-});
+};
 
 async function* delegate_return() {
   async function* inner() {
@@ -203,13 +203,13 @@ async function* catch_return() {
   }
 }
 
-(async () => {
+async () => {
   catch_return().throw(\\"\\").then(({ value }) => {
     if (value !== undefined) {
       (value: void); // error: number ~> void
     }
   });
-});
+};
 
 async function* yield_return() {
   try {
@@ -220,12 +220,12 @@ async function* yield_return() {
   }
 }
 
-(async () => {
+async () => {
   yield_return().throw(\\"\\").then(({ value }) => {
     if (value !== undefined) {
       (value: void); // error: number ~> void
     }
   });
-});
+};
 "
 `;

tests/flow/predicates-parsing/__snapshots__/jsfmt.spec.js.snap

@@ -113,14 +113,14 @@ var a0 = (x: mixed) => x !== null;
 
 var a1 = (x: mixed): %checks => x !== null;
 
-((x): %checks => x !== null);
+(x): %checks => x !== null;
 
 const insert_a_really_big_predicated_arrow_function_name_here = (x): %checks =>
   x !== null;
 
 declare var x;
 x;
-(checks => 123);
+checks => 123;
 
 type checks = any;
 

tests/function/__snapshots__/jsfmt.spec.js.snap

@@ -9,14 +9,20 @@ export default (function() {})();
 new (function() {});
 (function() {});
 a = function f() {} || b;
+(function() {} && a);
+a + function() {};
+new function() {};
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-(function() {}).length;
-typeof (function() {});
+(function() {}.length);
+typeof function() {};
 export default (function() {})();
 (function() {})()\`\`;
 (function() {})\`\`;
-new (function() {})();
+new function() {}();
 (function() {});
 a = function f() {} || b;
+(function() {} && a);
+a + function() {};
+new function() {}();
 "
 `;

tests/function/function_expression.js

@@ -6,3 +6,6 @@ export default (function() {})();
 new (function() {});
 (function() {});
 a = function f() {} || b;
+(function() {} && a);
+a + function() {};
+new function() {};

tests/objects/__snapshots__/jsfmt.spec.js.snap

@@ -11,7 +11,7 @@ a = () => ({}).x;
 ({} = 0);
 (({} = 0), 1);
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-(() => ({}\`\`));
+() => ({}\`\`);
 ({}\`\`);
 a = () => ({}.x);
 ({} && a, b);

tests/template/__snapshots__/jsfmt.spec.js.snap

@@ -107,7 +107,7 @@ async function f() {
 b()\`\`;
 
 // \\"ClassExpression\\"
-(class {})\`\`;
+(class {}\`\`);
 
 // \\"ConditionalExpression\\"
 (b ? c : d)\`\`;