feat(rome_formatter): improve the formatting of if-else chains
MODIFY
crates/rome_formatter/src/js/statements/if_statement.rs
crates/rome_formatter/src/js/statements/if_statement.rs
use crate::formatter_traits::{FormatOptionalTokenAndNode, FormatTokenAndNode};
+use crate::{block_indent, concat_elements, hard_group_elements, token};
use crate::{
- format_elements, group_elements, hard_group_elements, space_token, FormatElement, FormatResult,
- Formatter, ToFormatElement,
+ format_elements, space_token, FormatElement, FormatResult, Formatter, ToFormatElement,
};
-use rslint_parser::ast::JsIfStatementFields;
-use rslint_parser::ast::{JsAnyStatement, JsIfStatement};
+use rslint_parser::ast::{JsAnyStatement, JsElseClauseFields, JsIfStatement};
+use rslint_parser::ast::{JsElseClause, JsIfStatementFields};
+use rslint_parser::SyntaxToken;
impl ToFormatElement for JsIfStatement {
fn to_format_element(&self, formatter: &Formatter) -> FormatResult<FormatElement> {
+ let (head, mut else_clause) = format_if_element(formatter, None, self)?;
+
+ let mut if_chain = vec![head];
+ while let Some(clause) = else_clause.take() {
+ let JsElseClauseFields {
+ else_token,
+ alternate,
+ } = clause.as_fields();
+
+ match alternate? {
+ JsAnyStatement::JsIfStatement(stmt) => {
+ let (head, alternate) = format_if_element(formatter, Some(else_token?), &stmt)?;
+
+ if_chain.push(head);
+ else_clause = alternate;
+ }
+ alternate => {
+ if_chain.push(format_elements![
+ space_token(),
+ else_token.format(formatter)?,
+ space_token(),
+ into_block(formatter, alternate)?,
+ ]);
+ }
+ }
+ }
+
+ Ok(hard_group_elements(concat_elements(if_chain)))
+ }
+}
+
+/// Format a single `else? if(test) consequent` element, returning the next else clause
+fn format_if_element(
+ formatter: &Formatter,
+ else_token: Option<SyntaxToken>,
+ stmt: &JsIfStatement,
+) -> FormatResult<(FormatElement, Option<JsElseClause>)> {
let JsIfStatementFields {
if_token,
l_paren_token,
@@ -17,9 +55,14 @@ impl ToFormatElement for JsIfStatement {
r_paren_token,
consequent,
else_clause,
- } = self.as_fields();
+ } = stmt.as_fields();
- let head = group_elements(format_elements![
+ let head = format_elements![
+ else_token.format_with_or_empty(formatter, |token| format_elements![
+ space_token(),
+ token,
+ space_token(),
+ ])?,
if_token.format(formatter)?,
space_token(),
formatter.format_delimited_soft_block_indent(
@@ -28,45 +71,21 @@ impl ToFormatElement for JsIfStatement {
&r_paren_token?,
)?,
space_token(),
- ]);
+ into_block(formatter, consequent?)?,
+ ];
- let consequent = consequent?;
- let has_consequent_block = matches!(consequent, JsAnyStatement::JsBlockStatement(_));
-
- let has_else_block = else_clause
- .as_ref()
- .and_then(|else_clause| {
- let alternate = else_clause.alternate().ok()?;
- Some(matches!(
- alternate,
- JsAnyStatement::JsBlockStatement(_) | JsAnyStatement::JsIfStatement(_)
- ))
- })
- .unwrap_or(false);
-
- let else_clause = else_clause.format_with_or_empty(formatter, |else_clause| {
- format_elements![space_token(), else_clause]
- })?;
-
- match (has_consequent_block, has_else_block) {
- (false, false) => Ok(format_elements![
- hard_group_elements(head),
- consequent.format(formatter)?,
- else_clause,
- ]),
- (false, true) => Ok(format_elements![
- head,
- hard_group_elements(format_elements![consequent.format(formatter)?, else_clause]),
- ]),
- (true, false) => Ok(format_elements![
- hard_group_elements(format_elements![head, consequent.format(formatter)?]),
- else_clause
- ]),
- (true, true) => Ok(hard_group_elements(format_elements![
- head,
- consequent.format(formatter)?,
- else_clause
- ])),
+ Ok((head, else_clause))
}
+
+/// 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(_)) {
+ stmt.format(formatter)
+ } else {
+ Ok(format_elements![
+ token("{"),
+ block_indent(stmt.format(formatter)?),
+ token("}"),
+ ])
}
}

