Skip to content

Commit

Permalink
feat: biome-ignore as suppression comment
Browse files Browse the repository at this point in the history
  • Loading branch information
ematipico committed Sep 5, 2023
1 parent a97bb03 commit 66733f5
Show file tree
Hide file tree
Showing 23 changed files with 449 additions and 139 deletions.
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,12 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom
### Configuration
### Editors
### Formatter

#### Enhancements

- You can use `// biome-ignore` as suppression comment.
- The `// rome-ignore` suppression is deprecated.

### JavaScript APIs
### Linter

Expand Down Expand Up @@ -61,6 +67,9 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom
- Math.pow(a++, /**/ (2))
+ (a++) ** /**/ (2)
```
- You can use `// biome-ignore` as suppression comment.
- The `// rome-ignore` suppression is deprecated.


#### Bug fixes

Expand Down
17 changes: 16 additions & 1 deletion crates/rome_analyze/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -454,10 +454,23 @@ where
}
};

if matches!(kind, SuppressionKind::Deprecated) {
let signal = DiagnosticSignal::new(move || {
SuppressionDiagnostic::new(
category!("suppressions/deprecatedSuppressionComment"),
range,
"// rome-ignore is deprecated, use // biome-ignore instead",
)
.with_tags(DiagnosticTags::DEPRECATED_CODE)
});
(self.emit_signal)(&signal)?;
}

let rule = match kind {
SuppressionKind::Everything => None,
SuppressionKind::Rule(rule) => Some(rule),
SuppressionKind::MaybeLegacy(rule) => Some(rule),
SuppressionKind::Deprecated => None,
};

