Skip to content
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.

Sign up for GitHub

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

refactor(linter): severity and recommendation of lint rules #5133

Merged
merged 3 commits into from
Feb 19, 2025
Merged

refactor(linter): severity and recommendation of lint rules #5133

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
627948f
refactor(linter): severity and recommendation of lint rules
ematipico Feb 16, 2025
259fbb7
chore: tests, codegen and clippy
ematipico Feb 17, 2025
33ff868
chore: fix codegen
ematipico Feb 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
33 changes: 33 additions & 0 deletions .changeset/spotty-buckets-hang.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
---
"@biomejs/biome": major
---

Reworked some recommended rules recommended to be less pedantic and blocking. This is a **breaking change** if your project relied on those rules to block the CI in case of violations, you should raise their diagnostic to **error**.

Some rules aren't recommended anymore, and some others return a different severity:

The following rules return a **warning** diagnostic:
- `noDelete`
- `noForEach`
- `noSuspiciousSemicolonInJsx`
- `noThisInStatic`
- `noUnusedLabels`

The following rules return an **information** diagnostic:
- `noUselessCatch`
- `noUselessConstructor`
- `noUselessEmptyExport`
- `noUselessFragments`
- `noUselessLabel`
- `noUselessLoneBlockStatements`
- `noUselessSwitchCase`
- `noUselessTernary`
- `noUselessThisAlias`
- `noUselessTypeConstraint`

The following rules aren't recommended anymore:
- `noDelete`
- `noForEach`
- `noFlatMapIdentity`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this rule is no longer recommended?

Copy link
Member Author

@ematipico ematipico Feb 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clippy has this rule under the pedantic category, and it doesn't recommended it, I think we should follow their lead and stick with it.

Our recommendation set is very opinionated and we should reduce it. What do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many users like the fact that Biome is opinionated.
I accept that we could be more cautious about what we add in the recommended rule set.
Personally I could keep this rule in the recommended set because it brings one of the mission of Biome: making dev better dev by helping to learn JS API.
We could reduce the severity level to information.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good!


