Skip to content

Commit 12e971f

Browse files
authored
Use Rule for FileNoqaDirectives (#18966)
Summary -- See #18946 (comment). Test Plan -- Existing tests
1 parent 4eab76b commit 12e971f

File tree

2 files changed

+44
-20
lines changed

2 files changed

+44
-20
lines changed

crates/ruff_linter/src/checkers/noqa.rs

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,17 @@ pub(crate) fn check_noqa(
3535
// Identify any codes that are globally exempted (within the current file).
3636
let file_noqa_directives =
3737
FileNoqaDirectives::extract(locator, comment_ranges, &settings.external, path);
38-
let exemption = FileExemption::from(&file_noqa_directives);
3938

4039
// Extract all `noqa` directives.
4140
let mut noqa_directives =
4241
NoqaDirectives::from_commented_ranges(comment_ranges, &settings.external, path, locator);
4342

43+
if file_noqa_directives.is_empty() && noqa_directives.is_empty() {
44+
return Vec::new();
45+
}
46+
47+
let exemption = FileExemption::from(&file_noqa_directives);
48+
4449
// Indices of diagnostics that were ignored by a `noqa` directive.
4550
let mut ignored_diagnostics = vec![];
4651

@@ -55,7 +60,7 @@ pub(crate) fn check_noqa(
5560
continue;
5661
}
5762

58-
if exemption.contains(code) {
63+
if exemption.contains_secondary_code(code) {
5964
ignored_diagnostics.push(index);
6065
continue;
6166
}
@@ -72,13 +77,21 @@ pub(crate) fn check_noqa(
7277
{
7378
let suppressed = match &directive_line.directive {
7479
Directive::All(_) => {
75-
directive_line.matches.push(code.clone());
80+
let Ok(rule) = Rule::from_code(code) else {
81+
debug_assert!(false, "Invalid secondary code `{code}`");
82+
continue;
83+
};
84+
directive_line.matches.push(rule);
7685
ignored_diagnostics.push(index);
7786
true
7887
}
7988
Directive::Codes(directive) => {
8089
if directive.includes(code) {
81-
directive_line.matches.push(code.clone());
90+
let Ok(rule) = Rule::from_code(code) else {
91+
debug_assert!(false, "Invalid secondary code `{code}`");
92+
continue;
93+
};
94+
directive_line.matches.push(rule);
8295
ignored_diagnostics.push(index);
8396
true
8497
} else {
@@ -141,7 +154,7 @@ pub(crate) fn check_noqa(
141154
diag.secondary_code().is_some_and(|noqa| *noqa == code)
142155
})
143156
} else {
144-
matches.iter().any(|match_| *match_ == code)
157+
matches.iter().any(|match_| match_.noqa_code() == code)
145158
} || settings
146159
.external
147160
.iter()

crates/ruff_linter/src/noqa.rs

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -146,45 +146,48 @@ pub(crate) fn rule_is_ignored(
146146

147147
/// A summary of the file-level exemption as extracted from [`FileNoqaDirectives`].
148148
#[derive(Debug)]
149-
pub(crate) enum FileExemption<'a> {
149+
pub(crate) enum FileExemption {
150150
/// The file is exempt from all rules.
151-
All(Vec<&'a SecondaryCode>),
151+
All(Vec<Rule>),
152152
/// The file is exempt from the given rules.
153-
Codes(Vec<&'a SecondaryCode>),
153+
Codes(Vec<Rule>),
154154
}
155155

156-
impl FileExemption<'_> {
156+
impl FileExemption {
157157
/// Returns `true` if the file is exempt from the given rule, as identified by its noqa code.
158-
pub(crate) fn contains<T: for<'a> PartialEq<&'a str>>(&self, needle: &T) -> bool {
158+
pub(crate) fn contains_secondary_code(&self, needle: &SecondaryCode) -> bool {
159159
match self {
160160
FileExemption::All(_) => true,
161-
FileExemption::Codes(codes) => codes.iter().any(|code| *needle == code),
161+
FileExemption::Codes(codes) => codes.iter().any(|code| *needle == code.noqa_code()),
162162
}
163163
}
164164

165165
/// Returns `true` if the file is exempt from the given rule.
166166
pub(crate) fn includes(&self, needle: Rule) -> bool {
167-
self.contains(&needle.noqa_code())
167+
match self {
168+
FileExemption::All(_) => true,
169+
FileExemption::Codes(codes) => codes.contains(&needle),
170+
}
168171
}
169172

170173
/// Returns `true` if the file exemption lists the rule directly, rather than via a blanket
171174
/// exemption.
172175
pub(crate) fn enumerates(&self, needle: Rule) -> bool {
173-
let needle = needle.noqa_code();
174176
let codes = match self {
175177
FileExemption::All(codes) => codes,
176178
FileExemption::Codes(codes) => codes,
177179
};
178-
codes.iter().any(|code| needle == **code)
180+
codes.contains(&needle)
179181
}
180182
}
181183

182-
impl<'a> From<&'a FileNoqaDirectives<'a>> for FileExemption<'a> {
184+
impl<'a> From<&'a FileNoqaDirectives<'a>> for FileExemption {
183185
fn from(directives: &'a FileNoqaDirectives) -> Self {
184186
let codes = directives
185187
.lines()
186188
.iter()
187189
.flat_map(|line| &line.matches)
190+
.copied()
188191
.collect();
189192
if directives
190193
.lines()
@@ -206,7 +209,7 @@ pub(crate) struct FileNoqaDirectiveLine<'a> {
206209
/// The blanket noqa directive.
207210
pub(crate) parsed_file_exemption: Directive<'a>,
208211
/// The codes that are ignored by the parsed exemptions.
209-
pub(crate) matches: Vec<SecondaryCode>,
212+
pub(crate) matches: Vec<Rule>,
210213
}
211214

212215
impl Ranged for FileNoqaDirectiveLine<'_> {
@@ -271,9 +274,9 @@ impl<'a> FileNoqaDirectives<'a> {
271274
return None;
272275
}
273276

274-
if Rule::from_code(get_redirect_target(code).unwrap_or(code)).is_ok()
277+
if let Ok(rule) = Rule::from_code(get_redirect_target(code).unwrap_or(code))
275278
{
276-
Some(SecondaryCode::new(code.to_string()))
279+
Some(rule)
277280
} else {
278281
#[expect(deprecated)]
279282
let line = locator.compute_line_index(range.start());
@@ -306,6 +309,10 @@ impl<'a> FileNoqaDirectives<'a> {
306309
pub(crate) fn lines(&self) -> &[FileNoqaDirectiveLine] {
307310
&self.0
308311
}
312+
313+
pub(crate) fn is_empty(&self) -> bool {
314+
self.0.is_empty()
315+
}
309316
}
310317

311318
/// Output of lexing a `noqa` directive.
@@ -854,7 +861,7 @@ fn find_noqa_comments<'a>(
854861
continue;
855862
};
856863

857-
if exemption.contains(code) {
864+
if exemption.contains_secondary_code(code) {
858865
comments_by_line.push(None);
859866
continue;
860867
}
@@ -1010,7 +1017,7 @@ pub(crate) struct NoqaDirectiveLine<'a> {
10101017
/// The noqa directive.
10111018
pub(crate) directive: Directive<'a>,
10121019
/// The codes that are ignored by the directive.
1013-
pub(crate) matches: Vec<SecondaryCode>,
1020+
pub(crate) matches: Vec<Rule>,
10141021
/// Whether the directive applies to `range.end`.
10151022
pub(crate) includes_end: bool,
10161023
}
@@ -1135,6 +1142,10 @@ impl<'a> NoqaDirectives<'a> {
11351142
pub(crate) fn lines(&self) -> &[NoqaDirectiveLine] {
11361143
&self.inner
11371144
}
1145+
1146+
pub(crate) fn is_empty(&self) -> bool {
1147+
self.inner.is_empty()
1148+
}
11381149
}
11391150

11401151
/// Remaps offsets falling into one of the ranges to instead check for a noqa comment on the

0 commit comments

Comments
 (0)