Account for empty lines in long member call chain (#3035)

* Account for empty lines in member chain

* Add tests

* Account for parens

* Improve tests

* Add some comments

* Remove an outdated comment

* Fix lint errors

* Refactor a line of code

* Add one more comment for clarification

src/printer.js

@@ -3578,6 +3578,25 @@ function printMemberChain(path, options, print) {
   //   [Identifier, CallExpression, MemberExpression, CallExpression]
   const printedNodes = [];
 
+  // Here we try to retain one typed empty line after each call expression or
+  // the first group whether it is in parentheses or not
+  function shouldInsertEmptyLineAfter(node) {
+    const originalText = options.originalText;
+    const nextCharIndex = util.getNextNonSpaceNonCommentCharacterIndex(
+      originalText,
+      node
+    );
+    const nextChar = originalText.charAt(nextCharIndex);
+
+    // if it is cut off by a parenthesis, we only account for one typed empty
+    // line after that parenthesis
+    if (nextChar == ")") {
+      return util.isNextLineEmptyAfterIndex(originalText, nextCharIndex + 1);
+    }
+
+    return util.isNextLineEmpty(originalText, node);
+  }
+
   function rec(path) {
     const node = path.getValue();
     if (
@@ -3586,16 +3605,19 @@ function printMemberChain(path, options, print) {
     ) {
       printedNodes.unshift({
         node: node,
-        printed: comments.printComments(
-          path,
-          () =>
-            concat([
-              printOptionalToken(path),
-              printFunctionTypeParameters(path, options, print),
-              printArgumentsList(path, options, print)
-            ]),
-          options
-        )
+        printed: concat([
+          comments.printComments(
+            path,
+            () =>
+              concat([
+                printOptionalToken(path),
+                printFunctionTypeParameters(path, options, print),
+                printArgumentsList(path, options, print)
+              ]),
+            options
+          ),
+          shouldInsertEmptyLineAfter(node) ? hardline : ""
+        ])
       });
       path.call(callee => rec(callee), "callee");
     } else if (isMemberish(node)) {
@@ -3772,9 +3794,19 @@ function printMemberChain(path, options, print) {
     return group(oneLine);
   }
 
+  // Find out the last node in the first group and check if it has an
+  // empty line after
+  const lastNodeBeforeIndent = util.getLast(
+    shouldMerge ? groups.slice(1, 2)[0] : groups[0]
+  ).node;
+  const shouldHaveEmptyLineBeforeIndent =
+    lastNodeBeforeIndent.type !== "CallExpression" &&
+    shouldInsertEmptyLineAfter(lastNodeBeforeIndent);
+
   const expanded = concat([
     printGroup(groups[0]),
     shouldMerge ? concat(groups.slice(1, 2).map(printGroup)) : "",
+    shouldHaveEmptyLineBeforeIndent ? hardline : "",
     printIndentedGroup(groups.slice(shouldMerge ? 2 : 1))
   ]);
 
@@ -3799,7 +3831,7 @@ function printMemberChain(path, options, print) {
     // We only need to check `oneLine` because if `expanded` is chosen
     // that means that the parent group has already been broken
     // naturally
-    willBreak(oneLine) ? breakParent : "",
+    willBreak(oneLine) || shouldHaveEmptyLineBeforeIndent ? breakParent : "",
     conditionalGroup([oneLine, expanded])
   ]);
 }

src/util.js

@@ -178,9 +178,9 @@ function isPreviousLineEmpty(text, node) {
   return idx !== idx2;
 }
 
-function isNextLineEmpty(text, node) {
+function isNextLineEmptyAfterIndex(text, index) {
   let oldIdx = null;
-  let idx = locEnd(node);
+  let idx = index;
   while (idx !== oldIdx) {
     // We need to skip all the potential trailing inline comments
     oldIdx = idx;
@@ -193,7 +193,11 @@ function isNextLineEmpty(text, node) {
   return hasNewline(text, idx);
 }
 
-function getNextNonSpaceNonCommentCharacter(text, node) {
+function isNextLineEmpty(text, node) {
+  return isNextLineEmptyAfterIndex(text, locEnd(node));
+}
+
+function getNextNonSpaceNonCommentCharacterIndex(text, node) {
   let oldIdx = null;
   let idx = locEnd(node);
   while (idx !== oldIdx) {
@@ -203,7 +207,11 @@ function getNextNonSpaceNonCommentCharacter(text, node) {
     idx = skipTrailingComment(text, idx);
     idx = skipNewline(text, idx);
   }
-  return text.charAt(idx);
+  return idx;
+}
+
+function getNextNonSpaceNonCommentCharacter(text, node) {
+  return text.charAt(getNextNonSpaceNonCommentCharacterIndex(text, node));
 }
 
 function hasSpaces(text, index, opts) {
@@ -750,10 +758,12 @@ module.exports = {
   getParentExportDeclaration,
   getPenultimate,
   getLast,
+  getNextNonSpaceNonCommentCharacterIndex,
   getNextNonSpaceNonCommentCharacter,
   skipWhitespace,
   skipSpaces,
   skipNewline,
+  isNextLineEmptyAfterIndex,
   isNextLineEmpty,
   isPreviousLineEmpty,
   hasNewline,

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

@@ -412,3 +412,125 @@ function a() {
 }
 
 `;
+
+exports[`member-chain.js 1`] = `
+fooBar.doSomething('Hello World').doAnotherThing('Foo', { foo: bar })
+
+  // App configuration.
+  .doOneMoreThing(config)
+
+  .run(() => console.log('Bar'));
+
+bigDeal
+
+  .doSomething('Hello World')
+
+  // Hello world
+  .doAnotherThing('Foo', { foo: bar })
+
+  // App configuration.
+  .doOneMoreThing(config)
+
+  .run(() => console.log('Bar'));
+
+
+foo.bar.baz
+
+  .doSomething('Hello World')
+
+  // Hello world
+  .foo.bar.doAnotherThing('Foo', { foo: bar })
+
+  .doOneMoreThing(config)
+  .bar.run(() => console.log('Bar'));
+
+(
+  somethingGood ? thisIsIt : maybeNot
+)
+
+  // Hello world
+  .doSomething('Hello World')
+
+  .doAnotherThing('Foo', { foo: bar }) // Run this
+  .run(() => console.log('Bar')); // Do this
+
+helloWorld
+
+  .text()
+
+  .then(t => t);
+
+(veryLongVeryLongVeryLong ||
+ anotherVeryLongVeryLongVeryLong ||
+ veryVeryVeryLongError
+)
+
+  .map(tickets => TicketRecord.createFromSomeLongString())
+
+  .filter(obj => !!obj);
+
+const sel = this.connections
+
+  .concat(this.activities.concat(this.operators))
+  .filter(x => x.selected);
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+fooBar
+  .doSomething("Hello World")
+  .doAnotherThing("Foo", { foo: bar })
+
+  // App configuration.
+  .doOneMoreThing(config)
+
+  .run(() => console.log("Bar"));
+
+bigDeal
+
+  .doSomething("Hello World")
+
+  // Hello world
+  .doAnotherThing("Foo", { foo: bar })
+
+  // App configuration.
+  .doOneMoreThing(config)
+
+  .run(() => console.log("Bar"));
+
+foo.bar.baz
+
+  .doSomething("Hello World")
+
+  // Hello world
+  .foo.bar.doAnotherThing("Foo", { foo: bar })
+
+  .doOneMoreThing(config)
+  .bar.run(() => console.log("Bar"));
+
+(somethingGood ? thisIsIt : maybeNot)
+
+  // Hello world
+  .doSomething("Hello World")
+
+  .doAnotherThing("Foo", { foo: bar }) // Run this
+  .run(() => console.log("Bar")); // Do this
+
+helloWorld
+
+  .text()
+
+  .then(t => t);
+
+(veryLongVeryLongVeryLong ||
+  anotherVeryLongVeryLongVeryLong ||
+  veryVeryVeryLongError
+)
+
+  .map(tickets => TicketRecord.createFromSomeLongString())
+
+  .filter(obj => !!obj);
+
+const sel = this.connections
+
+  .concat(this.activities.concat(this.operators))
+  .filter(x => x.selected);
+
+`;

tests/preserve_line/member-chain.js

@@ -0,0 +1,59 @@
+fooBar.doSomething('Hello World').doAnotherThing('Foo', { foo: bar })
+
+  // App configuration.
+  .doOneMoreThing(config)
+
+  .run(() => console.log('Bar'));
+
+bigDeal
+
+  .doSomething('Hello World')
+
+  // Hello world
+  .doAnotherThing('Foo', { foo: bar })
+
+  // App configuration.
+  .doOneMoreThing(config)
+
+  .run(() => console.log('Bar'));
+
+
+foo.bar.baz
+
+  .doSomething('Hello World')
+
+  // Hello world
+  .foo.bar.doAnotherThing('Foo', { foo: bar })
+
+  .doOneMoreThing(config)
+  .bar.run(() => console.log('Bar'));
+
+(
+  somethingGood ? thisIsIt : maybeNot
+)
+
+  // Hello world
+  .doSomething('Hello World')
+
+  .doAnotherThing('Foo', { foo: bar }) // Run this
+  .run(() => console.log('Bar')); // Do this
+
+helloWorld
+
+  .text()
+
+  .then(t => t);
+
+(veryLongVeryLongVeryLong ||
+ anotherVeryLongVeryLongVeryLong ||
+ veryVeryVeryLongError
+)
+
+  .map(tickets => TicketRecord.createFromSomeLongString())
+
+  .filter(obj => !!obj);
+
+const sel = this.connections
+
+  .concat(this.activities.concat(this.operators))
+  .filter(x => x.selected);