Skip to content

Commit

Permalink
refactor(cli/rule): don't report unused suppression comments (#2900)
Browse files Browse the repository at this point in the history
  • Loading branch information
Conaclos authored May 17, 2024
1 parent beabe1b commit 938ea1a
Show file tree
Hide file tree
Showing 8 changed files with 173 additions and 16 deletions.
5 changes: 3 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,13 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b
You can also run a group of rules:

```shell
biome lint --rule=suspicous src/main.js
biome lint --rule=suspicious src/main.js
```

In this case, the severity level of a rule is not overridden.
Thus, disabled rules stay disabled.
Thus, the disabled rules stay disabled.
To ensure that the group is run, the `recommended` field of the group is turned on.
The `nursery` group cannot be passed because no rules are enabled in the nursery group by default.

The option is compatible with other options such as `--apply`, `--apply-unsafe` and `--reporter`.

Expand Down
74 changes: 70 additions & 4 deletions crates/biome_cli/tests/commands/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3367,6 +3367,41 @@ fn lint_rule_filter() {
));
}

#[test]
fn lint_rule_filter_ignore_suppression_comments() {
let mut fs = MemoryFileSystem::default();
let mut console = BufferConsole::default();
let content = r#"
debugger;
// biome-ignore lint/performance/noDelete: <explanation>
delete obj.prop;
"#;

let file_path = Path::new("check.js");
fs.insert(file_path.into(), content.as_bytes());

let result = run_cli(
DynRef::Borrowed(&mut fs),
&mut console,
Args::from(
[
("lint"),
"--rule=suspicious/noDebugger",
file_path.as_os_str().to_str().unwrap(),
]
.as_slice(),
),
);

assert_cli_snapshot(SnapshotPayload::new(
module_path!(),
"lint_rule_filter_ignore_suppression_comments",
fs,
console,
result,
));
}

