Fix trailing new lines preservation (#724)

There is an off by one error which made the algorithm not work all the time. In many cases, it actually is the opposite, so whenever you save, it adds/removes that line. I noticed it during the --debug-check run on our codebase but didn't investigate.

Fixes #723

src/util.js

@@ -200,7 +200,9 @@ function isPreviousLineEmpty(text, node) {
   let idx = locStart(node) - 1;
   idx = skipSpaces(text, idx, { backwards: true });
   idx = skipNewline(text, idx, { backwards: true });
-  return hasNewline(text, idx, { backwards: true });
+  idx = skipSpaces(text, idx, { backwards: true });
+  const idx2 = skipNewline(text, idx, { backwards: true });
+  return idx !== idx2;
 }
 
 function isNextLineEmpty(text, node) {

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

@@ -86,32 +86,6 @@ declare class Foo extends Qux<string> {
 "
 `;
 
-exports[`test dangling_array.js 1`] = `
-"expect(() => {}).toTriggerReadyStateChanges([
-  // Nothing.
-]);
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-expect(() => {}).toTriggerReadyStateChanges(
-  [
-    // Nothing.
-  ]
-);
-"
-`;
-
-exports[`test dangling_array.js 2`] = `
-"expect(() => {}).toTriggerReadyStateChanges([
-  // Nothing.
-]);
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-expect(() => {}).toTriggerReadyStateChanges(
-  [
-    // Nothing.
-  ]
-);
-"
-`;
-
 exports[`test first-line.js 1`] = `
 "a // comment
 b
@@ -825,7 +799,18 @@ exports[`test jsx.js 2`] = `
 `;
 
 exports[`test preserve-new-line-last.js 1`] = `
-"function name() {
+"function f() {
+  a
+  /* eslint-disable */
+}
+
+function f() {
+  a
+
+  /* eslint-disable */
+}
+
+function name() {
   // comment1
   func1()
 
@@ -836,12 +821,24 @@ exports[`test preserve-new-line-last.js 1`] = `
   // func3()
 }
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+function f() {
+  a;
+  /* eslint-disable */
+}
+
+function f() {
+  a;
+
+  /* eslint-disable */
+}
+
 function name() {
   // comment1
   func1();
 
   // comment2
   func2();
+
   // comment3 why func3 commented
   // func3()
 }
@@ -849,7 +846,18 @@ function name() {
 `;
 
 exports[`test preserve-new-line-last.js 2`] = `
-"function name() {
+"function f() {
+  a
+  /* eslint-disable */
+}
+
+function f() {
+  a
+
+  /* eslint-disable */
+}
+
+function name() {
   // comment1
   func1()
 
@@ -860,12 +868,24 @@ exports[`test preserve-new-line-last.js 2`] = `
   // func3()
 }
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+function f() {
+  a;
+  /* eslint-disable */
+}
+
+function f() {
+  a;
+
+  /* eslint-disable */
+}
+
 function name() {
   // comment1
   func1();
 
   // comment2
   func2();
+
   // comment3 why func3 commented
   // func3()
 }

tests/comments/preserve-new-line-last.js

@@ -1,3 +1,14 @@
+function f() {
+  a
+  /* eslint-disable */
+}
+
+function f() {
+  a
+
+  /* eslint-disable */
+}
+
 function name() {
   // comment1
   func1()

tests/flow/covariance/__snapshots__/jsfmt.spec.js.snap

@@ -65,6 +65,7 @@ var nv: NVerbose<number, *> = new NVerbose();
 nv.x = [0];
 (nv.x[0]: string); // error
 (nv.foo()[0]: string); // error
+
 /* TODO: use existentials for non-verbose covariance?
 
 type CovArray<X> = Array<*:X>;

tests/flow/coverage/__snapshots__/jsfmt.spec.js.snap

@@ -13,6 +13,7 @@ declare module bar {
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 declare module foo {
 }
+
 // TODO
 // This file triggers a violation of the \"disjoint-or-nested ranges invariant\"
 // that we implicitly assume in type-at-pos and coverage implementations. In

tests/flow/declare_module_exports/flow-typed/__snapshots__/jsfmt.spec.js.snap

@@ -48,6 +48,7 @@ declare module \"declare_m_e_with_other_type_declares\" {
 
 declare module \"DEPRECATED__declare_var_exports\" {
   declare var exports: number;
+
   /**
  * Ensure that, if both are present, \`declare module.exports\` wins
  */

tests/flow/destructuring/__snapshots__/jsfmt.spec.js.snap

@@ -388,10 +388,13 @@ exports[`test destructuring_param.js 1`] = `
 function f(a, { b }) {
   return a + b;
 }
+
 // Doesn\'t parse right now
+
 // function g(a, { a }) {
 //   return a;
 // }
+
 // function h({ a, { b } }, { c }, { { d } }) {
 //   return a + b + c + d;
 // }

tests/flow/more_generics/__snapshots__/jsfmt.spec.js.snap

@@ -69,7 +69,6 @@ function foo8<U>(x: U, y): U {
   y();
   return x;
 }
-
 /*
  foo8(0,void 0);
 */

tests/flow/overload/__snapshots__/jsfmt.spec.js.snap

@@ -83,6 +83,7 @@ a.bar({ a: true }); // error, function cannot be called on intersection type
 declare var x: { a: boolean } & { b: string };
 
 a.bar(x); // error with nested intersection info (outer for bar, inner for x)
+
 /********** tests **************
 interface Dummy<T> {
     dumb(foo: (x:number) => number):number;

tests/flow/reflection/__snapshots__/jsfmt.spec.js.snap

@@ -14,6 +14,7 @@ var c: typeof a = \"...\";
 
 type T = number;
 var x: T = \"...\";
+
 // what about recursive unions?
 "
 `;

tests/flow/return/__snapshots__/jsfmt.spec.js.snap

@@ -73,6 +73,7 @@ function i(x): ?number {
 }
 
 module.exports = C;
+
 //function fn(x:number) { return x; }
 //module.exports = fn;
 "

tests/flow/while/__snapshots__/jsfmt.spec.js.snap

@@ -30,7 +30,6 @@ function foo(x: boolean) {
     }
     return;
   }
-
   //console.log(\'this is still reachable\');
 }
 
@@ -39,7 +38,6 @@ function bar(x: boolean) {
   while (ii > 0) {
     return;
   }
-
   //console.log(\'this is still reachable\');
 }
 "

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

@@ -209,6 +209,7 @@ o = {
 
 o = {
   state
+
   // Comment
 };
 
@@ -284,6 +285,7 @@ o = {
 
 o = {
   state,
+
   // Comment
 };