Conversation
🦋 Changeset detectedLatest commit: b04d728 The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
bot
added
A-Project
Contributor
Parser conformance results onjs/262
jsx/babel
markdown/commonmark
symbols/microsoft
ts/babel
ts/microsoft
|
Merging this PR will not alter performance
Comparing 
Footnotes
|
ematipico
force-pushed
the
feat/use-baseline
branch
from
6526dc2 to
d1f881f
Compare
Contributor
Maybe we could have an agent kick off a pr to keep it up to date every month or so. Or at least have a standard prompt to use for the task. |
Contributor
WalkthroughAdds a nursery CSS lint rule Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
xtask/codegen/src/main.rs (1)
66-79:⚠️ Potential issue | 🟠 MajorHandle
LicenseandCssBaselinecommands gracefully whenexternal_datais disabled.When the
external_datafeature is off, the match arms exist but are empty (the function calls are#[cfg]'d out). Users can invoke these commands and get silent success with no work done—confusing UX.Either explicitly error with
bail!()as suggested, or feature-gate the enum variants themselves for compile-time consistency.Possible direction
+use anyhow::bail; TaskCommand::License => { #[cfg(feature = "external_data")] generate_license(Overwrite)?; + #[cfg(not(feature = "external_data"))] + bail!("`license` requires the `external_data` feature"); } @@ TaskCommand::CssBaseline => { #[cfg(feature = "external_data")] generate_css_baseline(Overwrite)?; + #[cfg(not(feature = "external_data"))] + bail!("`css-baseline` requires the `external_data` feature"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@xtask/codegen/src/main.rs` around lines 66 - 79, The match arms for TaskCommand::License and TaskCommand::CssBaseline are compiled empty when the external_data feature is disabled, causing silent no-ops; update handling so the commands fail fast or are unavailable: either (A) move the #[cfg(feature = "external_data")] gating to the TaskCommand enum (remove the variants when feature is off) so callers cannot invoke them at compile time, or (B) keep the variants but inside the TaskCommand::License and TaskCommand::CssBaseline arms call bail!() (or return an Err) with a clear message referencing generate_license and generate_css_baseline and stating that the external_data feature is required; pick one approach and apply consistently so TaskCommand and main.rs behavior align.
♻️ Duplicate comments (2)
xtask/codegen/src/generate_css_baseline.rs (1)
284-404:⚠️ Potential issue | 🔴 CriticalCritical:
binary_searchrequiresKNOWNto be sorted (it currently isn’t).The trailing
anchor/anchor-sizeentries make the slice unsorted, soKNOWN.binary_search(...)is unreliable and can silently drop functions from the generated baseline tables.Given this is codegen (not a hot path), the simplest fix is to ditch the sorted invariant:
Proposed patch
fn is_known_css_function(name: &str) -> bool { @@ - KNOWN.binary_search(&name).is_ok() + KNOWN.contains(&name) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@xtask/codegen/src/generate_css_baseline.rs` around lines 284 - 404, The KNOWN slice in is_known_css_function is not sorted so using KNOWN.binary_search(...) is incorrect; update is_known_css_function to use KNOWN.contains(&name) instead of binary_search (or alternatively sort the KNOWN array if you prefer to keep binary_search) so lookup is reliable; locate the is_known_css_function function and replace the binary_search call with contains.crates/biome_css_analyze/src/lint/nursery/use_baseline.rs (1)
570-608:⚠️ Potential issue | 🟠 MajorMap
CssFunctionAtRuleto"function".
CssFunctionAtRulecurrently returnsNone, preventing baseline checks and allowlist configuration for the@functionat-rule. Since"function"has a baseline entry (Limited tier) and the CSS Mixins spec defines@functionas a valid at-rule, it should map toSome("function")like other spec-compliant at-rules.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_css_analyze/src/lint/nursery/use_baseline.rs` around lines 570 - 608, The function at_rule_name currently maps AnyCssAtRule::CssFunctionAtRule to None, which prevents baseline/allowlist handling for the `@function` at-rule; update at_rule_name so that the CssFunctionAtRule arm returns Some("function") (i.e., change the match arm for AnyCssAtRule::CssFunctionAtRule from being grouped with internal/framework at-rules to return Some("function")) to enable baseline checks for "function".
🧹 Nitpick comments (3)
crates/biome_css_analyze/src/keywords.rs (1)
5756-5917:NAMED_COLORScurrently mixes colour and non-colour keywords.Line 5764 (
"auto"), Line 5866 ("none"), and CSS-wide keywords (for example Line 5823 and Line 5910) are not named colours. Keeping them here makes the exemption set broader than the constant name suggests.💡 Suggested split to keep intent explicit
+pub(crate) const CSS_WIDE_KEYWORDS: &[&str] = &[ + "inherit", + "initial", + "revert", + "revert-layer", + "unset", +]; + +pub(crate) const UNIVERSAL_NON_COLOR_KEYWORDS: &[&str] = &["auto", "none"]; + pub(crate) const NAMED_COLORS: &[&str] = &[ "aliceblue", "antiquewhite", "aqua", "aquamarine", - "auto", "azure", @@ - "inherit", - "initial", @@ - "none", @@ - "revert", - "revert-layer", @@ - "unset", ];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_css_analyze/src/keywords.rs` around lines 5756 - 5917, The NAMED_COLORS constant mixes true color names with non-color/CSS-wide keywords; split it into two explicit constants: keep only actual color token strings in NAMED_COLORS and move non-color entries (e.g., "auto", "none", CSS-wide keywords like "inherit", "initial", "unset", "revert", "revert-layer", and any other non-color tokens) into a new constant (e.g., CSS_WIDE_KEYWORDS or NON_COLOR_KEYWORDS). Update any code that referenced NAMED_COLORS for exemption checks to consult both lists where appropriate (use NAMED_COLORS for color-specific logic and the new constant for CSS-wide/non-color exemptions) and rename references if needed to preserve semantics.xtask/codegen/src/generate_css_baseline.rs (2)
186-195:update_bestshould probably also mergeyearwhen tiers tie.Right now, equal-tier entries keep the first-seen year. For year-based cut-offs, you likely want deterministic and “most permissive” behaviour:
- If both are
Newly, keep the earliest known year (min), or keepKnownoverUnknown.This is low-risk and makes the generated data more stable against upstream duplication.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@xtask/codegen/src/generate_css_baseline.rs` around lines 186 - 195, The update_best function currently replaces existing entries only when the new status has a higher tier; change its collision handling so when tiers are equal (using tier_rank/status.tier) it merges year and knowledge state deterministically: for Newly prefer the earliest year (min of status.year and existing.year), and treat Known as more permissive than Unknown (prefer Known over Unknown when one is Known), then update the existing Status accordingly; adjust the closure inside map.entry(...).and_modify(...) in update_best to perform this tie-handling/merge instead of leaving the first-seen year untouched.
561-562: Generated-file header command string looks too vague.
reformat_with_command(..., "baseline")will tell contributors to runbaseline, but the actual task seems to beCssBaseline(likelycargo xtask codegen css-baseline/just gen-css-baseline). Minor, but this text tends to live forever.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@xtask/codegen/src/generate_css_baseline.rs` around lines 561 - 562, The generated-file header uses a vague command string "baseline" which misleads contributors; update the call to xtask_glue::reformat_with_command(tokens.to_string(), "baseline") to use the precise task name used to regenerate the file (e.g. "CssBaseline" or the exact xtask/just invocation such as "cargo xtask codegen css-baseline" or "just gen-css-baseline") so the header shows the correct regeneration command; locate the call to reformat_with_command (the content variable assignment using tokens) and replace the second argument with the canonical command string used by the repository.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/biome_analyze/src/rule.rs`:
- Line 235: The Eslint CSS rule uses the wrong namespace and doc URL: update the
variant Self::EslintCss to produce the namespace "css" (not "eslint/css") and
change the documentation link constant used for EslintCss to point to the
correct GitHub path using "blob/main" and the ".md" file extension (e.g.,
.../blob/main/.../rule-name.md) so the canonical rule format and URL resolve
correctly; locate the namespace formatting logic around the Self::EslintCss
match arm and the documentation URL constant/format used for EslintCss and
adjust those strings accordingly.
In `@crates/biome_css_analyze/src/lint/nursery/use_baseline.rs`:
- Around line 405-416: The lookup fails when users provide mixed-case
keys/values because is_allowed_property_value lowercases the runtime key but
allow_property_values entries are not normalised; update UseBaselineOptions
(during deserialisation and/or its merge method) to normalise the
allow_property_values HashMap by converting all keys and values to a consistent
case (e.g., ASCII lowercase) when building the map so runtime matching in
is_allowed_property_value works case-insensitively; ensure you target the
UseBaselineOptions deserialisation/merge path that produces the
HashMap<String,String> used by is_allowed_property_value and preserve existing
semantics otherwise.
- Around line 381-495: is_inside_positive_supports is too broad: it treats any
node inside any positive `@supports` as exempt even if the `@supports` condition
doesn't test the same property/value; update is_inside_positive_supports (and
callers like check_property) to locate the nearest CssSupportsAtRule ancestor,
parse its condition AST (use CssSupportsFeatureDeclaration and
CssSupportsFeatureSelector / related nodes), extract tested property names and
values, and only return true when the supports condition explicitly tests the
current property or its value; leave behavior unchanged for negated conditions
(CssSupportsNotCondition) and fall back to false if no matching feature is
found.
In `@crates/biome_css_analyze/tests/specs/nursery/useBaseline/invalid.css`:
- Around line 39-45: The empty `@supports` selector(:has()) {} block should be
replaced with a non-empty trivial rule to avoid Stylelint's block-no-empty; edit
the `@supports` selector(:has()) { ... } block and add a harmless placeholder
declaration (for example a :root or a simple selector with a no-op custom
property or initial value) so the guard remains scoped but is not empty,
ensuring the semantics ("this guard doesn’t apply elsewhere") are preserved
without triggering lint errors.
In `@crates/biome_css_analyze/tests/specs/nursery/useBaseline/valid.css`:
- Around line 42-65: Tests use the non-existent CSS property background-filter
inside several rules (e.g., the rule blocks that read "a { accent-color: auto;
background-filter: auto }" and the nested/supports variants), which trips
property-no-unknown; update every occurrence of background-filter to the correct
backdrop-filter so the `@supports` blocks actually exercise backdrop-filter usage
(check the rules within the compound `@supports`, nested `@supports`, and the
case-insensitive `@SUPPORTS/`@SuPpOrTs blocks).
In `@crates/biome_rule_options/src/use_baseline.rs`:
- Around line 89-91: The field allow_property_values currently uses
HashMap<String, String>, which only permits a single value per CSS property;
change its type to HashMap<String, Vec<String>> so each property maps to an
array of allowed values (matching upstream ESLint's Record<string, string[]>),
keep or adjust the #[serde(skip_serializing_if = "HashMap::is_empty")] attribute
as needed, and update any code that builds, reads, or serializes/deserializes
allow_property_values to treat values as Vec<String> (constructors, defaults,
tests, and any places referencing allow_property_values must push/extend vectors
instead of assigning single strings).
In `@justfile`:
- Around line 59-61: The gen-css-baseline justfile recipe fails because
generate_css_baseline() is behind #[cfg(feature = "external_data")]; update the
recipe to enable that feature when invoking the xtask_codegen binary (i.e., run
cargo with the external_data feature enabled for package xtask_codegen) so the
feature-gated function is compiled and the css-baseline command succeeds.
In `@xtask/codegen/Cargo.toml`:
- Line 64: Make ureq an optional dependency and tie it to the existing
external_data feature, and add cfg guards around the unconditional imports.
Update Cargo.toml to declare ureq as optional and ensure the external_data
feature includes "ureq" (instead of listing ureq feature strings directly), then
add #[cfg(feature = "external_data")] before each unconditional use ureq;
statement in unicode.rs, generate_license.rs, and generate_css_baseline.rs so
the crate builds when the external_data feature is disabled.
In `@xtask/codegen/src/generate_css_baseline.rs`:
- Around line 10-11: The network fetch using WEB_FEATURES_DATA_URL currently
calls get(...).call() with no timeout and can block CI; update the HTTP client
creation to use
ureq::AgentBuilder::new().timeout(Duration::from_secs(...)).build() (or
equivalent) and replace the plain get(...) calls to use that agent so calls time
out; additionally pin or document the web-features source by changing
WEB_FEATURES_DATA_URL to a specific versioned URL (or add a comment explaining
why unpkg "latest" is acceptable) so generated output is stable over time.
---
Outside diff comments:
In `@xtask/codegen/src/main.rs`:
- Around line 66-79: The match arms for TaskCommand::License and
TaskCommand::CssBaseline are compiled empty when the external_data feature is
disabled, causing silent no-ops; update handling so the commands fail fast or
are unavailable: either (A) move the #[cfg(feature = "external_data")] gating to
the TaskCommand enum (remove the variants when feature is off) so callers cannot
invoke them at compile time, or (B) keep the variants but inside the
TaskCommand::License and TaskCommand::CssBaseline arms call bail!() (or return
an Err) with a clear message referencing generate_license and
generate_css_baseline and stating that the external_data feature is required;
pick one approach and apply consistently so TaskCommand and main.rs behavior
align.
---
Duplicate comments:
In `@crates/biome_css_analyze/src/lint/nursery/use_baseline.rs`:
- Around line 570-608: The function at_rule_name currently maps
AnyCssAtRule::CssFunctionAtRule to None, which prevents baseline/allowlist
handling for the `@function` at-rule; update at_rule_name so that the
CssFunctionAtRule arm returns Some("function") (i.e., change the match arm for
AnyCssAtRule::CssFunctionAtRule from being grouped with internal/framework
at-rules to return Some("function")) to enable baseline checks for "function".
In `@xtask/codegen/src/generate_css_baseline.rs`:
- Around line 284-404: The KNOWN slice in is_known_css_function is not sorted so
using KNOWN.binary_search(...) is incorrect; update is_known_css_function to use
KNOWN.contains(&name) instead of binary_search (or alternatively sort the KNOWN
array if you prefer to keep binary_search) so lookup is reliable; locate the
is_known_css_function function and replace the binary_search call with contains.
---
Nitpick comments:
In `@crates/biome_css_analyze/src/keywords.rs`:
- Around line 5756-5917: The NAMED_COLORS constant mixes true color names with
non-color/CSS-wide keywords; split it into two explicit constants: keep only
actual color token strings in NAMED_COLORS and move non-color entries (e.g.,
"auto", "none", CSS-wide keywords like "inherit", "initial", "unset", "revert",
"revert-layer", and any other non-color tokens) into a new constant (e.g.,
CSS_WIDE_KEYWORDS or NON_COLOR_KEYWORDS). Update any code that referenced
NAMED_COLORS for exemption checks to consult both lists where appropriate (use
NAMED_COLORS for color-specific logic and the new constant for
CSS-wide/non-color exemptions) and rename references if needed to preserve
semantics.
In `@xtask/codegen/src/generate_css_baseline.rs`:
- Around line 186-195: The update_best function currently replaces existing
entries only when the new status has a higher tier; change its collision
handling so when tiers are equal (using tier_rank/status.tier) it merges year
and knowledge state deterministically: for Newly prefer the earliest year (min
of status.year and existing.year), and treat Known as more permissive than
Unknown (prefer Known over Unknown when one is Known), then update the existing
Status accordingly; adjust the closure inside map.entry(...).and_modify(...) in
update_best to perform this tie-handling/merge instead of leaving the first-seen
year untouched.
- Around line 561-562: The generated-file header uses a vague command string
"baseline" which misleads contributors; update the call to
xtask_glue::reformat_with_command(tokens.to_string(), "baseline") to use the
precise task name used to regenerate the file (e.g. "CssBaseline" or the exact
xtask/just invocation such as "cargo xtask codegen css-baseline" or "just
gen-css-baseline") so the header shows the correct regeneration command; locate
the call to reformat_with_command (the content variable assignment using tokens)
and replace the second argument with the canonical command string used by the
repository.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4b6a9c56-1e8f-4c6f-a0da-163d532c45f9
⛔ Files ignored due to path filters (15)
Cargo.lockis excluded by!**/*.lockand included by**crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rsis excluded by!**/migrate/eslint_any_rule_to_biome.rsand included by**crates/biome_configuration/src/analyzer/linter/rules.rsis excluded by!**/rules.rsand included by**crates/biome_css_analyze/tests/specs/nursery/useBaseline/invalid.allow-lists.css.snapis excluded by!**/*.snapand included by**crates/biome_css_analyze/tests/specs/nursery/useBaseline/invalid.css.snapis excluded by!**/*.snapand included by**crates/biome_css_analyze/tests/specs/nursery/useBaseline/invalid.newly.css.snapis excluded by!**/*.snapand included by**crates/biome_css_analyze/tests/specs/nursery/useBaseline/invalid.year-2021.css.snapis excluded by!**/*.snapand included by**crates/biome_css_analyze/tests/specs/nursery/useBaseline/invalid.year-2022.css.snapis excluded by!**/*.snapand included by**crates/biome_css_analyze/tests/specs/nursery/useBaseline/valid.allow-lists.css.snapis excluded by!**/*.snapand included by**crates/biome_css_analyze/tests/specs/nursery/useBaseline/valid.css.snapis excluded by!**/*.snapand included by**crates/biome_css_analyze/tests/specs/nursery/useBaseline/valid.newly.css.snapis excluded by!**/*.snapand included by**crates/biome_css_analyze/tests/specs/nursery/useBaseline/valid.year.css.snapis excluded by!**/*.snapand included by**crates/biome_diagnostics_categories/src/categories.rsis excluded by!**/categories.rsand included by**packages/@biomejs/backend-jsonrpc/src/workspace.tsis excluded by!**/backend-jsonrpc/src/workspace.tsand included by**packages/@biomejs/biome/configuration_schema.jsonis excluded by!**/configuration_schema.jsonand included by**
📒 Files selected for processing (30)
.changeset/add-use-baseline-css-rule.mdcrates/biome_analyze/src/rule.rscrates/biome_css_analyze/Cargo.tomlcrates/biome_css_analyze/src/baseline_data.rscrates/biome_css_analyze/src/keywords.rscrates/biome_css_analyze/src/lib.rscrates/biome_css_analyze/src/lint/nursery/use_baseline.rscrates/biome_css_analyze/tests/specs/nursery/useBaseline/invalid.allow-lists.csscrates/biome_css_analyze/tests/specs/nursery/useBaseline/invalid.allow-lists.options.jsoncrates/biome_css_analyze/tests/specs/nursery/useBaseline/invalid.csscrates/biome_css_analyze/tests/specs/nursery/useBaseline/invalid.newly.csscrates/biome_css_analyze/tests/specs/nursery/useBaseline/invalid.newly.options.jsoncrates/biome_css_analyze/tests/specs/nursery/useBaseline/invalid.year-2021.csscrates/biome_css_analyze/tests/specs/nursery/useBaseline/invalid.year-2021.options.jsoncrates/biome_css_analyze/tests/specs/nursery/useBaseline/invalid.year-2022.csscrates/biome_css_analyze/tests/specs/nursery/useBaseline/invalid.year-2022.options.jsoncrates/biome_css_analyze/tests/specs/nursery/useBaseline/valid.allow-lists.csscrates/biome_css_analyze/tests/specs/nursery/useBaseline/valid.allow-lists.options.jsoncrates/biome_css_analyze/tests/specs/nursery/useBaseline/valid.csscrates/biome_css_analyze/tests/specs/nursery/useBaseline/valid.newly.csscrates/biome_css_analyze/tests/specs/nursery/useBaseline/valid.newly.options.jsoncrates/biome_css_analyze/tests/specs/nursery/useBaseline/valid.year.csscrates/biome_css_analyze/tests/specs/nursery/useBaseline/valid.year.options.jsoncrates/biome_rule_options/src/lib.rscrates/biome_rule_options/src/use_baseline.rsjustfilextask/codegen/Cargo.tomlxtask/codegen/src/generate_css_baseline.rsxtask/codegen/src/lib.rsxtask/codegen/src/main.rs
Comment on lines
+381
to
+495
| /// Check whether a syntax node is inside a positive (non-negated) `@supports` | ||
| /// condition. If so, the property/value is feature-gated and should not be | ||
| /// flagged. | ||
| fn is_inside_positive_supports(syntax: &biome_css_syntax::CssSyntaxNode) -> bool { | ||
| for anc in syntax.ancestors().skip(1) { | ||
| // If we encounter a `not` condition on the way up, it means we're | ||
| // inside `@supports not (...)` — not suppressed. | ||
| if CssSupportsNotCondition::can_cast(anc.kind()) { | ||
| return false; | ||
| } | ||
| // If we reach the top-level @supports at-rule (via its parent CssAtRule), | ||
| // we are inside a positive @supports. | ||
| if CssSupportsAtRule::can_cast(anc.kind()) { | ||
| return true; | ||
| } | ||
| } | ||
| false | ||
| } | ||
|
|
||
| fn in_allow_list(name: &TokenText, list: &[String]) -> bool { | ||
| list.iter() | ||
| .any(|item| item.eq_ignore_ascii_case(&name.to_ascii_lowercase_cow())) | ||
| } | ||
|
|
||
| fn is_allowed_property_value( | ||
| key: &TokenText, | ||
| value: &TokenText, | ||
| list: &HashMap<String, String>, | ||
| ) -> bool { | ||
| let this_value = list.get(key.to_ascii_lowercase_cow().as_ref()); | ||
| this_value.is_some_and(|this_value| { | ||
| value | ||
| .to_ascii_lowercase_cow() | ||
| .eq_ignore_ascii_case(this_value) | ||
| }) | ||
| } | ||
|
|
||
| fn is_named_color(name: &str) -> bool { | ||
| NAMED_COLORS.binary_search(&name).is_ok() | ||
| } | ||
|
|
||
| fn check_property( | ||
| prop: &CssGenericProperty, | ||
| options: &UseBaselineOptions, | ||
| ) -> Option<UseBaselineState> { | ||
| use biome_css_syntax::AnyCssValue; | ||
|
|
||
| // If inside a positive @supports, the feature is gated — skip | ||
| if is_inside_positive_supports(prop.syntax()) { | ||
| return None; | ||
| } | ||
|
|
||
| // Check the property name itself | ||
| let name_node = prop.name().ok()?; | ||
| let prop_name = name_node.as_css_identifier()?.value_token().ok()?; | ||
| let prop_text = prop_name.token_text_trimmed(); | ||
|
|
||
| // Skip custom properties (--foo) | ||
| if prop_text.text().starts_with("--") { | ||
| return None; | ||
| } | ||
|
|
||
| // Skip if in allow list | ||
| if in_allow_list(&prop_text, options.allow_properties.as_slice()) { | ||
| return None; | ||
| } | ||
|
|
||
| // Check property name baseline | ||
| if let Some(status) = find_baseline( | ||
| &BASELINE_PROPERTIES, | ||
| &prop_text.text().to_ascii_lowercase_cow(), | ||
| ) && !is_acceptable(options, status.tier, status.year) | ||
| { | ||
| return Some(UseBaselineState { | ||
| feature_name: prop_text.into(), | ||
| category: "property", | ||
| range: prop_name.text_trimmed_range(), | ||
| }); | ||
| } | ||
| // If not in our data, assume it's fine (unknown = not checked) | ||
|
|
||
| // Check property values (identifier keywords only) | ||
| for component in prop.value() { | ||
| if let Some(value) = component.as_any_css_value() | ||
| && let AnyCssValue::CssIdentifier(ident) = value | ||
| { | ||
| let tok = ident.value_token().ok()?; | ||
| let val_text = tok.token_text_trimmed(); | ||
|
|
||
| // Skip named colors | ||
| if is_named_color(&val_text.to_ascii_lowercase_cow()) { | ||
| continue; | ||
| } | ||
|
|
||
| // Skip if in property value allow list | ||
| if is_allowed_property_value(&prop_text, &val_text, &options.allow_property_values) { | ||
| continue; | ||
| } | ||
|
|
||
| if let Some(status) = find_property_value_baseline( | ||
| &prop_text.to_ascii_lowercase_cow(), | ||
| &val_text.to_ascii_lowercase_cow(), | ||
| ) && !is_acceptable(options, status.tier, status.year) | ||
| { | ||
| return Some(UseBaselineState { | ||
| feature_name: (prop_text, val_text).into(), | ||
| category: "property value", | ||
| range: tok.text_trimmed_range(), | ||
| }); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| None | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1. Check the full implementation of use_baseline.rs to understand the current logic
wc -l crates/biome_css_analyze/src/lint/nursery/use_baseline.rsRepository: biomejs/biome
Length of output: 118
🏁 Script executed:
#!/bin/bash
# 2. Look at the CST node types related to `@supports`
rg -n "CssSupports" crates/biome_css_syntax/src --type rust | head -30Repository: biomejs/biome
Length of output: 3636
🏁 Script executed:
#!/bin/bash
# 3. Check the test fixtures for use_baseline
find crates/biome_css_analyze/tests -name "*use_baseline*" -type fRepository: biomejs/biome
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Read the specific lines from use_baseline.rs
sed -n '381,495p' crates/biome_css_analyze/src/lint/nursery/use_baseline.rsRepository: biomejs/biome
Length of output: 3811
🏁 Script executed:
#!/bin/bash
# Search for test fixtures more broadly
find crates/biome_css_analyze/tests -name "*.rs" -o -name "*.md" | head -20Repository: biomejs/biome
Length of output: 146
🏁 Script executed:
#!/bin/bash
# Check the structure of CssSupportsFeatureDeclaration and related nodes
rg -A10 "pub struct CssSupportsFeatureDeclaration" crates/biome_css_syntax/src/generated/nodes.rs
rg -A10 "pub struct CssSupportsFeatureSelector" crates/biome_css_syntax/src/generated/nodes.rsRepository: biomejs/biome
Length of output: 1782
🏁 Script executed:
#!/bin/bash
# Check the spec test structure for use_baseline
find crates/biome_css_analyze/tests -type d -name "specs" | head -5Repository: biomejs/biome
Length of output: 93
🏁 Script executed:
#!/bin/bash
# Find test spec files for use_baseline
find crates/biome_css_analyze/tests/specs -name "*use_baseline*" -o -name "*baseline*" 2>/dev/null | head -10Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Get a broader view of test structure
ls -la crates/biome_css_analyze/tests/specs/Repository: biomejs/biome
Length of output: 570
🏁 Script executed:
#!/bin/bash
# Check nursery specs
ls -la crates/biome_css_analyze/tests/specs/nursery/Repository: biomejs/biome
Length of output: 427
🏁 Script executed:
#!/bin/bash
# Find use_baseline test file
find crates/biome_css_analyze/tests/specs/nursery -name "*use_baseline*" -o -name "*baseline*"Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Check the useBaseline test directory
ls -la crates/biome_css_analyze/tests/specs/nursery/useBaseline/Repository: biomejs/biome
Length of output: 1990
🏁 Script executed:
#!/bin/bash
# Read the test spec file for useBaseline
find crates/biome_css_analyze/tests/specs/nursery/useBaseline -name "*.md" -o -name "*.json"Repository: biomejs/biome
Length of output: 657
🏁 Script executed:
#!/bin/bash
# Read the main valid.css test file
cat crates/biome_css_analyze/tests/specs/nursery/useBaseline/valid.cssRepository: biomejs/biome
Length of output: 2689
🏁 Script executed:
#!/bin/bash
# Read the main invalid.css test file
cat crates/biome_css_analyze/tests/specs/nursery/useBaseline/invalid.cssRepository: biomejs/biome
Length of output: 1657
🏁 Script executed:
#!/bin/bash
# Check the snapshot files to see what diagnostics are expected
head -100 crates/biome_css_analyze/tests/specs/nursery/useBaseline/valid.css.snapRepository: biomejs/biome
Length of output: 2862
🏁 Script executed:
#!/bin/bash
# Check the invalid snapshot to see what diagnostics are expected
head -150 crates/biome_css_analyze/tests/specs/nursery/useBaseline/invalid.css.snapRepository: biomejs/biome
Length of output: 5398
🏁 Script executed:
#!/bin/bash
# Check what specific diagnostics are expected from the test files
grep -c "lint/nursery/useBaseline" crates/biome_css_analyze/tests/specs/nursery/useBaseline/invalid.css.snapRepository: biomejs/biome
Length of output: 59
🏁 Script executed:
#!/bin/bash
# Get the full invalid snapshot to see all expected diagnostics
cat crates/biome_css_analyze/tests/specs/nursery/useBaseline/invalid.css.snap | tail -200Repository: biomejs/biome
Length of output: 8102
🏁 Script executed:
#!/bin/bash
# Check if there are other functions that check `@supports` logic
rg -n "is_inside_positive_supports|CssSupportsAtRule|CssSupportsNotCondition" crates/biome_css_analyze/src/lint/nursery/use_baseline.rsRepository: biomejs/biome
Length of output: 487
🏁 Script executed:
#!/bin/bash
# Check the full use_baseline.rs to see all the logic around `@supports`
head -380 crates/biome_css_analyze/src/lint/nursery/use_baseline.rs | tail -100Repository: biomejs/biome
Length of output: 3492
🏁 Script executed:
#!/bin/bash
# Read more of the implementation to see if property values and functions are checked separately
sed -n '1,380p' crates/biome_css_analyze/src/lint/nursery/use_baseline.rs | grep -A 50 "pub fn rule"Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Look at the rule implementation structure
rg -n "impl UseBaseline|pub fn rule" crates/biome_css_analyze/src/lint/nursery/use_baseline.rs -A 30 | head -80Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Search for the declare_lint_rule macro for UseBaseline
rg -n "declare_lint_rule" crates/biome_css_analyze/src/lint/nursery/use_baseline.rs -A 20Repository: biomejs/biome
Length of output: 1992
🏁 Script executed:
#!/bin/bash
# Get the beginning of the file to see the rule declaration
head -150 crates/biome_css_analyze/src/lint/nursery/use_baseline.rsRepository: biomejs/biome
Length of output: 4693
🏁 Script executed:
#!/bin/bash
# Check if there are multiple rule checking handlers or just one
rg -n "impl Rule for UseBaseline|fn check_" crates/biome_css_analyze/src/lint/nursery/use_baseline.rsRepository: biomejs/biome
Length of output: 366
🏁 Script executed:
#!/bin/bash
# Read around where other node types are checked (at-rules, functions, etc)
sed -n '500,635p' crates/biome_css_analyze/src/lint/nursery/use_baseline.rsRepository: biomejs/biome
Length of output: 4873
🏁 Script executed:
#!/bin/bash
# Read the Rule implementation
sed -n '224,300p' crates/biome_css_analyze/src/lint/nursery/use_baseline.rsRepository: biomejs/biome
Length of output: 3030
The @supports exemption is too broad for properties alone—consider matching against the actual condition.
The concern is valid: is_inside_positive_supports() exempts ANY property name inside ANY positive @supports block without checking whether the condition actually tests that feature. However, this applies only to properties; functions, pseudo-classes, pseudo-elements, and media conditions are still flagged inside @supports blocks, as shown by the test fixtures (e.g., @supports (accent-color: auto) { accent-color: abs(...) } flags the function).
The current approach is permissive by design (per the rule docs: "Code inside @supports blocks is exempt"), but it could be more selective. A property should only be exempted if the nearest @supports condition actually tests that property or its value—not just any ancestor @supports. The test case at line 24 of invalid.css (@supports (clip-path: fill-box) { clip-path: stroke-box }) shows this gap: the condition guards fill-box, but stroke-box is still flagged because property values are checked separately.
If you want stricter matching, you'd need to extract features from the @supports condition (via CssSupportsFeatureDeclaration and CssSupportsFeatureSelector) and only suppress matching uses. That's more complex but more precise.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/biome_css_analyze/src/lint/nursery/use_baseline.rs` around lines 381 -
495, is_inside_positive_supports is too broad: it treats any node inside any
positive `@supports` as exempt even if the `@supports` condition doesn't test the
same property/value; update is_inside_positive_supports (and callers like
check_property) to locate the nearest CssSupportsAtRule ancestor, parse its
condition AST (use CssSupportsFeatureDeclaration and CssSupportsFeatureSelector
/ related nodes), extract tested property names and values, and only return true
when the supports condition explicitly tests the current property or its value;
leave behavior unchanged for negated conditions (CssSupportsNotCondition) and
fall back to false if no matching feature is found.
| "pulldown-cmark", | ||
| ] | ||
| license = ["serde", "serde_json", "ureq/default", "ureq/json"] | ||
| external_data = ["serde", "serde_json", "ureq/default", "ureq/json"] |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check ureq declaration and feature wiring
echo "=== Checking ureq dependency and external_data feature ==="
rg -n "^\s*ureq\s*=|^external_data\s*=" xtask/codegen/Cargo.toml -A3 -B2
# Check if ureq is used in source and whether it's feature-gated
echo -e "\n=== Checking ureq usage in source ==="
rg -n "ureq::|use ureq|cfg\(feature\s*=\s*\"external_data\"\)" xtask/codegen/src --max-count 20Repository: biomejs/biome
Length of output: 1201
ureq is non-optional but needs to be tied to external_data — and worse, it's imported unconditionally in three source files.
The dependency is always pulled in even when external_data is disabled. Additionally, ureq is imported without feature guards in unicode.rs, generate_license.rs, and generate_css_baseline.rs, which means disabling the feature breaks the build. Make ureq optional and gate it properly:
Suggested refactor
-ureq = "3.1.4"
+ureq = { version = "3.1.4", optional = true, default-features = false }
-external_data = ["serde", "serde_json", "ureq/default", "ureq/json"]
+external_data = ["dep:serde", "dep:serde_json", "dep:ureq", "ureq/json"]You'll also need to add #[cfg(feature = "external_data")] guards to the unconditional use ureq imports.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@xtask/codegen/Cargo.toml` at line 64, Make ureq an optional dependency and
tie it to the existing external_data feature, and add cfg guards around the
unconditional imports. Update Cargo.toml to declare ureq as optional and ensure
the external_data feature includes "ureq" (instead of listing ureq feature
strings directly), then add #[cfg(feature = "external_data")] before each
unconditional use ureq; statement in unicode.rs, generate_license.rs, and
generate_css_baseline.rs so the crate builds when the external_data feature is
disabled.
Comment on lines
+10
to
+11
| const WEB_FEATURES_DATA_URL: &str = "https://unpkg.com/web-features/data.json"; | ||
|
|
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the file structure
fd "generate_css_baseline.rs" --type fRepository: biomejs/biome
Length of output: 99
🏁 Script executed:
# Check the content at the specified lines
head -50 xtask/codegen/src/generate_css_baseline.rsRepository: biomejs/biome
Length of output: 1343
🏁 Script executed:
# Also check lines 532-565 to see the other location mentioned
wc -l xtask/codegen/src/generate_css_baseline.rsRepository: biomejs/biome
Length of output: 103
🌐 Web query:
ureq 3.1.4 HTTP request timeout configuration best practices
💡 Result:
What ureq 3.1.4 gives you (timeouts, defaults, precedence)
In ureq 3.x you can configure timeouts on the Agent (recommended for consistency) and, depending on API, sometimes per-request.
Key knobs on AgentBuilder:
timeout_connect: socket connect timeout. Default: 30s. If you set bothtimeout_connectandtimeout,timeout_connectwins for the connect phase. [1]timeout_read/timeout_write: per-socket-operation read/write timeouts. Default: none (so reads/writes can block forever). If you also settimeout,timeouttakes precedence over these. [1]timeout: overall request timeout including DNS resolution, connection time, redirects, and reading the response body; note that slow DNS may still make you exceed the timeout because DNS can’t be interrupted with available APIs. [1]
ureq also has a more granular timeout model (documented in 3.2.0) that splits timeouts by phase (resolve, connect, send_request, recv_response, recv_body, plus global and redirect-related per_call). This is useful as a mental model even if you stick to the AgentBuilder methods. [2]
Best-practice configuration patterns
-
Always set a finite “overall” timeout
- Use
AgentBuilder::timeout(...)as a hard upper bound so a request can’t hang forever (especially important because default read/write timeouts are “none”). [1]
- Use
-
Set a shorter connect timeout than your overall timeout
- Connect failures should usually be detected quickly (e.g., 250ms–2s in low-latency environments; higher if you expect cold networks/VPN/mobile).
- In ureq,
timeout_connectis also the one that can overridetimeoutfor the connect phase, so set it intentionally. [1]
-
Decide whether you need per-read/per-write timeouts
- If you have an overall
timeout, you may not needtimeout_read/write. - If you stream large bodies or want “stall detection” (fail if no progress), add
timeout_read(and possiblytimeout_write) to cap individual socket operations—just remembertimeout()overrides them when both are set. [1]
- If you have an overall
-
Account for redirects explicitly
- Because the overall timeout includes redirects, a low global timeout can fail on redirect-heavy flows. [1]
- If you adopt the granular model,
per_callexists specifically for “call-by-call when following redirects,” which can help avoid a single redirect hop consuming the whole budget. [2]
-
Be aware of DNS behavior
- ureq notes DNS may not be interruptible, so avoid extremely tight global budgets if you rely on system DNS and occasionally see slow resolution; alternatively use a custom resolver where appropriate. [1]
-
Consider upgrading if you see timeout edge cases
- The ureq changelog indicates fixes around
timeout_readbehavior landed after earlier 3.x versions; if timeout correctness is critical, validate on your exact version or upgrade. [3]
- The ureq changelog indicates fixes around
Example (sane defaults for many API clients)
use std::time::Duration;
let agent = ureq::builder()
.timeout_connect(Duration::from_secs(2)) // fail fast on connect
.timeout(Duration::from_secs(10)) // hard cap for whole request
.build();Sources: AgentBuilder timeout docs and defaults [1]; granular per-phase timeout model (3.2.0 docs) [2]; changelog notes about timeout fixes [3].
References:
[1] ureq AgentBuilder docs (timeout/timeout_connect/timeout_read/timeout_write semantics, defaults, precedence, DNS note). (world.pages.gitlab.gnome.org)
[2] ureq config::Timeouts (phase-specific timeout breakdown, redirect per_call). (docs.rs)
[3] ureq changelog (timeout-related fixes in later 3.x). (docs.rs)
Citations:
- 1: https://world.pages.gitlab.gnome.org/Health/ureq/struct.AgentBuilder.html
- 2: https://docs.rs/ureq/latest/ureq/config/struct.Timeouts.html
- 3: https://docs.rs/crate/ureq/latest/source/CHANGELOG.md?utm_source=openai
🏁 Script executed:
# Get the network request code section (lines 532-565)
sed -n '532,565p' xtask/codegen/src/generate_css_baseline.rsRepository: biomejs/biome
Length of output: 1203
🏁 Script executed:
# Also check around line 10 context and any other network calls
grep -n "ureq\|get\|timeout\|request" xtask/codegen/src/generate_css_baseline.rsRepository: biomejs/biome
Length of output: 381
🏁 Script executed:
# Check Cargo.toml to see ureq version
fd "Cargo.toml" xtask/codegen --exec grep -l ureq {} \;Repository: biomejs/biome
Length of output: 81
🏁 Script executed:
# Check ureq version in xtask/codegen/Cargo.toml
cat xtask/codegen/Cargo.toml | grep -A2 -B2 ureqRepository: biomejs/biome
Length of output: 389
🏁 Script executed:
# Let's also verify if there are any other network calls in the file or if this is the only one
wc -l xtask/codegen/src/generate_css_baseline.rs && tail -50 xtask/codegen/src/generate_css_baseline.rsRepository: biomejs/biome
Length of output: 1717
Add explicit HTTP timeouts (and consider pinning the web-features source).
Network requests without timeouts can hang CI/dev workflows. The code uses get(WEB_FEATURES_DATA_URL).call() without any timeout configuration; ureq 3.1.4 requires explicit setup (e.g. via AgentBuilder::timeout()) to prevent indefinite blocking. Also, unpkg.com/web-features/data.json is a "latest" endpoint, so output can drift over time—pin the version or document the intent.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@xtask/codegen/src/generate_css_baseline.rs` around lines 10 - 11, The network
fetch using WEB_FEATURES_DATA_URL currently calls get(...).call() with no
timeout and can block CI; update the HTTP client creation to use
ureq::AgentBuilder::new().timeout(Duration::from_secs(...)).build() (or
equivalent) and replace the plain get(...) calls to use that agent so calls time
out; additionally pin or document the web-features source by changing
WEB_FEATURES_DATA_URL to a specific versioned URL (or add a comment explaining
why unpkg "latest" is acceptable) so generated output is stable over time.
ematipico
force-pushed
the
feat/use-baseline
branch
from
72cdbf2 to
d2e9a72
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
crates/biome_css_analyze/src/lint/nursery/use_baseline.rs (1)
385-399:⚠️ Potential issue | 🟠 Major
@supports not (...)blocks are likely being treated as positive supports.Line 389 relies on ancestor scanning for
CssSupportsNotCondition, but declarations in the block body usually won’t have that condition node as an ancestor. In practice, Line 395 may returntruefor anyCssSupportsAtRule, suppressing diagnostics in@supports notfallback blocks.#!/bin/bash set -euo pipefail # 1) Inspect AST node APIs for supports at-rules and not-conditions. rg -n "pub struct CssSupportsAtRule|impl CssSupportsAtRule|fn prelude\\(|fn condition\\(|fn block\\(" crates/biome_css_syntax/src/generated/nodes.rs -A20 -B2 rg -n "pub struct CssSupportsNotCondition|impl CssSupportsNotCondition" crates/biome_css_syntax/src/generated/nodes.rs -A20 -B2 # 2) Re-open the current suppression logic. rg -n "fn is_inside_positive_supports|CssSupportsNotCondition|CssSupportsAtRule" crates/biome_css_analyze/src/lint/nursery/use_baseline.rs -C5 # 3) Check if there is explicit fixture coverage for `@supports` not. rg -n "@supports\\s+not" crates/biome_css_analyze/tests/specs/nursery/useBaseline -C2Expected verification outcome: if
CssSupportsNotConditionis only part of the prelude subtree and fixtures lack@supports notcoverage, this branch is a real false-negative risk.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_css_analyze/src/lint/nursery/use_baseline.rs` around lines 385 - 399, The current is_inside_positive_supports function incorrectly assumes a CssSupportsNotCondition will appear among ancestors of a declaration; instead locate the enclosing CssSupportsAtRule (use CssSupportsAtRule::can_cast while walking ancestors to find the at-rule node) then inspect its prelude/condition subtree (use CssSupportsAtRule::prelude or condition accessor and search that subtree) for any CssSupportsNotCondition nodes; update is_inside_positive_supports to return false if the at-rule's condition subtree contains a CssSupportsNotCondition and only return true when the at-rule exists and its condition subtree has no not-condition.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.changeset/add-use-baseline-css-rule.md:
- Line 5: Change the wording in the changelog so it says the rule "reports
diagnostics" instead of "warns" and add a brief note that the rule does not set
an explicit severity in declare_lint_rule!, so it defaults to
Severity::Information (RuleMetadata::default()). Update the sentence describing
the new nursery lint rule `useBaseline` to reflect that it reports diagnostics
for CSS properties, values, at-rules, media conditions, functions, or
pseudo-selectors not in the configured Baseline tier and mention the default
severity behavior via `declare_lint_rule!` / `RuleMetadata::default()`.
In
`@crates/biome_css_analyze/tests/specs/nursery/useBaseline/invalid.allow-lists.css`:
- Around line 16-17: The test case is masked by the `@supports` wrapper so the
intended rejection of "clip-path: stroke-box" never runs; update the fixture in
invalid.allow-lists.css so the property is checked outside a positive `@supports`
block (e.g., move "a { clip-path: stroke-box; }" out of "@supports (clip-path:
fill-box) { ... }" or change the supports condition to a negative/irrelevant
one) so the check in use_baseline.rs (the short-circuit at the property-checking
path) will exercise the negative allowPropertyValues case.
---
Duplicate comments:
In `@crates/biome_css_analyze/src/lint/nursery/use_baseline.rs`:
- Around line 385-399: The current is_inside_positive_supports function
incorrectly assumes a CssSupportsNotCondition will appear among ancestors of a
declaration; instead locate the enclosing CssSupportsAtRule (use
CssSupportsAtRule::can_cast while walking ancestors to find the at-rule node)
then inspect its prelude/condition subtree (use CssSupportsAtRule::prelude or
condition accessor and search that subtree) for any CssSupportsNotCondition
nodes; update is_inside_positive_supports to return false if the at-rule's
condition subtree contains a CssSupportsNotCondition and only return true when
the at-rule exists and its condition subtree has no not-condition.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 760af6cd-ac08-40a4-b921-c5f9f8a67e20
⛔ Files ignored due to path filters (13)
Cargo.lockis excluded by!**/*.lockand included by**crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rsis excluded by!**/migrate/eslint_any_rule_to_biome.rsand included by**crates/biome_configuration/src/analyzer/linter/rules.rsis excluded by!**/rules.rsand included by**crates/biome_css_analyze/tests/specs/nursery/useBaseline/invalid.allow-lists.css.snapis excluded by!**/*.snapand included by**crates/biome_css_analyze/tests/specs/nursery/useBaseline/invalid.css.snapis excluded by!**/*.snapand included by**crates/biome_css_analyze/tests/specs/nursery/useBaseline/invalid.newly.css.snapis excluded by!**/*.snapand included by**crates/biome_css_analyze/tests/specs/nursery/useBaseline/invalid.year-2021.css.snapis excluded by!**/*.snapand included by**crates/biome_css_analyze/tests/specs/nursery/useBaseline/invalid.year-2022.css.snapis excluded by!**/*.snapand included by**crates/biome_css_analyze/tests/specs/nursery/useBaseline/valid.allow-lists.css.snapis excluded by!**/*.snapand included by**crates/biome_css_analyze/tests/specs/nursery/useBaseline/valid.css.snapis excluded by!**/*.snapand included by**crates/biome_css_analyze/tests/specs/nursery/useBaseline/valid.newly.css.snapis excluded by!**/*.snapand included by**crates/biome_css_analyze/tests/specs/nursery/useBaseline/valid.year.css.snapis excluded by!**/*.snapand included by**crates/biome_diagnostics_categories/src/categories.rsis excluded by!**/categories.rsand included by**
📒 Files selected for processing (23)
.changeset/add-use-baseline-css-rule.mdcrates/biome_analyze/src/rule.rscrates/biome_css_analyze/Cargo.tomlcrates/biome_css_analyze/src/baseline_data.rscrates/biome_css_analyze/src/keywords.rscrates/biome_css_analyze/src/lib.rscrates/biome_css_analyze/src/lint/nursery/use_baseline.rscrates/biome_css_analyze/tests/specs/nursery/useBaseline/invalid.allow-lists.csscrates/biome_css_analyze/tests/specs/nursery/useBaseline/invalid.allow-lists.options.jsoncrates/biome_css_analyze/tests/specs/nursery/useBaseline/invalid.csscrates/biome_css_analyze/tests/specs/nursery/useBaseline/invalid.newly.csscrates/biome_css_analyze/tests/specs/nursery/useBaseline/invalid.newly.options.jsoncrates/biome_css_analyze/tests/specs/nursery/useBaseline/invalid.year-2021.csscrates/biome_css_analyze/tests/specs/nursery/useBaseline/invalid.year-2021.options.jsoncrates/biome_css_analyze/tests/specs/nursery/useBaseline/invalid.year-2022.csscrates/biome_css_analyze/tests/specs/nursery/useBaseline/invalid.year-2022.options.jsoncrates/biome_css_analyze/tests/specs/nursery/useBaseline/valid.allow-lists.csscrates/biome_css_analyze/tests/specs/nursery/useBaseline/valid.allow-lists.options.jsoncrates/biome_css_analyze/tests/specs/nursery/useBaseline/valid.csscrates/biome_css_analyze/tests/specs/nursery/useBaseline/valid.newly.csscrates/biome_css_analyze/tests/specs/nursery/useBaseline/valid.newly.options.jsoncrates/biome_css_analyze/tests/specs/nursery/useBaseline/valid.year.csscrates/biome_css_analyze/tests/specs/nursery/useBaseline/valid.year.options.json
✅ Files skipped from review due to trivial changes (1)
- crates/biome_css_analyze/Cargo.toml
🚧 Files skipped from review as they are similar to previous changes (11)
- crates/biome_css_analyze/tests/specs/nursery/useBaseline/valid.newly.options.json
- crates/biome_css_analyze/tests/specs/nursery/useBaseline/invalid.year-2022.options.json
- crates/biome_css_analyze/tests/specs/nursery/useBaseline/invalid.year-2021.options.json
- crates/biome_css_analyze/src/lib.rs
- crates/biome_css_analyze/tests/specs/nursery/useBaseline/valid.allow-lists.options.json
- crates/biome_analyze/src/rule.rs
- crates/biome_css_analyze/tests/specs/nursery/useBaseline/invalid.allow-lists.options.json
- crates/biome_css_analyze/tests/specs/nursery/useBaseline/invalid.newly.options.json
- crates/biome_css_analyze/src/keywords.rs
- crates/biome_css_analyze/tests/specs/nursery/useBaseline/invalid.year-2021.css
- crates/biome_css_analyze/tests/specs/nursery/useBaseline/invalid.year-2022.css
crates/biome_css_analyze/tests/specs/nursery/useBaseline/invalid.allow-lists.css
Outdated
Show resolved
Hide resolved
github-actions
bot
added
the
L-JavaScript
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
crates/biome_css_analyze/src/lint/nursery/use_baseline.rs (2)
623-627: Minor: same redundant conversion asin_allow_list.
namehere is&'static strfromat_rule_name, andeq_ignore_ascii_casedoesn't require pre-lowercasing.♻️ Suggested simplification
if options .allow_at_rules .iter() - .any(|item| item.eq_ignore_ascii_case(&name.to_ascii_lowercase_cow())) + .any(|item| item.eq_ignore_ascii_case(name)) { return None; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_css_analyze/src/lint/nursery/use_baseline.rs` around lines 623 - 627, The condition pre-lowercases `name` unnecessarily before calling `eq_ignore_ascii_case`, causing redundant conversion; update the check in the block that iterates `options.allow_at_rules` to call `eq_ignore_ascii_case` directly with `name` (e.g., `item.eq_ignore_ascii_case(name)`) instead of `item.eq_ignore_ascii_case(&name.to_ascii_lowercase_cow())`, and mirror the same simplification used in `in_allow_list` so no extra allocation or lowercase conversion occurs.
401-404: Minor: redundant case conversion.
eq_ignore_ascii_casealready handles case-insensitive comparison, so theto_ascii_lowercase_cow()call is unnecessary overhead.♻️ Suggested simplification
fn in_allow_list(name: &TokenText, list: &[String]) -> bool { list.iter() - .any(|item| item.eq_ignore_ascii_case(&name.to_ascii_lowercase_cow())) + .any(|item| item.eq_ignore_ascii_case(name.text())) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_css_analyze/src/lint/nursery/use_baseline.rs` around lines 401 - 404, The helper in_allow_list currently lowercases the TokenText before calling eq_ignore_ascii_case, which is redundant; update in_allow_list (and any call sites using to_ascii_lowercase_cow) to call eq_ignore_ascii_case directly with the original TokenText (i.e., replace item.eq_ignore_ascii_case(&name.to_ascii_lowercase_cow()) with item.eq_ignore_ascii_case(&name) or item.eq_ignore_ascii_case(name) as appropriate), removing the to_ascii_lowercase_cow() call to avoid unnecessary allocation and overhead.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/biome_js_analyze/src/lint/nursery/no_redundant_default_export.rs`:
- Line 18: The rustdoc summary contains a copy-paste gremlin and an unclosed
backtick: replace the broken sentence in the doc comment with a correct
description for this rule (e.g., "This rule reports the nursery lint rule
`no_redundant_default_export` when a `default` export references the same
identifier as a named export."), close all backticks, and remove the stray
"useBase" token so the rustdoc for no_redundant_default_export.rs and the rule
name `no_redundant_default_export` render correctly.
---
Nitpick comments:
In `@crates/biome_css_analyze/src/lint/nursery/use_baseline.rs`:
- Around line 623-627: The condition pre-lowercases `name` unnecessarily before
calling `eq_ignore_ascii_case`, causing redundant conversion; update the check
in the block that iterates `options.allow_at_rules` to call
`eq_ignore_ascii_case` directly with `name` (e.g.,
`item.eq_ignore_ascii_case(name)`) instead of
`item.eq_ignore_ascii_case(&name.to_ascii_lowercase_cow())`, and mirror the same
simplification used in `in_allow_list` so no extra allocation or lowercase
conversion occurs.
- Around line 401-404: The helper in_allow_list currently lowercases the
TokenText before calling eq_ignore_ascii_case, which is redundant; update
in_allow_list (and any call sites using to_ascii_lowercase_cow) to call
eq_ignore_ascii_case directly with the original TokenText (i.e., replace
item.eq_ignore_ascii_case(&name.to_ascii_lowercase_cow()) with
item.eq_ignore_ascii_case(&name) or item.eq_ignore_ascii_case(name) as
appropriate), removing the to_ascii_lowercase_cow() call to avoid unnecessary
allocation and overhead.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ef7693f1-cb84-4664-b753-8206a21381f4
⛔ Files ignored due to path filters (1)
crates/biome_css_analyze/tests/specs/nursery/useBaseline/invalid.allow-lists.css.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (4)
.changeset/add-use-baseline-css-rule.mdcrates/biome_css_analyze/src/lint/nursery/use_baseline.rscrates/biome_css_analyze/tests/specs/nursery/useBaseline/invalid.allow-lists.csscrates/biome_js_analyze/src/lint/nursery/no_redundant_default_export.rs
| /// Checks if a default export exports the same symbol as a named export. | ||
| /// | ||
| /// This rule warns when a `default` export references the same identifier as a named export. | ||
| /// This rule reports new nursery lint rule `useBase when a `default` export references the same identifier as a named export. |
Contributor
There was a problem hiding this comment.
Fix the rustdoc sentence (copy-paste gremlin + broken backtick).
Line 18 mentions useBase and has an unclosed inline code span, so the rule docs are misleading and render oddly.
Suggested patch
- /// This rule reports new nursery lint rule `useBase when a `default` export references the same identifier as a named export.
+ /// This rule reports when a `default` export references the same identifier as a named export.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/biome_js_analyze/src/lint/nursery/no_redundant_default_export.rs` at
line 18, The rustdoc summary contains a copy-paste gremlin and an unclosed
backtick: replace the broken sentence in the doc comment with a correct
description for this rule (e.g., "This rule reports the nursery lint rule
`no_redundant_default_export` when a `default` export references the same
identifier as a named export."), close all backticks, and remove the stray
"useBase" token so the rustdoc for no_redundant_default_export.rs and the rule
name `no_redundant_default_export` render correctly.
dyc3
approved these changes
Mar 6, 2026
| // CSS named colors are CssIdentifier tokens in property values. | ||
| // We exclude them from property-value baseline checks because they're | ||
| // universally supported regardless of baseline data. | ||
| pub(crate) const NAMED_COLORS: &[&str] = &[ |
Contributor
There was a problem hiding this comment.
I guess it wasn't worth it to make this a set?
Member
Author
There was a problem hiding this comment.
Argh, I missed it. Good catch
Member
Author
There was a problem hiding this comment.
So, looking at where NAMED_COLORS is actually placed, I think we should keep it as an array for now because all const inside the file are arrays.
If we want to use phf, I think we can send a PR to change all of them
chansuke
approved these changes
Mar 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR implements the rule
useBaseline, inspired by github.com/eslint/css/blob/main/docs/rules/use-baseline.mdI used AI to help me generate the codegen and implement the rule. However, the logic of the rule was a bit sloppy and unoptimized, so I made a few changes.
I added all the options from ESLint. Considering how new the rule is, I thought it made sense. However, I changed how the property values are handled. Instead of a
key:valuelist, I went for an object instead.Test Plan
Added various tests. There's a small problem with this rule: every time we update the metadata, there's a risk that tests need to be updated. Same for the docs.
Docs
Added in the rule