MODIFY
crates/rome_formatter/tests/specs/js/module/arrow/arrow_nested.js.snap
crates/rome_formatter/tests/specs/js/module/arrow/arrow_nested.js.snap
---
source: crates/rome_formatter/tests/spec_test.rs
-assertion_line: 157
+assertion_line: 163
expression: arrow_nested.js
---
@@ -59,7 +59,9 @@ const promiseFromCallback = (fn) =>
(resolve, reject) =>
fn(
(err, result) => {
- if (err) return reject(err);
+ if (err) {
+ return reject(err);
+ }
return resolve(result);
},
),

MODIFY
crates/rome_formatter/tests/specs/js/module/invalid/if_stmt_err.js.snap
crates/rome_formatter/tests/specs/js/module/invalid/if_stmt_err.js.snap
---
source: crates/rome_formatter/tests/spec_test.rs
-assertion_line: 157
+assertion_line: 163
expression: if_stmt_err.js
---
@@ -37,7 +37,9 @@ function test() {
let y = 100;
}
if (true) {
-} else var z = 191;
+} else {
+ var z = 191;
+}
if (false) {
let x = 99;
} else {

ADD
crates/rome_formatter/tests/specs/js/module/statement/if_chain.js
+if(1)1;else if(2)2;else 3;
+
+if(very_long_condition_1) very_long_statement_1(); else if (very_long_condition_2) very_long_statement_2(); else very_long_statement_3();

ADD
crates/rome_formatter/tests/specs/js/module/statement/if_chain.js.snap
+---
+source: crates/rome_formatter/tests/spec_test.rs
+assertion_line: 163
+expression: if_chain.js
+
+---
+# Input
+if(1)1;else if(2)2;else 3;
+
+if(very_long_condition_1) very_long_statement_1(); else if (very_long_condition_2) very_long_statement_2(); else very_long_statement_3();
+
+=============================
+# Outputs
+## Output 1
+-----
+Indent style: Tab
+Line width: 80
+-----
+if (1) {
+ 1;
+} else if (2) {
+ 2;
+} else {
+ 3;
+}
+
+if (very_long_condition_1) {
+ very_long_statement_1();
+} else if (very_long_condition_2) {
+ very_long_statement_2();
+} else {
+ very_long_statement_3();
+}
+

MODIFY
crates/rome_formatter/tests/specs/prettier/classes/ternary.js.snap
crates/rome_formatter/tests/specs/prettier/classes/ternary.js.snap
---
source: crates/rome_formatter/tests/prettier_tests.rs
-assertion_line: 57
+assertion_line: 121
expression: ternary.js
---
@@ -12,7 +12,9 @@ if (1) (class {}) ? 1 : 2;
# Output
```js
-if (1) (class {}) ? 1 : 2;
+if (1) {
+ (class {}) ? 1 : 2;
+}
```

MODIFY
crates/rome_formatter/tests/specs/prettier/comments/if.js.snap
crates/rome_formatter/tests/specs/prettier/comments/if.js.snap
@@ -76,37 +76,53 @@ if (1) // comment
{
false;
} // comment
-else if (2) true; // multi
+else if (2) {
+ true;
+} // multi
// ple
// lines
-else if (3) // existing comment
-true; // okay?
+else if (3) {
+ // existing comment
+ true;
+} // okay?
else if (4) {
// empty with existing comment
} // comment
else {
}
-if (5) true; // comment
+if (5) { // comment
+ true;
+}
if (6) { // comment
true;
-} else if (7) true; else { // comment // comment
+} else if (7) { // comment
+ true;
+} else { // comment
true;
}
if (8) // comment // comment
{
true;
-} else if (9) // comment // comment
-true; else // comment // comment
+} else if (9) { // comment
+ // comment
+ true;
+} else // comment // comment
{
true;
}
-if (10) { /* comment */ // comment
+if (10) /* comment */ { // comment
+ true;
+} else if (11) /* comment */ {
+ true;
+} else if (12) { // comment /* comment */ // comment
true;
-} else if (11) /* comment */ true; else if (12) true; else if (13) true; else /* comment */ { // comment /* comment */ // comment /* comment */ /* comment */ // comment
+} else if (13) /* comment */ /* comment */ { // comment
+ true;
+} else /* comment */ {
true;
}
@@ -114,9 +130,11 @@ if (14) /* comment */ // comment
// comment
{
true;
-} else if (15) /* comment */ // comment
+} else if (15) { // comment
+ /* comment */
/* comment */ // comment
true;
+}
```

MODIFY
crates/rome_formatter/tests/specs/prettier/do/do.js.snap
crates/rome_formatter/tests/specs/prettier/do/do.js.snap
---
source: crates/rome_formatter/tests/prettier_tests.rs
-assertion_line: 83
+assertion_line: 121
expression: do.js
---
@@ -71,7 +71,9 @@ const envSpecific = {
if(env === 'production') 'https://abc.mno.com/';
else
-if (env === "development") "http://localhost:4000";
+if (env === "development") {
+ "http://localhost:4000";
+}
}
}

MODIFY
crates/rome_formatter/tests/specs/prettier/empty-statement/body.js.snap
crates/rome_formatter/tests/specs/prettier/empty-statement/body.js.snap
---
source: crates/rome_formatter/tests/prettier_tests.rs
-assertion_line: 83
+assertion_line: 121
expression: body.js
---
@@ -19,7 +19,7 @@ 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)

MODIFY
crates/rome_formatter/tests/specs/prettier/if/comment_before_else.js.snap
crates/rome_formatter/tests/specs/prettier/if/comment_before_else.js.snap
@@ -33,8 +33,12 @@ else {
stuff;
}
-if (cond) stuff; // comment
-else stuff;
+if (cond) {
+ stuff;
+} // comment
+else {
+ stuff;
+}
```

