feat(css_analyze): add an ignore option to noUnknownFunction, noUnknownProperty, noUnknownPseudoClass & noUnknownPseudoElement#8814
Conversation
🦋 Changeset detectedLatest commit: 92bcf47 The changes in this PR will be included in the next version bump. This PR includes changesets to release 15 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 |
WalkthroughAdds a case-insensitive Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@crates/biome_rule_options/src/no_unknown_function.rs`:
- Around line 6-9: The doc comment on the NoUnknownFunctionOptions::ignore field
incorrectly mentions "at-rule names" — update the comment to refer to unknown
CSS function names (case-insensitive) instead; locate the struct
NoUnknownFunctionOptions and change the documentation above the pub ignore:
Vec<String> field to something like "A list of unknown function names to ignore
(case-insensitive)." and keep the existing serde attribute and field name
unchanged.
In `@crates/biome_rule_options/src/no_unknown_property.rs`:
- Around line 6-9: Update the doc comment on NoUnknownPropertyOptions::ignore to
refer to CSS properties (not at‑rules); change the wording to something like "A
list of unknown CSS property names to ignore (case-insensitive)." Keep the serde
attribute #[serde(skip_serializing_if = "Vec::is_empty")] and the field name
ignore unchanged.
In `@crates/biome_rule_options/src/no_unknown_pseudo_class.rs`:
- Around line 6-9: Update the doc comment on the NoUnknownPseudoClassOptions
struct (and specifically the ignore field) to reference unknown pseudo-class
names instead of at‑rules; keep the existing details (that the list is
case-insensitive and that serde should skip serializing when empty) but correct
the wording to say "unknown pseudo‑class names to ignore (case-insensitive)" so
the documentation matches the option's purpose.
In `@crates/biome_rule_options/src/no_unknown_pseudo_element.rs`:
- Around line 6-10: The doc comment on the NoUnknownPseudoElementOptions struct
incorrectly refers to "at-rule names"; update the comment for the struct field
ignore to accurately describe that it is "A list of unknown pseudo-element names
to ignore (case-insensitive)." Keep the serde attribute and field name ignore
unchanged; only adjust the documentation text to reference pseudo-elements and
preserve the note about case-insensitivity.
🧹 Nitpick comments (1)
crates/biome_css_analyze/src/lint/correctness/no_unknown_property.rs (1)
145-152: Consider using iterator.any()for a more idiomatic approach.The current implementation works correctly and is consistent with the other
should_ignoreimplementations across the codebase. However, this could be slightly more idiomatic:♻️ Optional refactor
fn should_ignore(name: &str, options: &NoUnknownPropertyOptions) -> bool { - for ignore_pattern in &options.ignore { - if name.eq_ignore_ascii_case(ignore_pattern) { - return true; - } - } - false + options + .ignore + .iter() + .any(|pattern| name.eq_ignore_ascii_case(pattern)) }If you adopt this, you might want to apply it consistently to the other three
should_ignoreimplementations inno_unknown_function.rs,no_unknown_pseudo_class.rs, andno_unknown_pseudo_element.rsas well.
CodSpeed Performance ReportMerging this PR will degrade performance by 16.22%Comparing Summary
Performance Changes
Footnotes
|
Netail
force-pushed
the
feat/ignore-list
branch
from
c747239 to
ed919f0
Compare
Netail
changed the title
noUnknownFunction, noUnknownProperty, noUnknownPseudoClass & noUnknownPseudoElement
ematipico
left a comment
ematipico
left a comment
There was a problem hiding this comment.
These aren't nursery rules, which means you're adding a feature. So it's a minor.
Also, technically, we should have a changeset for each rule. If they were four PRs, you would create a changeset for each pr
| /// It doesn't trigger the rule if the pseudo-element name isn't a vendor prefix or is a pseudo-element | ||
| fn should_not_trigger(pseudo_element_name: &str) -> bool { | ||
| fn should_not_trigger(pseudo_element_name: &str, options: &NoUnknownPseudoElementOptions) -> bool { | ||
| let lowercase = pseudo_element_name.to_ascii_lowercase_cow().to_string(); |
There was a problem hiding this comment.
Why are we allocating a string now?
Into the next branch? |
|
That too :) |
Netail
force-pushed
the
feat/ignore-list
branch
from
03ddd64 to
8b374ff
Compare
github-actions
bot
added
A-CLI
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.changeset/rich-flowers-hunt.md:
- Line 5: The changeset contains a documentation URL referencing the incorrect
rule slug `no-unknown-selector-pseudo-element`; update that URL and any mention
in this changeset to use the correct kebab-case slug `no-unknown-pseudo-element`
(e.g., change the link to
https://biomejs.dev/linter/rules/no-unknown-pseudo-element) so it matches the
configuration schema, backend, and CHANGELOG references.
🧹 Nitpick comments (1)
crates/biome_css_analyze/src/lint/correctness/no_unknown_pseudo_element.rs (1)
155-162: Consider using iterator style.A minor stylistic preference - the loop could be simplified:
♻️ Optional refactor using iterators
fn should_ignore(name: &str, options: &NoUnknownPseudoElementOptions) -> bool { - for ignore_pattern in &options.ignore { - if name.eq_ignore_ascii_case(ignore_pattern) { - return true; - } - } - false + options + .ignore + .iter() + .any(|pattern| name.eq_ignore_ascii_case(pattern)) }
| "@biomejs/biome": minor | ||
| --- | ||
|
|
||
| Added `ignore` option to [`noUnknownPseudoElement`](https://biomejs.dev/linter/rules/no-unknown-selector-pseudo-element). If an unknown pseudo-element name matches any of the items provided in `ignore`, a diagnostic won't be emitted. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for references to this rule and its URL patterns
echo "=== Searching for noUnknownPseudoElement references ==="
rg -i "noUnknownPseudoElement|no-unknown.*pseudo" --type md --type json --type ts --type js | head -20
echo -e "\n=== Checking changeset files for URL patterns ==="
find .changeset -name "*.md" -type f -exec grep -H "biomejs.dev" {} \; | head -10
echo -e "\n=== Checking if rule slug exists in codebase ==="
rg "no-unknown-pseudo-element|no-unknown-selector-pseudo" --type md --type json | head -10Repository: biomejs/biome
Length of output: 5955
Fix documentation URL in changeset.
Change no-unknown-selector-pseudo-element to no-unknown-pseudo-element — all current references in the codebase (configuration schema, backend, CHANGELOG) use the correct kebab-case slug without the redundant "selector" prefix.
🤖 Prompt for AI Agents
In @.changeset/rich-flowers-hunt.md at line 5, The changeset contains a
documentation URL referencing the incorrect rule slug
`no-unknown-selector-pseudo-element`; update that URL and any mention in this
changeset to use the correct kebab-case slug `no-unknown-pseudo-element` (e.g.,
change the link to https://biomejs.dev/linter/rules/no-unknown-pseudo-element)
so it matches the configuration schema, backend, and CHANGELOG references.
Netail
force-pushed
the
feat/ignore-list
branch
from
0c54560 to
8ab36e5
Compare
github-actions
bot
removed
A-Parser
Netail
removed
A-CLI
|
@Netail you should have acknowledged the benchmarks before merging... Or wait for me 😂 Please double check next time |
Ahh sorry, my apologies. Didn't knew that. Thought with the approval you already acknowledged the benchmarks😅 |
Well, you still merged a PR with a red workflow :D |
Ahh it needs to be approved through a link in the Codespeed comment, thought merging it when it's red will mark it as acknowledged for Codespeed too. That the failing workflow only indicated a lower benchmark |
2 participants
Summary
Add an ignore list to
noUnknownFunction,noUnknownProperty,noUnknownPseudoClass&noUnknownPseudoElementCloses #7625 & #5554
Test Plan
Add appropriate unit tests
Docs