Skip to content

Commit

Permalink
fix(js_formatter): correctly handle comments placement for jsx spread…
Browse files Browse the repository at this point in the history
… expression (#2358)
  • Loading branch information
ah-yu authored Apr 10, 2024
1 parent d9f7047 commit 62e5aab
Show file tree
Hide file tree
Showing 10 changed files with 232 additions and 233 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,10 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b

### Formatter

#### Bug fixes

- Fix [#2291](https://github.com/biomejs/biome/issues/2291) by correctly handle comment placement for JSX spread attributes and JSX spread children. Contributed by @ah-yu

### JavaScript APIs

### Linter
Expand Down
31 changes: 29 additions & 2 deletions crates/biome_js_formatter/src/jsx/attribute/spread_attribute.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::prelude::*;

use crate::utils::format_node_without_comments::FormatAnyJsExpressionWithoutComments;
use biome_formatter::write;
use biome_js_syntax::{JsxSpreadAttribute, JsxSpreadAttributeFields};

Expand All @@ -16,8 +17,34 @@ impl FormatNodeRule<JsxSpreadAttribute> for FormatJsxSpreadAttribute {
} = node.as_fields();

let argument = argument?;
let format_inner =
format_with(|f| write!(f, [dotdotdot_token.format(), argument.format(),]));
let format_inner = format_with(|f| {
if f.comments().is_suppressed(argument.syntax()) {
write!(
f,
[
dotdotdot_token.format(),
argument.format(),
line_suffix_boundary()
]
)
} else {
write!(
f,
[
format_leading_comments(argument.syntax()),
dotdotdot_token.format()
]
)?;
FormatAnyJsExpressionWithoutComments.fmt(&argument, f)?;
write!(
f,
[
format_dangling_comments(argument.syntax()).with_soft_block_indent(),
format_trailing_comments(argument.syntax()),
]
)
}
});

write!(f, [l_curly_token.format()])?;

Expand Down
36 changes: 28 additions & 8 deletions crates/biome_js_formatter/src/jsx/auxiliary/spread_child.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::prelude::*;
use biome_formatter::write;

use crate::utils::format_node_without_comments::FormatAnyJsExpressionWithoutComments;
use biome_js_syntax::{JsxSpreadChild, JsxSpreadChildFields};

#[derive(Debug, Clone, Default)]
Expand All @@ -18,14 +19,33 @@ impl FormatNodeRule<JsxSpreadChild> for FormatJsxSpreadChild {
let expression = expression?;

let format_inner = format_with(|f| {
write!(
f,
[
dotdotdot_token.format(),
expression.format(),
line_suffix_boundary()
]
)
if f.comments().is_suppressed(expression.syntax()) {
write!(
f,
[
dotdotdot_token.format(),
expression.format(),
line_suffix_boundary()
]
)
} else {
write!(
f,
[
format_leading_comments(expression.syntax()),
dotdotdot_token.format(),
]
)?;
FormatAnyJsExpressionWithoutComments.fmt(&expression, f)?;
write!(
f,
[
format_dangling_comments(expression.syntax()).with_soft_block_indent(),
format_trailing_comments(expression.syntax()),
line_suffix_boundary()
]
)
}
});

write!(f, [l_curly_token.format()])?;
Expand Down
154 changes: 154 additions & 0 deletions crates/biome_js_formatter/src/utils/format_node_without_comments.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,154 @@
use crate::js::bogus::bogus_expression::FormatJsBogusExpression;
use crate::js::expressions::array_expression::FormatJsArrayExpression;
use crate::js::expressions::arrow_function_expression::FormatJsArrowFunctionExpression;
use crate::js::expressions::assignment_expression::FormatJsAssignmentExpression;
use crate::js::expressions::await_expression::FormatJsAwaitExpression;
use crate::js::expressions::bigint_literal_expression::FormatJsBigintLiteralExpression;
use crate::js::expressions::binary_expression::FormatJsBinaryExpression;
use crate::js::expressions::boolean_literal_expression::FormatJsBooleanLiteralExpression;
use crate::js::expressions::call_expression::FormatJsCallExpression;
use crate::js::expressions::class_expression::FormatJsClassExpression;
use crate::js::expressions::computed_member_expression::FormatJsComputedMemberExpression;
use crate::js::expressions::conditional_expression::FormatJsConditionalExpression;
use crate::js::expressions::function_expression::FormatJsFunctionExpression;
use crate::js::expressions::import_call_expression::FormatJsImportCallExpression;
use crate::js::expressions::import_meta_expression::FormatJsImportMetaExpression;
use crate::js::expressions::in_expression::FormatJsInExpression;
use crate::js::expressions::instanceof_expression::FormatJsInstanceofExpression;
use crate::js::expressions::logical_expression::FormatJsLogicalExpression;
use crate::js::expressions::new_expression::FormatJsNewExpression;
use crate::js::expressions::new_target_expression::FormatJsNewTargetExpression;
use crate::js::expressions::null_literal_expression::FormatJsNullLiteralExpression;
use crate::js::expressions::number_literal_expression::FormatJsNumberLiteralExpression;
use crate::js::expressions::object_expression::FormatJsObjectExpression;
use crate::js::expressions::parenthesized_expression::FormatJsParenthesizedExpression;
use crate::js::expressions::post_update_expression::FormatJsPostUpdateExpression;
use crate::js::expressions::pre_update_expression::FormatJsPreUpdateExpression;
use crate::js::expressions::regex_literal_expression::FormatJsRegexLiteralExpression;
use crate::js::expressions::sequence_expression::FormatJsSequenceExpression;
use crate::js::expressions::static_member_expression::FormatJsStaticMemberExpression;
use crate::js::expressions::string_literal_expression::FormatJsStringLiteralExpression;
use crate::js::expressions::super_expression::FormatJsSuperExpression;
use crate::js::expressions::template_expression::FormatJsTemplateExpression;
use crate::js::expressions::this_expression::FormatJsThisExpression;
use crate::js::expressions::unary_expression::FormatJsUnaryExpression;
use crate::js::expressions::yield_expression::FormatJsYieldExpression;
use crate::jsx::expressions::tag_expression::FormatJsxTagExpression;
use crate::ts::expressions::as_expression::FormatTsAsExpression;
use crate::ts::expressions::instantiation_expression::FormatTsInstantiationExpression;
use crate::ts::expressions::non_null_assertion_expression::FormatTsNonNullAssertionExpression;
use crate::ts::expressions::satisfies_expression::FormatTsSatisfiesExpression;
use crate::ts::expressions::type_assertion_expression::FormatTsTypeAssertionExpression;
use crate::{js::expressions::identifier_expression::FormatJsIdentifierExpression, prelude::*};
use biome_js_syntax::{AnyJsExpression, AnyJsLiteralExpression};

#[derive(Debug, Clone, Default)]
pub(crate) struct FormatAnyJsExpressionWithoutComments;

impl FormatRule<AnyJsExpression> for FormatAnyJsExpressionWithoutComments {
type Context = JsFormatContext;
fn fmt(&self, node: &AnyJsExpression, f: &mut JsFormatter) -> FormatResult<()> {
match node {
AnyJsExpression::AnyJsLiteralExpression(literal_expr) => match literal_expr {
AnyJsLiteralExpression::JsBigintLiteralExpression(node) => {
FormatJsBigintLiteralExpression.fmt_node(node, f)
}
AnyJsLiteralExpression::JsBooleanLiteralExpression(node) => {
FormatJsBooleanLiteralExpression.fmt_node(node, f)
}
AnyJsLiteralExpression::JsNullLiteralExpression(node) => {
FormatJsNullLiteralExpression.fmt_node(node, f)
}
AnyJsLiteralExpression::JsNumberLiteralExpression(node) => {
FormatJsNumberLiteralExpression.fmt_node(node, f)
}
AnyJsLiteralExpression::JsRegexLiteralExpression(node) => {
FormatJsRegexLiteralExpression.fmt_node(node, f)
}
AnyJsLiteralExpression::JsStringLiteralExpression(node) => {
FormatJsStringLiteralExpression.fmt_node(node, f)
}
},
AnyJsExpression::JsArrayExpression(node) => {
FormatJsArrayExpression::default().fmt_node(node, f)
}
AnyJsExpression::JsArrowFunctionExpression(node) => {
FormatJsArrowFunctionExpression::default().fmt_node(node, f)
}
AnyJsExpression::JsAssignmentExpression(node) => {
FormatJsAssignmentExpression.fmt_node(node, f)
}
AnyJsExpression::JsAwaitExpression(node) => FormatJsAwaitExpression.fmt_node(node, f),
AnyJsExpression::JsBinaryExpression(node) => FormatJsBinaryExpression.fmt_node(node, f),
AnyJsExpression::JsBogusExpression(node) => FormatJsBogusExpression.fmt(node, f),
AnyJsExpression::JsCallExpression(node) => FormatJsCallExpression.fmt_node(node, f),
AnyJsExpression::JsClassExpression(node) => FormatJsClassExpression.fmt_node(node, f),
AnyJsExpression::JsComputedMemberExpression(node) => {
FormatJsComputedMemberExpression.fmt_node(node, f)
}
AnyJsExpression::JsConditionalExpression(node) => {
FormatJsConditionalExpression::default().fmt_node(node, f)
}
AnyJsExpression::JsFunctionExpression(node) => {
FormatJsFunctionExpression::default().fmt_node(node, f)
}
AnyJsExpression::JsIdentifierExpression(node) => {
FormatJsIdentifierExpression.fmt_node(node, f)
}
AnyJsExpression::JsImportCallExpression(node) => {
FormatJsImportCallExpression.fmt_node(node, f)
}
AnyJsExpression::JsImportMetaExpression(node) => {
FormatJsImportMetaExpression.fmt_node(node, f)
}
AnyJsExpression::JsInExpression(node) => FormatJsInExpression.fmt_node(node, f),
AnyJsExpression::JsInstanceofExpression(node) => {
FormatJsInstanceofExpression.fmt_node(node, f)
}
AnyJsExpression::JsLogicalExpression(node) => {
FormatJsLogicalExpression.fmt_node(node, f)
}
AnyJsExpression::JsNewExpression(node) => FormatJsNewExpression.fmt_node(node, f),
AnyJsExpression::JsNewTargetExpression(node) => {
FormatJsNewTargetExpression.fmt_node(node, f)
}
AnyJsExpression::JsObjectExpression(node) => FormatJsObjectExpression.fmt_node(node, f),
AnyJsExpression::JsParenthesizedExpression(node) => {
FormatJsParenthesizedExpression.fmt_node(node, f)
}
AnyJsExpression::JsPostUpdateExpression(node) => {
FormatJsPostUpdateExpression.fmt_node(node, f)
}
AnyJsExpression::JsPreUpdateExpression(node) => {
FormatJsPreUpdateExpression.fmt_node(node, f)
}
AnyJsExpression::JsSequenceExpression(node) => {
FormatJsSequenceExpression.fmt_node(node, f)
}
AnyJsExpression::JsStaticMemberExpression(node) => {
FormatJsStaticMemberExpression.fmt_node(node, f)
}
AnyJsExpression::JsSuperExpression(node) => FormatJsSuperExpression.fmt_node(node, f),
AnyJsExpression::JsTemplateExpression(node) => {
FormatJsTemplateExpression.fmt_node(node, f)
}
AnyJsExpression::JsThisExpression(node) => FormatJsThisExpression.fmt_node(node, f),
AnyJsExpression::JsUnaryExpression(node) => FormatJsUnaryExpression.fmt_node(node, f),
AnyJsExpression::JsYieldExpression(node) => FormatJsYieldExpression.fmt_node(node, f),
AnyJsExpression::JsxTagExpression(node) => FormatJsxTagExpression.fmt_node(node, f),
AnyJsExpression::TsAsExpression(node) => FormatTsAsExpression.fmt_node(node, f),
AnyJsExpression::TsInstantiationExpression(node) => {
FormatTsInstantiationExpression.fmt_node(node, f)
}
AnyJsExpression::TsNonNullAssertionExpression(node) => {
FormatTsNonNullAssertionExpression.fmt_node(node, f)
}
AnyJsExpression::TsSatisfiesExpression(node) => {
FormatTsSatisfiesExpression.fmt_node(node, f)
}
AnyJsExpression::TsTypeAssertionExpression(node) => {
FormatTsTypeAssertionExpression.fmt_node(node, f)
}
}
}
}
1 change: 1 addition & 0 deletions crates/biome_js_formatter/src/utils/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ pub mod string_utils;

pub(crate) mod format_class;
pub(crate) mod format_modifiers;
pub(crate) mod format_node_without_comments;
pub(crate) mod function_body;
pub mod jsx;
pub(crate) mod member_chain;
Expand Down
6 changes: 5 additions & 1 deletion crates/biome_js_formatter/tests/specs/jsx/comments.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,8 @@
a>;

<a></ /* block1 */ // test
a>;
a>;

<div {.../* comment */a}></div>;

<div>{.../* comment */a}</div>;
9 changes: 7 additions & 2 deletions crates/biome_js_formatter/tests/specs/jsx/comments.jsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
source: crates/biome_formatter_test/src/snapshot_builder.rs
info: jsx/comments.jsx
---

# Input

```jsx
Expand All @@ -11,6 +10,10 @@ a>;
<a></ /* block1 */ // test
a>;
<div {.../* comment */a}></div>;
<div>{.../* comment */a}</div>;
```
Expand Down Expand Up @@ -44,6 +47,8 @@ a>;
/* block1 */ // test
a
>;
```
<div {/* comment */ ...a}></div>;
<div>{/* comment */ ...a}</div>;
```
Loading

0 comments on commit 62e5aab

Please sign in to comment.