Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(formatter): prevent line break when comment follows end of attributes, irrespective of bracketSameLine value #3534

Merged
merged 6 commits into from
Jul 29, 2024
+106 −20
Merged

fix(formatter): prevent line break when comment follows end of attributes, irrespective of bracketSameLine value #3534

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2a63e39
fix: check last attribute coments
satojin219 Jul 27, 2024
e7529da
test: bracket_same_line.jsx
satojin219 Jul 27, 2024
59e78bd
fix: make char string single quotation
satojin219 Jul 27, 2024
7c52276
refactor: remove unnecessary comment
satojin219 Jul 27, 2024
809b098
refactor: remove unnecessary println
satojin219 Jul 27, 2024
d47cc05
fix: remove is_self_closing conditional branch
satojin219 Jul 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 29 additions & 5 deletions crates/biome_js_formatter/src/jsx/tag/opening_element.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ declare_node_union! {
impl Format<JsFormatContext> for AnyJsxOpeningElement {
fn fmt(&self, f: &mut Formatter<JsFormatContext>) -> FormatResult<()> {
let layout = self.compute_layout(f.context().comments())?;

let l_angle_token = self.l_angle_token()?;
let name = self.name()?;
let type_arguments = self.type_arguments();
Expand Down Expand Up @@ -69,7 +68,10 @@ impl Format<JsFormatContext> for AnyJsxOpeningElement {
]
)
}
OpeningElementLayout::IndentAttributes { name_has_comments } => {
OpeningElementLayout::IndentAttributes {
name_has_comments,
last_attribute_has_comments,
} => {
let format_inner = format_with(|f| {
write!(
f,
Expand All @@ -86,6 +88,8 @@ impl Format<JsFormatContext> for AnyJsxOpeningElement {

if self.is_self_closing() {
write!(f, [soft_line_break_or_space(), format_close])
} else if force_bracket_same_line && last_attribute_has_comments {
write!(f, [soft_line_break(), format_close])
} else if force_bracket_same_line || wants_bracket_same_line {
write!(f, [format_close])
} else {
Expand All @@ -96,7 +100,6 @@ impl Format<JsFormatContext> for AnyJsxOpeningElement {
let has_multiline_string_attribute = attributes
.iter()
.any(|attribute| is_multiline_string_literal_attribute(&attribute));

write![
f,
[group(&format_inner).should_expand(has_multiline_string_attribute)]
Expand Down Expand Up @@ -166,7 +169,10 @@ impl AnyJsxOpeningElement {
{
OpeningElementLayout::SingleStringAttribute
} else {
OpeningElementLayout::IndentAttributes { name_has_comments }
OpeningElementLayout::IndentAttributes {
name_has_comments,
last_attribute_has_comments: has_last_attribute_comments(self, comments),
}
};

Ok(layout)
Expand Down Expand Up @@ -200,7 +206,10 @@ enum OpeningElementLayout {
/// moreAttributes={withSomeExpression}
/// ></div>;
/// ```
IndentAttributes { name_has_comments: bool },
IndentAttributes {
name_has_comments: bool,
last_attribute_has_comments: bool,
},
}

/// Returns `true` if this is an attribute with a [JsxString] initializer that does not contain any new line characters.
Expand Down Expand Up @@ -240,3 +249,18 @@ fn as_string_literal_attribute_value(attribute: &AnyJsxAttribute) -> Option<JsxS
JsxSpreadAttribute(_) => None,
}
}

fn has_last_attribute_comments(element: &AnyJsxOpeningElement, comments: &JsComments) -> bool {
let has_comments_on_last_attribute = element
.attributes()
.last()
.map_or(false, |attribute| comments.has_comments(attribute.syntax()));

let last_attribute_has_comments = element
.syntax()
.tokens()
.map(|token| token.text().contains('>') && token.has_leading_comments())
.any(|has_comment| has_comment);

has_comments_on_last_attribute || last_attribute_has_comments
}
31 changes: 23 additions & 8 deletions crates/biome_js_formatter/tests/specs/jsx/bracket_same_line/bracket_same_line.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,33 @@
const a = <div></div>;

<Foo
className={style}
reallyLongAttributeName1={longComplexValue}
reallyLongAttributeName2={anotherLongValue}
className={style}
reallyLongAttributeName1={longComplexValue}
reallyLongAttributeName2={anotherLongValue}
/>;

<Foo
className={style}
reallyLongAttributeName1={longComplexValue}
reallyLongAttributeName2={anotherLongValue}
className={style}
reallyLongAttributeName1={longComplexValue}
reallyLongAttributeName2={anotherLongValue} // comment
/>;

<Foo
className={style}
reallyLongAttributeName1={longComplexValue}
reallyLongAttributeName2={anotherLongValue}
>
Hi
</Foo>;

<Foo
className={style}
reallyLongAttributeName1={longComplexValue}
reallyLongAttributeName2={anotherLongValue}
// comment
>
Hi
Hi
</Foo>;

<div className="hi" />;
<div className="hi"></div>;
<div className="hi"></div>;
61 changes: 54 additions & 7 deletions crates/biome_js_formatter/tests/specs/jsx/bracket_same_line/bracket_same_line.jsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
---
source: crates/biome_formatter_test/src/snapshot_builder.rs
assertion_line: 212
info: jsx/bracket_same_line/bracket_same_line.jsx
---
# Input
Expand All @@ -8,21 +9,37 @@ info: jsx/bracket_same_line/bracket_same_line.jsx
const a = <div></div>;

<Foo
className={style}
reallyLongAttributeName1={longComplexValue}
reallyLongAttributeName2={anotherLongValue}
className={style}
reallyLongAttributeName1={longComplexValue}
reallyLongAttributeName2={anotherLongValue}
/>;

<Foo
className={style}
reallyLongAttributeName1={longComplexValue}
reallyLongAttributeName2={anotherLongValue} // comment
/>;

<Foo
className={style}
reallyLongAttributeName1={longComplexValue}
reallyLongAttributeName2={anotherLongValue}
className={style}
reallyLongAttributeName1={longComplexValue}
reallyLongAttributeName2={anotherLongValue}
>
Hi
Hi
</Foo>;

<Foo
className={style}
reallyLongAttributeName1={longComplexValue}
reallyLongAttributeName2={anotherLongValue}
// comment
>
Hi
</Foo>;

<div className="hi" />;
<div className="hi"></div>;

```


Expand Down Expand Up @@ -57,6 +74,12 @@ const a = <div></div>;
reallyLongAttributeName2={anotherLongValue}
/>;

<Foo
className={style}
reallyLongAttributeName1={longComplexValue}
reallyLongAttributeName2={anotherLongValue} // comment
/>;

<Foo
className={style}
reallyLongAttributeName1={longComplexValue}
Expand All @@ -65,6 +88,15 @@ const a = <div></div>;
Hi
</Foo>;

<Foo
className={style}
reallyLongAttributeName1={longComplexValue}
reallyLongAttributeName2={anotherLongValue}
// comment
>
Hi
</Foo>;

<div className="hi" />;
<div className="hi"></div>;
```
Expand Down Expand Up @@ -96,13 +128,28 @@ const a = <div></div>;
reallyLongAttributeName2={anotherLongValue}
/>;

<Foo
className={style}
reallyLongAttributeName1={longComplexValue}
reallyLongAttributeName2={anotherLongValue} // comment
/>;

<Foo
className={style}
reallyLongAttributeName1={longComplexValue}
reallyLongAttributeName2={anotherLongValue}>
Hi
</Foo>;

<Foo
className={style}
reallyLongAttributeName1={longComplexValue}
reallyLongAttributeName2={anotherLongValue}
// comment
>
Hi
</Foo>;

<div className="hi" />;
<div className="hi"></div>;
```