From 97fc6b723c4a531d293a395348276b7962532de2 Mon Sep 17 00:00:00 2001 From: IWANABETHATGUY Date: Wed, 27 Apr 2022 18:03:40 +0800 Subject: [PATCH] =?UTF-8?q?fix(rome=5Fjs=5Fformatter):=20=F0=9F=90=9B=20pr?= =?UTF-8?q?eserve=20new-lines=20after=20directives=20(#2500)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/rome_formatter/src/format_element.rs | 36 +++++++++---------- .../src/js/lists/directive_list.rs | 28 +++++++++++++-- .../tests/specs/js/module/newlines.js.snap | 3 ++ .../prettier/js/directives/newline.js.snap | 3 +- .../specs/prettier/js/directives/test.js.snap | 5 +++ 5 files changed, 53 insertions(+), 22 deletions(-) diff --git a/crates/rome_formatter/src/format_element.rs b/crates/rome_formatter/src/format_element.rs index e5f0dda1480..7aa6f0ab974 100644 --- a/crates/rome_formatter/src/format_element.rs +++ b/crates/rome_formatter/src/format_element.rs @@ -819,30 +819,30 @@ where join_elements_with(elements, hard_line_break) } +/// Get the number of line breaks between two consecutive SyntaxNodes in the tree +pub fn get_lines_before(next_node: &SyntaxNode) -> usize { + // Count the newlines in the leading trivia of the next node + if let Some(leading_trivia) = next_node.first_leading_trivia() { + leading_trivia + .pieces() + .take_while(|piece| { + // Stop at the first comment piece, the comment printer + // will handle newlines between the comment and the node + !piece.is_comments() + }) + .filter(|piece| piece.is_newline()) + .count() + } else { + 0 + } +} + #[inline] pub fn join_elements_with(elements: I, separator: fn() -> FormatElement) -> FormatElement where I: IntoIterator, FormatElement)>, L: Language, { - /// Get the number of line breaks between two consecutive SyntaxNodes in the tree - fn get_lines_before(next_node: &SyntaxNode) -> usize { - // Count the newlines in the leading trivia of the next node - if let Some(leading_trivia) = next_node.first_leading_trivia() { - leading_trivia - .pieces() - .take_while(|piece| { - // Stop at the first comment piece, the comment printer - // will handle newlines between the comment and the node - !piece.is_comments() - }) - .filter(|piece| piece.is_newline()) - .count() - } else { - 0 - } - } - concat_elements(IntersperseFn::new( elements.into_iter(), |_, next_node, next_elem| { diff --git a/crates/rome_js_formatter/src/js/lists/directive_list.rs b/crates/rome_js_formatter/src/js/lists/directive_list.rs index 9762a5538a0..ab0f7bb2a4e 100644 --- a/crates/rome_js_formatter/src/js/lists/directive_list.rs +++ b/crates/rome_js_formatter/src/js/lists/directive_list.rs @@ -1,14 +1,36 @@ use crate::{empty_element, format_elements, hard_line_break, Format, FormatElement, Formatter}; -use rome_formatter::FormatResult; +use rome_formatter::{empty_line, format_element::get_lines_before, FormatResult}; use rome_js_syntax::JsDirectiveList; -use rome_rowan::AstNodeList; +use rome_rowan::{AstNode, AstNodeList}; impl Format for JsDirectiveList { fn format(&self, formatter: &Formatter) -> FormatResult { if !self.is_empty() { + let syntax_node = self.syntax(); + let next_sibling = syntax_node.next_sibling(); + // if next_sibling's first leading_trivia has more than one new_line, we should add an extra empty line at the end of + // JsDirectiveList, for example: + //```js + // "use strict"; <- first leading new_line + // <- second leading new_line + // function foo() { + + // } + //``` + // so we should keep an extra empty line after JsDirectiveList + let need_extra_empty_line = if let Some(next_sibling) = next_sibling { + get_lines_before(&next_sibling) > 1 + } else { + false + }; Ok(format_elements![ formatter.format_list(self.clone()), - hard_line_break() + hard_line_break(), + if need_extra_empty_line { + empty_line() + } else { + empty_element() + } ]) } else { Ok(empty_element()) diff --git a/crates/rome_js_formatter/tests/specs/js/module/newlines.js.snap b/crates/rome_js_formatter/tests/specs/js/module/newlines.js.snap index 2ee3addbac4..3b657d87c45 100644 --- a/crates/rome_js_formatter/tests/specs/js/module/newlines.js.snap +++ b/crates/rome_js_formatter/tests/specs/js/module/newlines.js.snap @@ -1,6 +1,8 @@ --- source: crates/rome_js_formatter/tests/spec_test.rs +assertion_line: 242 expression: newlines.js + --- # Input "directive"; @@ -97,6 +99,7 @@ Quote style: Double Quotes "directive"; "directive"; + statement(); // comment statement(); diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/directives/newline.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/directives/newline.js.snap index c671e030a0a..fef62742722 100644 --- a/crates/rome_js_formatter/tests/specs/prettier/js/directives/newline.js.snap +++ b/crates/rome_js_formatter/tests/specs/prettier/js/directives/newline.js.snap @@ -1,6 +1,6 @@ --- source: crates/rome_js_formatter/tests/prettier_tests.rs -assertion_line: 144 +assertion_line: 175 expression: newline.js --- @@ -21,6 +21,7 @@ a(); /* @flow */ "use strict"; + import a from "a"; a(); diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/directives/test.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/directives/test.js.snap index b6a636a3d18..66e5bd03687 100644 --- a/crates/rome_js_formatter/tests/specs/prettier/js/directives/test.js.snap +++ b/crates/rome_js_formatter/tests/specs/prettier/js/directives/test.js.snap @@ -1,6 +1,8 @@ --- source: crates/rome_js_formatter/tests/prettier_tests.rs +assertion_line: 175 expression: test.js + --- # Input ```js @@ -35,6 +37,7 @@ function f4() { # Output ```js "use strict"; + function f1() { "use strict"; } @@ -46,11 +49,13 @@ function f2() { function f3() { "ngInject"; + Object.assign(this, { $log, $uibModal }); } function f4() { "ngInject"; + Object.assign(this, { $log, $uibModal }); }