The rule `noRenderReturnValue` is only recommended when the `react` domain is enabled.
2 changes: 1 addition & 1 deletion crates/biome_analyze/src/rule.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ impl RuleSource {

pub fn to_rule_url(&self) -> String {
match self {
Self::Clippy(rule_name) => format!("https://rust-lang.github.io/rust-clippy/master/#/{rule_name}"),
Self::Clippy(rule_name) => format!("https://rust-lang.github.io/rust-clippy/master/#{rule_name}"),
Self::Eslint(rule_name) => format!("https://eslint.org/docs/latest/rules/{rule_name}"),
Self::EslintGraphql(rule_name) => format!("https://the-guild.dev/graphql/eslint/rules/{rule_name}"),
Self::EslintGraphqlSchemaLinter(rule_name) => format!("https://github.com/cjoudrey/graphql-schema-linter?tab=readme-ov-file#{rule_name}"),
Expand Down
3 changes: 2 additions & 1 deletion crates/biome_css_analyze/tests/plugin/useLowercaseColors.grit.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
---
source: crates/biome_css_analyze/tests/spec_tests.rs
expression: useLowercaseColors.grit
snapshot_kind: text
---
# Input
```css
Expand All @@ -16,7 +17,7 @@ div {
```
useLowercaseColors.grit:3:12 plugin ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Prefer lowercase colors
i Prefer lowercase colors
1 │ div {
2 │ color: red;
Expand Down
4 changes: 1 addition & 3 deletions crates/biome_css_analyze/tests/spec_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use biome_analyze::{
use biome_css_parser::{parse_css, CssParserOptions};
use biome_css_syntax::{CssFileSource, CssLanguage};
use biome_diagnostics::advice::CodeSuggestionAdvice;
use biome_diagnostics::{DiagnosticExt, Severity};
use biome_fs::OsFileSystem;
use biome_plugin_loader::AnalyzerGritPlugin;
use biome_rowan::AstNode;
Expand Down Expand Up @@ -146,8 +145,7 @@ pub(crate) fn analyze_and_snap(
}
}

let error = diag.with_severity(Severity::Warning);
diagnostics.push(diagnostic_to_string(file_name, input_code, error));
diagnostics.push(diagnostic_to_string(file_name, input_code, diag.into()));
return ControlFlow::Continue(());
}

Expand Down
15 changes: 8 additions & 7 deletions crates/biome_css_analyze/tests/specs/a11y/useGenericFontNames/invalid.css.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
---
source: crates/biome_css_analyze/tests/spec_tests.rs
expression: invalid.css
snapshot_kind: text
---
# Input
```css
Expand All @@ -17,7 +18,7 @@ a { font: 1em Lucida Grande, Arial, "sans-serif" }
```
invalid.css:1:18 lint/a11y/useGenericFontNames ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Generic font family missing.
× Generic font family missing.
> 1 │ a { font-family: Arial; }
│ ^^^^^
Expand All @@ -39,7 +40,7 @@ invalid.css:1:18 lint/a11y/useGenericFontNames ━━━━━━━━━━━
```
invalid.css:2:44 lint/a11y/useGenericFontNames ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Generic font family missing.
× Generic font family missing.
1 │ a { font-family: Arial; }
> 2 │ a { font-family: 'Arial', "Lucida Grande", Arial; }
Expand All @@ -62,7 +63,7 @@ invalid.css:2:44 lint/a11y/useGenericFontNames ━━━━━━━━━━━
```
invalid.css:3:18 lint/a11y/useGenericFontNames ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Generic font family missing.
× Generic font family missing.
1 │ a { font-family: Arial; }
2 │ a { font-family: 'Arial', "Lucida Grande", Arial; }
Expand All @@ -86,7 +87,7 @@ invalid.css:3:18 lint/a11y/useGenericFontNames ━━━━━━━━━━━
```
invalid.css:4:18 lint/a11y/useGenericFontNames ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Generic font family missing.
× Generic font family missing.
2 │ a { font-family: 'Arial', "Lucida Grande", Arial; }
3 │ a { fOnT-fAmIlY: ' Lucida Grande '; }
Expand All @@ -110,7 +111,7 @@ invalid.css:4:18 lint/a11y/useGenericFontNames ━━━━━━━━━━━
```
invalid.css:5:39 lint/a11y/useGenericFontNames ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Generic font family missing.
× Generic font family missing.
3 │ a { fOnT-fAmIlY: ' Lucida Grande '; }
4 │ a { font-family: Times; }
Expand All @@ -134,7 +135,7 @@ invalid.css:5:39 lint/a11y/useGenericFontNames ━━━━━━━━━━━
```
invalid.css:6:43 lint/a11y/useGenericFontNames ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Generic font family missing.
× Generic font family missing.
4 │ a { font-family: Times; }
5 │ a { FONT: italic 300 16px/30px Arial, " Arial"; }
Expand All @@ -157,7 +158,7 @@ invalid.css:6:43 lint/a11y/useGenericFontNames ━━━━━━━━━━━
```
invalid.css:7:37 lint/a11y/useGenericFontNames ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Generic font family missing.
× Generic font family missing.
5 │ a { FONT: italic 300 16px/30px Arial, " Arial"; }
6 │ a { font: normal 14px/32px -apple-system, BlinkMacSystemFont; }
Expand Down
53 changes: 27 additions & 26 deletions ...e_css_analyze/tests/specs/correctness/noInvalidDirectionInLinearGradient/invalid.css.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
---
source: crates/biome_css_analyze/tests/spec_tests.rs
expression: invalid.css
snapshot_kind: text
---
# Input
```css
Expand Down Expand Up @@ -94,7 +95,7 @@ expression: invalid.css
```
invalid.css:2:30 lint/correctness/noInvalidDirectionInLinearGradient ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Unexpected nonstandard direction
× Unexpected nonstandard direction
1 │ .foo {
> 2 │ background: linear-gradient(bottom, #fff, #000);
Expand All @@ -112,7 +113,7 @@ invalid.css:2:30 lint/correctness/noInvalidDirectionInLinearGradient ━━━
```
invalid.css:5:30 lint/correctness/noInvalidDirectionInLinearGradient ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Unexpected nonstandard direction
× Unexpected nonstandard direction
3 │ }
4 │ .foo {
Expand All @@ -131,7 +132,7 @@ invalid.css:5:30 lint/correctness/noInvalidDirectionInLinearGradient ━━━
```
invalid.css:8:30 lint/correctness/noInvalidDirectionInLinearGradient ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Unexpected nonstandard direction
× Unexpected nonstandard direction
6 │ }
7 │ .foo {
Expand All @@ -150,7 +151,7 @@ invalid.css:8:30 lint/correctness/noInvalidDirectionInLinearGradient ━━━
```
invalid.css:11:30 lint/correctness/noInvalidDirectionInLinearGradient ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Unexpected nonstandard direction
× Unexpected nonstandard direction
9 │ }
10 │ .foo {
Expand All @@ -169,7 +170,7 @@ invalid.css:11:30 lint/correctness/noInvalidDirectionInLinearGradient ━━━
```
invalid.css:14:30 lint/correctness/noInvalidDirectionInLinearGradient ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Unexpected nonstandard direction
× Unexpected nonstandard direction
12 │ }
13 │ .foo {
Expand All @@ -188,7 +189,7 @@ invalid.css:14:30 lint/correctness/noInvalidDirectionInLinearGradient ━━━
```
invalid.css:17:30 lint/correctness/noInvalidDirectionInLinearGradient ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Unexpected nonstandard direction
× Unexpected nonstandard direction
15 │ }
16 │ .foo {
Expand All @@ -207,7 +208,7 @@ invalid.css:17:30 lint/correctness/noInvalidDirectionInLinearGradient ━━━
```
invalid.css:20:30 lint/correctness/noInvalidDirectionInLinearGradient ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Unexpected nonstandard direction
× Unexpected nonstandard direction
18 │ }
19 │ .foo {
Expand All @@ -226,7 +227,7 @@ invalid.css:20:30 lint/correctness/noInvalidDirectionInLinearGradient ━━━
```
invalid.css:23:30 lint/correctness/noInvalidDirectionInLinearGradient ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Unexpected nonstandard direction
× Unexpected nonstandard direction
21 │ }
22 │ .foo {
Expand All @@ -245,7 +246,7 @@ invalid.css:23:30 lint/correctness/noInvalidDirectionInLinearGradient ━━━
```
invalid.css:26:30 lint/correctness/noInvalidDirectionInLinearGradient ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Unexpected nonstandard direction
× Unexpected nonstandard direction
24 │ }
25 │ .foo {
Expand All @@ -264,7 +265,7 @@ invalid.css:26:30 lint/correctness/noInvalidDirectionInLinearGradient ━━━
```
invalid.css:29:30 lint/correctness/noInvalidDirectionInLinearGradient ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Unexpected nonstandard direction
× Unexpected nonstandard direction
27 │ }
28 │ .foo {
Expand All @@ -283,7 +284,7 @@ invalid.css:29:30 lint/correctness/noInvalidDirectionInLinearGradient ━━━
```
invalid.css:32:30 lint/correctness/noInvalidDirectionInLinearGradient ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Unexpected nonstandard direction
× Unexpected nonstandard direction
30 │ }
31 │ .foo {
Expand All @@ -302,7 +303,7 @@ invalid.css:32:30 lint/correctness/noInvalidDirectionInLinearGradient ━━━
```
invalid.css:35:30 lint/correctness/noInvalidDirectionInLinearGradient ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Unexpected nonstandard direction
× Unexpected nonstandard direction
33 │ }
34 │ .foo {
Expand All @@ -321,7 +322,7 @@ invalid.css:35:30 lint/correctness/noInvalidDirectionInLinearGradient ━━━
```
invalid.css:38:30 lint/correctness/noInvalidDirectionInLinearGradient ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Unexpected nonstandard direction
× Unexpected nonstandard direction
36 │ }
37 │ .foo {
Expand All @@ -340,7 +341,7 @@ invalid.css:38:30 lint/correctness/noInvalidDirectionInLinearGradient ━━━
```
invalid.css:41:38 lint/correctness/noInvalidDirectionInLinearGradient ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Unexpected nonstandard direction
× Unexpected nonstandard direction
39 │ }
40 │ .foo {
Expand All @@ -359,7 +360,7 @@ invalid.css:41:38 lint/correctness/noInvalidDirectionInLinearGradient ━━━
```
invalid.css:44:35 lint/correctness/noInvalidDirectionInLinearGradient ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Unexpected nonstandard direction
× Unexpected nonstandard direction
42 │ }
43 │ .foo {
Expand All @@ -378,7 +379,7 @@ invalid.css:44:35 lint/correctness/noInvalidDirectionInLinearGradient ━━━
```
invalid.css:47:33 lint/correctness/noInvalidDirectionInLinearGradient ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Unexpected nonstandard direction
× Unexpected nonstandard direction
45 │ }
46 │ .foo {
Expand All @@ -397,7 +398,7 @@ invalid.css:47:33 lint/correctness/noInvalidDirectionInLinearGradient ━━━
```
invalid.css:50:52 lint/correctness/noInvalidDirectionInLinearGradient ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Unexpected nonstandard direction
× Unexpected nonstandard direction
48 │ }
49 │ .foo {
Expand All @@ -416,7 +417,7 @@ invalid.css:50:52 lint/correctness/noInvalidDirectionInLinearGradient ━━━
```
invalid.css:53:49 lint/correctness/noInvalidDirectionInLinearGradient ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Unexpected nonstandard direction
× Unexpected nonstandard direction
51 │ }
52 │ .foo {
Expand All @@ -435,7 +436,7 @@ invalid.css:53:49 lint/correctness/noInvalidDirectionInLinearGradient ━━━
```
invalid.css:56:47 lint/correctness/noInvalidDirectionInLinearGradient ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Unexpected nonstandard direction
× Unexpected nonstandard direction
54 │ }
55 │ .foo {
Expand All @@ -454,7 +455,7 @@ invalid.css:56:47 lint/correctness/noInvalidDirectionInLinearGradient ━━━
```
invalid.css:59:38 lint/correctness/noInvalidDirectionInLinearGradient ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Unexpected nonstandard direction
× Unexpected nonstandard direction
57 │ }
58 │ .foo {
Expand All @@ -473,7 +474,7 @@ invalid.css:59:38 lint/correctness/noInvalidDirectionInLinearGradient ━━━
```
invalid.css:62:49 lint/correctness/noInvalidDirectionInLinearGradient ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Unexpected nonstandard direction
× Unexpected nonstandard direction
60 │ }
61 │ .foo {
Expand All @@ -492,7 +493,7 @@ invalid.css:62:49 lint/correctness/noInvalidDirectionInLinearGradient ━━━
```
invalid.css:66:33 lint/correctness/noInvalidDirectionInLinearGradient ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Unexpected nonstandard direction
× Unexpected nonstandard direction
64 │ }
65 │ .foo {
Expand All @@ -511,7 +512,7 @@ invalid.css:66:33 lint/correctness/noInvalidDirectionInLinearGradient ━━━
```
invalid.css:69:52 lint/correctness/noInvalidDirectionInLinearGradient ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Unexpected nonstandard direction
× Unexpected nonstandard direction
67 │ }
68 │ .foo {
Expand All @@ -530,7 +531,7 @@ invalid.css:69:52 lint/correctness/noInvalidDirectionInLinearGradient ━━━
```
invalid.css:73:49 lint/correctness/noInvalidDirectionInLinearGradient ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Unexpected nonstandard direction
× Unexpected nonstandard direction
71 │ }
72 │ .foo {
Expand All @@ -549,7 +550,7 @@ invalid.css:73:49 lint/correctness/noInvalidDirectionInLinearGradient ━━━
```
invalid.css:77:47 lint/correctness/noInvalidDirectionInLinearGradient ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Unexpected nonstandard direction
× Unexpected nonstandard direction
75 │ }
76 │ .foo {
Expand All @@ -568,7 +569,7 @@ invalid.css:77:47 lint/correctness/noInvalidDirectionInLinearGradient ━━━
```
invalid.css:81:48 lint/correctness/noInvalidDirectionInLinearGradient ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Unexpected nonstandard direction
× Unexpected nonstandard direction
79 │ }
80 │ .foo {
Expand Down
Loading
Loading