if let Some(rule) = rule {
Expand Down Expand Up @@ -507,7 +520,7 @@ where
if has_legacy && range_match(self.range, range) {
let signal = DiagnosticSignal::new(move || {
SuppressionDiagnostic::new(
category!("suppressions/deprecatedSyntax"),
category!("suppressions/deprecatedSuppressionComment"),
range,
"Suppression is using a deprecated syntax",
)
Expand Down Expand Up @@ -617,6 +630,8 @@ pub enum SuppressionKind<'a> {
Rule(&'a str),
/// A suppression using the legacy syntax to disable a specific rule eg. `// rome-ignore lint(style/useWhile)`
MaybeLegacy(&'a str),
/// `rome-ignore` is legacy
Deprecated,
}

fn update_suppression<L: Language>(
Expand Down
2 changes: 1 addition & 1 deletion crates/rome_analyze/src/rule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ pub trait Rule: RuleMeta + Sized {
<Self::Group as RuleGroup>::NAME,
Self::METADATA.name
);
let suppression_text = format!("rome-ignore {}", rule_category);
let suppression_text = format!("biome-ignore {}", rule_category);
let root = ctx.root();
let token = root.syntax().token_at_offset(text_range.start());
let mut mutation = root.begin();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,32 +23,14 @@ check ━━━━━━━━━━━━━━━━━━━━━━━━
# Emitted Messages

```block
file.js:1:1 suppressions/deprecatedSyntax FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
file.js:1:1 suppressions/deprecatedSuppressionComment ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Suppression is using a deprecated syntax
! // rome-ignore is deprecated, use // biome-ignore instead
> 1 │ // rome-ignore lint(suspicious/noDoubleEquals): test
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
2 │ a == b;
i Safe fix: Rewrite suppression to use the newer syntax
1 │ - //·rome-ignore·lint(suspicious/noDoubleEquals):·test
1 │ + //·rome-ignore·lint/suspicious/noDoubleEquals:·test
2 2 │ a == b;
```

```block
file.js organizeImports ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
i Import statements could be sorted:
1 │ - //·rome-ignore·lint(suspicious/noDoubleEquals):·test
1 │ + //·rome-ignore·lint/suspicious/noDoubleEquals:·test
2 2 │ a == b;
```

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,14 @@ a == b;
# Emitted Messages

```block
file.js:1:1 suppressions/deprecatedSyntax FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
file.js:1:1 suppressions/deprecatedSuppressionComment ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Suppression is using a deprecated syntax
! // rome-ignore is deprecated, use // biome-ignore instead
> 1 │ // rome-ignore lint(suspicious/noDoubleEquals): test
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
2 │ a == b;
i Safe fix: Rewrite suppression to use the newer syntax
1 │ - //·rome-ignore·lint(suspicious/noDoubleEquals):·test
1 │ + //·rome-ignore·lint/suspicious/noDoubleEquals:·test
2 2 │ a == b;
```

Expand Down
3 changes: 2 additions & 1 deletion crates/rome_diagnostics_categories/src/categories.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ define_categories! {
"lint/nursery/noVoid": "https://biomejs.dev/linter/rules/no-void",
"lint/nursery/useAriaPropTypes": "https://biomejs.dev/linter/rules/use-aria-prop-types",
"lint/nursery/useArrowFunction": "https://biomejs.dev/linter/rules/use-arrow-function",
"lint/nursery/useBiomeSuppressionComment": "https://biomejs.dev/lint/rules/use-biome-suppression-comment",
"lint/nursery/useExhaustiveDependencies": "https://biomejs.dev/linter/rules/use-exhaustive-dependencies",
"lint/nursery/useGetterReturn": "https://biomejs.dev/linter/rules/use-getter-return",
"lint/nursery/useGroupedTypeImport": "https://biomejs.dev/linter/rules/use-grouped-type-import",
Expand Down Expand Up @@ -217,7 +218,7 @@ define_categories! {
"suppressions/unknownGroup",
"suppressions/unknownRule",
"suppressions/unused",
"suppressions/deprecatedSyntax",
"suppressions/deprecatedSuppressionComment",

// Used in tests and examples
"args/fileNotFound",
Expand Down
87 changes: 47 additions & 40 deletions crates/rome_js_analyze/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,12 @@ where

for comment in parse_suppression_comment(text) {
let categories = match comment {
Ok(comment) => comment.categories,
Ok(comment) => {
if comment.is_legacy {
result.push(Ok(SuppressionKind::Deprecated));
}
comment.categories
}
Err(err) => {
result.push(Err(err));
continue;
Expand Down Expand Up @@ -234,7 +239,10 @@ mod tests {
String::from_utf8(buffer).unwrap()
}

const SOURCE: &str = r#"a[f].c = a[f].c;"#;
const SOURCE: &str = r#"// rome-ignore lint: test
let a = 1;
"#;

let parsed = parse(SOURCE, JsFileSource::tsx(), JsParserOptions::default());

Expand All @@ -245,7 +253,7 @@ mod tests {
closure_index: Some(0),
dependencies_index: Some(1),
};
let rule_filter = RuleFilter::Rule("nursery", "noSelfAssign");
let rule_filter = RuleFilter::Rule("nursery", "useBiomeSuppressionComment");
options.configuration.rules.push_rule(
RuleKey::new("nursery", "useHookAtTopLevel"),
RuleOptions::new(HooksOptions { hooks: vec![hook] }),
Expand All @@ -264,7 +272,7 @@ mod tests {
error_ranges.push(diag.location().span.unwrap());
let error = diag
.with_severity(Severity::Warning)
.with_file_path("ahahah")
.with_file_path("dummyFile")
.with_file_source_code(SOURCE);
let text = markup_to_string(markup! {
{PrintDiagnostic::verbose(&error)}
Expand All @@ -274,6 +282,7 @@ mod tests {

for action in signal.actions() {
let new_code = action.mutation.commit();
eprintln!("new code!!!");
eprintln!("{new_code}");
}

Expand All @@ -289,47 +298,47 @@ mod tests {
const SOURCE: &str = "
function checkSuppressions1(a, b) {
a == b;
// rome-ignore lint/suspicious:whole group
// biome-ignore lint/suspicious:whole group
a == b;
// rome-ignore lint/suspicious/noDoubleEquals: single rule
// biome-ignore lint/suspicious/noDoubleEquals: single rule
a == b;
/* rome-ignore lint/style/useWhile: multiple block comments */ /* rome-ignore lint/suspicious/noDoubleEquals: multiple block comments */
/* biome-ignore lint/style/useWhile: multiple block comments */ /* biome-ignore lint/suspicious/noDoubleEquals: multiple block comments */
a == b;
// rome-ignore lint/style/useWhile: multiple line comments
// rome-ignore lint/suspicious/noDoubleEquals: multiple line comments
// biome-ignore lint/style/useWhile: multiple line comments
// biome-ignore lint/suspicious/noDoubleEquals: multiple line comments
a == b;
a == b;
}
// rome-ignore lint/suspicious/noDoubleEquals: do not suppress warning for the whole function
// biome-ignore lint/suspicious/noDoubleEquals: do not suppress warning for the whole function
function checkSuppressions2(a, b) {
a == b;
}
function checkSuppressions3(a, b) {
a == b;
// rome-ignore lint(suspicious): whole group
// rome-ignore lint/suspicious: whole group
a == b;
// rome-ignore lint(suspicious/noDoubleEquals): single rule
// rome-ignore lint/suspicious/noDoubleEquals: single rule
a == b;
/* rome-ignore lint(style/useWhile): multiple block comments */ /* rome-ignore lint(suspicious/noDoubleEquals): multiple block comments */
/* rome-ignore lint/style/useWhile: multiple block comments */ /* rome-ignore lint(suspicious/noDoubleEquals): multiple block comments */
a == b;
// rome-ignore lint(style/useWhile): multiple line comments
// rome-ignore lint(suspicious/noDoubleEquals): multiple line comments
// rome-ignore lint/style/useWhile: multiple line comments
// rome-ignore lint/suspicious/noDoubleEquals: multiple line comments
a == b;
a == b;
}
// rome-ignore lint(suspicious/noDoubleEquals): do not suppress warning for the whole function
// biome-ignore lint(suspicious/noDoubleEquals): do not suppress warning for the whole function
function checkSuppressions4(a, b) {
a == b;
}
function checkSuppressions5() {
// rome-ignore format explanation
// rome-ignore format(:
// rome-ignore (value): explanation
// rome-ignore unknown: explanation
// biome-ignore format explanation
// biome-ignore format(:
// biome-ignore (value): explanation
// biome-ignore unknown: explanation
}
";

Expand Down Expand Up @@ -366,56 +375,54 @@ mod tests {
parse_ranges.push(span.unwrap());
}

if code == category!("suppressions/deprecatedSyntax") {
assert!(signal.actions().len() > 0);
if code == category!("suppressions/deprecatedSuppressionComment") {
warn_ranges.push(span.unwrap());
}
}

ControlFlow::<Never>::Continue(())
},
);

assert_eq!(
lint_ranges.as_slice(),
&[
TextRange::new(TextSize::from(67), TextSize::from(69)),
TextRange::new(TextSize::from(635), TextSize::from(637)),
TextRange::new(TextSize::from(828), TextSize::from(830)),
TextRange::new(TextSize::from(915), TextSize::from(917)),
TextRange::new(TextSize::from(1490), TextSize::from(1492)),
TextRange::new(TextSize::from(1684), TextSize::from(1686)),
TextRange::new(TextSize::from(641), TextSize::from(643)),
TextRange::new(TextSize::from(835), TextSize::from(837)),
TextRange::new(TextSize::from(922), TextSize::from(924)),
TextRange::new(TextSize::from(1492), TextSize::from(1494)),
TextRange::new(TextSize::from(1687), TextSize::from(1689)),
]
);

assert_eq!(
parse_ranges.as_slice(),
&[
TextRange::new(TextSize::from(1787), TextSize::from(1798)),
TextRange::new(TextSize::from(1837), TextSize::from(1838)),
TextRange::new(TextSize::from(1870), TextSize::from(1871)),
TextRange::new(TextSize::from(1922), TextSize::from(1929)),
TextRange::new(TextSize::from(1791), TextSize::from(1802)),
TextRange::new(TextSize::from(1842), TextSize::from(1843)),
TextRange::new(TextSize::from(1876), TextSize::from(1877)),
TextRange::new(TextSize::from(1929), TextSize::from(1936)),
]
);

assert_eq!(
warn_ranges.as_slice(),
&[
TextRange::new(TextSize::from(937), TextSize::from(981)),
TextRange::new(TextSize::from(1022), TextSize::from(1081)),
TextRange::new(TextSize::from(1122), TextSize::from(1185)),
TextRange::new(TextSize::from(1186), TextSize::from(1260)),
TextRange::new(TextSize::from(1301), TextSize::from(1360)),
TextRange::new(TextSize::from(1377), TextSize::from(1447)),
TextRange::new(TextSize::from(1523), TextSize::from(1617)),
TextRange::new(TextSize::from(944), TextSize::from(987)),
TextRange::new(TextSize::from(1028), TextSize::from(1086)),
TextRange::new(TextSize::from(1127), TextSize::from(1189)),
TextRange::new(TextSize::from(1190), TextSize::from(1264)),
TextRange::new(TextSize::from(1305), TextSize::from(1363)),
TextRange::new(TextSize::from(1380), TextSize::from(1449)),
TextRange::new(TextSize::from(1525), TextSize::from(1620)),
]
);
}

#[test]
fn suppression_syntax() {
const SOURCE: &str = "
// rome-ignore lint/suspicious/noDoubleEquals: single rule
// biome-ignore lint/suspicious/noDoubleEquals: single rule
a == b;
";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,19 @@ function SuppressionComments2() {
```

# Diagnostics
```
SuppressionComments.js:1:1 suppressions/deprecatedSuppressionComment ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! // rome-ignore is deprecated, use // biome-ignore instead
> 1 │ // rome-ignore lint/correctness/noUnreachable: this comment does nothing
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
2 │ function SuppressionComments1() {
3 │ beforeReturn();
```

```
SuppressionComments.js:5:5 lint/correctness/noUnreachable ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Expand All @@ -43,6 +56,21 @@ SuppressionComments.js:5:5 lint/correctness/noUnreachable ━━━━━━━
6 │ }
```

```
SuppressionComments.js:11:5 suppressions/deprecatedSuppressionComment ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! // rome-ignore is deprecated, use // biome-ignore instead
9 │ beforeReturn();
10 │ return;
> 11 │ // rome-ignore lint/correctness/noUnreachable: supress warning
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
12 │ afterReturn();
13 │ }
```

```
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ invalid.jsx:2:5 lint/a11y/useKeyWithClickEvents FIXABLE ━━━━━━━
1 1 │ <>
2 │ - ····<div·onClick={()·=>·{}}·/>
2 │ + ····{/*·rome-ignore·lint/a11y/useKeyWithClickEvents:·<explanation>·*/}
2 │ + ····{/*·biome-ignore·lint/a11y/useKeyWithClickEvents:·<explanation>·*/}
3 │ + ····<div·onClick={()·=>·{}}·/>
3 4 │ <div {...spread} onClick={() => {}} />
4 5 │ <div {...spread} onClick={() => {}}>foo</div>
Expand Down
Loading

0 comments on commit 66733f5

Please sign in to comment.