From e30482d4402032a1cf8c93acc792e1bd765a8abd Mon Sep 17 00:00:00 2001 From: Christopher Chedeau Date: Thu, 16 Feb 2017 06:56:12 -0800 Subject: [PATCH] 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 | 4 +- .../comments/__snapshots__/jsfmt.spec.js.snap | 76 ++++++++++++------- tests/comments/preserve-new-line-last.js | 11 +++ .../__snapshots__/jsfmt.spec.js.snap | 1 + .../coverage/__snapshots__/jsfmt.spec.js.snap | 1 + .../__snapshots__/jsfmt.spec.js.snap | 1 + .../__snapshots__/jsfmt.spec.js.snap | 3 + .../__snapshots__/jsfmt.spec.js.snap | 1 - .../overload/__snapshots__/jsfmt.spec.js.snap | 1 + .../__snapshots__/jsfmt.spec.js.snap | 1 + .../return/__snapshots__/jsfmt.spec.js.snap | 1 + .../while/__snapshots__/jsfmt.spec.js.snap | 2 - .../__snapshots__/jsfmt.spec.js.snap | 2 + 13 files changed, 73 insertions(+), 32 deletions(-) diff --git a/src/util.js b/src/util.js index 9825b625..861808e1 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) { diff --git a/tests/comments/__snapshots__/jsfmt.spec.js.snap b/tests/comments/__snapshots__/jsfmt.spec.js.snap index 86404ad0..038f3416 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 { " `; -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 6588beb8..25176705 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 2eff10a8..0f489a99 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 = new NVerbose(); nv.x = [0]; (nv.x[0]: string); // error (nv.foo()[0]: string); // error + /* TODO: use existentials for non-verbose covariance? type CovArray = Array<*:X>; diff --git a/tests/flow/coverage/__snapshots__/jsfmt.spec.js.snap b/tests/flow/coverage/__snapshots__/jsfmt.spec.js.snap index d74df174..af5ab756 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 33226b29..9e17ec1f 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 094ccefb..aef73dd8 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 9c1efd5b..6540e6e8 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(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 6782e926..50fb19d9 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 { 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 a60e7be2..003aebc8 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 7aad9fe5..f27b1a2b 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 fee2ed5f..c2d26091 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 5f2e3336..6dc4aca6 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 };