From 8fe1d5ff84073ddf4c1a29475019d25439f04eaa Mon Sep 17 00:00:00 2001 From: Denis Bezrukov <6227442+denbezrukov@users.noreply.github.com> Date: Sat, 8 Jun 2024 23:09:22 +0300 Subject: [PATCH] fix(css_formatter): CSS formatter converts variable declarations and function calls to lowercase #3068 --- CHANGELOG.md | 2 +- crates/biome_css_formatter/src/comments.rs | 4 +- .../src/css/auxiliary/function.rs | 24 ++++-- .../src/css/lists/layer_name_list.rs | 10 ++- .../src/css/value/identifier.rs | 33 +++++++- crates/biome_css_formatter/src/separated.rs | 77 ++++++++++++++++++- .../tests/specs/css/atrule/import.css.snap | 2 +- .../tests/specs/css/atrule/layer.css | 11 ++- .../tests/specs/css/atrule/layer.css.snap | 21 ++++- .../tests/specs/css/casing.css.snap | 7 +- .../tests/specs/css/properties/all.css | 9 +++ .../tests/specs/css/properties/all.css.snap | 18 +++++ .../specs/prettier/css/case/case.css.snap | 3 - .../css/color/color-adjuster.css.snap | 3 - .../specs/prettier/css/loose/loose.css.snap | 3 - .../specs/prettier/css/parens/parens.css.snap | 9 +-- .../src/syntax/property/mod.rs | 12 ++- 17 files changed, 200 insertions(+), 48 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e07866e7dc2e..8039ae724be2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,7 +19,7 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b - Fix [#3069](https://github.com/biomejs/biome/issues/3069), prevent overwriting paths when using `--staged` or `--changed` options. Contributed by @unvalley - Fix the bug where whitespace after the & character in CSS nesting was incorrectly trimmed, ensuring proper targeting of child classes [#3061](https://github.com/biomejs/biome/issues/3061). Contributed by @denbezrukov - +- Fix [#3068](https://github.com/biomejs/biome/issues/3068) where the CSS formatter was inadvertently converting variable declarations and function calls to lowercase. Contributed by @denbezrukov ### Configuration diff --git a/crates/biome_css_formatter/src/comments.rs b/crates/biome_css_formatter/src/comments.rs index d145cbf57eaa..988e22ca3a71 100644 --- a/crates/biome_css_formatter/src/comments.rs +++ b/crates/biome_css_formatter/src/comments.rs @@ -1,5 +1,5 @@ use crate::prelude::*; -use biome_css_syntax::{AnyCssDeclarationName, CssFunction, CssLanguage, TextLen}; +use biome_css_syntax::{AnyCssDeclarationName, CssFunction, CssIdentifier, CssLanguage, TextLen}; use biome_diagnostics::category; use biome_formatter::comments::{ is_doc_comment, CommentKind, CommentPlacement, CommentStyle, CommentTextPosition, Comments, @@ -123,7 +123,7 @@ fn handle_function_comment( }; let is_inside_function = CssFunction::can_cast(comment.enclosing_node().kind()); - let is_after_name = AnyCssDeclarationName::can_cast(preceding_node.kind()); + let is_after_name = CssIdentifier::can_cast(preceding_node.kind()); if is_inside_function && is_after_name { CommentPlacement::leading(following_node.clone(), comment) } else { diff --git a/crates/biome_css_formatter/src/css/auxiliary/function.rs b/crates/biome_css_formatter/src/css/auxiliary/function.rs index 20795d08dc47..847c68ea1e59 100644 --- a/crates/biome_css_formatter/src/css/auxiliary/function.rs +++ b/crates/biome_css_formatter/src/css/auxiliary/function.rs @@ -1,6 +1,8 @@ +use crate::css::value::identifier::FormatCssIdentifierOptions; use crate::prelude::*; use biome_css_syntax::{CssFunction, CssFunctionFields}; use biome_formatter::{format_args, write}; + #[derive(Debug, Clone, Default)] pub(crate) struct FormatCssFunction; impl FormatNodeRule<CssFunction> for FormatCssFunction { @@ -12,16 +14,22 @@ impl FormatNodeRule<CssFunction> for FormatCssFunction { r_paren_token, } = node.as_fields(); + if let Ok(name) = name { + write!( + f, + [name + .format() + .with_options(FormatCssIdentifierOptions::default().prevent_lowercase(true))] + )?; + } + write!( f, - [ - name.format(), - group(&format_args![ - l_paren_token.format(), - soft_block_indent(&items.format()), - r_paren_token.format() - ]) - ] + [group(&format_args![ + l_paren_token.format(), + soft_block_indent(&items.format()), + r_paren_token.format() + ])] ) } } diff --git a/crates/biome_css_formatter/src/css/lists/layer_name_list.rs b/crates/biome_css_formatter/src/css/lists/layer_name_list.rs index e863bc99a967..9fd71ffc6bd5 100644 --- a/crates/biome_css_formatter/src/css/lists/layer_name_list.rs +++ b/crates/biome_css_formatter/src/css/lists/layer_name_list.rs @@ -1,10 +1,18 @@ +use crate::css::value::identifier::FormatCssIdentifierOptions; use crate::prelude::*; +use crate::separated::FormatAstSeparatedListWithOptionsExtension; use biome_css_syntax::CssLayerNameList; + #[derive(Debug, Clone, Default)] pub(crate) struct FormatCssLayerNameList; impl FormatRule<CssLayerNameList> for FormatCssLayerNameList { type Context = CssFormatContext; fn fmt(&self, node: &CssLayerNameList, f: &mut CssFormatter) -> FormatResult<()> { - f.join().entries(node.format_separated(".")).finish() + f.join() + .entries(node.format_separated_with_options1( + ".", + FormatCssIdentifierOptions::default().prevent_lowercase(true), + )) + .finish() } } diff --git a/crates/biome_css_formatter/src/css/value/identifier.rs b/crates/biome_css_formatter/src/css/value/identifier.rs index 40699a133bbd..5e79f9a63a3a 100644 --- a/crates/biome_css_formatter/src/css/value/identifier.rs +++ b/crates/biome_css_formatter/src/css/value/identifier.rs @@ -1,14 +1,41 @@ use crate::{prelude::*, utils::string_utils::FormatTokenAsLowercase}; use biome_css_syntax::{CssIdentifier, CssIdentifierFields}; -use biome_formatter::write; +use biome_formatter::{write, FormatRuleWithOptions}; -#[derive(Debug, Clone, Default)] -pub(crate) struct FormatCssIdentifier; +#[derive(Default, Debug, Clone, Copy)] +pub(crate) struct FormatCssIdentifier { + prevent_lowercase: bool, +} + +#[derive(Default, Debug, Clone, Copy)] +pub(crate) struct FormatCssIdentifierOptions { + pub(crate) prevent_lowercase: bool, +} + +impl FormatCssIdentifierOptions { + pub(crate) fn prevent_lowercase(mut self, prevent_lowercase: bool) -> Self { + self.prevent_lowercase = prevent_lowercase; + self + } +} + +impl FormatRuleWithOptions<CssIdentifier> for FormatCssIdentifier { + type Options = FormatCssIdentifierOptions; + + fn with_options(mut self, options: Self::Options) -> Self { + self.prevent_lowercase = options.prevent_lowercase; + self + } +} impl FormatNodeRule<CssIdentifier> for FormatCssIdentifier { fn fmt_fields(&self, node: &CssIdentifier, f: &mut CssFormatter) -> FormatResult<()> { let CssIdentifierFields { value_token } = node.as_fields(); + if self.prevent_lowercase { + return write!(f, [value_token.format()]); + } + // Identifiers in CSS are used all over the place. Type selectors, // declaration names, value definitions, and plenty more. For the most // part, these identifiers are case-insensitive, meaning they can diff --git a/crates/biome_css_formatter/src/separated.rs b/crates/biome_css_formatter/src/separated.rs index b3d989941b90..fa9d5b69b3b9 100644 --- a/crates/biome_css_formatter/src/separated.rs +++ b/crates/biome_css_formatter/src/separated.rs @@ -1,10 +1,10 @@ use crate::prelude::*; use crate::FormatCssSyntaxToken; -use biome_css_syntax::{CssLanguage, CssSyntaxToken}; +use biome_css_syntax::{CssIdentifier, CssLanguage, CssSyntaxToken}; use biome_formatter::separated::{ FormatSeparatedElementRule, FormatSeparatedIter, TrailingSeparator, }; -use biome_formatter::FormatRefWithRule; +use biome_formatter::{FormatRefWithRule, FormatRuleWithOptions}; use biome_rowan::{AstNode, AstSeparatedList, AstSeparatedListElementsIterator}; use std::marker::PhantomData; @@ -30,8 +30,6 @@ where } } -// Unused currently because CSS formatting is very barebones for now -#[allow(unused)] type CssFormatSeparatedIter<Node> = FormatSeparatedIter< AstSeparatedListElementsIterator<CssLanguage, Node>, Node, @@ -59,3 +57,74 @@ pub(crate) trait FormatAstSeparatedListExtension: } impl<T> FormatAstSeparatedListExtension for T where T: AstSeparatedList<Language = CssLanguage> {} + +#[derive(Default, Debug, Clone, Copy)] +pub(crate) struct CssFormatSeparatedElementRuleWithOptions<N, O> { + node: PhantomData<N>, + options: O, +} + +impl<N, O> CssFormatSeparatedElementRuleWithOptions<N, O> { + pub(crate) fn new(options: O) -> Self { + Self { + node: PhantomData, + options, + } + } +} + +impl<N, O, R> FormatSeparatedElementRule<N> for CssFormatSeparatedElementRuleWithOptions<N, O> +where + O: Clone + Copy, + R: FormatNodeRule<N> + FormatRuleWithOptions<N, Context = CssFormatContext, Options = O>, + N: AstNode<Language = CssLanguage> + + for<'a> AsFormat<CssFormatContext, Format<'a> = FormatRefWithRule<'a, N, R>> + + 'static, +{ + type Context = CssFormatContext; + type FormatNode<'a> = FormatRefWithRule<'a, N, R>; + type FormatSeparator<'a> = FormatRefWithRule<'a, CssSyntaxToken, FormatCssSyntaxToken>; + + fn format_node<'a>(&self, node: &'a N) -> Self::FormatNode<'a> { + node.format().with_options(self.options) + } + + fn format_separator<'a>(&self, separator: &'a CssSyntaxToken) -> Self::FormatSeparator<'a> { + separator.format() + } +} + +type CssFormatSeparatedIterWithOptions<Node, Options> = FormatSeparatedIter< + AstSeparatedListElementsIterator<CssLanguage, Node>, + Node, + CssFormatSeparatedElementRuleWithOptions<Node, Options>, +>; + +/// AST Separated list formatting extension methods with options +pub(crate) trait FormatAstSeparatedListWithOptionsExtension<O>: + AstSeparatedList<Language = CssLanguage> +{ + /// Prints a separated list of nodes with options + /// + /// Trailing separators will be reused from the original list or created by + /// calling the `separator_factory` function. The last trailing separator + /// will not be printed by default. Use `with_trailing_separator` to add it + /// in where necessary. + fn format_separated_with_options1( + &self, + separator: &'static str, + options: O, + ) -> CssFormatSeparatedIterWithOptions<Self::Node, O> { + FormatSeparatedIter::new( + self.elements(), + separator, + CssFormatSeparatedElementRuleWithOptions::new(options), + ) + .with_trailing_separator(TrailingSeparator::Disallowed) + } +} + +impl<T, O> FormatAstSeparatedListWithOptionsExtension<O> for T where + T: AstSeparatedList<Language = CssLanguage, Node = CssIdentifier> +{ +} diff --git a/crates/biome_css_formatter/tests/specs/css/atrule/import.css.snap b/crates/biome_css_formatter/tests/specs/css/atrule/import.css.snap index 9f70710ac635..46e7360091c8 100644 --- a/crates/biome_css_formatter/tests/specs/css/atrule/import.css.snap +++ b/crates/biome_css_formatter/tests/specs/css/atrule/import.css.snap @@ -335,7 +335,7 @@ st.css"); @import url("./test.css") screen and (min-width: 400px); @import url("./test.css") layer(default) supports(display: flex) screen and (min-width: 400px); -@import url("./test.css") layer(default) supports(display: flex) +@import url("./test.css") layer(DEFAULT) supports(display: flex) screen and (min-width: 400px); @import url("./test.css") /* Comment */ layer(/* Comment */ /* Comment */ default) /* Comment */ diff --git a/crates/biome_css_formatter/tests/specs/css/atrule/layer.css b/crates/biome_css_formatter/tests/specs/css/atrule/layer.css index 1908c39a783f..4f3e69464a06 100644 --- a/crates/biome_css_formatter/tests/specs/css/atrule/layer.css +++ b/crates/biome_css_formatter/tests/specs/css/atrule/layer.css @@ -18,7 +18,7 @@ framework } @layer {} -@layer { +@layer { } @layer @@ -66,3 +66,12 @@ reset.type h1, h2 { color: maroon; } } } + +@layer reSet, MyLayer; + +@layer reSet, MyLayer { + audio[controls] { + /* specificity of 0,1,1 - explicit "reset" layer */ + display: block; + } +} diff --git a/crates/biome_css_formatter/tests/specs/css/atrule/layer.css.snap b/crates/biome_css_formatter/tests/specs/css/atrule/layer.css.snap index adf72a578f3d..d29445daae3f 100644 --- a/crates/biome_css_formatter/tests/specs/css/atrule/layer.css.snap +++ b/crates/biome_css_formatter/tests/specs/css/atrule/layer.css.snap @@ -2,7 +2,6 @@ source: crates/biome_formatter_test/src/snapshot_builder.rs info: css/atrule/layer.css --- - # Input ```css @@ -26,7 +25,7 @@ framework } @layer {} -@layer { +@layer { } @layer @@ -75,6 +74,15 @@ reset.type } } +@layer reSet, MyLayer; + +@layer reSet, MyLayer { + audio[controls] { + /* specificity of 0,1,1 - explicit "reset" layer */ + display: block; + } +} + ``` @@ -185,6 +193,13 @@ Quote style: Double Quotes } } } -``` +@layer reSet, MyLayer; +@layer reSet, MyLayer { + audio[controls] { + /* specificity of 0,1,1 - explicit "reset" layer */ + display: block; + } +} +``` diff --git a/crates/biome_css_formatter/tests/specs/css/casing.css.snap b/crates/biome_css_formatter/tests/specs/css/casing.css.snap index 1220fb0f6bb4..90f8c28c9b29 100644 --- a/crates/biome_css_formatter/tests/specs/css/casing.css.snap +++ b/crates/biome_css_formatter/tests/specs/css/casing.css.snap @@ -2,7 +2,6 @@ source: crates/biome_formatter_test/src/snapshot_builder.rs info: css/casing.css --- - # Input ```css @@ -72,8 +71,8 @@ div.classNames#AND_Ids.ArePreserved { } div { - --preserved-casing: blue; - color: var(--Preserved-Casing); + --Preserved-Casing: blue; + color: VAR(--Preserved-Casing); } @font-palette-values --AnyCASInG-works { @@ -88,5 +87,3 @@ div { @page ThisIsPreserved:first { } ``` - - diff --git a/crates/biome_css_formatter/tests/specs/css/properties/all.css b/crates/biome_css_formatter/tests/specs/css/properties/all.css index e6cc6f95f1db..b1842c3a7b3b 100644 --- a/crates/biome_css_formatter/tests/specs/css/properties/all.css +++ b/crates/biome_css_formatter/tests/specs/css/properties/all.css @@ -22,3 +22,12 @@ div { 180deg ); } + +.variable { + --myVariableName: 10px; + margin: var(--myVariableName); +} + +.function { + transform: translateX(10px) translateY(10px); +} diff --git a/crates/biome_css_formatter/tests/specs/css/properties/all.css.snap b/crates/biome_css_formatter/tests/specs/css/properties/all.css.snap index 911f821885fd..4544da2300ce 100644 --- a/crates/biome_css_formatter/tests/specs/css/properties/all.css.snap +++ b/crates/biome_css_formatter/tests/specs/css/properties/all.css.snap @@ -30,6 +30,15 @@ div { ); } +.variable { + --myVariableName: 10px; + margin: var(--myVariableName); +} + +.function { + transform: translateX(10px) translateY(10px); +} + ``` @@ -68,4 +77,13 @@ div { 180deg ); } + +.variable { + --myVariableName: 10px; + margin: var(--myVariableName); +} + +.function { + transform: translateX(10px) translateY(10px); +} ``` diff --git a/crates/biome_css_formatter/tests/specs/prettier/css/case/case.css.snap b/crates/biome_css_formatter/tests/specs/prettier/css/case/case.css.snap index d66973ad9955..1d198720302a 100644 --- a/crates/biome_css_formatter/tests/specs/prettier/css/case/case.css.snap +++ b/crates/biome_css_formatter/tests/specs/prettier/css/case/case.css.snap @@ -2,7 +2,6 @@ source: crates/biome_formatter_test/src/snapshot_builder.rs info: css/case/case.css --- - # Input ```css @@ -326,5 +325,3 @@ case.css:1:44 parse ━━━━━━━━━━━━━━━━━━━━ ``` - - diff --git a/crates/biome_css_formatter/tests/specs/prettier/css/color/color-adjuster.css.snap b/crates/biome_css_formatter/tests/specs/prettier/css/color/color-adjuster.css.snap index dfb92d0cf302..b5bc0090a51c 100644 --- a/crates/biome_css_formatter/tests/specs/prettier/css/color/color-adjuster.css.snap +++ b/crates/biome_css_formatter/tests/specs/prettier/css/color/color-adjuster.css.snap @@ -2,7 +2,6 @@ source: crates/biome_formatter_test/src/snapshot_builder.rs info: css/color/color-adjuster.css --- - # Input ```css @@ -1837,5 +1836,3 @@ color-adjuster.css:120:52 parse ━━━━━━━━━━━━━━━━ ``` 109: color: color(color-mod(0deg blue(10%)) rgb(+ 0 10 0) hue(+ 10deg) tint(10%) lightness(+ 10%) saturation(+ 10%) blend(rebeccapurple 50%)); ``` - - diff --git a/crates/biome_css_formatter/tests/specs/prettier/css/loose/loose.css.snap b/crates/biome_css_formatter/tests/specs/prettier/css/loose/loose.css.snap index 791deaf3e0aa..da4364900bb6 100644 --- a/crates/biome_css_formatter/tests/specs/prettier/css/loose/loose.css.snap +++ b/crates/biome_css_formatter/tests/specs/prettier/css/loose/loose.css.snap @@ -2,7 +2,6 @@ source: crates/biome_formatter_test/src/snapshot_builder.rs info: css/loose/loose.css --- - # Input ```css @@ -52,5 +51,3 @@ div { background-image: url() center center no-repeat black; } ``` - - diff --git a/crates/biome_css_formatter/tests/specs/prettier/css/parens/parens.css.snap b/crates/biome_css_formatter/tests/specs/prettier/css/parens/parens.css.snap index 15721ae5d595..c4dcfa331042 100644 --- a/crates/biome_css_formatter/tests/specs/prettier/css/parens/parens.css.snap +++ b/crates/biome_css_formatter/tests/specs/prettier/css/parens/parens.css.snap @@ -267,12 +267,7 @@ a { ```diff --- Prettier +++ Biome -@@ -53,14 +53,14 @@ - } - - .foo { -- --paddingC: calc(var(--widthB) / 2); -+ --paddingc: calc(var(--widthB) / 2); +@@ -57,10 +57,10 @@ content: attr(data-title); color: var(--main-bg-color); background-color: rgb(255, 0, 0); @@ -425,7 +420,7 @@ a { } .foo { - --paddingc: calc(var(--widthB) / 2); + --paddingC: calc(var(--widthB) / 2); content: attr(data-title); color: var(--main-bg-color); background-color: rgb(255, 0, 0); diff --git a/crates/biome_css_parser/src/syntax/property/mod.rs b/crates/biome_css_parser/src/syntax/property/mod.rs index abba7279ba73..467e249835fa 100644 --- a/crates/biome_css_parser/src/syntax/property/mod.rs +++ b/crates/biome_css_parser/src/syntax/property/mod.rs @@ -5,8 +5,9 @@ use crate::syntax::css_modules::{ }; use crate::syntax::parse_error::{expected_component_value, expected_identifier}; use crate::syntax::{ - is_at_any_value, is_at_identifier, is_at_string, parse_any_value, - parse_custom_identifier_with_keywords, parse_regular_identifier, parse_string, + is_at_any_value, is_at_dashed_identifier, is_at_identifier, is_at_string, parse_any_value, + parse_custom_identifier_with_keywords, parse_dashed_identifier, parse_regular_identifier, + parse_string, }; use biome_css_syntax::CssSyntaxKind::*; use biome_css_syntax::{CssSyntaxKind, T}; @@ -165,7 +166,12 @@ fn parse_generic_property(p: &mut CssParser) -> ParsedSyntax { } let m = p.start(); - parse_regular_identifier(p).ok(); + + if is_at_dashed_identifier(p) { + parse_dashed_identifier(p).ok(); + } else { + parse_regular_identifier(p).ok(); + } p.expect(T![:]);