#[test]
fn lint_rule_filter_with_config() {
let mut fs = MemoryFileSystem::default();
Expand Down Expand Up @@ -3418,7 +3453,7 @@ fn lint_rule_filter_with_config() {
}

#[test]
fn lint_rule_filter_with_recommened_disabled() {
fn lint_rule_filter_with_recommended_disabled() {
let mut fs = MemoryFileSystem::default();
let mut console = BufferConsole::default();
let config = r#"{
Expand All @@ -3443,7 +3478,7 @@ fn lint_rule_filter_with_recommened_disabled() {
Args::from(
[
("lint"),
"--rule=style/useNamingConvention",
"--rule=lint/style/useNamingConvention",
file_path.as_os_str().to_str().unwrap(),
]
.as_slice(),
Expand All @@ -3452,7 +3487,7 @@ fn lint_rule_filter_with_recommened_disabled() {

assert_cli_snapshot(SnapshotPayload::new(
module_path!(),
"lint_rule_filter_with_recommened_disabled",
"lint_rule_filter_with_recommended_disabled",
fs,
console,
result,
Expand Down Expand Up @@ -3545,6 +3580,37 @@ fn lint_rule_filter_group() {
));
}

#[test]
fn lint_rule_filter_nursery_group() {
let mut fs = MemoryFileSystem::default();
let mut console = BufferConsole::default();
let content = "";

let file_path = Path::new("check.js");
fs.insert(file_path.into(), content.as_bytes());

let result = run_cli(
DynRef::Borrowed(&mut fs),
&mut console,
Args::from(
[
("lint"),
"--rule=nursery",
file_path.as_os_str().to_str().unwrap(),
]
.as_slice(),
),
);

assert_cli_snapshot(SnapshotPayload::new(
module_path!(),
"lint_rule_filter_nursery_group",
fs,
console,
result,
));
}

#[test]
fn lint_rule_filter_group_with_disabled_rule() {
let mut fs = MemoryFileSystem::default();
Expand Down Expand Up @@ -3575,7 +3641,7 @@ fn lint_rule_filter_group_with_disabled_rule() {
Args::from(
[
("lint"),
"--rule=suspicious",
"--rule=lint/suspicious",
file_path.as_os_str().to_str().unwrap(),
]
.as_slice(),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
---
source: crates/biome_cli/tests/snap_test.rs
expression: content
---
## `check.js`

```js

debugger;
// biome-ignore lint/performance/noDelete: <explanation>
delete obj.prop;

```

# Termination Message

```block
lint ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
× Some errors were emitted while running checks.
```

# Emitted Messages

```block
check.js:2:9 lint/suspicious/noDebugger FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
× This is an unexpected use of the debugger statement.
> 2 │ debugger;
│ ^^^^^^^^^
3 │ // biome-ignore lint/performance/noDelete: <explanation>
4 │ delete obj.prop;
i Unsafe fix: Remove debugger statement
1 │ -
2 │ - ········debugger;
1 │ +
3 2 │ // biome-ignore lint/performance/noDelete: <explanation>
4 3 │ delete obj.prop;
```

```block
Checked 1 file in <TIME>. No fixes needed.
Found 1 error.
```
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
---
source: crates/biome_cli/tests/snap_test.rs
expression: content
---
## `check.js`

```js

```

# Termination Message

```block
flags/invalid ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
× Failed to parse CLI arguments.
Caused by:
couldn't parse `nursery`: The `nursery` group cannot be selected. Select a specific nursery rule instead.
```
15 changes: 11 additions & 4 deletions crates/biome_configuration/src/linter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,17 +231,24 @@ impl From<RuleSelector> for RuleFilter<'static> {

impl FromStr for RuleSelector {
type Err = &'static str;
fn from_str(s: &str) -> Result<Self, Self::Err> {
if let Some((group_name, rule_name)) = s.split_once('/') {
fn from_str(selector: &str) -> Result<Self, Self::Err> {
let selector = selector.strip_prefix("lint/").unwrap_or(selector);
if let Some((group_name, rule_name)) = selector.split_once('/') {
let group = RuleGroup::from_str(group_name)?;
if let Some(rule_name) = Rules::has_rule(group, rule_name) {
Ok(RuleSelector::Rule(group, rule_name))
} else {
Err("This rule doesn't exist.")
}
} else {
match RuleGroup::from_str(s) {
Ok(group) => Ok(RuleSelector::Group(group)),
match RuleGroup::from_str(selector) {
Ok(group) => {
if matches!(group, RuleGroup::Nursery) {
Err("The `nursery` group cannot be selected. Select a specific nursery rule instead.")
} else {
Ok(RuleSelector::Group(group))
}
}
Err(_) => Err(
"This group doesn't exist. Use the syntax `<group>/<rule>` to specify a rule.",
),
Expand Down
9 changes: 6 additions & 3 deletions crates/biome_service/src/file_handlers/javascript.rs
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,11 @@ pub(crate) fn lint(params: LintParams) -> LintResults {
.filter(|diag| diag.severity() <= Severity::Error)
.count();

let has_lint = filter.categories.contains(RuleCategories::LINT);
// Do not report unused suppression comment diagnostics if:
// - it is a syntax-only analyzer pass, or
// - if a single rule is run.
let ignores_suppression_comment =
!filter.categories.contains(RuleCategories::LINT) || params.rule.is_some();

info!("Analyze file {}", params.path.display());
let (_, analyze_diagnostics) = analyze(
Expand All @@ -382,8 +386,7 @@ pub(crate) fn lint(params: LintParams) -> LintResults {
params.manifest,
|signal| {
if let Some(mut diagnostic) = signal.diagnostic() {
// Do not report unused suppression comment diagnostics if this is a syntax-only analyzer pass
if !has_lint
if ignores_suppression_comment
&& diagnostic.category() == Some(category!("suppressions/unused"))
{
return ControlFlow::<Never>::Continue(());
Expand Down
11 changes: 8 additions & 3 deletions crates/biome_service/src/file_handlers/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -330,12 +330,17 @@ fn lint(params: LintParams) -> LintResults {
compute_analyzer_options(&params.settings, PathBuf::from(params.path.as_path()));
let mut filter = AnalysisFilter::from_enabled_rules(Some(rule_filter_list.as_slice()));
filter.categories = params.categories;
let has_lint = filter.categories.contains(RuleCategories::LINT);

// Do not report unused suppression comment diagnostics if:
// - it is a syntax-only analyzer pass, or
// - if a single rule is run.
let ignores_suppression_comment =
!filter.categories.contains(RuleCategories::LINT) || params.rule.is_some();

let (_, analyze_diagnostics) = analyze(&root, filter, &analyzer_options, |signal| {
if let Some(mut diagnostic) = signal.diagnostic() {
// Do not report unused suppression comment diagnostics if this is a syntax-only analyzer pass
if !has_lint && diagnostic.category() == Some(category!("suppressions/unused"))
if ignores_suppression_comment
&& diagnostic.category() == Some(category!("suppressions/unused"))
{
return ControlFlow::<Never>::Continue(());
}
Expand Down

0 comments on commit 938ea1a

Please sign in to comment.