Propagate breaks upwards automatically, introduce breakParent, and deprecate multilineGroup (#440)

bin/prettier.js

@@ -125,7 +125,12 @@ if (stdin) {
         output = format(input);
       } catch (e) {
         process.exitCode = 2;
-        console.error(filename + ": " + e);
+        if(e.loc) {
+          console.error(filename + ": " + e);
+        }
+        else {
+          console.error(filename + ":", e);
+        }
         return;
       }
 

src/comments.js

@@ -7,6 +7,7 @@ var docBuilders = require("./doc-builders");
 var fromString = docBuilders.fromString;
 var concat = docBuilders.concat;
 var hardline = docBuilders.hardline;
+var breakParent = docBuilders.breakParent;
 var util = require("./util");
 var comparePos = util.comparePos;
 var childNodesCacheKey = Symbol("child-nodes");

src/doc-builders.js

@@ -1,7 +1,7 @@
 "use strict";
 const assert = require("assert");
 const utils = require("./doc-utils");
-const hasHardLine = utils.hasHardLine;
+const willBreak = utils.willBreak;
 
 function assertDoc(val) {
   assert(
@@ -38,7 +38,7 @@ function group(contents, opts) {
 function multilineGroup(contents, opts) {
   return group(
     contents,
-    Object.assign(opts || {}, { shouldBreak: hasHardLine(contents) })
+    Object.assign(opts || {}, { shouldBreak: willBreak(contents) })
   );
 }
 
@@ -60,9 +60,10 @@ function ifBreak(breakContents, flatContents) {
   return { type: "if-break", breakContents, flatContents };
 }
 
+const breakParent =  { type: "break-parent" };
 const line = { type: "line" };
 const softline = { type: "line", soft: true };
-const hardline = { type: "line", hard: true };
+const hardline = concat([{ type: "line", hard: true }, breakParent ]);
 const literalline = { type: "line", hard: true, literal: true };
 
 function join(sep, arr) {
@@ -89,6 +90,7 @@ module.exports = {
   group,
   multilineGroup,
   conditionalGroup,
+  breakParent,
   ifBreak,
   indent
 };

src/doc-debug.js

@@ -1,9 +1,5 @@
 "use strict";
 function flattenDoc(doc) {
-  if (!doc || typeof doc === "string" || doc.type === "line") {
-    return doc;
-  }
-
   if (doc.type === "concat") {
     var res = [];
 
@@ -20,26 +16,25 @@ function flattenDoc(doc) {
     }
 
     return Object.assign({}, doc, { parts: res });
-  }
 
-  if (doc.type === "indent") {
-    return Object.assign({}, doc, { contents: flattenDoc(doc.contents) });
-  }
-
-  if (doc.type === "if-break") {
+  } else if (doc.type === "if-break") {
     return Object.assign({}, doc, {
-      breakContents: flattenDoc(doc.breakContents),
-      flatContents: flattenDoc(doc.flatContents)
+      breakContents: doc.breakContents ? flattenDoc(doc.breakContents) : null,
+      flatContents: doc.flatContents ? flattenDoc(doc.flatContents) : null
     });
-  }
 
-  if (doc.type === "group") {
+  } else if (doc.type === "group") {
     return Object.assign({}, doc, {
       contents: flattenDoc(doc.contents),
       expandedStates: doc.expandedStates
         ? doc.expandedStates.map(flattenDoc)
         : doc.expandedStates
     });
+
+  } else if (doc.contents) {
+    return Object.assign({}, doc, { contents: flattenDoc(doc.contents) });
+  } else {
+    return doc;
   }
 }
 
@@ -61,6 +56,10 @@ function printDoc(doc) {
     return "line";
   }
 
+  if (doc.type === "break-parent") {
+    return "breakParent";
+  }
+
   if (doc.type === "concat") {
     return "[" + doc.parts.map(printDoc).join(", ") + "]";
   }
@@ -77,14 +76,14 @@ function printDoc(doc) {
   }
 
   if (doc.type === "group") {
-    return (doc.break
-      ? "multilineGroup"
-      : doc.expandedStates ? "conditionalGroup" : "group") +
+    if(doc.expandedStates) {
+      return "conditionalGroup(" +
+        "[" + doc.expandedStates.map(printDoc).join(",") + "])";
+    }
+
+    return (doc.break ? "wrappedGroup" : "group") +
       "(" +
       printDoc(doc.contents) +
-      (doc.expandedStates
-        ? ", [" + doc.expandedStates.map(printDoc).join(",") + "]"
-        : "") +
       ")";
   }
 }

src/doc-utils.js

@@ -1,38 +1,26 @@
 "use strict";
-function iterDoc(topDoc, func) {
-  const docs = [ topDoc ];
-  while (docs.length !== 0) {
-    const doc = docs.pop();
-    let res = undefined;
+function traverseDoc(doc, onEnter, onExit) {
+  if(onEnter) {
+    onEnter(doc);
+  }
 
-    if (typeof doc === "string") {
-      const res = func("string", doc);
-
-      if (res) {
-        return res;
-      }
-    } else {
-      const res = func(doc.type, doc);
-
-      if (res) {
-        return res;
-      }
-
-      if (doc.type === "concat") {
-        for (var i = doc.parts.length - 1; i >= 0; i--) {
-          docs.push(doc.parts[i]);
-        }
-      } else if (doc.type === "if-break") {
-        if (doc.breakContents) {
-          docs.push(doc.breakContents);
-        }
-        if (doc.flatContents) {
-          docs.push(doc.flatContents);
-        }
-      } else if (doc.type !== "line") {
-        docs.push(doc.contents);
-      }
+  if (doc.type === "concat") {
+    for (var i = 0; i < doc.parts.length; i++) {
+      traverseDoc(doc.parts[i], onEnter, onExit);
     }
+  } else if (doc.type === "if-break") {
+    if (doc.breakContents) {
+      traverseDoc(doc.breakContents, onEnter, onExit);
+    }
+    if (doc.flatContents) {
+      traverseDoc(doc.flatContents, onEnter, onExit);
+    }
+  } else if (doc.contents) {
+    traverseDoc(doc.contents, onEnter, onExit);
+  }
+
+  if(onExit) {
+    onExit(doc);
   }
 }
 
@@ -41,26 +29,91 @@ function isEmpty(n) {
 }
 
 function getFirstString(doc) {
-  return iterDoc(doc, (type, doc) => {
-    if (type === "string" && doc.trim().length !== 0) {
-      return doc;
+  let firstString = null;
+  traverseDoc(doc, doc => {
+    if (
+      typeof doc === "string" && doc.trim().length !== 0 && firstString === null
+    ) {
+      firstString = doc;
     }
   });
+  return firstString;
 }
 
-function hasHardLine(doc) {
-  // TODO: If we hit a group, check if it's already marked as a
-  // multiline group because they should be marked bottom-up.
-  return !!iterDoc(doc, (type, doc) => {
-    switch (type) {
+function isLineNext(doc) {
+  let flag = null;
+  traverseDoc(doc, doc => {
+    if(flag === null) {
+      if (typeof doc === "string") {
+        flag = false;
+      }
+      if (doc.type === "line") {
+        flag = true;
+      }
+    }
+  });
+  return !!flag;
+}
+
+function willBreak(doc) {
+  let willBreak = false;
+  traverseDoc(doc, doc => {
+    switch (doc.type) {
+      case "group":
+        if(doc.break) {
+          willBreak = true;
+        }
       case "line":
         if (doc.hard) {
-          return true;
+          willBreak = true;
         }
 
         break;
     }
   });
+  return willBreak;
 }
 
-module.exports = { isEmpty, getFirstString, hasHardLine };
+function breakParentGroup(groupStack) {
+  if(groupStack.length > 0) {
+    const parentGroup = groupStack[groupStack.length - 1];
+    // Breaks are not propagated through conditional groups because
+    // the user is expected to manually handle what breaks.
+    if(!parentGroup.expandedStates) {
+      parentGroup.break = true;
+    }
+  }
+  return null;
+}
+
+function propagateBreaks(doc) {
+  const groupStack = [];
+  traverseDoc(
+    doc,
+    doc => {
+      if (doc.type === "break-parent") {
+        breakParentGroup(groupStack);
+      }
+      if (doc.type === "group") {
+        groupStack.push(doc);
+      }
+    },
+    doc => {
+      if (doc.type === "group") {
+        const group = groupStack.pop();
+        if(group.break) {
+          breakParentGroup(groupStack);
+        }
+      }
+    }
+  );
+}
+
+module.exports = {
+  isEmpty,
+  getFirstString,
+  willBreak,
+  isLineNext,
+  traverseDoc,
+  propagateBreaks
+};

src/printer.js

@@ -19,7 +19,8 @@ var conditionalGroup = docBuilders.conditionalGroup;
 var ifBreak = docBuilders.ifBreak;
 
 var docUtils = require("./doc-utils");
-var hasHardLine = docUtils.hasHardLine;
+var willBreak = docUtils.willBreak;
+var isLineNext = docUtils.isLineNext;
 var getFirstString = docUtils.getFirstString;
 var isEmpty = docUtils.isEmpty;
 
@@ -226,7 +227,7 @@ function genericPrintNoParens(path, options, print) {
 
       parts.push(
         path.call(print, "typeParameters"),
-        multilineGroup(
+        group(
           concat([
             printFunctionParams(path, print, options),
             printReturnType(path, print)
@@ -255,7 +256,7 @@ function genericPrintNoParens(path, options, print) {
         parts.push(path.call(print, "params", 0));
       } else {
         parts.push(
-          multilineGroup(
+          group(
             concat([
               printFunctionParams(path, print, options),
               printReturnType(path, print)
@@ -808,7 +809,10 @@ function genericPrintNoParens(path, options, print) {
         if (hasBraces && !isEmpty) {
           parts.push(" else");
         } else {
-          parts.push(concat([ hardline, "else" ]));
+          // We use `conditionalGroup` to suppress break propagation.
+          // This allows us to provide a hardline without forcing the
+          // entire `if` clause to break up.
+          parts.push(conditionalGroup([concat([ hardline, "else" ])]));
         }
 
         parts.push(
@@ -1053,7 +1057,7 @@ function genericPrintNoParens(path, options, print) {
         return concat([ "{", path.call(print, "expression"), "}" ]);
       }
 
-      return multilineGroup(
+      return group(
         concat([
           "{",
           indent(
@@ -1684,7 +1688,7 @@ function printArgumentsList(path, options, print) {
     lastArg.type === "NewExpression";
 
   if (groupLastArg) {
-    const shouldBreak = printed.slice(0, -1).some(hasHardLine);
+    const shouldBreak = printed.slice(0, -1).some(willBreak);
     return conditionalGroup(
       [
         concat([ "(", join(concat([ ", " ]), printed), ")" ]),
@@ -2151,13 +2155,13 @@ function printJSXChildren(path, options, print) {
 function printJSXElement(path, options, print) {
   const n = path.getValue();
 
-  // turn <div></div> into <div />
+  // Turn <div></div> into <div />
   if (isEmptyJSXElement(n)) {
     n.openingElement.selfClosing = true;
     delete n.closingElement;
   }
 
-  // if no children, just print the opening element
+  // If no children, just print the opening element
   const openingLines = path.call(print, "openingElement");
   if (n.openingElement.selfClosing) {
     assert.ok(!n.closingElement);
@@ -2165,49 +2169,64 @@ function printJSXElement(path, options, print) {
   }
 
   const children = printJSXChildren(path, options, print);
-  let hasAnyHardLine = false;
+  let forcedBreak = false;
 
-  // trim trailing whitespace, recording if there was a hardline
-  while (children.length && util.getLast(children).type === "line") {
-    if (hasHardLine(util.getLast(children))) hasAnyHardLine = true;
+  // Trim trailing whitespace, recording if there was a hardline
+  while (children.length && isLineNext(util.getLast(children))) {
+    if (willBreak(util.getLast(children))) {
+      forcedBreak = true;
+    }
     children.pop();
   }
 
-  // trim leading whitespace, recording if there was a hardline
-  while (children.length && children[0].type === "line") {
-    if (hasHardLine(children[0])) hasAnyHardLine = true;
+  // Trim leading whitespace, recording if there was a hardline
+  while (children.length && isLineNext(children[0])) {
+    if (willBreak(children[0])) {
+      forcedBreak = true;
+    }
     children.shift();
   }
 
-  // group by line, recording if there was a hardline.
+  // Group by line, recording if there was a hardline.
   let childrenGroupedByLine;
   if (children.length === 1) {
-    if (!hasAnyHardLine && hasHardLine(children[0])) hasAnyHardLine = true;
+    if (!forcedBreak && willBreak(children[0])) {
+      forcedBreak = true;
+    }
 
-    // no need for groups, and a lone jsx whitespace doesn't work otherwise
-    childrenGroupedByLine = [ hardline, children[0] ];
+    // Don't wrap a single child in a group as there is no need, and a
+    // lone JSX whitespace doesn't work otherwise because the group
+    // will be printed in flat mode, and we need to print `{' '}` in
+    // break mode.
+    childrenGroupedByLine = [concat([ hardline, children[0] ])];
   } else {
-    //  prefill leading newline, and initialize the first line's group
-    childrenGroupedByLine = [ hardline, group(concat([])) ];
+    // Prefill leading newline, and initialize the first line's group
+    let groups = [[]];
     children.forEach((child, i) => {
       let prev = children[i - 1];
-      if (prev && hasHardLine(prev)) {
-        hasAnyHardLine = true;
+      if (prev && willBreak(prev)) {
+        forcedBreak = true;
 
-        // on a new line, so create a new group and put this element in it
-        const newLineGroup = group(concat([ child ]));
-        childrenGroupedByLine.push(newLineGroup);
+        // On a new line, so create a new group and put this element
+        // in it.
+        groups.push([ child ]);
       } else {
-        // not on a newline, so add this element to the current group
-        const currentLineGroup = util.getLast(childrenGroupedByLine);
-        currentLineGroup.contents.parts.push(child);
+        // Not on a newline, so add this element to the current group.
+        util.getLast(groups).push(child);
       }
 
-      // ensure we record hardline of last element
-      if (!hasAnyHardLine && i === children.length - 1) {
-        if (hasHardLine(child)) hasAnyHardLine = true;
+      // Ensure we record hardline of last element.
+      if (!forcedBreak && i === children.length - 1) {
+        if (willBreak(child)) forcedBreak = true;
       }
     });
+
+    childrenGroupedByLine = [
+      hardline,
+      // Conditional groups suppress break propagation; we want output
+      // hard lines without breaking up the entire jsx element.
+      concat(groups.map(contents => conditionalGroup([concat(contents)])))
+    ];
   }
 
   const closingLines = path.call(print, "closingElement");
@@ -2224,17 +2243,14 @@ function printJSXElement(path, options, print) {
     ])
   );
 
-  let elem;
-  if (hasAnyHardLine) {
-    elem = multiLineElem;
-  } else {
-    elem = conditionalGroup([
-      group(concat([ openingLines, concat(children), closingLines ])),
-      multiLineElem
-    ]);
+  if (forcedBreak) {
+    return multiLineElem;
   }
 
-  return elem;
+  return conditionalGroup([
+    group(concat([ openingLines, concat(children), closingLines ])),
+    multiLineElem
+  ]);
 }
 
 function maybeWrapJSXElementInParens(path, elem, options) {
@@ -2435,7 +2451,9 @@ function printAstToDoc(ast, options) {
     );
   }
 
-  return printGenerically(FastPath.from(ast));
+  const doc = printGenerically(FastPath.from(ast));
+  docUtils.propagateBreaks(doc);
+  return doc;
 }
 
 module.exports = { printAstToDoc };

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

@@ -149,11 +149,16 @@ export type AsyncExecuteOptions = child_process$execFileOpts & {
 // optional trailing comma gets moved all the way to the beginning
 const regex = new RegExp(
   // optional trailing comma
-  \"^\\\\s*\" + // beginning of the line
-    \"name\\\\s*=\\\\s*\" + // name =
-    \"[\'\\\"]\" + // opening quotation mark
-    escapeStringRegExp(target.name) + // target name
-    \"[\'\\\"]\" + // closing quotation mark
+  \"^\\\\s*\" +
+    // beginning of the line
+    \"name\\\\s*=\\\\s*\" +
+    // name =
+    \"[\'\\\"]\" +
+    // opening quotation mark
+    escapeStringRegExp(target.name) +
+    // target name
+    \"[\'\\\"]\" +
+    // closing quotation mark
     \",?$\"
 );
 
@@ -345,11 +350,16 @@ export type AsyncExecuteOptions = child_process$execFileOpts & {
 // optional trailing comma gets moved all the way to the beginning
 const regex = new RegExp(
   // optional trailing comma
-  \"^\\\\s*\" + // beginning of the line
-    \"name\\\\s*=\\\\s*\" + // name =
-    \"[\'\\\"]\" + // opening quotation mark
-    escapeStringRegExp(target.name) + // target name
-    \"[\'\\\"]\" + // closing quotation mark
+  \"^\\\\s*\" +
+    // beginning of the line
+    \"name\\\\s*=\\\\s*\" +
+    // name =
+    \"[\'\\\"]\" +
+    // opening quotation mark
+    escapeStringRegExp(target.name) +
+    // target name
+    \"[\'\\\"]\" +
+    // closing quotation mark
     \",?$\"
 );
 

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

@@ -388,7 +388,8 @@ new (function() {
 (function() {
 });
 a = function f() {
-} || b;
+} ||
+  b;
 "
 `;
 

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

@@ -2490,7 +2490,8 @@ type Breakfast = Apple | Orange | Broccoli | Carrot;
 function bar(x: Breakfast) {
   if (x.kind === \"Fruit\") {
     (x.taste: \"Good\");
-  } // error, Apple.taste = Bad else (x.raw: \"No\"); // error, Carrot.raw = Maybe
+  } // error, Apple.taste = Bad else
+    (x.raw: \"No\"); // error, Carrot.raw = Maybe
 }
 
 function qux(x: Breakfast) {

tests/type-destructors/__snapshots__/jsfmt.spec.js.snap

@@ -17,7 +17,8 @@ function foo(x: ?string): $NonMaybeType<?string> {
 function foo(x: ?string): $NonMaybeType<?string> {
   if (x != null) {
     return x;
-  } else return 0; // this should be an error
+  } else
+    return 0; // this should be an error
 }
 
 //(foo(): string); // should not be necessary to expose the error above