MODIFY
crates/rome_formatter/tests/specs/prettier/if/else.js.snap
crates/rome_formatter/tests/specs/prettier/if/else.js.snap
@@ -32,18 +32,26 @@ function f() {
// Both functions below should be formatted exactly the same
function f() {
- if (position) return { name: pair }; else return {
+ if (position) {
+ return { name: pair };
+ } else {
+ return {
name: pair.substring(0, position),
value: pair.substring(position + 1),
};
}
+}
function f() {
- if (position) return { name: pair }; else return {
+ if (position) {
+ return { name: pair };
+ } else {
+ return {
name: pair.substring(0, position),
value: pair.substring(position + 1),
};
}
+}
```

MODIFY
crates/rome_formatter/tests/specs/prettier/if/expr_and_same_line_comments.js.snap
crates/rome_formatter/tests/specs/prettier/if/expr_and_same_line_comments.js.snap
@@ -30,25 +30,45 @@ else if (a === 2) looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo
# Output
```js
-if (a === 0) doSomething(); else if (a === 1) doSomethingElse(); else if ( // comment A1 // comment B1
- a === 2
-) doSomethingElse(); // comment C1
+if (a === 0) {
+ doSomething(); // comment A1
+} else if (a === 1) {
+ doSomethingElse(); // comment B1
+} else if (a === 2) {
+ doSomethingElse(); // comment C1
+}
-if (a === 0) doSomething(); /* comment A2 */ else if (a === 1) doSomethingElse(); /* comment B2 */ else if (
- a === 2
-) doSomethingElse(); /* comment C2 */
+if (a === 0) {
+ doSomething(); /* comment A2 */
+} else if (a === 1) {
+ doSomethingElse(); /* comment B2 */
+} else if (a === 2) {
+ doSomethingElse(); /* comment C2 */
+}
-if (a === 0) expr; else if (a === 1) expr; else if (a === 2) expr; // comment A3 // comment B3 // comment C3
+if (a === 0) {
+ expr; // comment A3
+} else if (a === 1) {
+ expr; // comment B3
+} else if (a === 2) {
+ expr; // comment C3
+}
-if (a === 0) expr; /* comment A4 */ else if (a === 1) expr; /* comment B4 */ else if (
- a === 2
-) expr; /* comment C4 */
+if (a === 0) {
+ expr; /* comment A4 */
+} else if (a === 1) {
+ expr; /* comment B4 */
+} else if (a === 2) {
+ expr; /* comment C4 */
+}
-if (a === 0) looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong; else if ( // comment A5
- a === 1
-) looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong; else if ( // comment B5
- a === 2
-) looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong; // comment C5
+if (a === 0) {
+ looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong; // comment A5
+} else if (a === 1) {
+ looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong; // comment B5
+} else if (a === 2) {
+ looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong; // comment C5
+}
```

MODIFY
crates/rome_formatter/tests/specs/prettier/if/if_comments.js.snap
crates/rome_formatter/tests/specs/prettier/if/if_comments.js.snap
@@ -78,14 +78,23 @@ async function f1() {
}
async function f2() {
- if (untrackedChoice === 0) /* Cancel */ null; else if (untrackedChoice === 1) shouldAmend = /* Add */
- true; else if (untrackedChoice === 2) /* Allow Untracked */ allowUntracked =
- true;
+ if (untrackedChoice === 0) /* Cancel */ {
+ null;
+ } else if (untrackedChoice === 1) /* Add */ {
+ shouldAmend = true;
+ } else if (untrackedChoice === 2) /* Allow Untracked */ {
+ allowUntracked = true;
+ }
}
async function f3() {
- if (untrackedChoice === 0) null; else if (untrackedChoice === 1) shouldAmend = /* Cancel */ // Cancel /* Add */ // Add
- true; else if (untrackedChoice === 2) allowUntracked = true; /* Allow Untracked */ // Allow Untracked
+ if (untrackedChoice === 0) /* Cancel */ { // Cancel
+ null;
+ } else if (untrackedChoice === 1) /* Add */ { // Add
+ shouldAmend = true;
+ } else if (untrackedChoice === 2) /* Allow Untracked */ { // Allow Untracked
+ allowUntracked = true;
+ }
}
async function f4() {

MODIFY
crates/rome_formatter/tests/specs/prettier/no-semi/no-semi.js.snap
crates/rome_formatter/tests/specs/prettier/no-semi/no-semi.js.snap
@@ -127,7 +127,9 @@ class X {}
// don't semicolon if it doesn't start statement
-if (true) (() => {})();
+if (true) {
+ (() => {})();
+}
// check indentation
@@ -139,9 +141,15 @@ if (true) {
// check statement clauses
do break; while (false);
-if (true) do break; while (false);
+if (true) {
+ do break; while (false);
+}
-if (true) 1; else 2;
+if (true) {
+ 1;
+} else {
+ 2;
+}
for (;;)
for (x of y)

MODIFY
crates/rome_formatter/tests/specs/prettier/sloppy-mode/function-declaration-in-if.js.snap
crates/rome_formatter/tests/specs/prettier/sloppy-mode/function-declaration-in-if.js.snap
---
source: crates/rome_formatter/tests/prettier_tests.rs
-assertion_line: 83
+assertion_line: 121
expression: function-declaration-in-if.js
---
@@ -12,7 +12,9 @@ if (false) function foo(){}
# Output
```js
-if (false) function foo(){}
+if (false) {
+ function foo(){}
+}
```

MODIFY
crates/rome_formatter/tests/specs/prettier/tab-width/class.js.snap
crates/rome_formatter/tests/specs/prettier/tab-width/class.js.snap
---
source: crates/rome_formatter/tests/prettier_tests.rs
-assertion_line: 57
+assertion_line: 121
expression: class.js
---
@@ -26,7 +26,9 @@ class A {
var x = 1;
while (typeof x == "number" || typeof x == "string") {
x = x + 1;
- if (true) x = "";
+ if (true) {
+ x = "";
+ }
}
var z = x;
}

feat(rome_formatter): improve the formatting of if-else chains (#2149)