generator: rewrite automatic parentheses insertion to be smarter, includes some buffer state that automatically triggers a parentheses to be pushed also has the positive side effect of cleaning up a lot of redundant code - fixes #2064

packages/babel/src/generation/buffer.js

@@ -10,6 +10,8 @@ import isNumber from "lodash/lang/isNumber";
 
 export default class Buffer {
   constructor(position, format) {
+    this.parenPushNewlineState = null;
+
     this.position = position;
     this._indent  = format.indent.base;
     this.format   = format;
@@ -118,6 +120,40 @@ export default class Buffer {
     this.position.unshift(cha);
   }
 
+  /**
+   * Set some state that will be modified if a newline has been inserted before any
+   * non-space characters.
+   *
+   * This is to prevent breaking semantics for terminatorless separator nodes. eg:
+   *
+   *    return foo;
+   *
+   * returns `foo`. But if we do:
+   *
+   *   return
+   *   foo;
+   *
+   *  `undefined` will be returned and not `foo` due to the terminator.
+   */
+
+  startTerminatorless() {
+    return this.parenPushNewlineState = {
+      printed: false
+    };
+  }
+
+  /**
+   * Print an ending parentheses if a starting one has been printed.
+   */
+
+  endTerminatorless(state) {
+    if (state.printed) {
+      this.dedent();
+      this.newline();
+      this.push(")");
+    }
+  }
+
   /**
    * Add a newline (or many newlines), maintaining formatting.
    * Strips multiple newlines if removeLast is true.
@@ -217,6 +253,27 @@ export default class Buffer {
    */
 
   _push(str) {
+    // see startTerminatorless() instance method
+    var parenPushNewlineState = this.parenPushNewlineState;
+    if (parenPushNewlineState) {
+      for (var i = 0; i < str.length; i++) {
+        var cha = str[i];
+
+        // we can ignore spaces since they wont interupt a terminatorless separator
+        if (cha === " ") continue;
+
+        this.parenPushNewlineState = null;
+
+        if (cha === "\n") {
+          // we're going to break this terminator expression so we need to add a parentheses
+          this._push("(");
+          this.indent();
+          parenPushNewlineState.printed = true;
+        }
+      }
+    }
+
+    //
     this.position.push(str);
     this.buf += str;
   }

packages/babel/src/generation/generators/expressions.js

@@ -168,7 +168,9 @@ var buildYieldAwait = function (keyword) {
 
     if (node.argument) {
       this.push(" ");
+      var terminatorState = this.startTerminatorless();
       print.plain(node.argument);
+      this.endTerminatorless(terminatorState);
     }
   };
 };

packages/babel/src/generation/generators/statements.js

@@ -114,14 +114,16 @@ export function DoWhileStatement(node, print) {
  * Prints label (or key).
  */
 
-var buildLabelStatement = function (prefix, key) {
+var buildLabelStatement = function (prefix, key = "label") {
   return function (node, print) {
     this.push(prefix);
 
-    var label = node[key || "label"];
+    var label = node[key];
     if (label) {
       this.push(" ");
+      var terminatorState = this.startTerminatorless();
       print.plain(label);
+      this.endTerminatorless(terminatorState);
     }
 
     this.semicolon();
@@ -135,6 +137,7 @@ var buildLabelStatement = function (prefix, key) {
 export var ContinueStatement = buildLabelStatement("continue");
 export var ReturnStatement   = buildLabelStatement("return", "argument");
 export var BreakStatement    = buildLabelStatement("break");
+export var ThrowStatement    = buildLabelStatement("throw", "argument");
 
 /**
  * Prints LabeledStatement, prints label and body.
@@ -183,16 +186,6 @@ export function CatchClause(node, print) {
   print.plain(node.body);
 }
 
-/**
- * Prints ThrowStatement, prints argument.
- */
-
-export function ThrowStatement(node, print) {
-  this.push("throw ");
-  print.plain(node.argument);
-  this.semicolon();
-}
-
 /**
  * Prints SwitchStatement, prints discriminant and cases.
  */

packages/babel/src/generation/index.js

@@ -161,20 +161,13 @@ class CodeGenerator {
    * [Please add a description.]
    */
 
-  catchUp(node, parent, leftParenPrinted) {
+  catchUp(node) {
     // catch up to this nodes newline if we're behind
     if (node.loc && this.format.retainLines && this.buffer.buf) {
-      var needsParens = false;
-      if (!leftParenPrinted && parent && this.position.line < node.loc.start.line && t.isTerminatorless(parent)) {
-        needsParens = true;
-        this._push("(");
-      }
       while (this.position.line < node.loc.start.line) {
         this._push("\n");
       }
-      return needsParens;
     }
-    return false;
   }
 
   /**
@@ -231,15 +224,12 @@ class CodeGenerator {
       throw new ReferenceError(`unknown node of type ${JSON.stringify(node.type)} with constructor ${JSON.stringify(node && node.constructor.name)}`);
     }
 
-    var needsNoLineTermParens = n.needsParensNoLineTerminator(node, parent);
-    var needsParens           = needsNoLineTermParens || n.needsParens(node, parent);
-
+    var needsParens = n.needsParens(node, parent);
     if (needsParens) this.push("(");
-    if (needsNoLineTermParens) this.indent();
 
     this.printLeadingComments(node, parent);
 
-    var needsParensFromCatchup = this.catchUp(node, parent, needsParens);
+    this.catchUp(node);
 
     this._printNewline(true, node, parent, opts);
 
@@ -248,11 +238,7 @@ class CodeGenerator {
 
     this[node.type](node, this.buildPrint(node), parent);
 
-    if (needsNoLineTermParens) {
-      this.newline();
-      this.dedent();
-    }
-    if (needsParens || needsParensFromCatchup) this.push(")");
+    if (needsParens) this.push(")");
 
     this.map.mark(node, "end");
     if (opts.after) opts.after();

packages/babel/src/generation/node/index.js

@@ -110,21 +110,6 @@ export default class Node {
 
     return find(parens, node, parent);
   }
-
-  /**
-   * [Please add a description.]
-   */
-
-  static needsParensNoLineTerminator(node, parent) {
-    if (!parent) return false;
-
-    // no comments
-    if (!node.leadingComments || !node.leadingComments.length) {
-      return false;
-    }
-
-    return t.isTerminatorless(parent);
-  }
 }
 
 /**

packages/babel/test/fixtures/generation/edgecase/return-with-retainlines-option/actual.js

@@ -3,3 +3,9 @@ function foo(l) {
     l
   );
 }
+
+function foo() {
+  return (
+    1 && 2
+  ) || 3;
+}

packages/babel/test/fixtures/generation/edgecase/return-with-retainlines-option/expected.js

@@ -1,3 +1,10 @@
 function foo(l) {
   return (
-  l);}
+    l);}
+
+
+
+function foo() {
+  return (
+    1 && 2 || 
+    3);}