Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/ruff/suppressions.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,35 @@ def f():
# ruff:disable[E741,F841]
I = 1 # noqa: E741,F841
# ruff:enable[E741,F841]


def f():
# TODO: Duplicate codes should be counted as duplicate, not unused
Copy link
Member

Choose a reason for hiding this comment

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

It's fine if one of them counts as unused, for as long as we remove the right one.

# ruff: disable[F841, F841]
foo = 0


def f():
# Overlapping range suppressions, one should be marked as used,
# and the other should trigger an unused suppression diagnostic
# ruff: disable[F841]
# ruff: disable[F841]
foo = 0


def f():
# Multiple codes but only one is used
# ruff: disable[E741, F401, F841]
foo = 0


def f():
# Multiple codes but only two are used
# ruff: disable[E741, F401, F841]
I = 0


def f():
# Multiple codes but none are used
# ruff: disable[E741, F401, F841]
print("hello")
13 changes: 11 additions & 2 deletions crates/ruff_linter/src/checkers/noqa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,9 @@ pub(crate) fn check_noqa(
}
}

// Diagnostics for unused/invalid range suppressions
suppressions.check_suppressions(context, locator);

// Enforce that the noqa directive was actually used (RUF100), unless RUF100 was itself
// suppressed.
if context.is_rule_enabled(Rule::UnusedNOQA)
Expand All @@ -140,8 +143,13 @@ pub(crate) fn check_noqa(
Directive::All(directive) => {
if matches.is_empty() {
let edit = delete_comment(directive.range(), locator);
let mut diagnostic = context
.report_diagnostic(UnusedNOQA { codes: None }, directive.range());
let mut diagnostic = context.report_diagnostic(
UnusedNOQA {
codes: None,
kind: ruff::rules::UnusedNOQAKind::Noqa,
},
directive.range(),
);
diagnostic.add_primary_tag(ruff_db::diagnostic::DiagnosticTag::Unnecessary);
diagnostic.set_fix(Fix::safe_edit(edit));
}
Expand Down Expand Up @@ -236,6 +244,7 @@ pub(crate) fn check_noqa(
.map(|code| (*code).to_string())
.collect(),
}),
kind: ruff::rules::UnusedNOQAKind::Noqa,
},
directive.range(),
);
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/noqa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -879,7 +879,7 @@ fn find_noqa_comments<'a>(
exemption: &'a FileExemption,
directives: &'a NoqaDirectives,
noqa_line_for: &NoqaMapping,
suppressions: &Suppressions,
suppressions: &'a Suppressions,
) -> Vec<Option<NoqaComment<'a>>> {
// List of noqa comments, ordered to match up with `messages`
let mut comments_by_line: Vec<Option<NoqaComment<'a>>> = vec![];
Expand Down
30 changes: 25 additions & 5 deletions crates/ruff_linter/src/rules/ruff/rules/unused_noqa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,29 @@ use ruff_macros::{ViolationMetadata, derive_message_formats};

use crate::AlwaysFixableViolation;

#[derive(Debug, PartialEq, Eq)]
#[derive(Debug, PartialEq, Eq, Default)]
pub(crate) struct UnusedCodes {
pub disabled: Vec<String>,
pub duplicated: Vec<String>,
pub unknown: Vec<String>,
pub unmatched: Vec<String>,
}

#[derive(Debug, PartialEq, Eq)]
pub(crate) enum UnusedNOQAKind {
Noqa,
Suppression,
}

impl UnusedNOQAKind {
fn as_str(&self) -> &str {
match self {
UnusedNOQAKind::Noqa => "`noqa` directive",
UnusedNOQAKind::Suppression => "suppression",
}
}
}

/// ## What it does
/// Checks for `noqa` directives that are no longer applicable.
///
Expand Down Expand Up @@ -46,6 +61,7 @@ pub(crate) struct UnusedCodes {
#[violation_metadata(stable_since = "v0.0.155")]
pub(crate) struct UnusedNOQA {
pub codes: Option<UnusedCodes>,
pub kind: UnusedNOQAKind,
}

impl AlwaysFixableViolation for UnusedNOQA {
Expand Down Expand Up @@ -95,16 +111,20 @@ impl AlwaysFixableViolation for UnusedNOQA {
));
}
if codes_by_reason.is_empty() {
"Unused `noqa` directive".to_string()
format!("Unused {}", self.kind.as_str())
} else {
format!("Unused `noqa` directive ({})", codes_by_reason.join("; "))
format!(
"Unused {} ({})",
self.kind.as_str(),
codes_by_reason.join("; ")
)
}
}
None => "Unused blanket `noqa` directive".to_string(),
None => format!("Unused blanket {}", self.kind.as_str()),
}
}

fn fix_title(&self) -> String {
"Remove unused `noqa` directive".to_string()
format!("Remove unused {}", self.kind.as_str())
}
}
Loading