feat(linter): implement noInlineStyles HTML lint rule#9319
feat(linter): implement noInlineStyles HTML lint rule#9319Beltranss wants to merge 1 commit intobiomejs:mainfrom
noInlineStyles HTML lint rule#9319Conversation
Closes biomejs#9062. Added a new nursery lint rule `noInlineStyles` that disallows the use of the `style` attribute on HTML elements. Inline styles are discouraged because they mix content with presentation, reduce maintainability, and can prevent effective use of a strict Content Security Policy. The rule provides an unsafe auto-fix that removes the `style` attribute.
|
WalkthroughThis pull request introduces a new HTML linter rule Suggested labels
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🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/biome_rule_options/src/no_inline_styles.rs (1)
3-6: Add rustdoc to the options type.Tiny one, but this struct is public and should be documented like other rule options.
As per coding guidelines, "Use inline rustdoc documentation for rules, assists, and their options".Proposed tweak
#[derive(Default, Clone, Debug, Deserialize, Deserializable, Merge, Eq, PartialEq, Serialize)] #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] #[serde(rename_all = "camelCase", deny_unknown_fields, default)] +/// Options for the `noInlineStyles` rule. +/// +/// This rule currently has no configurable options. pub struct NoInlineStylesOptions {}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_rule_options/src/no_inline_styles.rs` around lines 3 - 6, Add a one- or two-line rustdoc comment to the public options struct NoInlineStylesOptions explaining its purpose and usage (e.g., what the option controls and any default behavior) so it matches the project's inline documentation standard; place the /// doc comment immediately above the pub struct NoInlineStylesOptions declaration and keep the wording concise and descriptive consistent with other rule options' docs.
🤖 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_html_analyze/src/lint/nursery/no_inline_styles.rs`:
- Around line 103-107: The is_style_attribute function currently compares the
attribute name with a case-sensitive "style", which misses variants like "STYLE"
or "Style"; update the comparison inside is_style_attribute (working with
HtmlAttribute and name.value_token().text_trimmed()) to use a case-insensitive
comparison (e.g., call eq_ignore_ascii_case on the trimmed text) so the rule
correctly detects style attributes regardless of case, matching the approach
used in no_autofocus.rs and use_scoped_styles.rs.
---
Nitpick comments:
In `@crates/biome_rule_options/src/no_inline_styles.rs`:
- Around line 3-6: Add a one- or two-line rustdoc comment to the public options
struct NoInlineStylesOptions explaining its purpose and usage (e.g., what the
option controls and any default behavior) so it matches the project's inline
documentation standard; place the /// doc comment immediately above the pub
struct NoInlineStylesOptions declaration and keep the wording concise and
descriptive consistent with other rule options' docs.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
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_diagnostics_categories/src/categories.rsis excluded by!**/categories.rsand included by**crates/biome_html_analyze/tests/specs/nursery/noInlineStyles/invalid.html.snapis excluded by!**/*.snapand included by**crates/biome_html_analyze/tests/specs/nursery/noInlineStyles/valid.html.snapis excluded by!**/*.snapand 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 (5)
crates/biome_html_analyze/src/lint/nursery/no_inline_styles.rscrates/biome_html_analyze/tests/specs/nursery/noInlineStyles/invalid.htmlcrates/biome_html_analyze/tests/specs/nursery/noInlineStyles/valid.htmlcrates/biome_rule_options/src/lib.rscrates/biome_rule_options/src/no_inline_styles.rs
| fn is_style_attribute(node: &HtmlAttribute) -> bool { | ||
| node.name().is_ok_and(|name| { | ||
| name.value_token() | ||
| .is_ok_and(|value_token| value_token.text_trimmed() == "style") | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Read the source file to see the actual code
cat -n crates/biome_html_analyze/src/lint/nursery/no_inline_styles.rs | head -120 | tail -25Repository: biomejs/biome
Length of output: 1128
🏁 Script executed:
# Check test fixtures for coverage of uppercase variants
ls -la crates/biome_html_analyze/tests/specs/nursery/noInlineStyles/ 2>/dev/null || echo "Path not found"Repository: biomejs/biome
Length of output: 428
🏁 Script executed:
# Search for test fixtures
find crates/biome_html_analyze/tests -name "*noInlineStyles*" -o -name "*no_inline_styles*" 2>/dev/nullRepository: biomejs/biome
Length of output: 117
🏁 Script executed:
# Read the test fixtures to see what cases are covered
cat crates/biome_html_analyze/tests/specs/nursery/noInlineStyles/invalid.htmlRepository: biomejs/biome
Length of output: 318
🏁 Script executed:
# Read the valid fixtures too
cat crates/biome_html_analyze/tests/specs/nursery/noInlineStyles/valid.htmlRepository: biomejs/biome
Length of output: 335
🏁 Script executed:
# Check the rule metadata to see if fix_kind is present
head -80 crates/biome_html_analyze/src/lint/nursery/no_inline_styles.rsRepository: biomejs/biome
Length of output: 2682
🏁 Script executed:
# Check if HTML attribute names are case-sensitive in HTML spec
# Also check if the parser normalizes them
rg -n "HtmlAttribute|lowercase|case" crates/biome_html_analyze/src/lint/nursery/no_inline_styles.rs -A2 -B2Repository: biomejs/biome
Length of output: 565
🏁 Script executed:
# Check how the HTML parser tokenizes attribute names
# Look for any case normalization in the HTML syntax/parsing layer
rg -n "text_trimmed|lowercase|to_lowercase|eq_ignore" crates/biome_html_syntax/ -i --type rs | head -20Repository: biomejs/biome
Length of output: 83
🏁 Script executed:
# Check if there's any documentation or tests about attribute name handling
rg -n "style.*attribute|attribute.*case" crates/biome_html_analyze/tests/ -i -A2 -B2Repository: biomejs/biome
Length of output: 29043
🏁 Script executed:
# Search for how HtmlAttribute name is extracted and used elsewhere
rg -n "HtmlAttribute.*name\(\)|\.name\(\)" crates/biome_html_analyze/src/ -A3 | head -40Repository: biomejs/biome
Length of output: 3546
🏁 Script executed:
# Check the HTML spec or parser behavior by looking at other attribute checks
rg -n "value_token\(\).*text_trimmed" crates/biome_html_analyze/src/ -B3 -A3Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
# Verify how the HTML parser/tokenizer handles attribute names
# Check if there's any normalization happening before reaching is_style_attribute
find crates/biome_html -name "*.rs" -type f | xargs rg -l "HtmlAttribute" | head -5Repository: biomejs/biome
Length of output: 313
🏁 Script executed:
# Check if there are any comments or documentation about case sensitivity
rg -n "case.sensitive|case.insensitive|uppercase|lowercase" crates/biome_html_analyze/src/lint/nursery/no_inline_styles.rsRepository: biomejs/biome
Length of output: 39
🏁 Script executed:
# Create a test to verify the actual behaviour
cat > /tmp/test_case_sensitivity.html << 'EOF'
<!-- lowercase - should be detected -->
<div style="color: red;"></div>
<!-- uppercase - will this be detected? -->
<div STYLE="color: red;"></div>
<!-- mixed case -->
<div Style="color: red;"></div>
EOFRepository: biomejs/biome
Length of output: 39
🏁 Script executed:
# Find the HTML syntax crate
find crates -name "*html*" -type d | head -10Repository: biomejs/biome
Length of output: 502
🏁 Script executed:
# Check how attribute names are parsed - look at the actual parser
rg -n "HtmlAttribute\|attribute.*name" crates/biome_html_analyze/src/ -l | head -5Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
# Look at other rules that check attribute names to see if they use case-insensitive comparison
rg -n "text_trimmed.*==" crates/biome_html_analyze/src/lint/ -B2 -A2 | head -40Repository: biomejs/biome
Length of output: 3932
🏁 Script executed:
# Check if HTML spec defines attributes as case-insensitive
# Let's look at how autofocus and other attributes are checked
rg -n "eq_ignore_ascii_case" crates/biome_html_analyze/src/lint/ -B3 -A1Repository: biomejs/biome
Length of output: 13461
🏁 Script executed:
# Quick confirmation: check that use_scoped_styles also handles style attribute/element names with case-insensitivity
rg -n "use_scoped_styles" crates/biome_html_analyze/src/lint/nursery/ -lRepository: biomejs/biome
Length of output: 120
🏁 Script executed:
# Verify no_duplicate_classes uses case-insensitive check (earlier results showed it uses ==)
head -80 crates/biome_html_analyze/src/assist/source/no_duplicate_classes.rs | tail -15Repository: biomejs/biome
Length of output: 569
Use case-insensitive comparison for style attribute.
Line 106 uses strict equality to "style", which misses uppercase or mixed-case variants like STYLE or Style. HTML attributes are case-insensitive, so the rule can be bypassed. Align with the pattern used elsewhere in the codebase (e.g. no_autofocus.rs, use_scoped_styles.rs) by using eq_ignore_ascii_case:
Proposed fix
fn is_style_attribute(node: &HtmlAttribute) -> bool {
node.name().is_ok_and(|name| {
name.value_token()
- .is_ok_and(|value_token| value_token.text_trimmed() == "style")
+ .is_ok_and(|value_token| value_token.text_trimmed().eq_ignore_ascii_case("style"))
})
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fn is_style_attribute(node: &HtmlAttribute) -> bool { | |
| node.name().is_ok_and(|name| { | |
| name.value_token() | |
| .is_ok_and(|value_token| value_token.text_trimmed() == "style") | |
| }) | |
| fn is_style_attribute(node: &HtmlAttribute) -> bool { | |
| node.name().is_ok_and(|name| { | |
| name.value_token() | |
| .is_ok_and(|value_token| value_token.text_trimmed().eq_ignore_ascii_case("style")) | |
| }) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/biome_html_analyze/src/lint/nursery/no_inline_styles.rs` around lines
103 - 107, The is_style_attribute function currently compares the attribute name
with a case-sensitive "style", which misses variants like "STYLE" or "Style";
update the comparison inside is_style_attribute (working with HtmlAttribute and
name.value_token().text_trimmed()) to use a case-insensitive comparison (e.g.,
call eq_ignore_ascii_case on the trimmed text) so the rule correctly detects
style attributes regardless of case, matching the approach used in
no_autofocus.rs and use_scoped_styles.rs.
dyc3
left a comment
There was a problem hiding this comment.
We should add a js rule that detects this in jsx as well!
| fn is_style_attribute(node: &HtmlAttribute) -> bool { | ||
| node.name().is_ok_and(|name| { | ||
| name.value_token() | ||
| .is_ok_and(|value_token| value_token.text_trimmed() == "style") | ||
| }) |
There was a problem hiding this comment.
nit: tbh, i would just inline this instead of having another function for it
Merging this PR will not alter performance
Comparing Footnotes
|
ematipico
left a comment
There was a problem hiding this comment.
The PR isn't ready. It doesn't handle components, which means components with style attribute are triggered.
Please implement that and tests
|
Duplicate of #9146, but this implementation seems better. However, this PR does not seem to handle JSX, while the other does |
|
Yeah but both didn't address out comments yet |
Summary
Closes #9062.
Implements the
noInlineStylesnursery lint rule for HTML that disallows the use of thestyleattribute on HTML elements. This is a port of the html-eslintno-inline-stylesrule.Inline styles are discouraged because they:
The rule provides an unsafe auto-fix that removes the
styleattribute.Example
AI Assistance Disclosure
This PR was written primarily with the assistance of Claude Code (Claude Opus). The implementation, tests, documentation, and commit message were all generated with AI assistance, reviewed and approved by the contributor.
Test plan
invalid.htmltest with 5 cases covering various elements with inline stylesvalid.htmltest with cases using classes, IDs, and<style>blockscargo test -p biome_html_analyze)just gen-analyzer)🤖 Generated with Claude Code