fix(css_formatter): don't preserve empty lines for selector lists (#1309

)

crates/biome_css_formatter/src/css/lists/compound_selector_list.rs

@@ -5,9 +5,13 @@ pub(crate) struct FormatCssCompoundSelectorList;
 impl FormatRule<CssCompoundSelectorList> for FormatCssCompoundSelectorList {
     type Context = CssFormatContext;
     fn fmt(&self, node: &CssCompoundSelectorList, f: &mut CssFormatter) -> FormatResult<()> {
-        let mut joiner = f.join_nodes_with_soft_line();
+        // Using `join_with` instead of `join_nodes_with_soft_line` to avoid
+        // preserving empty lines from the input source. See the comment in
+        // [FormatCssSelectorList] for more information.
+        let separator = soft_line_break_or_space();
+        let mut joiner = f.join_with(&separator);
 
-        for (rule, formatted) in node.elements().zip(node.format_separated(",")) {
+        for formatted in node.format_separated(",") {
             // 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
@@ -18,7 +22,7 @@ impl FormatRule<CssCompoundSelectorList> for FormatCssCompoundSelectorList {
             // For example, a selector like `div span a` is structured like
             // `[div, [span, [a]]]`, so `a` would end up double-indented if it
             // was handled by the selector rather than here.
-            joiner.entry(rule.node()?.syntax(), &group(&indent(&formatted)));
+            joiner.entry(&group(&indent(&formatted)));
         }
 
         joiner.finish()

crates/biome_css_formatter/src/css/lists/declaration_list.rs

@@ -6,6 +6,8 @@ pub(crate) struct FormatCssDeclarationList;
 impl FormatRule<CssDeclarationList> for FormatCssDeclarationList {
     type Context = CssFormatContext;
     fn fmt(&self, node: &CssDeclarationList, f: &mut CssFormatter) -> FormatResult<()> {
+        // This is one of the few cases where we _do_ want to respect empty
+        // lines from the input, so we can use `join_nodes_with_hardline`.
         let mut joiner = f.join_nodes_with_hardline();
 
         for (rule, formatted) in node.elements().zip(

crates/biome_css_formatter/src/css/lists/keyframes_item_list.rs

@@ -6,6 +6,8 @@ pub(crate) struct FormatCssKeyframesItemList;
 impl FormatRule<CssKeyframesItemList> for FormatCssKeyframesItemList {
     type Context = CssFormatContext;
     fn fmt(&self, node: &CssKeyframesItemList, f: &mut CssFormatter) -> FormatResult<()> {
+        // This is one of the few cases where we _do_ want to respect empty
+        // lines from the input, so we can use `join_nodes_with_hardline`.
         let mut joiner = f.join_nodes_with_hardline();
 
         for item in node.iter() {

crates/biome_css_formatter/src/css/lists/keyframes_selector_list.rs

@@ -5,9 +5,24 @@ pub(crate) struct FormatCssKeyframesSelectorList;
 impl FormatRule<CssKeyframesSelectorList> for FormatCssKeyframesSelectorList {
     type Context = CssFormatContext;
     fn fmt(&self, node: &CssKeyframesSelectorList, f: &mut CssFormatter) -> FormatResult<()> {
-        let mut joiner = f.join_nodes_with_soft_line();
+        // Using `join_with` and a manual separator instead of `join_nodes_with_soft_line`
+        // here allows ensures that the list won't try to read the input source to
+        // preserve line breaks and empty lines. That way, an input like:
+        //     h1,
+        //
+        //     h2 {}
+        // gets formatted to
+        //     h1, h2 {}
+        // and only breaks when the result doesn't fit on a single line.
+        //
+        // For top-level selector lists, like as the `prelude` of a `CssRule`, the
+        // parent node can wrap this list in a group and set `should_expand(true)`
+        // to have the separator expand into full line breaks instead of trying to
+        // fit on a single line.
+        let separator = soft_line_break_or_space();
+        let mut joiner = f.join_with(&separator);
 
-        for (rule, formatted) in node.elements().zip(node.format_separated(",")) {
+        for formatted in node.format_separated(",") {
             // 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
@@ -18,7 +33,7 @@ impl FormatRule<CssKeyframesSelectorList> for FormatCssKeyframesSelectorList {
             // For example, a selector like `div span a` is structured like
             // `[div, [span, [a]]]`, so `a` would end up double-indented if it
             // was handled by the selector rather than here.
-            joiner.entry(rule.node()?.syntax(), &group(&indent(&formatted)));
+            joiner.entry(&group(&indent(&formatted)));
         }
 
         joiner.finish()

crates/biome_css_formatter/src/css/lists/parameter_list.rs

@@ -6,10 +6,14 @@ pub(crate) struct FormatCssParameterList;
 impl FormatRule<CssParameterList> for FormatCssParameterList {
     type Context = CssFormatContext;
     fn fmt(&self, node: &CssParameterList, f: &mut CssFormatter) -> FormatResult<()> {
-        let mut joiner = f.join_nodes_with_soft_line();
+        // Using `join_with` instead of `join_nodes_with_soft_line` to avoid
+        // preserving empty lines from the input source. See the comment in
+        // [FormatCssSelectorList] for more information.
+        let separator = soft_line_break_or_space();
+        let mut joiner = f.join_with(&separator);
 
-        for (rule, formatted) in node.elements().zip(node.format_separated(",")) {
-            joiner.entry(rule.node()?.syntax(), &formatted);
+        for formatted in node.format_separated(",") {
+            joiner.entry(&formatted);
         }
 
         joiner.finish()

crates/biome_css_formatter/src/css/lists/pseudo_value_list.rs

@@ -6,10 +6,14 @@ pub(crate) struct FormatCssPseudoValueList;
 impl FormatRule<CssPseudoValueList> for FormatCssPseudoValueList {
     type Context = CssFormatContext;
     fn fmt(&self, node: &CssPseudoValueList, f: &mut CssFormatter) -> FormatResult<()> {
-        let mut joiner = f.join_nodes_with_soft_line();
+        // Using `join_with` instead of `join_nodes_with_soft_line` to avoid
+        // preserving empty lines from the input source. See the comment in
+        // [FormatCssSelectorList] for more information.
+        let separator = soft_line_break_or_space();
+        let mut joiner = f.join_with(&separator);
 
-        for (rule, formatted) in node.elements().zip(node.format_separated(",")) {
-            joiner.entry(rule.node()?.syntax(), &formatted);
+        for formatted in node.format_separated(",") {
+            joiner.entry(&formatted);
         }
 
         joiner.finish()

crates/biome_css_formatter/src/css/lists/relative_selector_list.rs

@@ -6,9 +6,13 @@ pub(crate) struct FormatCssRelativeSelectorList;
 impl FormatRule<CssRelativeSelectorList> for FormatCssRelativeSelectorList {
     type Context = CssFormatContext;
     fn fmt(&self, node: &CssRelativeSelectorList, f: &mut CssFormatter) -> FormatResult<()> {
-        let mut joiner = f.join_nodes_with_soft_line();
+        // Using `join_with` instead of `join_nodes_with_soft_line` to avoid
+        // preserving empty lines from the input source. See the comment in
+        // [FormatCssSelectorList] for more information.
+        let separator = soft_line_break_or_space();
+        let mut joiner = f.join_with(&separator);
 
-        for (rule, formatted) in node.elements().zip(node.format_separated(",")) {
+        for formatted in node.format_separated(",") {
             // 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
@@ -19,7 +23,7 @@ impl FormatRule<CssRelativeSelectorList> for FormatCssRelativeSelectorList {
             // For example, a selector like `div span a` is structured like
             // `[div, [span, [a]]]`, so `a` would end up double-indented if it
             // was handled by the selector rather than here.
-            joiner.entry(rule.node()?.syntax(), &group(&indent(&formatted)));
+            joiner.entry(&group(&indent(&formatted)));
         }
 
         joiner.finish()

crates/biome_css_formatter/src/css/lists/rule_list.rs

@@ -5,6 +5,8 @@ pub(crate) struct FormatCssRuleList;
 impl FormatRule<CssRuleList> for FormatCssRuleList {
     type Context = CssFormatContext;
     fn fmt(&self, node: &CssRuleList, f: &mut CssFormatter) -> FormatResult<()> {
+        // This is one of the few cases where we _do_ want to respect empty
+        // lines from the input, so we can use `join_nodes_with_hardline`.
         let mut join = f.join_nodes_with_hardline();
 
         for rule in node {

crates/biome_css_formatter/src/css/lists/selector_list.rs

@@ -6,9 +6,24 @@ pub(crate) struct FormatCssSelectorList;
 impl FormatRule<CssSelectorList> for FormatCssSelectorList {
     type Context = CssFormatContext;
     fn fmt(&self, node: &CssSelectorList, f: &mut CssFormatter) -> FormatResult<()> {
-        let mut joiner = f.join_nodes_with_soft_line();
+        let separator = soft_line_break_or_space();
+        // Using `join_with` and a manual separator instead of `join_nodes_with_soft_line`
+        // here allows ensures that the list won't try to read the input source to
+        // preserve line breaks and empty lines. That way, an input like:
+        //     h1,
+        //
+        //     h2 {}
+        // gets formatted to
+        //     h1, h2 {}
+        // and only breaks when the result doesn't fit on a single line.
+        //
+        // For top-level selector lists, like as the `prelude` of a `CssRule`, the
+        // parent node can wrap this list in a group and set `should_expand(true)`
+        // to have the separator expand into full line breaks instead of trying to
+        // fit on a single line.
+        let mut joiner = f.join_with(&separator);
 
-        for (rule, formatted) in node.elements().zip(node.format_separated(",")) {
+        for formatted in node.format_separated(",") {
             // 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
@@ -19,7 +34,7 @@ impl FormatRule<CssSelectorList> for FormatCssSelectorList {
             // For example, a selector like `div span a` is structured like
             // `[div, [span, [a]]]`, so `a` would end up double-indented if it
             // was handled by the selector rather than here.
-            joiner.entry(rule.node()?.syntax(), &group(&indent(&formatted)));
+            joiner.entry(&group(&indent(&formatted)));
         }
 
         joiner.finish()

crates/biome_css_formatter/src/css/selectors/pseudo_class_nth_selector.rs

@@ -12,7 +12,7 @@ impl FormatNodeRule<CssPseudoClassNthSelector> for FormatCssPseudoClassNthSelect
     ) -> FormatResult<()> {
         let CssPseudoClassNthSelectorFields { nth, of_selector } = node.as_fields();
 
-        write!(f, [nth.format()])?;
+        write!(f, [group(&nth.format())])?;
 
         if of_selector.is_some() {
             write!(f, [space(), of_selector.format()])?;

crates/biome_css_formatter/tests/specs/css/atrule/keyframes.css.snap

@@ -346,7 +346,6 @@ Line width: 80
 	}
 
 	68%,
-
 	72% {
 		left: 50px;
 	}

crates/biome_css_formatter/tests/specs/css/pseudo/pseudo_class_function_compound_selector_list.css.snap

@@ -33,13 +33,7 @@ Line width: 80
 ```css
 :-webkit-any(i, p, :link, span:focus) {
 }
-:-webkit-any(
-		i,
-		p,
-		:link,
-
-		span:focus
-	) {
+:-webkit-any(i, p, :link, span:focus) {
 }
 ```
 

crates/biome_css_formatter/tests/specs/css/pseudo/pseudo_class_function_nth.css

@@ -30,5 +30,8 @@ odd) {}
     .test) {}
 
 :nth-child(2n+1
-     of
-     li, .test) {}
+    of
+    li, .test) {}
+:nth-child(2n+1
+        of
+        li, .test, .anotherLongClassName, #aSelectorLongEnoughToBreak, .OverMultipleLinesWithIndentation) {}

crates/biome_css_formatter/tests/specs/css/pseudo/pseudo_class_function_nth.css.snap

@@ -38,8 +38,11 @@ odd) {}
     .test) {}
 
 :nth-child(2n+1
-     of
-     li, .test) {}
+    of
+    li, .test) {}
+:nth-child(2n+1
+        of
+        li, .test, .anotherLongClassName, #aSelectorLongEnoughToBreak, .OverMultipleLinesWithIndentation) {}
 ```
 
 
@@ -83,16 +86,19 @@ Line width: 80
 
 :nth-child(2n + 1 of li, .test) {
 }
-:nth-child(
-		2n
-		+ 1 of li,
-
-		.test
-	) {
+:nth-child(2n + 1 of li, .test) {
 }
 
 :nth-child(2n + 1 of li, .test) {
 }
+:nth-child(
+		2n + 1 of li,
+		.test,
+		.anotherLongClassName,
+		#aSelectorLongEnoughToBreak,
+		.OverMultipleLinesWithIndentation
+	) {
+}
 ```
 
 

crates/biome_css_formatter/tests/specs/css/pseudo/pseudo_class_function_value_list.css.snap

@@ -38,11 +38,7 @@ Line width: 80
 ```css
 :lang(de, fr) {
 }
-:lang(
-		de,
-
-		fr
-	) {
+:lang(de, fr) {
 }
 
 :lang(de) {

crates/biome_css_formatter/tests/specs/css/pseudo/where.css.snap

@@ -47,7 +47,6 @@ Line width: 80
 ```css
 :where(
 		#p0:checked ~ #play:checked ~ #c1:checked,
-
 		#p1:checked ~ #play:checked ~ #c2:checked,
 		#p2:checked ~ #play:checked ~ #cO:checked
 	)