feat(lint): add NoInlineStyles nursery rule#9146
feat(lint): add NoInlineStyles nursery rule#9146mcmxcdev wants to merge 2 commits intobiomejs:mainfrom
Conversation
🦋 Changeset detectedLatest commit: d5b4b48 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
| "@biomejs/biome": patch | ||
| --- | ||
|
|
||
| Added the nursery rule [`noInlineStyles`](https://biomejs.dev/linter/rules/no-inline-styles/). The rule disallows the use of inline `style` attributes in HTML and the `style` prop in JSX, including `React.createElement` calls. Inline styles make code harder to maintain and can interfere with Content Security Policy. |
There was a problem hiding this comment.
I have added the rule for HTML and JSX only so far since those are the languages we would like to have covered at Dune. Should this already be extended to Vue, Svelte and Astro? Or in a follow up?
There was a problem hiding this comment.
I am also not sure if this PR is properly adding the rule in the correct way across languages like HTML and JSX.
There was a problem hiding this comment.
Should this already be extended to Vue, Svelte and Astro?
It should just work because you've already implemented it for HTML, but you can add snapshot tests for those to verify.
I am also not sure if this PR is properly adding the rule in the correct way across languages like HTML and JSX.
As long as the rule has the same name in js_analyze and html_analyze, you should be good.
WalkthroughThis pull request introduces a new nursery linter rule 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 Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
crates/biome_html_analyze/src/lint/nursery/no_inline_styles.rs (1)
84-86: Diagnostic note is slightly inconsistent with the JSX counterpart.The HTML rule says
"Use a CSS class instead."while the JS rule (seecrates/biome_js_analyze/src/lint/nursery/no_inline_styles.rsline 144) says"Use a CSS class or a styling library instead."— worth aligning for a consistent user experience.📝 Suggested fix
- "Use a CSS class instead." + "Use a CSS class or a styling library instead."🤖 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 84 - 86, Update the diagnostic note in the no_inline_styles lint so it matches the JS rule: replace the .note(markup! { "Use a CSS class instead." }) in crates/biome_html_analyze/src/lint/nursery/no_inline_styles.rs with .note(markup! { "Use a CSS class or a styling library instead." }) so the message is consistent with the JS counterpart (see the JS rule in crates/biome_js_analyze/src/lint/nursery/no_inline_styles.rs).crates/biome_rule_options/src/no_inline_styles.rs (1)
3-6: Add a doc comment toNoInlineStylesOptions.The struct is missing rustdoc, which is expected even for empty options types — it's the doc surface users see in generated references.
📝 Suggested addition
+/// Options for the [`noInlineStyles`](https://biomejs.dev/linter/rules/no-inline-styles/) rule. #[derive(Default, Clone, Debug, Deserialize, Deserializable, Merge, Eq, PartialEq, Serialize)]As per coding guidelines: "Use inline rustdoc documentation for rules, assists, and their options."
🤖 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 an inline rustdoc comment to the NoInlineStylesOptions struct explaining its purpose and that it is an options type (even if empty) for the "no inline styles" rule; place the doc comment immediately above the pub struct NoInlineStylesOptions {} and include a short one- or two-sentence description suitable for generated docs (mention it contains no configurable fields if applicable) so rustdoc and generated references show the rule's options surface..changeset/add-no-inline-styles-rule.md (1)
5-5: Add an invalid code example to the changeset.The description covers the what and why nicely, but is missing a concrete invalid snippet — required per project guidelines.
📝 Suggested addition
Added the nursery rule [`noInlineStyles`](https://biomejs.dev/linter/rules/no-inline-styles/). The rule disallows the use of inline `style` attributes in HTML and the `style` prop in JSX, including `React.createElement` calls. Inline styles make code harder to maintain and can interfere with Content Security Policy. + +**Invalid examples:** +```html +<div style="color: red;"></div> +``` +```jsx +<div style={{ color: "red" }}>Error</div>; +```Based on learnings: "For new lint rules in changesets, show an example of invalid case in inline code or code block."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.changeset/add-no-inline-styles-rule.md at line 5, The changeset for the new nursery rule noInlineStyles is missing a required invalid code example — add concrete invalid snippets (both HTML and JSX) inside the changeset content: include an HTML example like a div with a style attribute (e.g., <div style="color: red;"></div>) and a JSX example using the style prop (e.g., <div style={{ color: "red" }}>Error</div>), formatted as an inline code block or fenced code block so reviewers and docs clearly show the invalid cases for the noInlineStyles rule.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.changeset/add-no-inline-styles-rule.md:
- Line 5: The changeset for the new nursery rule noInlineStyles is missing a
required invalid code example — add concrete invalid snippets (both HTML and
JSX) inside the changeset content: include an HTML example like a div with a
style attribute (e.g., <div style="color: red;"></div>) and a JSX example using
the style prop (e.g., <div style={{ color: "red" }}>Error</div>), formatted as
an inline code block or fenced code block so reviewers and docs clearly show the
invalid cases for the noInlineStyles rule.
In `@crates/biome_html_analyze/src/lint/nursery/no_inline_styles.rs`:
- Around line 84-86: Update the diagnostic note in the no_inline_styles lint so
it matches the JS rule: replace the .note(markup! { "Use a CSS class instead."
}) in crates/biome_html_analyze/src/lint/nursery/no_inline_styles.rs with
.note(markup! { "Use a CSS class or a styling library instead." }) so the
message is consistent with the JS counterpart (see the JS rule in
crates/biome_js_analyze/src/lint/nursery/no_inline_styles.rs).
In `@crates/biome_rule_options/src/no_inline_styles.rs`:
- Around line 3-6: Add an inline rustdoc comment to the NoInlineStylesOptions
struct explaining its purpose and that it is an options type (even if empty) for
the "no inline styles" rule; place the doc comment immediately above the pub
struct NoInlineStylesOptions {} and include a short one- or two-sentence
description suitable for generated docs (mention it contains no configurable
fields if applicable) so rustdoc and generated references show the rule's
options surface.
dyc3
left a comment
dyc3
left a comment
There was a problem hiding this comment.
A very nice first draft! Mostly nitpicks.
| } | ||
|
|
||
| impl Rule for NoInlineStyles { | ||
| type Query = Semantic<AnyJsElementWithStyle>; |
There was a problem hiding this comment.
I don't know why you would need the semantic model for this. It should just be a simple ban on the style attribute
| sources: &[ | ||
| RuleSource::HtmlEslint("no-inline-styles").inspired(), | ||
| ], | ||
| domains: &[RuleDomain::React], |
There was a problem hiding this comment.
not sure i would put this in the react domain
| @@ -0,0 +1,15 @@ | |||
| // Invalid cases - should trigger the rule | |||
There was a problem hiding this comment.
this should be our magic comment /* should generate diagnostics */
| @@ -0,0 +1,13 @@ | |||
| <!-- Invalid cases - should trigger the rule --> | |||
There was a problem hiding this comment.
this should be our magic comment <!-- should generate diagnostics -->
| @@ -0,0 +1,11 @@ | |||
| <!-- Valid cases - should NOT trigger the rule --> | |||
There was a problem hiding this comment.
this should be our magic comment <!-- should not generate diagnostics -->
| /// | ||
| /// Inline styles are specified using the `style` attribute directly on an element. | ||
| /// They make code harder to maintain and override, prevent reusability of styling, and | ||
| /// can be a security concern when implementing a strict Content Security Policy (CSP). |
There was a problem hiding this comment.
At the bottom of the docs, or here, we should have a link to CSP. Possibly from MDN website
| let name = attribute.name().ok()?; | ||
| let name_token = name.value_token().ok()?; | ||
|
|
||
| if name_token.text_trimmed() == "style" { |
There was a problem hiding this comment.
Attributes are case insensitive. Use to_lowercase_cow and add some tests for different variants of style e.g. STYLE
| use biome_rule_options::no_inline_styles::NoInlineStylesOptions; | ||
|
|
||
| declare_lint_rule! { | ||
| /// Disallow the use of inline styles in JSX. |
There was a problem hiding this comment.
| /// Disallow the use of inline styles in JSX. | |
| /// Disallow the use of inline styles. |
Redundant
| /// ### Invalid | ||
| /// | ||
| /// ```jsx,expect_diagnostic | ||
| /// <div style={{ color: "red" }}>Error</div> |
There was a problem hiding this comment.
| /// <div style={{ color: "red" }}>Error</div> | |
| /// <div style="color: red">Error</div> |
It seems useless to cover two examples that show the same style attribute
| } | ||
| } | ||
| AnyJsElementWithStyle::JsCallExpression(call_expression) => { | ||
| if let Some(react_create_element) = |
There was a problem hiding this comment.
The code is full if let Ok and let Some. Use ok() with the try operator. It will make the code easier to read
| 13 │ React.createElement("div", { style: { color: "red" } }); | ||
| 14 │ | ||
| > 15 │ React.createElement("button", { style: { background: "blue" } }, "Click"); | ||
| │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ |
| Some( | ||
| RuleDiagnostic::new( | ||
| rule_category!(), | ||
| state.range(), |
| type Signals = Option<Self::State>; | ||
| type Options = NoInlineStylesOptions; | ||
|
|
||
| fn run(ctx: &RuleContext<Self>) -> Self::Signals { |
There was a problem hiding this comment.
The JSX rule needs more tests and logic. We shouldn't trigger the rule for Components.
@mcmxcdev did you copy the tests from the eslint rule? If not, I ask you to do so, don't let the agent take over because it doesn't know anything
Merging this PR will not alter performance
Comparing 
Footnotes
|
3 participants
Summary
Added the nursery rule
NoInlineStyleswhich disallows the usage ofstyleprop in HTML and JSX files.Closes #9062
Test Plan
styleprop in use in HTML and JSXclass(HTML),className(JSX)data-style,idand other propsDocs
Documentation is included as rustdoc examples in the rule implementation with
expect_diagnosticannotations.