Skip to content

Commit

Permalink
fix(css_formatter): fix CSS formatter converts variable declarations …
Browse files Browse the repository at this point in the history
…and function calls to lowercase #3068 (#3130)
  • Loading branch information
denbezrukov authored Jun 8, 2024
1 parent 2f4997b commit 3e53475
Show file tree
Hide file tree
Showing 25 changed files with 258 additions and 121 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 2 additions & 2 deletions crates/biome_css_formatter/src/comments.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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 {
Expand Down
24 changes: 16 additions & 8 deletions crates/biome_css_formatter/src/css/auxiliary/function.rs
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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()
])]
)
}
}
10 changes: 9 additions & 1 deletion crates/biome_css_formatter/src/css/lists/layer_name_list.rs
Original file line number Diff line number Diff line change
@@ -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_options(
".",
FormatCssIdentifierOptions::default().prevent_lowercase(true),
))
.finish()
}
}
33 changes: 30 additions & 3 deletions crates/biome_css_formatter/src/css/value/identifier.rs
Original file line number Diff line number Diff line change
@@ -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
Expand Down
77 changes: 73 additions & 4 deletions crates/biome_css_formatter/src/separated.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -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,
Expand Down Expand Up @@ -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_options(
&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>
{
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down
11 changes: 10 additions & 1 deletion crates/biome_css_formatter/tests/specs/css/atrule/layer.css
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ framework
}

@layer {}
@layer {
@layer {
}

@layer
Expand Down Expand Up @@ -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;
}
}
21 changes: 18 additions & 3 deletions crates/biome_css_formatter/tests/specs/css/atrule/layer.css.snap
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
source: crates/biome_formatter_test/src/snapshot_builder.rs
info: css/atrule/layer.css
---

# Input

```css
Expand All @@ -26,7 +25,7 @@ framework
}

@layer {}
@layer {
@layer {
}

@layer
Expand Down Expand Up @@ -75,6 +74,15 @@ reset.type
}
}

@layer reSet, MyLayer;

@layer reSet, MyLayer {
audio[controls] {
/* specificity of 0,1,1 - explicit "reset" layer */
display: block;
}
}

```


Expand Down Expand Up @@ -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;
}
}
```
7 changes: 2 additions & 5 deletions crates/biome_css_formatter/tests/specs/css/casing.css.snap
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
source: crates/biome_formatter_test/src/snapshot_builder.rs
info: css/casing.css
---

# Input

```css
Expand Down Expand Up @@ -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 {
Expand All @@ -88,5 +87,3 @@ div {
@page ThisIsPreserved:first {
}
```


9 changes: 9 additions & 0 deletions crates/biome_css_formatter/tests/specs/css/properties/all.css
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,12 @@ div {
180deg
);
}

.variable {
--myVariableName: 10px;
margin: var(--myVariableName);
}

.function {
transform: translateX(10px) translateY(10px);
}
18 changes: 18 additions & 0 deletions crates/biome_css_formatter/tests/specs/css/properties/all.css.snap
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,15 @@ div {
);
}

.variable {
--myVariableName: 10px;
margin: var(--myVariableName);
}

.function {
transform: translateX(10px) translateY(10px);
}

```


Expand Down Expand Up @@ -68,4 +77,13 @@ div {
180deg
);
}

.variable {
--myVariableName: 10px;
margin: var(--myVariableName);
}

.function {
transform: translateX(10px) translateY(10px);
}
```
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
source: crates/biome_formatter_test/src/snapshot_builder.rs
info: css/case/case.css
---

# Input

```css
Expand Down Expand Up @@ -326,5 +325,3 @@ case.css:1:44 parse ━━━━━━━━━━━━━━━━━━━━
```


Loading

0 comments on commit 3e53475

Please sign in to comment.