From 9b208a517af0aa088878ee16f1bccb2f3cfaabc0 Mon Sep 17 00:00:00 2001
From: Ben Newman <3439902+divanutq@users.noreply.github.com>
Date: Wed, 14 May 2014 16:14:28 -0400
Subject: [PATCH] Make path.needsParens(true) ignore statement parsing
 ambiguities.

As @spicyj pointed out in
https://github.com/benjamn/recast/issues/81#issuecomment-42729389, the
NodePath.prototype.needsParens method has been conflating precedence logic
with garden-path parser lookahead quirks.

Rather than removing that edge case logic from the function, I have made
it possible to skip the canBeFirstInStatement/firstInStatement checks by
explicitly passing true to needsParens.

Since the behavior of needsParens differs only if client code passes true
for the assumeExpressionContext parameter, this change is completely
backwards-compatible and does not require a minor version bump.
---
 lib/node-path.js | 28 ++++++++++++++++++++++------
 test/run.js      | 18 ++++++++++++++++++
 2 files changed, 40 insertions(+), 6 deletions(-)

diff --git a/lib/node-path.js b/lib/node-path.js
index 8a90126..99a037b 100644
--- a/lib/node-path.js
+++ b/lib/node-path.js
@@ -97,7 +97,20 @@ NPp.getValueProperty = function(name) {
     return types.getFieldValue(this.value, name);
 };
 
-NPp.needsParens = function() {
+/**
+ * Determine whether this.node needs to be wrapped in parentheses in order
+ * for a parser to reproduce the same local AST structure.
+ *
+ * For instance, in the expression `(1 + 2) * 3`, the BinaryExpression
+ * whose operator is "+" needs parentheses, because `1 + 2 * 3` would
+ * parse differently.
+ *
+ * If assumeExpressionContext === true, we don't worry about edge cases
+ * like an anonymous FunctionExpression appearing lexically first in its
+ * enclosing statement and thus needing parentheses to avoid being parsed
+ * as a FunctionDeclaration with a missing name.
+ */
+NPp.needsParens = function(assumeExpressionContext) {
     if (!this.parent)
         return false;
 
@@ -215,11 +228,8 @@ NPp.needsParens = function() {
         }
     }
 
-    if (n.FunctionExpression.check(node) &&
-        this.firstInStatement())
-        return true;
-
-    if (n.ObjectExpression.check(node) &&
+    if (assumeExpressionContext !== true &&
+        !this.canBeFirstInStatement() &&
         this.firstInStatement())
         return true;
 
@@ -266,6 +276,12 @@ function containsCallExpression(node) {
     return false;
 }
 
+NPp.canBeFirstInStatement = function() {
+    var node = this.node;
+    return !n.FunctionExpression.check(node)
+        && !n.ObjectExpression.check(node);
+};
+
 NPp.firstInStatement = function() {
     return firstInStatement(this);
 };
diff --git a/test/run.js b/test/run.js
index 47cee88..67f4c52 100644
--- a/test/run.js
+++ b/test/run.js
@@ -489,6 +489,24 @@ describe("NodePath", function() {
         var sequenceAssignmentPath = new NodePath(sequenceAssignmentAST);
         assert.ok(sequenceAssignmentPath.get("right").needsParens());
     });
+
+    it("should support .needsParens(true)", function() {
+        var programPath = new NodePath(parse("(function(){})"));
+        var funExpPath = programPath.get("body", 0, "expression");
+        n.FunctionExpression.assert(funExpPath.value);
+        assert.strictEqual(funExpPath.needsParens(), true);
+        assert.strictEqual(funExpPath.canBeFirstInStatement(), false);
+        assert.strictEqual(funExpPath.firstInStatement(), true);
+        assert.strictEqual(funExpPath.needsParens(true), false);
+
+        programPath = new NodePath(parse("({ foo: 42 })"));
+        var objLitPath = programPath.get("body", 0, "expression");
+        n.ObjectExpression.assert(objLitPath.value);
+        assert.strictEqual(objLitPath.needsParens(), true);
+        assert.strictEqual(objLitPath.canBeFirstInStatement(), false);
+        assert.strictEqual(objLitPath.firstInStatement(), true);
+        assert.strictEqual(objLitPath.needsParens(true), false);
+    });
 });
 
 describe("path.replace", function() {