Repository: jlongster/prettier
Pull request #450 (2017-01-24 18:08:20)

Remove `multilineGroup`
Starting to convert all `multilineGroup`s into normal `group`s. This inverts the logic in some places and we need to be sure that the right nodes break the parents correctly. This is making code a bit more consistent already.

jlongster: This PR now removes all instances of `multilineGroup`. The rest of them were easy and didn't have any changes on the output.

Patch:
diff --git a/commands.md b/commands.md
index c49bd99ab..134e2d0eb 100644
--- a/commands.md
+++ b/commands.md
@@ -24,11 +24,12 @@ line, but if it doesn't fit it will break the outermost group first
 and try again. It will continue breaking groups until everything fits
 (or there are no more groups to break).
 
-### multilineGroup
-
-This is the same as `group`, but with an additional behavior: if this
-group spans any other groups that have hard breaks (see below) this
-group *always* breaks. Otherwise it acts the same as `group`.
+A document can force parent groups to break by including `breakParent`
+(see below). A hard and literal line automatically include this so
+they always break parent groups. Breaks are propagated to all parent
+groups, so if a deeply nested expression has a hard break, everything
+with break. This only matters for "hard" breaks, i.e. newlines that
+are printed no matter what and can be statically analyzed.
 
 For example, an array will try to fit on one line:
 
@@ -53,6 +54,20 @@ Functions always break after the opening curly brace no matter what,
 so the array breaks as well for consistent formatting. See the
 implementation of `ArrayExpression` for an example.
 
+### breakParent
+
+Include this anywhere to force all parent groups to break. See `group`
+for more info. Example:
+
+```js
+group(concat([
+  " ",
+  expr,
+  " ",
+  breakParent
+]))
+```
+
 ### join
 
 Join an array of items with a separator.
diff --git a/src/doc-builders.js b/src/doc-builders.js
index 7a0c1010f..b24ae6189 100644
--- a/src/doc-builders.js
+++ b/src/doc-builders.js
@@ -1,13 +1,11 @@
 "use strict";
-const assert = require("assert");
 const utils = require("./doc-utils");
 const willBreak = utils.willBreak;
 
 function assertDoc(val) {
-  assert(
-    typeof val === "string" || val != null && typeof val.type === "string",
-    "Value is a valid document"
-  );
+  if (!(typeof val === "string" || val != null && typeof val.type === "string")) {
+    throw new Error("Value " + JSON.stringify(val) + " is not a valid document");
+  }
 }
 
 function concat(parts) {
@@ -35,13 +33,6 @@ function group(contents, opts) {
   };
 }
 
