fix(css_formatter): don't wrap selector identation after css comments (…

…#4614)
biomejs · Nov 26, 2024 · cd1c8ec · cd1c8ec
1 parent ac73177
commit cd1c8ec
Show file tree

Hide file tree

Showing 8 changed files with 204 additions and 74 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -56,7 +56,7 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b
 
 - Fix [#4026](https://github.com/biomejs/biome/issues/4026), don't move comments in `grid-template`. Contributed by @fireairforce
 
-- Fix [#4533](URL_ADDRESS.com/biomejs/biome/issues/4533), don't throw error when pseudeo class after a webkit scrollbar pseudeo element.
+- Fix [#4533](https://github.com/biomejs/biome/issues/4533), don't throw error when pseudeo class after a webkit scrollbar pseudeo element.
 
   The follow code will not report:
 
@@ -66,6 +66,8 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b
 
   Contributed by @fireairforce
 
+- Fix [#4575](https://github.com/biomejs/biome/issues/4575), don't wrap selector identation after css comments. Contributed by @fireairforce
+
 ### JavaScript APIs
 
 ### Linter

diff --git a/crates/biome_css_formatter/src/css/lists/selector_list.rs b/crates/biome_css_formatter/src/css/lists/selector_list.rs
@@ -24,6 +24,38 @@ impl FormatRule<CssSelectorList> for FormatCssSelectorList {
         let mut joiner = f.join_with(&separator);
 
         for formatted in node.format_separated(",") {
+            let computed_selector =
+                formatted
+                    .node()?
+                    .as_css_complex_selector()
+                    .and_then(|complex_selector| {
+                        complex_selector
+                            .left()
+                            .ok()?
+                            .as_css_compound_selector()
+                            .cloned()
+                    });
+
+            if let Some(computed_selector) = computed_selector {
+                let simple_selector_has_leading_comments = computed_selector
+                    .simple_selector()
+                    .and_then(|simple_selector| simple_selector.as_css_type_selector().cloned())
+                    .and_then(|type_selector| type_selector.ident().ok()?.value_token().ok())
+                    .is_some_and(|value_token| value_token.has_leading_comments());
+
+                let sub_selector_has_leading_comments = computed_selector
+                    .sub_selectors()
+                    .first()
+                    .and_then(|sub_selector| sub_selector.as_css_class_selector().cloned())
+                    .and_then(|class_selector| class_selector.dot_token().ok())
+                    .is_some_and(|value_token| value_token.has_leading_comments());
+
+                if simple_selector_has_leading_comments || sub_selector_has_leading_comments {
+                    joiner.entry(&group(&formatted));
+                    continue;
+                }
+            }
+
             // Each selector gets `indent` added in case it breaks over multiple
             // lines. The break is added here rather than in each selector both
             // for simplicity and to avoid recursively adding indents when

diff --git a/crates/biome_css_formatter/src/css/selectors/complex_selector.rs b/crates/biome_css_formatter/src/css/selectors/complex_selector.rs
@@ -23,16 +23,47 @@ impl FormatNodeRule<CssComplexSelector> for FormatCssComplexSelector {
                 write!(f, [combinator.format()])
             }
         });
+        let mut has_leading_comments = false;
 
-        write!(
-            f,
-            [
-                left.format(),
-                soft_line_break_or_space(),
-                formatted_combinator,
-                space(),
-                right.format()
-            ]
-        )
+        let is_selector_list_first_child = node.syntax().parent().is_some_and(|parent| {
+            matches!(parent.kind(), CssSyntaxKind::CSS_SELECTOR_LIST)
+                && node.syntax().prev_sibling().is_none()
+        });
+
+        if let Some(computed_selector) = left.clone()?.as_css_compound_selector() {
+            let simple_selector_has_leading_comments = computed_selector
+                .simple_selector()
+                .and_then(|simple_selector| simple_selector.as_css_type_selector().cloned())
+                .and_then(|type_selector| type_selector.ident().ok()?.value_token().ok())
+                .is_some_and(|value_token| value_token.has_leading_comments());
+
+            let sub_selector_has_leading_comments = computed_selector
+                .sub_selectors()
+                .first()
+                .and_then(|sub_selector| sub_selector.as_css_class_selector().cloned())
+                .and_then(|class_selector| class_selector.dot_token().ok())
+                .is_some_and(|value_token| value_token.has_leading_comments());
+
+            has_leading_comments =
+                simple_selector_has_leading_comments || sub_selector_has_leading_comments;
+        }
+
+        if has_leading_comments && !is_selector_list_first_child {
+            write!(
+                f,
+                [left.format(), formatted_combinator, space(), right.format()]
+            )
+        } else {
+            write!(
+                f,
+                [
+                    left.format(),
+                    soft_line_break_or_space(),
+                    formatted_combinator,
+                    space(),
+                    right.format()
+                ]
+            )
+        }
     }
 }
diff --git a/crates/biome_css_formatter/tests/quick_test.rs b/crates/biome_css_formatter/tests/quick_test.rs
@@ -13,13 +13,14 @@ mod language {
 // use this test check if your snippet prints as you wish, without using a snapshot
 fn quick_test() {
     let src = r#"
-#grid {
-	grid-template-columns:
-	  1fr /* first comment */
-	  auto /* second comment */
-      60px /* fourth comment */
-	  2fr /* third comment */  
-  }
+/* 1some medium long comment */
+.line1 selector,
+/* 2some medium long comment */
+.line1 selector,
+/* 3some medium long comment */
+div selector {
+  background: green;
+}
 "#;
     let parse = parse_css(src, CssParserOptions::default());
     println!("{parse:#?}");

diff --git a/crates/biome_css_formatter/tests/specs/css/issue_3229.css.snap b/crates/biome_css_formatter/tests/specs/css/issue_3229.css.snap
@@ -84,17 +84,17 @@ foo
 
 /* input */
 foo
-	/* a comment */
+/* a comment */
 
-	.aRule {
+.aRule {
 	color: red;
 }
 
 /* first format */
 foo
-	/* a comment */
+/* a comment */
 
-	.aRule {
+.aRule {
 	color: red;
 }
 ```
diff --git a/crates/biome_css_formatter/tests/specs/css/selectors/selector_lists.css b/crates/biome_css_formatter/tests/specs/css/selectors/selector_lists.css
@@ -14,4 +14,28 @@ div.with.really.long#selector#content, div.another.really.long#selector.that.goe
 h1, h2     , h3
 
 
-, h4,    h5,      h6 {}
+, h4,    h5,      h6 {}
+
+
+/* 1some medium long comment */
+.line1 selector,
+/* 2some medium long comment */
+.line1 selector {
+  background: red;
+}
+
+/* 1some medium long comment */
+.line1 selector,
+/* 2some medium long comment */
+div selector {
+  background: blue;
+}
+
+/* 1some medium long comment */
+.line1 selector,
+/* 2some medium long comment */
+.line1 selector,
+/* 3some medium long comment */
+div selector {
+  background: green;
+}
diff --git a/crates/biome_css_formatter/tests/specs/css/selectors/selector_lists.css.snap b/crates/biome_css_formatter/tests/specs/css/selectors/selector_lists.css.snap
@@ -2,7 +2,6 @@
 source: crates/biome_formatter_test/src/snapshot_builder.rs
 info: css/selectors/selector_lists.css
 ---
-
 # Input
 
 ```css
@@ -23,6 +22,31 @@ h1, h2     , h3
 
 
 , h4,    h5,      h6 {}
+
+
+/* 1some medium long comment */
+.line1 selector,
+/* 2some medium long comment */
+.line1 selector {
+  background: red;
+}
+
+/* 1some medium long comment */
+.line1 selector,
+/* 2some medium long comment */
+div selector {
+  background: blue;
+}
+
+/* 1some medium long comment */
+.line1 selector,
+/* 2some medium long comment */
+.line1 selector,
+/* 3some medium long comment */
+div selector {
+  background: green;
+}
+
 ```
 
 
@@ -62,11 +86,32 @@ h4,
 h5,
 h6 {
 }
+
+/* 1some medium long comment */
+.line1 selector,
+/* 2some medium long comment */
+.line1 selector {
+	background: red;
+}
+
+/* 1some medium long comment */
+.line1 selector,
+/* 2some medium long comment */
+div selector {
+	background: blue;
+}
+
+/* 1some medium long comment */
+.line1 selector,
+/* 2some medium long comment */
+.line1 selector,
+/* 3some medium long comment */
+div selector {
+	background: green;
+}
 ```
 
 # Lines exceeding max width of 80 characters
 ```
    12: div.another.really.long#selector.that.goes.past.the.line.length.with.a.single.selector {
 ```
-
-