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.

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

fix(css): reformat issue in comments between compound selectors #4537

Merged
merged 1 commit into from
Nov 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b

- Fix [#4121](https://github.com/biomejs/biome/issues/4326), don't ident a CSS selector when has leading comments. Contributed by @fireairforce
- Fix [#4334](https://github.com/biomejs/biome/issues/4334), don't insert trailing comma on type import statement. Contributed by @fireairforce

- Fix [#3229](https://github.com/biomejs/biome/issues/3229), where Biome wasn't idempotent when block comments were placed inside compound selectors. Contributed by @ematipico
### JavaScript APIs

### Linter
Expand Down Expand Up @@ -130,7 +130,7 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b
Also, this will bring the Biome rule closer to the [no-undef ESLint rule](https://eslint.org/docs/latest/rules/no-undef).

Contributed by @Conaclos

- Add [noGlobalDirnameFilename](https://biomejs.dev/linter/rules/no-global-dirname-filename/). Contributed by @unvalley

#### Enhancements
Expand Down
33 changes: 23 additions & 10 deletions crates/biome_css_formatter/src/comments.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use crate::prelude::*;
use biome_css_syntax::{AnyCssDeclarationName, CssFunction, CssIdentifier, CssLanguage, TextLen};
use biome_css_syntax::{
AnyCssDeclarationName, CssComplexSelector, CssFunction, CssIdentifier, CssLanguage, TextLen,
};
use biome_diagnostics::category;
use biome_formatter::comments::{
is_doc_comment, CommentKind, CommentPlacement, CommentStyle, CommentTextPosition, Comments,
Expand Down Expand Up @@ -89,15 +91,15 @@ impl CommentStyle for CssCommentStyle {
comment: DecoratedComment<Self::Language>,
) -> CommentPlacement<Self::Language> {
match comment.text_position() {
CommentTextPosition::EndOfLine => {
handle_function_comment(comment).or_else(handle_declaration_name_comment)
}
CommentTextPosition::OwnLine => {
handle_function_comment(comment).or_else(handle_declaration_name_comment)
}
CommentTextPosition::SameLine => {
handle_function_comment(comment).or_else(handle_declaration_name_comment)
}
CommentTextPosition::EndOfLine => handle_function_comment(comment)
.or_else(handle_declaration_name_comment)
.or_else(handle_complex_selector_comment),
CommentTextPosition::OwnLine => handle_function_comment(comment)
.or_else(handle_declaration_name_comment)
.or_else(handle_complex_selector_comment),
CommentTextPosition::SameLine => handle_function_comment(comment)
.or_else(handle_declaration_name_comment)
.or_else(handle_complex_selector_comment),
}
}
}
Expand Down Expand Up @@ -130,3 +132,14 @@ fn handle_function_comment(
CommentPlacement::Default(comment)
}
}

fn handle_complex_selector_comment(
comment: DecoratedComment<CssLanguage>,
) -> CommentPlacement<CssLanguage> {
if let Some(complex) = CssComplexSelector::cast_ref(comment.enclosing_node()) {
if let Ok(right) = complex.right() {
return CommentPlacement::leading(right.into_syntax(), comment);
}
}
CommentPlacement::Default(comment)
}
8 changes: 6 additions & 2 deletions crates/biome_css_formatter/tests/quick_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,12 @@ mod language {
#[test]
// use this test check if your snippet prints as you wish, without using a snapshot
fn quick_test() {
let src = r#"
@charset "UTF-8";
let src = r#"foo
/* a comment */

.aRule {
color: red;
}
"#;
let parse = parse_css(src, CssParserOptions::default());
println!("{parse:#?}");
Expand Down
39 changes: 39 additions & 0 deletions crates/biome_css_formatter/tests/specs/css/issue_3229.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
foo /* a comment */
.aRule {
color: red;
}

foo /* a comment */

.aRule {
color: red;
}

foo


/* a comment */

.aRule {
color: red;
}

/* input */
foo

/* a comment */

.aRule {
color: red
}




/* first format */
foo
/* a comment */

.aRule {
color: red;
}
100 changes: 100 additions & 0 deletions crates/biome_css_formatter/tests/specs/css/issue_3229.css.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
---
source: crates/biome_formatter_test/src/snapshot_builder.rs
info: css/issue_3229.css
snapshot_kind: text
---
# Input

```css
foo /* a comment */
.aRule {
color: red;
}

foo /* a comment */

.aRule {
color: red;
}

foo


/* a comment */

.aRule {
color: red;
}

/* input */
foo

/* a comment */

.aRule {
color: red
}




/* first format */
foo
/* a comment */

.aRule {
color: red;
}

```


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

# Outputs

## Output 1

-----
Indent style: Tab
Indent width: 2
Line ending: LF
Line width: 80
Quote style: Double Quotes
-----

```css
foo /* a comment */ .aRule {
color: red;
}

foo
/* a comment */

.aRule {
color: red;
}

foo
/* a comment */

.aRule {
color: red;
}

/* input */
foo
/* a comment */

.aRule {
color: red;
}

/* first format */
foo
/* a comment */

.aRule {
color: red;
}
```
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
---
source: crates/biome_formatter_test/src/snapshot_builder.rs
info: css/atrule/supports.css
snapshot_kind: text
---
# Input

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
---
source: crates/biome_formatter_test/src/snapshot_builder.rs
info: css/bom/bom.css
snapshot_kind: text
---
# Input

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
---
source: crates/biome_formatter_test/src/snapshot_builder.rs
info: css/comments/custom-properties.css
snapshot_kind: text
---
# Input

Expand Down
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I gave up checking regressions with Prettier tests, because many of them include unsupported syntax, so we have parsing errors. At the end, they aren't reliable

Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
---
source: crates/biome_formatter_test/src/snapshot_builder.rs
info: css/comments/selectors.css
snapshot_kind: text
---
# Input

Expand Down Expand Up @@ -231,8 +232,8 @@ input:not(/* comment 125 */[/* comment 126 */disabled/* comment 127 */]/* commen

-/* comment 29 */
-.element /* comment 30 */ > /* comment 31 */ .element /* comment 32 */ {
+/* comment 29 */ .element /* comment 30 */
+ > /* comment 31 */ .element /* comment 32 */ {
+/* comment 29 */ .element
+ > /* comment 30 */ /* comment 31 */ .element /* comment 32 */ {
}
/* comment 33 */
.element
Expand All @@ -249,8 +250,8 @@ input:not(/* comment 125 */[/* comment 126 */disabled/* comment 127 */]/* commen

-/* comment 37 */
-.element /* comment 38 */ + /* comment 39 */ .element /* comment 40 */ {
+/* comment 37 */ .element /* comment 38 */
+ + /* comment 39 */ .element /* comment 40 */ {
+/* comment 37 */ .element
+ + /* comment 38 */ /* comment 39 */ .element /* comment 40 */ {
}
/* comment 41 */
.element
Expand All @@ -267,8 +268,8 @@ input:not(/* comment 125 */[/* comment 126 */disabled/* comment 127 */]/* commen

-/* comment 45 */
-.element /* comment 46 */ ~ /* comment 47 */ .element /* comment 48 */ {
+/* comment 45 */ .element /* comment 46 */
+ ~ /* comment 47 */ .element /* comment 48 */ {
+/* comment 45 */ .element
+ ~ /* comment 46 */ /* comment 47 */ .element /* comment 48 */ {
}
/* comment 49 */
.element
Expand Down Expand Up @@ -442,8 +443,8 @@ input:not(/* comment 125 */[/* comment 126 */disabled/* comment 127 */]/* commen
-/* comment 167 */
-article /* comment 168 */ :--heading /* comment 169 */ + /* comment 170 */ p /* comment 171 */ {
+/* comment 167 */ article
+ /* comment 168 */ :--heading /* comment 169 */
+ + /* comment 170 */ p /* comment 171 */ {
+ /* comment 168 */ :--heading
+ + /* comment 169 */ /* comment 170 */ p /* comment 171 */ {
}

-/* comment 172 */
Expand Down Expand Up @@ -534,8 +535,8 @@ input:not(/* comment 125 */[/* comment 126 */disabled/* comment 127 */]/* commen
{
}

/* comment 29 */ .element /* comment 30 */
> /* comment 31 */ .element /* comment 32 */ {
/* comment 29 */ .element
> /* comment 30 */ /* comment 31 */ .element /* comment 32 */ {
}
/* comment 33 */
.element
Expand All @@ -545,8 +546,8 @@ input:not(/* comment 125 */[/* comment 126 */disabled/* comment 127 */]/* commen
{
}

/* comment 37 */ .element /* comment 38 */
+ /* comment 39 */ .element /* comment 40 */ {
/* comment 37 */ .element
+ /* comment 38 */ /* comment 39 */ .element /* comment 40 */ {
}
/* comment 41 */
.element
Expand All @@ -556,8 +557,8 @@ input:not(/* comment 125 */[/* comment 126 */disabled/* comment 127 */]/* commen
{
}

/* comment 45 */ .element /* comment 46 */
~ /* comment 47 */ .element /* comment 48 */ {
/* comment 45 */ .element
~ /* comment 46 */ /* comment 47 */ .element /* comment 48 */ {
}
/* comment 49 */
.element
Expand Down Expand Up @@ -679,8 +680,8 @@ input:not(
} /* comment 166 */

/* comment 167 */ article
/* comment 168 */ :--heading /* comment 169 */
+ /* comment 170 */ p /* comment 171 */ {
/* comment 168 */ :--heading
+ /* comment 169 */ /* comment 170 */ p /* comment 171 */ {
}

/* comment 172 */ .foo
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
---
source: crates/biome_formatter_test/src/snapshot_builder.rs
info: css/front-matter/custom-parser.css
snapshot_kind: text
---
# Input

Expand All @@ -21,7 +22,7 @@ description: Description
```diff
--- Prettier
+++ Biome
@@ -1,8 +1,6 @@
@@ -1,8 +1,7 @@
---mycustomparser
-title: Title
-description: Description
Expand All @@ -31,7 +32,8 @@ description: Description
-.something {
+ title:Title
+ description:Description
+ --- /* comment */
+ ---
+ /* comment */
+ .something {
}
```
Expand All @@ -42,7 +44,8 @@ description: Description
---mycustomparser
title:Title
description:Description
--- /* comment */
---
/* comment */
.something {
}
```
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
---
source: crates/biome_formatter_test/src/snapshot_builder.rs
info: css/front-matter/empty.css
snapshot_kind: text
---
# Input

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
---
source: crates/biome_formatter_test/src/snapshot_builder.rs
info: css/yaml/comment_after.css
snapshot_kind: text
---
# Input

Expand Down
Loading