-function multilineGroup(contents, opts) {
-  return group(
-    contents,
-    Object.assign(opts || {}, { shouldBreak: willBreak(contents) })
-  );
-}
-
 function conditionalGroup(states, opts) {
   return group(
     states[0],
@@ -64,7 +55,7 @@ const breakParent =  { type: "break-parent" };
 const line = { type: "line" };
 const softline = { type: "line", soft: true };
 const hardline = concat([{ type: "line", hard: true }, breakParent ]);
-const literalline = { type: "line", hard: true, literal: true };
+const literalline = concat([{ type: "line", hard: true, literal: true }, breakParent ]);
 
 function join(sep, arr) {
   var res = [];
@@ -88,7 +79,6 @@ module.exports = {
   hardline,
   literalline,
   group,
-  multilineGroup,
   conditionalGroup,
   breakParent,
   ifBreak,
diff --git a/src/doc-debug.js b/src/doc-debug.js
index 2ae5b3f2f..b89382de1 100644
--- a/src/doc-debug.js
+++ b/src/doc-debug.js
@@ -19,8 +19,8 @@ function flattenDoc(doc) {
 
   } else if (doc.type === "if-break") {
     return Object.assign({}, doc, {
-      breakContents: doc.breakContents ? flattenDoc(doc.breakContents) : null,
-      flatContents: doc.flatContents ? flattenDoc(doc.flatContents) : null
+      breakContents: doc.breakContents != null ? flattenDoc(doc.breakContents) : null,
+      flatContents: doc.flatContents != null ? flattenDoc(doc.flatContents) : null
     });
 
   } else if (doc.type === "group") {
diff --git a/src/printer.js b/src/printer.js
index 91262bf49..bb249fcca 100644
--- a/src/printer.js
+++ b/src/printer.js
@@ -13,7 +13,6 @@ var hardline = docBuilders.hardline;
 var softline = docBuilders.softline;
 var literalline = docBuilders.literalline;
 var group = docBuilders.group;
-var multilineGroup = docBuilders.multilineGroup;
 var indent = docBuilders.indent;
 var conditionalGroup = docBuilders.conditionalGroup;
 var ifBreak = docBuilders.ifBreak;
@@ -270,6 +269,8 @@ function genericPrintNoParens(path, options, print) {
       const body = path.call(print, "body");
       const collapsed = concat([ concat(parts), " ", body ]);
 
+      // We want to always keep these types of nodes on the same line
+      // as the arrow.
       if (
         n.body.type === "ArrayExpression" ||
           n.body.type === "ObjectExpression" ||
@@ -278,13 +279,21 @@ function genericPrintNoParens(path, options, print) {
         return group(collapsed);
       }
 
-      return conditionalGroup([
-        collapsed,
-        concat([
-          concat(parts),
-          indent(options.tabWidth, concat([ line, body ]))
-        ])
-      ]);
+      // These nested groups are a little wonky, but because
+      // `conditionalGroup` suppresses break propagation, we want to
+      // re-propagate it. We still want to allow the printer to choose
+      // the more collapsed version, but still break parents if there
+      // are any hard breaks in the content.
+      return group(
+        conditionalGroup([
+          collapsed,
+          concat([
+            concat(parts),
+            indent(options.tabWidth, concat([ line, body ]))
+          ])
+        ]),
+        { shouldBreak: willBreak(body) }
+      );
     case "MethodDefinition":
       if (n.static) {
         parts.push("static ");
@@ -425,7 +434,7 @@ function genericPrintNoParens(path, options, print) {
 
         if (grouped.length > 0) {
           parts.push(
-            multilineGroup(
+            group(
               concat([
                 "{",
                 indent(
@@ -560,7 +569,7 @@ function genericPrintNoParens(path, options, print) {
       if (props.length === 0) {
         return "{}";
       } else {
-        return multilineGroup(
+        return group(
           concat([
             leftBrace,
             indent(
@@ -641,7 +650,7 @@ function genericPrintNoParens(path, options, print) {
           lastElem === null;
 
         parts.push(
-          multilineGroup(
+          group(
             concat([
               "[",
               indent(
@@ -781,7 +790,7 @@ function genericPrintNoParens(path, options, print) {
         parts.push(";");
       }
 
-      return multilineGroup(concat(parts));
+      return group(concat(parts));
     case "VariableDeclarator":
       return n.init
         ? concat([ path.call(print, "id"), " = ", path.call(print, "init") ])
@@ -1086,17 +1095,15 @@ function genericPrintNoParens(path, options, print) {
         concat([
           "<",
           path.call(print, "name"),
-          multilineGroup(
-            concat([
-              indent(
-                options.tabWidth,
-                concat(
-                  path.map(attr => concat([ line, print(attr) ]), "attributes")
-                )
-              ),
-              n.selfClosing ? line : softline
-            ])
-          ),
+          concat([
+            indent(
+              options.tabWidth,
+              concat(
+                path.map(attr => concat([ line, print(attr) ]), "attributes")
+              )
+            ),
+            n.selfClosing ? line : softline
+          ]),
           n.selfClosing ? "/>" : ">"
         ])
       );
@@ -1310,7 +1317,7 @@ function genericPrintNoParens(path, options, print) {
 
       parts.push(path.call(print, "typeParameters"));
 
-      parts.push(multilineGroup(printFunctionParams(path, print, options)));
+      parts.push(group(printFunctionParams(path, print, options)));
 
       // The returnType is not wrapped in a TypeAnnotation, so the colon
       // needs to be added separately.
@@ -1655,7 +1662,7 @@ function printMethod(path, options, print) {
   parts.push(
     key,
     path.call(print, "value", "typeParameters"),
-    multilineGroup(
+    group(
       concat([
         path.call(
           function(valuePath) {
@@ -1726,7 +1733,7 @@ function printArgumentsList(path, options, print) {
     );
   }
 
-  return multilineGroup(
+  return group(
     concat([
       "(",
       indent(
@@ -1811,7 +1818,7 @@ function printObjectMethod(path, options, print) {
   }
 
   parts.push(
-    multilineGroup(
+    group(
       concat([
         printFunctionParams(path, print, options),
         printReturnType(path, print)
@@ -1879,7 +1886,7 @@ function printExportDeclaration(path, options, print) {
       } else {
         parts.push(
           decl.exportKind === "type" ? "type " : "",
-          multilineGroup(
+          group(
             concat([
               "{",
               indent(
@@ -2276,7 +2283,7 @@ function maybeWrapJSXElementInParens(path, elem, options) {
     return elem;
   }
 
-  return multilineGroup(
+  return group(
     concat([
       ifBreak("("),
       indent(options.tabWidth, concat([ softline, elem ])),


Test patch:
diff --git a/tests/comments/__snapshots__/jsfmt.spec.js.snap b/tests/comments/__snapshots__/jsfmt.spec.js.snap
index da0065a31..6156980c4 100644
--- a/tests/comments/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/comments/__snapshots__/jsfmt.spec.js.snap
@@ -179,7 +179,8 @@ Observable
 
 /* Comments in JSX tag are placed in a non optimal way*/
 <div
-/* comment*/ />;
+/* comment*/
+/>;
 
 // Comments disappear in empty blocks
 if (1) {
@@ -379,7 +380,8 @@ Observable
 
 // Comments in JSX tag are placed in a non optimal way
 <// comment
-div />;
+div
+/>;
 
 // Comments disappear in empty blocks
 if (1) {
diff --git a/tests/trailing_whitespace/__snapshots__/jsfmt.spec.js.snap b/tests/trailing_whitespace/__snapshots__/jsfmt.spec.js.snap
index 967746b7b..f2c9a816f 100644
--- a/tests/trailing_whitespace/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/trailing_whitespace/__snapshots__/jsfmt.spec.js.snap
@@ -18,7 +18,8 @@ export type Result<T, V> =
 \`
    
    
-\` + \`
+\` +
+  \`
     
     
 \`;