Skip to content

Commit

Permalink
fix(js_formatter): ensure flat conditional content is always written …
Browse files Browse the repository at this point in the history
…in `RemoveSoftLinesBuffer` (#1357)

Co-authored-by: Emanuele Stoppa <[email protected]>
  • Loading branch information
faultyserver and ematipico authored Dec 28, 2023
1 parent cd8b0c9 commit c212dd6
Show file tree
Hide file tree
Showing 6 changed files with 140 additions and 8 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ Biome now scores 97% compatibility with Prettier and features more than 180 lint

- Fix [#1220](https://github.com/biomejs/biome/issues/1220). Avoid duplicating comments in type unions for mapped, empty object, and empty tuple types. [#1240](https://github.com/biomejs/biome/pull/1240) Contributed by @faultyserver

- Fix [#1356](https://github.com/biomejs/biome/issues/1356). Ensure `if_group_fits_on_line` content is always written in `RemoveSoftLinesBuffer`s. [#1357](https://github.com/biomejs/biome/pull/1357) Contributed by @faultyserver

- Fix [#1171](https://github.com/biomejs/biome/issues/1171). Correctly format empty statement with comment inside arrow body when used as single argument in call expression. Contributed by @kalleep

### JavaScript APIs
Expand Down
30 changes: 22 additions & 8 deletions crates/biome_formatter/src/buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -606,16 +606,30 @@ impl<Context> Buffer for RemoveSoftLinesBuffer<'_, Context> {

fn write_element(&mut self, element: FormatElement) -> FormatResult<()> {
let element = match element {
FormatElement::Tag(Tag::StartConditionalContent(condition))
if condition.mode == PrintMode::Expanded =>
{
self.is_in_expanded_conditional_content = true;
return Ok(());
FormatElement::Tag(Tag::StartConditionalContent(condition)) => {
match condition.mode {
PrintMode::Expanded => {
// Mark that we're within expanded content so that it
// can be dropped in all future writes until the ending
// tag.
self.is_in_expanded_conditional_content = true;
return Ok(());
}
PrintMode::Flat => {
// Flat groups have the conditional tag dropped as
// well, since the content within it will _always_ be
// printed by this buffer.
return Ok(());
}
}
}
FormatElement::Tag(Tag::EndConditionalContent)
if self.is_in_expanded_conditional_content =>
{
FormatElement::Tag(Tag::EndConditionalContent) => {
// NOTE: This assumes that conditional content cannot be nested.
// This is true for all practical cases, but it's _possible_ to
// write IR that breaks this.
self.is_in_expanded_conditional_content = false;
// No matter if this was flat or expanded content, the ending
// tag gets dropped, since the starting tag was also dropped.
return Ok(());
}
// All content within an expanded conditional gets dropped. If there's a
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"cases": [
{
"semicolon": "AsNeeded",
"line_width": 40
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// Ensure that the type annotation preserves the semicolon in the type
// annotation, even though the parent group breaks. This was only an issue when
// `semicolon: AsNeeded` was set, since the semicolon became conditional on
// whether the group was printed inline.
//
// https://github.com/biomejs/biome/issues/1356.

foo((args: {a: string; b: string}) => {
return a;
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
---
source: crates/biome_formatter_test/src/snapshot_builder.rs
info: ts/parameters/issue-1356/parameter_type_annotation_semicolon.ts
---

# Input

```ts
// Ensure that the type annotation preserves the semicolon in the type
// annotation, even though the parent group breaks. This was only an issue when
// `semicolon: AsNeeded` was set, since the semicolon became conditional on
// whether the group was printed inline.
//
// https://github.com/biomejs/biome/issues/1356.

foo((args: {a: string; b: string}) => {
return a;
})
```


=============================

# Outputs

## Output 1

-----
Indent style: Tab
Indent width: 2
Line ending: LF
Line width: 80
Quote style: Double Quotes
JSX quote style: Double Quotes
Quote properties: As needed
Trailing comma: All
Semicolons: Always
Arrow parentheses: Always
Bracket spacing: true
Bracket same line: false
-----

```ts
// Ensure that the type annotation preserves the semicolon in the type
// annotation, even though the parent group breaks. This was only an issue when
// `semicolon: AsNeeded` was set, since the semicolon became conditional on
// whether the group was printed inline.
//
// https://github.com/biomejs/biome/issues/1356.

foo((args: { a: string; b: string }) => {
return a;
});
```

## Output 2

-----
Indent style: Tab
Indent width: 2
Line ending: LF
Line width: 40
Quote style: Double Quotes
JSX quote style: Double Quotes
Quote properties: As needed
Trailing comma: All
Semicolons: Always
Arrow parentheses: Always
Bracket spacing: true
Bracket same line: false
-----

```ts
// Ensure that the type annotation preserves the semicolon in the type
// annotation, even though the parent group breaks. This was only an issue when
// `semicolon: AsNeeded` was set, since the semicolon became conditional on
// whether the group was printed inline.
//
// https://github.com/biomejs/biome/issues/1356.

foo(
(args: { a: string; b: string }) => {
return a;
},
);
```

# Lines exceeding max width of 40 characters
```
1: // Ensure that the type annotation preserves the semicolon in the type
2: // annotation, even though the parent group breaks. This was only an issue when
3: // `semicolon: AsNeeded` was set, since the semicolon became conditional on
6: // https://github.com/biomejs/biome/issues/1356.
```


2 changes: 2 additions & 0 deletions website/src/content/docs/internals/changelog.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ Biome now scores 97% compatibility with Prettier and features more than 180 lint

- Fix [#1220](https://github.com/biomejs/biome/issues/1220). Avoid duplicating comments in type unions for mapped, empty object, and empty tuple types. [#1240](https://github.com/biomejs/biome/pull/1240) Contributed by @faultyserver

- Fix [#1356](https://github.com/biomejs/biome/issues/1356). Ensure `if_group_fits_on_line` content is always written in `RemoveSoftLinesBuffer`s. [#1357](https://github.com/biomejs/biome/pull/1357) Contributed by @faultyserver

- Fix [#1171](https://github.com/biomejs/biome/issues/1171). Correctly format empty statement with comment inside arrow body when used as single argument in call expression. Contributed by @kalleep

### JavaScript APIs
Expand Down

0 comments on commit c212dd6

Please sign in to comment.