Repository: jlongster/prettier (js)

Issue:
Last line comment is not stable
```js
function f() {
  a
  /* eslint-disable */
}
```

turns into

```js
function f() {
  a;

  /* eslint-disable */
}
```

and back

https://jlongster.github.io/prettier/#%7B%22content%22%3A%22function%20f()%20%7B%5Cn%20%20a%5Cn%20%20%2F*%20eslint-disable%20*%2F%5Cn%7D%5Cn%5Cnfunction%20f()%20%7B%5Cn%20%20a%3B%5Cn%5Cn%20%20%2F*%20eslint-disable%20*%2F%5Cn%7D%5Cn%22%2C%22options%22%3A%7B%22printWidth%22%3A80%2C%22tabWidth%22%3A2%2C%22singleQuote%22%3Afalse%2C%22trailingComma%22%3Afalse%2C%22bracketSpacing%22%3Atrue%2C%22jsxBracketSameLine%22%3Afalse%2C%22doc%22%3Afalse%7D%7D

Originally reported by @alekhurst in #717

Pull request description:
Fix trailing new lines preservation
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

Patch:
diff --git a/src/util.js b/src/util.js
index 9825b6259..861808e1f 100644
--- a/src/util.js
+++ b/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) {


Test patch:
diff --git a/tests/comments/__snapshots__/jsfmt.spec.js.snap b/tests/comments/__snapshots__/jsfmt.spec.js.snap
index 86404ad09..038f3416f 100644
--- a/tests/comments/__snapshots__/jsfmt.spec.js.snap
+++ b/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()
 }
diff --git a/tests/comments/preserve-new-line-last.js b/tests/comments/preserve-new-line-last.js
index 6588beb8d..251767053 100644
--- a/tests/comments/preserve-new-line-last.js
+++ b/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()
diff --git a/tests/flow/covariance/__snapshots__/jsfmt.spec.js.snap b/tests/flow/covariance/__snapshots__/jsfmt.spec.js.snap
index 2eff10a82..0f489a995 100644
--- a/tests/flow/covariance/__snapshots__/jsfmt.spec.js.snap
+++ b/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>;
diff --git a/tests/flow/coverage/__snapshots__/jsfmt.spec.js.snap b/tests/flow/coverage/__snapshots__/jsfmt.spec.js.snap
index d74df1742..af5ab7562 100644
--- a/tests/flow/coverage/__snapshots__/jsfmt.spec.js.snap
+++ b/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
diff --git a/tests/flow/declare_module_exports/flow-typed/__snapshots__/jsfmt.spec.js.snap b/tests/flow/declare_module_exports/flow-typed/__snapshots__/jsfmt.spec.js.snap
index 33226b290..9e17ec1f7 100644
--- a/tests/flow/declare_module_exports/flow-typed/__snapshots__/jsfmt.spec.js.snap
+++ b/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
  */
diff --git a/tests/flow/destructuring/__snapshots__/jsfmt.spec.js.snap b/tests/flow/destructuring/__snapshots__/jsfmt.spec.js.snap
index 094ccefb8..aef73dd80 100644
--- a/tests/flow/destructuring/__snapshots__/jsfmt.spec.js.snap
+++ b/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;
 // }
diff --git a/tests/flow/more_generics/__snapshots__/jsfmt.spec.js.snap b/tests/flow/more_generics/__snapshots__/jsfmt.spec.js.snap
index 9c1efd5be..6540e6e8e 100644
--- a/tests/flow/more_generics/__snapshots__/jsfmt.spec.js.snap
+++ b/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);
 */
diff --git a/tests/flow/overload/__snapshots__/jsfmt.spec.js.snap b/tests/flow/overload/__snapshots__/jsfmt.spec.js.snap
index 6782e926b..50fb19d9a 100644
--- a/tests/flow/overload/__snapshots__/jsfmt.spec.js.snap
+++ b/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;
diff --git a/tests/flow/reflection/__snapshots__/jsfmt.spec.js.snap b/tests/flow/reflection/__snapshots__/jsfmt.spec.js.snap
index a60e7be20..003aebc88 100644
--- a/tests/flow/reflection/__snapshots__/jsfmt.spec.js.snap
+++ b/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?
 "
 `;
diff --git a/tests/flow/return/__snapshots__/jsfmt.spec.js.snap b/tests/flow/return/__snapshots__/jsfmt.spec.js.snap
index 7aad9fe5b..f27b1a2b8 100644
--- a/tests/flow/return/__snapshots__/jsfmt.spec.js.snap
+++ b/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;
 "
diff --git a/tests/flow/while/__snapshots__/jsfmt.spec.js.snap b/tests/flow/while/__snapshots__/jsfmt.spec.js.snap
index fee2ed5f1..c2d260910 100644
--- a/tests/flow/while/__snapshots__/jsfmt.spec.js.snap
+++ b/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\');
 }
 "
diff --git a/tests/trailing_comma/__snapshots__/jsfmt.spec.js.snap b/tests/trailing_comma/__snapshots__/jsfmt.spec.js.snap
index 5f2e3336a..6dc4aca65 100644
--- a/tests/trailing_comma/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/trailing_comma/__snapshots__/jsfmt.spec.js.snap
@@ -209,6 +209,7 @@ o = {
 
 o = {
   state
+
   // Comment
 };
 
@@ -284,6 +285,7 @@ o = {
 
 o = {
   state,
+
   // Comment
 };