fix(rome_js_formatter): correctly format sequence expressions
MODIFY
crates/rome_js_formatter/src/js/expressions/sequence_expression.rs
crates/rome_js_formatter/src/js/expressions/sequence_expression.rs
use crate::prelude::*;
-
use crate::FormatNodeFields;
-use rome_js_syntax::{JsSequenceExpression, JsSequenceExpressionFields};
+use rome_formatter::ConcatBuilder;
+use rome_js_syntax::{JsSequenceExpression, JsSequenceExpressionFields, JsSyntaxKind};
use rome_rowan::AstNode;
impl FormatNodeFields<JsSequenceExpression> for FormatNodeRule<JsSequenceExpression> {
@@ -10,6 +10,32 @@ impl FormatNodeFields<JsSequenceExpression> for FormatNodeRule<JsSequenceExpress
formatter: &Formatter<JsFormatOptions>,
) -> FormatResult<FormatElement> {
let mut current = node.clone();
+ let parent = current.syntax().parent();
+
+ let has_already_indentation = parent.map_or(false, |parent| {
+ // Return statement already does the indentation for us
+ // Arrow function body can't have a sequence expression unless it's parenthesized, otherwise
+ // would be a syntax error
+ if matches!(parent.kind(), JsSyntaxKind::JS_RETURN_STATEMENT) {
+ true
+ } else if matches!(parent.kind(), JsSyntaxKind::JS_PARENTHESIZED_EXPRESSION) {
+ // In case we are inside a sequence expression, we have to go up a level and see the great parent.
+ // Arrow function body and return statements applying indentation for us, so we signal the
+ // sequence expression to not add other indentation levels
+ let great_parent = parent.parent().map(|gp| gp.kind());
+
+ matches!(
+ great_parent,
+ Some(
+ JsSyntaxKind::JS_ARROW_FUNCTION_EXPRESSION
+ | JsSyntaxKind::JS_RETURN_STATEMENT
+ | JsSyntaxKind::JS_PROPERTY_OBJECT_MEMBER
+ )
+ )
+ } else {
+ false
+ }
+ });
// Find the left most sequence expression
while let Some(sequence_expression) =
@@ -25,36 +51,68 @@ impl FormatNodeFields<JsSequenceExpression> for FormatNodeRule<JsSequenceExpress
right,
} = current.as_fields();
- let mut formatted = vec![formatted![
+ let mut formatted = ConcatBuilder::default();
+
+ formatted.entry(formatted![
formatter,
- [
- left.format(),
- comma_token.format(),
- space_token(),
- right.format(),
- ]
- ]?];
+ [left.format()?, comma_token.format()?]
+ ]?);
+
+ let mut previous_right = right;
// Traverse upwards again and concatenate the sequence expression until we find the first non-sequence expression
- while let Some(parent) = current.syntax().parent() {
- if let Some(parent_sequence) = JsSequenceExpression::cast(parent) {
+ while let Some(parent_sequence) = current
+ .syntax()
+ .parent()
+ .and_then(JsSequenceExpression::cast)
+ {
let JsSequenceExpressionFields {
left: _left,
comma_token,
right,
} = parent_sequence.as_fields();
- formatted.push(formatted![
+ if has_already_indentation {
+ formatted.entry(formatted![
formatter,
- [comma_token.format(), space_token(), right.format()]
+ [
+ soft_line_break_or_space(),
+ previous_right.format()?,
+ comma_token.format()?,
+ ]
]?);
-
- current = parent_sequence;
} else {
- break;
+ formatted.entry(formatted![
+ formatter,
+ [indent(formatted![
+ formatter,
+ [
+ soft_line_break_or_space(),
+ previous_right.format()?,
+ comma_token.format()?,
+ ]
+ ]?),]
+ ]?)
+ }
+ previous_right = right;
+ current = parent_sequence;
}
+
+ if has_already_indentation {
+ formatted.entry(formatted![
+ formatter,
+ [soft_line_break_or_space(), previous_right.format()?,]
+ ]?);
+ } else {
+ formatted.entry(formatted![
+ formatter,
+ [indent(formatted![
+ formatter,
+ [soft_line_break_or_space(), previous_right.format()?,]
+ ]?),]
+ ]?)
}
- Ok(concat_elements(formatted))
+ Ok(group_elements(formatted.finish()))
}
}

MODIFY
crates/rome_js_formatter/src/lib.rs
crates/rome_js_formatter/src/lib.rs
@@ -379,7 +379,9 @@ mod test {
#[ignore]
// use this test check if your snippet prints as you wish, without using a snapshot
fn quick_test() {
- let src = r#"xyz.a(b!).a(b!).a(b!)
+ let src = r#"(function(){
+ return aLongIdentifierName, aLongIdentifierName, aLongIdentifierName, aLongIdentifierName;
+});
"#;
let syntax = SourceType::jsx();

MODIFY
crates/rome_js_formatter/tests/specs/js/module/expression/sequence_expression.js
crates/rome_js_formatter/tests/specs/js/module/expression/sequence_expression.js
a,b
+
+const f = () => (
+ ____________first, ____________second, ____________third, ____________third, ____________third, ____________third, ____________third
+
+);
+
+(
+ ____________first, ____________second, ____________third, ____________third, ____________third, ____________third, ____________third
+
+)
+
+function a() {
+ return ____________first, ____________second, ____________third, ____________third, ____________third, ____________third, ____________third
+}
+
+const object ={
+ something: (
+ ____________first, ____________second, ____________third, ____________third, ____________third, ____________third, ____________third
+ )
+
+}
+
+aLongIdentifierName,
+ aLongIdentifierName,
+ aLongIdentifierName,
+ aLongIdentifierName;
\ No newline at end of file

MODIFY
crates/rome_js_formatter/tests/specs/js/module/expression/sequence_expression.js.snap
crates/rome_js_formatter/tests/specs/js/module/expression/sequence_expression.js.snap
---
source: crates/rome_js_formatter/tests/spec_test.rs
+assertion_line: 244
expression: sequence_expression.js
---
# Input
a,b
+const f = () => (
+ ____________first, ____________second, ____________third, ____________third, ____________third, ____________third, ____________third
+
+);
+
+(
+ ____________first, ____________second, ____________third, ____________third, ____________third, ____________third, ____________third
+
+)
+
+function a() {
+ return ____________first, ____________second, ____________third, ____________third, ____________third, ____________third, ____________third
+}
+
+const object ={
+ something: (
+ ____________first, ____________second, ____________third, ____________third, ____________third, ____________third, ____________third
+ )
+
+}
+
+aLongIdentifierName,
+ aLongIdentifierName,
+ aLongIdentifierName,
+ aLongIdentifierName;
=============================
# Outputs
## Output 1
@@ -15,3 +41,52 @@ Quote style: Double Quotes
-----
a, b;
+const f = () => (
+ ____________first,
+ ____________second,
+ ____________third,
+ ____________third,
+ ____________third,
+ ____________third,
+ ____________third
+);
+
+(
+ ____________first,
+ ____________second,
+ ____________third,
+ ____________third,
+ ____________third,
+ ____________third,
+ ____________third
+);
+
+function a() {
+ return (
+ ____________first,
+ ____________second,
+ ____________third,
+ ____________third,
+ ____________third,
+ ____________third,
+ ____________third
+ );
+}
+
+const object = {
+ something: (
+ ____________first,
+ ____________second,
+ ____________third,
+ ____________third,
+ ____________third,
+ ____________third,
+ ____________third
+ ),
+};
+
+aLongIdentifierName,
+ aLongIdentifierName,
+ aLongIdentifierName,
+ aLongIdentifierName;
+

MODIFY
crates/rome_js_formatter/tests/specs/prettier/js/objects/expression.js.snap
crates/rome_js_formatter/tests/specs/prettier/js/objects/expression.js.snap
---
source: crates/rome_js_formatter/tests/prettier_tests.rs
+assertion_line: 181
expression: expression.js
---
# Input
@@ -57,7 +58,9 @@ const a2 = {
const a3 = {
someKey: (
- longLongLongLongLongLongLongLongLongLongLongLongLongLongName, longLongLongLongLongLongLongLongLongLongLongLongLongLongName, longLongLongLongLongLongLongLongLongLongLongLongLongLongName
+ longLongLongLongLongLongLongLongLongLongLongLongLongLongName,
+ longLongLongLongLongLongLongLongLongLongLongLongLongLongName,
+ longLongLongLongLongLongLongLongLongLongLongLongLongLongName
),
};
@@ -80,8 +83,4 @@ error[SyntaxError]: expected `')'` but instead found `:`
```
-# Lines exceeding max width of 80 characters
-```
- 25: longLongLongLongLongLongLongLongLongLongLongLongLongLongName, longLongLongLongLongLongLongLongLongLongLongLongLongLongName, longLongLongLongLongLongLongLongLongLongLongLongLongLongName
-```

MODIFY
crates/rome_js_formatter/tests/specs/prettier/js/sequence-break/break.js.snap
crates/rome_js_formatter/tests/specs/prettier/js/sequence-break/break.js.snap
---
source: crates/rome_js_formatter/tests/prettier_tests.rs
+assertion_line: 181
expression: break.js
---
# Input
@@ -27,25 +28,35 @@ const f = (
argument2,
argument3,
) => (
- doSomethingWithArgument(
- argument1,
- ), doSomethingWithArgument(argument2), argument1
+ doSomethingWithArgument(argument1),
+ doSomethingWithArgument(argument2),
+ argument1
);
(function () {
return (
- aLongIdentifierName, aLongIdentifierName, aLongIdentifierName, aLongIdentifierName
+ aLongIdentifierName,
+ aLongIdentifierName,
+ aLongIdentifierName,
+ aLongIdentifierName
);
});
-aLongIdentifierName, aLongIdentifierName, aLongIdentifierName, aLongIdentifierName;
+aLongIdentifierName,
+ aLongIdentifierName,
+ aLongIdentifierName,
+ aLongIdentifierName;
a.then(
() => (
- aLongIdentifierName, aLongIdentifierName, aLongIdentifierName, aLongIdentifierName
+ aLongIdentifierName,
+ aLongIdentifierName,
+ aLongIdentifierName,
+ aLongIdentifierName
),
);
for (
- aLongIdentifierName =
- 0, aLongIdentifierName =
- 0, aLongIdentifierName = 0, aLongIdentifierName = 0;
+ aLongIdentifierName = 0,
+ aLongIdentifierName = 0,
+ aLongIdentifierName = 0,
+ aLongIdentifierName = 0;
test;
update
) {}
@@ -54,22 +65,26 @@ for (
b ? c : function () {
return 0;
}
-), (
+),
+ (
a =
b ? c : function () {
return 0;
}
-), (
+ ),
+ (
a =
b ? c : function () {
return 0;
}
-), (
+ ),
+ (
a =
b ? c : function () {
return 0;
}
-), (
+ ),
+ (
a =
b ? c : function () {
return 0;
@@ -78,10 +93,4 @@ for (
```
-# Lines exceeding max width of 80 characters
-```
- 12: aLongIdentifierName, aLongIdentifierName, aLongIdentifierName, aLongIdentifierName
- 15: aLongIdentifierName, aLongIdentifierName, aLongIdentifierName, aLongIdentifierName;
- 18: aLongIdentifierName, aLongIdentifierName, aLongIdentifierName, aLongIdentifierName
-```

fix(rome_js_formatter): correctly format sequence expressions (#2541)