Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

Commit

Permalink
fix(rome_js_formatter): print empty statements differently
Browse files Browse the repository at this point in the history
  • Loading branch information
ematipico committed Apr 27, 2022
1 parent 97fc6b7 commit f02529d
Show file tree
Hide file tree
Showing 8 changed files with 67 additions and 36 deletions.
13 changes: 10 additions & 3 deletions crates/rome_js_formatter/src/js/statements/do_while_statement.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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(),
Expand All @@ -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),
])
Expand Down
19 changes: 14 additions & 5 deletions crates/rome_js_formatter/src/js/statements/empty_statement.rs
Original file line number Diff line number Diff line change
@@ -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()))
}
}
}
31 changes: 12 additions & 19 deletions crates/rome_js_formatter/src/js/statements/if_statement.rs
Original file line number Diff line number Diff line change
@@ -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};

Expand Down Expand Up @@ -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)?,
]);
}
Expand Down Expand Up @@ -72,7 +68,6 @@ fn format_if_element(
test.format(formatter)?,
&r_paren_token?,
)?,
space_token(),
into_block(formatter, consequent?)?,
];

Expand All @@ -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("}"),
])
])
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,7 @@ do { // trailing
var foo = 4
}

while (something)
while (something)


do; while(true);
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
---
source: crates/rome_js_formatter/tests/spec_test.rs
assertion_line: 242
expression: do_while.js

---
# Input
do {
Expand All @@ -15,6 +17,9 @@ do { // trailing
}

while (something)


do; while(true);
=============================
# Outputs
## Output 1
Expand All @@ -32,3 +37,6 @@ do {
var foo = 4;
} while (something);

do;
while (true);

Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
if(a);
if(a); else ;

if (Math.random() > 0.5) {
console.log(1)
} else if (Math.random() > 0.5) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -122,6 +131,5 @@ if (true) {

if (true) {
that();
} else {
}
} else;

Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
---
source: crates/rome_js_formatter/tests/prettier_tests.rs
assertion_line: 144
assertion_line: 175
expression: body.js

---
Expand All @@ -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);
```

Expand Down

0 comments on commit f02529d

Please sign in to comment.