From 96c78b2885b143db12399bb2a934d7195739dfb9 Mon Sep 17 00:00:00 2001 From: zoomdong <1344492820@qq.com> Date: Wed, 30 Oct 2024 22:54:46 +0800 Subject: [PATCH 1/3] fix(css_formatter): don't ident css selector when has leading comments --- CHANGELOG.md | 4 ++ .../src/css/lists/relative_selector_list.rs | 41 ++++++++++++++----- .../tests/specs/css/declaration_list.css | 16 ++++++++ .../tests/specs/css/declaration_list.css.snap | 32 +++++++++++++++ 4 files changed, 82 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5386d6e9bf1f..12fe2a0bb908 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,10 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b ### Formatter +### Bug fixes + +- Fix [#4121](https://github.com/biomejs/biome/issues/4326), don't ident a CSS selector when has leading comments. Contributed by @fireairforce + ### JavaScript APIs ### Linter diff --git a/crates/biome_css_formatter/src/css/lists/relative_selector_list.rs b/crates/biome_css_formatter/src/css/lists/relative_selector_list.rs index 566e6e53982a..0561cc0dbb30 100644 --- a/crates/biome_css_formatter/src/css/lists/relative_selector_list.rs +++ b/crates/biome_css_formatter/src/css/lists/relative_selector_list.rs @@ -13,17 +13,36 @@ impl FormatRule for FormatCssRelativeSelectorList { let mut joiner = f.join_with(&separator); 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 - // selectors are nested within other rules. The group is then added - // around the indent to ensure that it tries using a flat layout - // first and only expands when the single selector can't fit the line. - // - // 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(&group(&indent(&formatted))); + let mut has_comments_before = false; + if let Some(relative_selector) = formatted.node()?.as_css_relative_selector() { + if let Some(computed_selector) = + relative_selector.selector()?.as_css_compound_selector() + { + if let Some(simple_selector) = computed_selector.simple_selector() { + if let Some(type_selector) = simple_selector.as_css_type_selector() { + has_comments_before = + type_selector.ident()?.value_token()?.has_leading_comments(); + } + } + } + } + + if has_comments_before { + // Computed Selector which contains a leading comments should be formatted without indent. + joiner.entry(&group(&formatted)); + } else { + // 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 + // selectors are nested within other rules. The group is then added + // around the indent to ensure that it tries using a flat layout + // first and only expands when the single selector can't fit the line. + // + // 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(&group(&indent(&formatted))); + } } joiner.finish() diff --git a/crates/biome_css_formatter/tests/specs/css/declaration_list.css b/crates/biome_css_formatter/tests/specs/css/declaration_list.css index 2d9252e42926..780622ef405b 100644 --- a/crates/biome_css_formatter/tests/specs/css/declaration_list.css +++ b/crates/biome_css_formatter/tests/specs/css/declaration_list.css @@ -34,4 +34,20 @@ a { a { color: red;;;; +} + +.with-comments { + /* hello */ + a, + /* world */ + button { + color: blue; + } +} + +.without-comments { + a, + button { + color: blue; + } } \ No newline at end of file diff --git a/crates/biome_css_formatter/tests/specs/css/declaration_list.css.snap b/crates/biome_css_formatter/tests/specs/css/declaration_list.css.snap index e168c99e2562..9db6650632b6 100644 --- a/crates/biome_css_formatter/tests/specs/css/declaration_list.css.snap +++ b/crates/biome_css_formatter/tests/specs/css/declaration_list.css.snap @@ -42,6 +42,22 @@ a { a { color: red;;;; } + +.with-comments { + /* hello */ + a, + /* world */ + button { + color: blue; + } +} + +.without-comments { + a, + button { + color: blue; + } +} ``` @@ -103,4 +119,20 @@ a { a { color: red; } + +.with-comments { + /* hello */ + a, + /* world */ + button { + color: blue; + } +} + +.without-comments { + a, + button { + color: blue; + } +} ``` From 4d1f892b31a89eec8f4c670c4c91fe3132dfac7a Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Thu, 31 Oct 2024 11:01:20 +0000 Subject: [PATCH 2/3] refactor using a more FP approach --- .../src/css/lists/relative_selector_list.rs | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/crates/biome_css_formatter/src/css/lists/relative_selector_list.rs b/crates/biome_css_formatter/src/css/lists/relative_selector_list.rs index 0561cc0dbb30..1ec24bda134a 100644 --- a/crates/biome_css_formatter/src/css/lists/relative_selector_list.rs +++ b/crates/biome_css_formatter/src/css/lists/relative_selector_list.rs @@ -13,21 +13,23 @@ impl FormatRule for FormatCssRelativeSelectorList { let mut joiner = f.join_with(&separator); for formatted in node.format_separated(",") { - let mut has_comments_before = false; - if let Some(relative_selector) = formatted.node()?.as_css_relative_selector() { - if let Some(computed_selector) = - relative_selector.selector()?.as_css_compound_selector() - { - if let Some(simple_selector) = computed_selector.simple_selector() { - if let Some(type_selector) = simple_selector.as_css_type_selector() { - has_comments_before = - type_selector.ident()?.value_token()?.has_leading_comments(); - } - } - } - } + let has_leading_comments = formatted + .node()? + .as_css_relative_selector() + .and_then(|relative_selector| { + relative_selector + .selector() + .ok()? + .as_css_compound_selector() + .cloned() + }) + .and_then(|computed_selector| 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()) + .map(|value_token| value_token.has_leading_comments()) + .unwrap_or_default(); - if has_comments_before { + if has_leading_comments { // Computed Selector which contains a leading comments should be formatted without indent. joiner.entry(&group(&formatted)); } else { From 4678a239e720f6e16480e43689411219163fad8e Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Thu, 31 Oct 2024 11:04:48 +0000 Subject: [PATCH 3/3] clippy suggestion --- .../src/css/lists/relative_selector_list.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/biome_css_formatter/src/css/lists/relative_selector_list.rs b/crates/biome_css_formatter/src/css/lists/relative_selector_list.rs index 1ec24bda134a..4f103d8f9b06 100644 --- a/crates/biome_css_formatter/src/css/lists/relative_selector_list.rs +++ b/crates/biome_css_formatter/src/css/lists/relative_selector_list.rs @@ -26,8 +26,7 @@ impl FormatRule for FormatCssRelativeSelectorList { .and_then(|computed_selector| 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()) - .map(|value_token| value_token.has_leading_comments()) - .unwrap_or_default(); + .is_some_and(|value_token| value_token.has_leading_comments()); if has_leading_comments { // Computed Selector which contains a leading comments should be formatted without indent.