From f02529da76d8a352e0460d112848dda6d6826ad9 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa <my.burning@gmail.com> Date: Wed, 27 Apr 2022 15:27:08 +0100 Subject: [PATCH 1/2] fix(rome_js_formatter): print empty statements differently --- .../src/js/statements/do_while_statement.rs | 13 ++++++-- .../src/js/statements/empty_statement.rs | 19 +++++++++--- .../src/js/statements/if_statement.rs | 31 +++++++------------ .../specs/js/module/statement/do_while.js | 5 ++- .../js/module/statement/do_while.js.snap | 8 +++++ .../specs/js/module/statement/if_else.js | 3 ++ .../specs/js/module/statement/if_else.js.snap | 12 +++++-- .../prettier/js/empty-statement/body.js.snap | 12 +++---- 8 files changed, 67 insertions(+), 36 deletions(-) diff --git a/crates/rome_js_formatter/src/js/statements/do_while_statement.rs b/crates/rome_js_formatter/src/js/statements/do_while_statement.rs index d088db9a660..bc889a0ce0c 100644 --- a/crates/rome_js_formatter/src/js/statements/do_while_statement.rs +++ b/crates/rome_js_formatter/src/js/statements/do_while_statement.rs @@ -1,5 +1,5 @@ use crate::format_traits::FormatOptional; -use rome_formatter::FormatResult; +use rome_formatter::{hard_line_break, FormatResult}; use crate::{ format_elements, hard_group_elements, space_token, token, Format, FormatElement, FormatNode, @@ -21,7 +21,7 @@ impl FormatNode for JsDoWhileStatement { semicolon_token, } = self.as_fields(); - let head = format_elements![do_token.format(formatter)?, space_token(),]; + let head = do_token.format(formatter)?; let tail = format_elements![ space_token(), @@ -39,12 +39,19 @@ impl FormatNode for JsDoWhileStatement { if matches!(body, JsAnyStatement::JsBlockStatement(_)) { Ok(hard_group_elements(format_elements![ head, + space_token(), body.format(formatter)?, tail, ])) + } else if matches!(body, JsAnyStatement::JsEmptyStatement(_)) { + Ok(format_elements![ + hard_group_elements(format_elements![head, body.format(formatter)?,]), + hard_line_break(), + tail, + ]) } else { Ok(format_elements![ - hard_group_elements(head), + hard_group_elements(format_elements![head, space_token()]), body.format(formatter)?, hard_group_elements(tail), ]) diff --git a/crates/rome_js_formatter/src/js/statements/empty_statement.rs b/crates/rome_js_formatter/src/js/statements/empty_statement.rs index ac2344f8164..250c6eef800 100644 --- a/crates/rome_js_formatter/src/js/statements/empty_statement.rs +++ b/crates/rome_js_formatter/src/js/statements/empty_statement.rs @@ -1,13 +1,22 @@ -use crate::{empty_element, FormatElement, FormatNode, Formatter}; +use crate::{empty_element, Format, FormatElement, FormatNode, Formatter}; use rome_formatter::FormatResult; - -use rome_js_syntax::JsEmptyStatement; use rome_js_syntax::JsEmptyStatementFields; +use rome_js_syntax::{JsEmptyStatement, JsSyntaxKind}; +use rome_rowan::AstNode; impl FormatNode for JsEmptyStatement { fn format_fields(&self, formatter: &Formatter) -> FormatResult<FormatElement> { let JsEmptyStatementFields { semicolon_token } = self.as_fields(); - - Ok(formatter.format_replaced(&semicolon_token?, empty_element())) + let parent_kind = self.syntax().parent().map(|p| p.kind()); + if matches!( + parent_kind, + Some(JsSyntaxKind::JS_DO_WHILE_STATEMENT) + | Some(JsSyntaxKind::JS_IF_STATEMENT) + | Some(JsSyntaxKind::JS_ELSE_CLAUSE) + ) { + semicolon_token.format(formatter) + } else { + Ok(formatter.format_replaced(&semicolon_token?, empty_element())) + } } } diff --git a/crates/rome_js_formatter/src/js/statements/if_statement.rs b/crates/rome_js_formatter/src/js/statements/if_statement.rs index e3783338807..996c5439e12 100644 --- a/crates/rome_js_formatter/src/js/statements/if_statement.rs +++ b/crates/rome_js_formatter/src/js/statements/if_statement.rs @@ -1,9 +1,6 @@ use crate::format_traits::FormatOptional; -use crate::{ - block_indent, concat_elements, group_elements, hard_group_elements, hard_line_break, token, - Format, -}; -use rome_formatter::FormatResult; +use crate::{block_indent, concat_elements, group_elements, hard_group_elements, token, Format}; +use rome_formatter::{hard_line_break, FormatResult}; use crate::{format_elements, space_token, FormatElement, FormatNode, Formatter}; @@ -33,7 +30,6 @@ impl FormatNode for JsIfStatement { if_chain.push(format_elements![ space_token(), else_token.format(formatter)?, - space_token(), into_block(formatter, alternate)?, ]); } @@ -72,7 +68,6 @@ fn format_if_element( test.format(formatter)?, &r_paren_token?, )?, - space_token(), into_block(formatter, consequent?)?, ]; @@ -82,23 +77,21 @@ fn format_if_element( /// Wraps the statement into a block if its not already a JsBlockStatement fn into_block(formatter: &Formatter, stmt: JsAnyStatement) -> FormatResult<FormatElement> { if matches!(stmt, JsAnyStatement::JsBlockStatement(_)) { - return stmt.format(formatter); + return Ok(format_elements![space_token(), stmt.format(formatter)?]); } // If the body is an empty statement, force a line break to ensure behavior // is coherent with `is_non_collapsable_empty_block` if matches!(stmt, JsAnyStatement::JsEmptyStatement(_)) { - return Ok(format_elements![ - token("{"), - stmt.format(formatter)?, - hard_line_break(), - token("}") - ]); + return Ok(format_elements![stmt.format(formatter)?, hard_line_break()]); } - Ok(group_elements(format_elements![ - token("{"), - block_indent(stmt.format(formatter)?), - token("}"), - ])) + Ok(format_elements![ + space_token(), + group_elements(format_elements![ + token("{"), + block_indent(stmt.format(formatter)?), + token("}"), + ]) + ]) } diff --git a/crates/rome_js_formatter/tests/specs/js/module/statement/do_while.js b/crates/rome_js_formatter/tests/specs/js/module/statement/do_while.js index f96148103cb..ed3b1987b91 100644 --- a/crates/rome_js_formatter/tests/specs/js/module/statement/do_while.js +++ b/crates/rome_js_formatter/tests/specs/js/module/statement/do_while.js @@ -9,4 +9,7 @@ do { // trailing var foo = 4 } -while (something) \ No newline at end of file +while (something) + + +do; while(true); \ No newline at end of file diff --git a/crates/rome_js_formatter/tests/specs/js/module/statement/do_while.js.snap b/crates/rome_js_formatter/tests/specs/js/module/statement/do_while.js.snap index 368c97287f4..c29b039de94 100644 --- a/crates/rome_js_formatter/tests/specs/js/module/statement/do_while.js.snap +++ b/crates/rome_js_formatter/tests/specs/js/module/statement/do_while.js.snap @@ -1,6 +1,8 @@ --- source: crates/rome_js_formatter/tests/spec_test.rs +assertion_line: 242 expression: do_while.js + --- # Input do { @@ -15,6 +17,9 @@ do { // trailing } while (something) + + +do; while(true); ============================= # Outputs ## Output 1 @@ -32,3 +37,6 @@ do { var foo = 4; } while (something); +do; +while (true); + diff --git a/crates/rome_js_formatter/tests/specs/js/module/statement/if_else.js b/crates/rome_js_formatter/tests/specs/js/module/statement/if_else.js index fb71b3d26da..032107bbc7d 100644 --- a/crates/rome_js_formatter/tests/specs/js/module/statement/if_else.js +++ b/crates/rome_js_formatter/tests/specs/js/module/statement/if_else.js @@ -1,3 +1,6 @@ +if(a); +if(a); else ; + if (Math.random() > 0.5) { console.log(1) } else if (Math.random() > 0.5) { diff --git a/crates/rome_js_formatter/tests/specs/js/module/statement/if_else.js.snap b/crates/rome_js_formatter/tests/specs/js/module/statement/if_else.js.snap index bfce207d498..0e8f34c4238 100644 --- a/crates/rome_js_formatter/tests/specs/js/module/statement/if_else.js.snap +++ b/crates/rome_js_formatter/tests/specs/js/module/statement/if_else.js.snap @@ -1,8 +1,13 @@ --- source: crates/rome_js_formatter/tests/spec_test.rs +assertion_line: 242 expression: if_else.js + --- # Input +if(a); +if(a); else ; + if (Math.random() > 0.5) { console.log(1) } else if (Math.random() > 0.5) { @@ -71,6 +76,10 @@ Indent style: Tab Line width: 80 Quote style: Double Quotes ----- +if (a); +if (a); +else; + if (Math.random() > 0.5) { console.log(1); } else if (Math.random() > 0.5) { @@ -122,6 +131,5 @@ if (true) { if (true) { that(); -} else { -} +} else; diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/empty-statement/body.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/empty-statement/body.js.snap index 4010b708008..b237c09a195 100644 --- a/crates/rome_js_formatter/tests/specs/prettier/js/empty-statement/body.js.snap +++ b/crates/rome_js_formatter/tests/specs/prettier/js/empty-statement/body.js.snap @@ -1,6 +1,6 @@ --- source: crates/rome_js_formatter/tests/prettier_tests.rs -assertion_line: 144 +assertion_line: 175 expression: body.js --- @@ -19,15 +19,15 @@ do; while(1); # Output ```js with (a); -if (1) { -} else if (2) { -} else { -} +if (1); +else if (2); +else; for (;;); while (1); for (var i in o); for (var i of o); -do while (1); +do; +while (1); ``` From c7aa984fe7c0bb0cc9a15d6f0a1763284971e743 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa <my.burning@gmail.com> Date: Wed, 27 Apr 2022 17:19:06 +0100 Subject: [PATCH 2/2] chore: add empty block on empty statements --- .../src/js/statements/do_while_statement.rs | 10 ++++---- .../src/js/statements/empty_statement.rs | 7 +++--- .../src/js/statements/if_statement.rs | 23 ++++++++----------- .../specs/js/module/statement/if_else.js.snap | 11 +++++---- .../prettier/js/empty-statement/body.js.snap | 11 +++++---- 5 files changed, 31 insertions(+), 31 deletions(-) diff --git a/crates/rome_js_formatter/src/js/statements/do_while_statement.rs b/crates/rome_js_formatter/src/js/statements/do_while_statement.rs index bc889a0ce0c..4770827299c 100644 --- a/crates/rome_js_formatter/src/js/statements/do_while_statement.rs +++ b/crates/rome_js_formatter/src/js/statements/do_while_statement.rs @@ -1,5 +1,5 @@ -use crate::format_traits::FormatOptional; -use rome_formatter::{hard_line_break, FormatResult}; +use crate::format_traits::{FormatOptional, FormatWith}; +use rome_formatter::FormatResult; use crate::{ format_elements, hard_group_elements, space_token, token, Format, FormatElement, FormatNode, @@ -21,7 +21,7 @@ impl FormatNode for JsDoWhileStatement { semicolon_token, } = self.as_fields(); - let head = do_token.format(formatter)?; + let head = do_token.format_with(formatter, |e| format_elements![e, space_token()])?; let tail = format_elements![ space_token(), @@ -39,19 +39,17 @@ impl FormatNode for JsDoWhileStatement { if matches!(body, JsAnyStatement::JsBlockStatement(_)) { Ok(hard_group_elements(format_elements![ head, - space_token(), body.format(formatter)?, tail, ])) } else if matches!(body, JsAnyStatement::JsEmptyStatement(_)) { Ok(format_elements![ hard_group_elements(format_elements![head, body.format(formatter)?,]), - hard_line_break(), tail, ]) } else { Ok(format_elements![ - hard_group_elements(format_elements![head, space_token()]), + hard_group_elements(head), body.format(formatter)?, hard_group_elements(tail), ]) diff --git a/crates/rome_js_formatter/src/js/statements/empty_statement.rs b/crates/rome_js_formatter/src/js/statements/empty_statement.rs index 250c6eef800..586747f5542 100644 --- a/crates/rome_js_formatter/src/js/statements/empty_statement.rs +++ b/crates/rome_js_formatter/src/js/statements/empty_statement.rs @@ -1,5 +1,5 @@ -use crate::{empty_element, Format, FormatElement, FormatNode, Formatter}; -use rome_formatter::FormatResult; +use crate::{empty_element, FormatElement, FormatNode, Formatter}; +use rome_formatter::{format_elements, hard_line_break, token, FormatResult}; use rome_js_syntax::JsEmptyStatementFields; use rome_js_syntax::{JsEmptyStatement, JsSyntaxKind}; use rome_rowan::AstNode; @@ -14,7 +14,8 @@ impl FormatNode for JsEmptyStatement { | Some(JsSyntaxKind::JS_IF_STATEMENT) | Some(JsSyntaxKind::JS_ELSE_CLAUSE) ) { - semicolon_token.format(formatter) + let new_body = format_elements![token("{"), hard_line_break(), token("}")]; + Ok(formatter.format_replaced(&semicolon_token?, new_body)) } else { Ok(formatter.format_replaced(&semicolon_token?, empty_element())) } diff --git a/crates/rome_js_formatter/src/js/statements/if_statement.rs b/crates/rome_js_formatter/src/js/statements/if_statement.rs index 996c5439e12..a87d6c76e7b 100644 --- a/crates/rome_js_formatter/src/js/statements/if_statement.rs +++ b/crates/rome_js_formatter/src/js/statements/if_statement.rs @@ -1,9 +1,7 @@ use crate::format_traits::FormatOptional; use crate::{block_indent, concat_elements, group_elements, hard_group_elements, token, Format}; -use rome_formatter::{hard_line_break, FormatResult}; - use crate::{format_elements, space_token, FormatElement, FormatNode, Formatter}; - +use rome_formatter::FormatResult; use rome_js_syntax::JsSyntaxToken; use rome_js_syntax::{JsAnyStatement, JsElseClauseFields, JsIfStatement}; use rome_js_syntax::{JsElseClause, JsIfStatementFields}; @@ -30,6 +28,7 @@ impl FormatNode for JsIfStatement { if_chain.push(format_elements![ space_token(), else_token.format(formatter)?, + space_token(), into_block(formatter, alternate)?, ]); } @@ -68,6 +67,7 @@ fn format_if_element( test.format(formatter)?, &r_paren_token?, )?, + space_token(), into_block(formatter, consequent?)?, ]; @@ -77,21 +77,18 @@ fn format_if_element( /// Wraps the statement into a block if its not already a JsBlockStatement fn into_block(formatter: &Formatter, stmt: JsAnyStatement) -> FormatResult<FormatElement> { if matches!(stmt, JsAnyStatement::JsBlockStatement(_)) { - return Ok(format_elements![space_token(), stmt.format(formatter)?]); + return stmt.format(formatter); } // If the body is an empty statement, force a line break to ensure behavior // is coherent with `is_non_collapsable_empty_block` if matches!(stmt, JsAnyStatement::JsEmptyStatement(_)) { - return Ok(format_elements![stmt.format(formatter)?, hard_line_break()]); + return Ok(format_elements![stmt.format(formatter)?]); } - Ok(format_elements![ - space_token(), - group_elements(format_elements![ - token("{"), - block_indent(stmt.format(formatter)?), - token("}"), - ]) - ]) + Ok(group_elements(format_elements![ + token("{"), + block_indent(stmt.format(formatter)?), + token("}"), + ])) } diff --git a/crates/rome_js_formatter/tests/specs/js/module/statement/if_else.js.snap b/crates/rome_js_formatter/tests/specs/js/module/statement/if_else.js.snap index 0e8f34c4238..397f1cbe02f 100644 --- a/crates/rome_js_formatter/tests/specs/js/module/statement/if_else.js.snap +++ b/crates/rome_js_formatter/tests/specs/js/module/statement/if_else.js.snap @@ -76,9 +76,11 @@ Indent style: Tab Line width: 80 Quote style: Double Quotes ----- -if (a); -if (a); -else; +if (a) { +} +if (a) { +} else { +} if (Math.random() > 0.5) { console.log(1); @@ -131,5 +133,6 @@ if (true) { if (true) { that(); -} else; +} else { +} diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/empty-statement/body.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/empty-statement/body.js.snap index b237c09a195..67e58f31b0b 100644 --- a/crates/rome_js_formatter/tests/specs/prettier/js/empty-statement/body.js.snap +++ b/crates/rome_js_formatter/tests/specs/prettier/js/empty-statement/body.js.snap @@ -19,15 +19,16 @@ do; while(1); # Output ```js with (a); -if (1); -else if (2); -else; +if (1) { +} else if (2) { +} else { +} for (;;); while (1); for (var i in o); for (var i of o); -do; -while (1); +do { +} while (1); ```