-
-
Notifications
You must be signed in to change notification settings - Fork 918
feat(lint): add NoInlineStyles nursery rule #9146
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@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. | ||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,89 @@ | ||
| use biome_analyze::{ | ||
| Ast, Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule, | ||
| }; | ||
| use biome_console::markup; | ||
| use biome_html_syntax::HtmlAttribute; | ||
| use biome_rowan::AstNode; | ||
| use biome_rule_options::no_inline_styles::NoInlineStylesOptions; | ||
|
|
||
| declare_lint_rule! { | ||
| /// Disallow the use of inline styles on elements. | ||
| /// | ||
| /// 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). | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. At the bottom of the docs, or here, we should have a link to CSP. Possibly from MDN website |
||
| /// | ||
| /// Instead of inline styles, use CSS classes defined in external stylesheets or | ||
| /// `<style>` blocks. This promotes separation of concerns and makes styles easier | ||
| /// to manage, reuse, and override. | ||
| /// | ||
| /// ## Examples | ||
| /// | ||
| /// ### Invalid | ||
| /// | ||
| /// ```html,expect_diagnostic | ||
| /// <div style="color: red;"></div> | ||
| /// ``` | ||
| /// | ||
| /// ```html,expect_diagnostic | ||
| /// <button style="background: blue; color: white;">Click</button> | ||
| /// ``` | ||
| /// | ||
| /// ### Valid | ||
| /// | ||
| /// ```html | ||
| /// <div class="text-red"></div> | ||
| /// ``` | ||
| /// | ||
| /// ```html | ||
| /// <button class="btn btn-primary">Click</button> | ||
| /// ``` | ||
| /// | ||
| pub NoInlineStyles { | ||
| version: "next", | ||
| name: "noInlineStyles", | ||
| language: "html", | ||
| sources: &[ | ||
| RuleSource::HtmlEslint("no-inline-styles").same(), | ||
| ], | ||
| recommended: false, | ||
| } | ||
| } | ||
|
|
||
| impl Rule for NoInlineStyles { | ||
| type Query = Ast<HtmlAttribute>; | ||
| type State = (); | ||
| type Signals = Option<Self::State>; | ||
| type Options = NoInlineStylesOptions; | ||
|
|
||
| fn run(ctx: &RuleContext<Self>) -> Self::Signals { | ||
| let attribute = ctx.query(); | ||
| let name = attribute.name().ok()?; | ||
| let name_token = name.value_token().ok()?; | ||
|
|
||
| if name_token.text_trimmed() == "style" { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Attributes are case insensitive. Use |
||
| return Some(()); | ||
| } | ||
|
|
||
| None | ||
| } | ||
|
|
||
| fn diagnostic(ctx: &RuleContext<Self>, _state: &Self::State) -> Option<RuleDiagnostic> { | ||
| let attribute = ctx.query(); | ||
| Some( | ||
| RuleDiagnostic::new( | ||
| rule_category!(), | ||
| attribute.range(), | ||
| markup! { | ||
| "Avoid using the "<Emphasis>"style"</Emphasis>" attribute." | ||
| }, | ||
| ) | ||
| .note(markup! { | ||
| "Inline styles make code harder to maintain, override, and can interfere with Content Security Policy." | ||
| }) | ||
| .note(markup! { | ||
| "Use a CSS class instead." | ||
| }), | ||
| ) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| <!-- Invalid cases - should trigger the rule --> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this should be our magic comment |
||
|
|
||
| <div style="color: red;"></div> | ||
|
|
||
| <button style="background: blue; color: white;">Click</button> | ||
|
|
||
| <p style="margin: 0; padding: 10px;">Paragraph</p> | ||
|
|
||
| <span style="">Empty inline style</span> | ||
|
|
||
| <img style="width: 100px;" src="image.png" alt="Image"> | ||
|
|
||
| <a style="text-decoration: none;" href="#">Link</a> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,147 @@ | ||
| --- | ||
| source: crates/biome_html_analyze/tests/spec_tests.rs | ||
| expression: invalid.html | ||
| --- | ||
| # Input | ||
| ```html | ||
| <!-- Invalid cases - should trigger the rule --> | ||
|
|
||
| <div style="color: red;"></div> | ||
|
|
||
| <button style="background: blue; color: white;">Click</button> | ||
|
|
||
| <p style="margin: 0; padding: 10px;">Paragraph</p> | ||
|
|
||
| <span style="">Empty inline style</span> | ||
|
|
||
| <img style="width: 100px;" src="image.png" alt="Image"> | ||
|
|
||
| <a style="text-decoration: none;" href="#">Link</a> | ||
|
|
||
| ``` | ||
|
|
||
| # Diagnostics | ||
| ``` | ||
| invalid.html:3:6 lint/nursery/noInlineStyles ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ | ||
|
|
||
| i Avoid using the style attribute. | ||
|
|
||
| 1 │ <!-- Invalid cases - should trigger the rule --> | ||
| 2 │ | ||
| > 3 │ <div style="color: red;"></div> | ||
| │ ^^^^^^^^^^^^^^^^^^^ | ||
| 4 │ | ||
| 5 │ <button style="background: blue; color: white;">Click</button> | ||
|
|
||
| i Inline styles make code harder to maintain, override, and can interfere with Content Security Policy. | ||
|
|
||
| i Use a CSS class instead. | ||
|
|
||
| i This rule belongs to the nursery group, which means it is not yet stable and may change in the future. Visit https://biomejs.dev/linter/#nursery for more information. | ||
|
|
||
|
|
||
| ``` | ||
|
|
||
| ``` | ||
| invalid.html:5:9 lint/nursery/noInlineStyles ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ | ||
|
|
||
| i Avoid using the style attribute. | ||
|
|
||
| 3 │ <div style="color: red;"></div> | ||
| 4 │ | ||
| > 5 │ <button style="background: blue; color: white;">Click</button> | ||
| │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| 6 │ | ||
| 7 │ <p style="margin: 0; padding: 10px;">Paragraph</p> | ||
|
|
||
| i Inline styles make code harder to maintain, override, and can interfere with Content Security Policy. | ||
|
|
||
| i Use a CSS class instead. | ||
|
|
||
| i This rule belongs to the nursery group, which means it is not yet stable and may change in the future. Visit https://biomejs.dev/linter/#nursery for more information. | ||
|
|
||
|
|
||
| ``` | ||
|
|
||
| ``` | ||
| invalid.html:7:4 lint/nursery/noInlineStyles ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ | ||
|
|
||
| i Avoid using the style attribute. | ||
|
|
||
| 5 │ <button style="background: blue; color: white;">Click</button> | ||
| 6 │ | ||
| > 7 │ <p style="margin: 0; padding: 10px;">Paragraph</p> | ||
| │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| 8 │ | ||
| 9 │ <span style="">Empty inline style</span> | ||
|
|
||
| i Inline styles make code harder to maintain, override, and can interfere with Content Security Policy. | ||
|
|
||
| i Use a CSS class instead. | ||
|
|
||
| i This rule belongs to the nursery group, which means it is not yet stable and may change in the future. Visit https://biomejs.dev/linter/#nursery for more information. | ||
|
|
||
|
|
||
| ``` | ||
|
|
||
| ``` | ||
| invalid.html:9:7 lint/nursery/noInlineStyles ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ | ||
|
|
||
| i Avoid using the style attribute. | ||
|
|
||
| 7 │ <p style="margin: 0; padding: 10px;">Paragraph</p> | ||
| 8 │ | ||
| > 9 │ <span style="">Empty inline style</span> | ||
| │ ^^^^^^^^ | ||
| 10 │ | ||
| 11 │ <img style="width: 100px;" src="image.png" alt="Image"> | ||
|
|
||
| i Inline styles make code harder to maintain, override, and can interfere with Content Security Policy. | ||
|
|
||
| i Use a CSS class instead. | ||
|
|
||
| i This rule belongs to the nursery group, which means it is not yet stable and may change in the future. Visit https://biomejs.dev/linter/#nursery for more information. | ||
|
|
||
|
|
||
| ``` | ||
|
|
||
| ``` | ||
| invalid.html:11:6 lint/nursery/noInlineStyles ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ | ||
|
|
||
| i Avoid using the style attribute. | ||
|
|
||
| 9 │ <span style="">Empty inline style</span> | ||
| 10 │ | ||
| > 11 │ <img style="width: 100px;" src="image.png" alt="Image"> | ||
| │ ^^^^^^^^^^^^^^^^^^^^^ | ||
| 12 │ | ||
| 13 │ <a style="text-decoration: none;" href="#">Link</a> | ||
|
|
||
| i Inline styles make code harder to maintain, override, and can interfere with Content Security Policy. | ||
|
|
||
| i Use a CSS class instead. | ||
|
|
||
| i This rule belongs to the nursery group, which means it is not yet stable and may change in the future. Visit https://biomejs.dev/linter/#nursery for more information. | ||
|
|
||
|
|
||
| ``` | ||
|
|
||
| ``` | ||
| invalid.html:13:4 lint/nursery/noInlineStyles ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ | ||
|
|
||
| i Avoid using the style attribute. | ||
|
|
||
| 11 │ <img style="width: 100px;" src="image.png" alt="Image"> | ||
| 12 │ | ||
| > 13 │ <a style="text-decoration: none;" href="#">Link</a> | ||
| │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| 14 │ | ||
|
|
||
| i Inline styles make code harder to maintain, override, and can interfere with Content Security Policy. | ||
|
|
||
| i Use a CSS class instead. | ||
|
|
||
| i This rule belongs to the nursery group, which means it is not yet stable and may change in the future. Visit https://biomejs.dev/linter/#nursery for more information. | ||
|
|
||
|
|
||
| ``` |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| <!-- Valid cases - should NOT trigger the rule --> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this should be our magic comment |
||
|
|
||
| <div class="text-red"></div> | ||
|
|
||
| <button class="btn btn-primary">Click</button> | ||
|
|
||
| <p class="no-margin padded">Paragraph</p> | ||
|
|
||
| <div id="container"></div> | ||
|
|
||
| <span data-style="not-inline">Not inline style</span> | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am also not sure if this PR is properly adding the rule in the correct way across languages like HTML and JSX.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should just work because you've already implemented it for HTML, but you can add snapshot tests for those to verify.
As long as the rule has the same name in js_analyze and html_analyze, you should be good.