Skip to content

Commit 5ebddd7

Browse files
amyreeseamyreese
committed
Basic suppression comment matching
1 parent ff89bf7 commit 5ebddd7

File tree

2 files changed

+272
-26
lines changed

2 files changed

+272
-26
lines changed

‎crates/ruff_linter/src/linter.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ use crate::rules::ruff::rules::test_rules::{self, TEST_RULES, TestRule};
3232
use crate::settings::types::UnsafeFixes;
3333
use crate::settings::{LinterSettings, TargetVersion, flags};
3434
use crate::source_kind::SourceKind;
35-
use crate::suppression::Suppression;
35+
use crate::suppression::Suppressions;
3636
use crate::{Locator, directives, fs};
3737

3838
pub(crate) mod float;
@@ -140,7 +140,7 @@ pub fn check_path(
140140
let comment_ranges = indexer.comment_ranges();
141141

142142
// Gather all ruff:directive suppressions
143-
let _suppression_comments = Suppression::load(locator.contents(), comment_ranges);
143+
let _suppressions = Suppressions::load(locator.contents(), comment_ranges);
144144

145145
// Collect doc lines. This requires a rare mix of tokens (for comments) and AST
146146
// (for docstrings), which demands special-casing at this level.

‎crates/ruff_linter/src/suppression.rs‎

Lines changed: 270 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
use core::fmt;
2+
use ruff_source_file::LineRanges;
23
use std::{error::Error, fmt::Formatter};
34
use thiserror::Error;
45

5-
use ruff_python_trivia::{CommentRanges, Cursor};
6+
use ruff_python_trivia::{CommentRanges, Cursor, is_python_whitespace};
67
use ruff_text_size::{TextRange, TextSize};
78
use smallvec::{SmallVec, smallvec};
89

@@ -12,20 +13,7 @@ enum SuppressionAction {
1213
Enable,
1314
}
1415

15-
#[allow(unused)]
1616
#[derive(Clone, Debug, Eq, PartialEq)]
17-
enum SuppressionKind {
18-
/// multi-line range suppression
19-
Range,
20-
21-
/// next-line suppression
22-
NextLine,
23-
24-
/// end-of-line suppression
25-
EndOfLine,
26-
}
27-
28-
#[derive(Debug, Eq, PartialEq)]
2917
pub(crate) struct SuppressionComment {
3018
/// Range containing the entire suppression comment
3119
range: TextRange,
@@ -40,30 +28,136 @@ pub(crate) struct SuppressionComment {
4028
reason: TextRange,
4129
}
4230

31+
impl SuppressionComment {
32+
/// Return the suppressed codes as strings
33+
fn codes_as_str(&self, source: &str) -> Vec<String> {
34+
self.codes
35+
.iter()
36+
.map(|range| source[*range].to_string())
37+
.collect()
38+
}
39+
40+
/// Whether the comment is an own-line comment, and how indented it is
41+
fn own_line_indent(&self, source: &str) -> Option<usize> {
42+
let before =
43+
&source[TextRange::new(source.line_start(self.range.start()), self.range.start())];
44+
let is_own_line = before.chars().all(is_python_whitespace);
45+
is_own_line.then(|| before.chars().count())
46+
}
47+
}
48+
4349
#[allow(unused)]
4450
#[derive(Debug)]
4551
pub(crate) struct Suppression {
46-
kind: SuppressionKind,
47-
4852
/// The lint code being suppressed
4953
code: String,
5054

5155
/// Range for which the suppression applies
5256
range: TextRange,
5357

5458
/// Any comments associated with the suppression
55-
comments: Vec<SuppressionComment>,
59+
comments: SmallVec<[SuppressionComment; 2]>,
60+
}
61+
62+
#[allow(unused)]
63+
#[derive(Debug)]
64+
pub(crate) struct Suppressions {
65+
valid: Vec<Suppression>,
66+
invalid: Vec<SuppressionComment>,
67+
errors: Vec<ParseError>,
5668
}
5769

58-
impl Suppression {
59-
pub(crate) fn load(source: &str, comment_ranges: &CommentRanges) -> Vec<SuppressionComment> {
60-
comment_ranges
70+
impl Suppressions {
71+
pub(crate) fn load(source: &str, comment_ranges: &CommentRanges) -> Self {
72+
let mut suppression_comments = comment_ranges
6173
.iter()
62-
.flat_map(|comment_range| {
74+
.map(|comment_range| {
6375
let mut parser = SuppressionParser::new(source, *comment_range);
6476
parser.parse_comment()
6577
})
66-
.collect::<Vec<_>>()
78+
.collect::<Vec<_>>();
79+
80+
let mut index = 0;
81+
let mut suppressions: Vec<Suppression> = Vec::new();
82+
let mut invalid: Vec<SuppressionComment> = Vec::new();
83+
let mut errors: Vec<ParseError> = Vec::new();
84+
85+
// Process all parsed comments in order generating appropriate suppression ranges
86+
while index < suppression_comments.len() {
87+
let mut remove_index: Option<usize> = None;
88+
match &suppression_comments[index] {
89+
Ok(comment) => {
90+
match comment.action {
91+
SuppressionAction::Enable => {
92+
let Some(_indent) = comment.own_line_indent(source) else {
93+
invalid.push(comment.clone());
94+
continue;
95+
};
96+
97+
invalid.push(comment.clone());
98+
}
99+
SuppressionAction::Disable => {
100+
let Some(indent) = comment.own_line_indent(source) else {
101+
invalid.push(comment.clone());
102+
continue;
103+
};
104+
105+
// Look for matching "enable" comments.
106+
// Match by indentation and suppressed codes.
107+
// TODO: search only within the same scope
108+
let codes = comment.codes_as_str(source);
109+
if let Some(other_index) =
110+
suppression_comments[index + 1..].iter().position(|k| {
111+
k.as_ref().is_ok_and(|other| {
112+
other.action == SuppressionAction::Enable
113+
&& other.own_line_indent(source) == Some(indent)
114+
&& other.codes_as_str(source) == codes
115+
})
116+
})
117+
{
118+
// Offset from current position
119+
let other_index = index + 1 + other_index;
120+
121+
// Create a suppression range spanning from the starting disable
122+
// comment to the ending enable comment.
123+
let other = suppression_comments[other_index].as_ref().unwrap();
124+
let combined_range =
125+
TextRange::new(comment.range.start(), other.range.end());
126+
for code in codes {
127+
suppressions.push(Suppression {
128+
code,
129+
range: combined_range,
130+
comments: smallvec![comment.clone(), other.clone()],
131+
});
132+
}
133+
// Mark the matched enable comment to be removed from the vector
134+
// so that it doesn't get processed and treated as unmatched.
135+
remove_index = Some(other_index);
136+
} else {
137+
invalid.push(comment.clone());
138+
}
139+
}
140+
}
141+
}
142+
Err(error) => {
143+
if error.kind != ParseErrorKind::NotASuppression {
144+
errors.push(error.clone());
145+
}
146+
}
147+
}
148+
// Remove a marked comment from the vector.
149+
if let Some(remove_index) = remove_index {
150+
suppression_comments.remove(remove_index).ok();
151+
}
152+
index += 1;
153+
}
154+
155+
suppressions.shrink_to_fit();
156+
Self {
157+
valid: suppressions,
158+
invalid,
159+
errors,
160+
}
67161
}
68162
}
69163

@@ -248,17 +342,19 @@ impl<'src> SuppressionParser<'src> {
248342
#[cfg(test)]
249343
mod tests {
250344
use insta::assert_debug_snapshot;
345+
use ruff_python_trivia::CommentRanges;
251346
use ruff_text_size::{TextRange, TextSize};
252347
use similar::DiffableStr;
253348

254349
use crate::suppression::{
255-
ParseError, SuppressionAction, SuppressionComment, SuppressionParser,
350+
ParseError, SuppressionAction, SuppressionComment, SuppressionParser, Suppressions,
256351
};
257352

258353
fn parse_suppression_comment(source: &str) -> Result<SuppressionComment, ParseError> {
354+
let offset = TextSize::new(source.find('#').unwrap_or(0).try_into().unwrap());
259355
let mut parser = SuppressionParser::new(
260356
source,
261-
TextRange::new(0.into(), TextSize::try_from(source.len()).unwrap()),
357+
TextRange::new(offset, TextSize::try_from(source.len()).unwrap()),
262358
);
263359
parser.parse_comment()
264360
}
@@ -430,6 +526,156 @@ mod tests {
430526
);
431527
}
432528

529+
#[test]
530+
fn trailing_comment() {
531+
let source = "print('hello world') # ruff: enable[some-thing]";
532+
let comment = parse_suppression_comment(source);
533+
assert_debug_snapshot!(
534+
comment,
535+
@r"
536+
Ok(
537+
SuppressionComment {
538+
range: 22..48,
539+
action: Enable,
540+
codes: [
541+
37..47,
542+
],
543+
reason: 48..48,
544+
},
545+
)
546+
",
547+
);
548+
assert_debug_snapshot!(
549+
comment.unwrap().own_line_indent(source),
550+
@"None",
551+
);
552+
}
553+
554+
#[test]
555+
fn indented_comment() {
556+
let source = " # ruff: enable[some-thing]";
557+
let comment = parse_suppression_comment(source);
558+
assert_debug_snapshot!(
559+
comment,
560+
@r"
561+
Ok(
562+
SuppressionComment {
563+
range: 4..30,
564+
action: Enable,
565+
codes: [
566+
19..29,
567+
],
568+
reason: 30..30,
569+
},
570+
)
571+
",
572+
);
573+
assert_debug_snapshot!(
574+
comment.unwrap().own_line_indent(source),
575+
@r"
576+
Some(
577+
4,
578+
)
579+
",
580+
);
581+
}
582+
583+
#[test]
584+
fn load_no_comments() {
585+
let source = "print('hello world')";
586+
let suppressions = Suppressions::load(source, &CommentRanges::new(vec![]));
587+
assert_debug_snapshot!(
588+
suppressions,
589+
@r"
590+
Suppressions {
591+
valid: [],
592+
invalid: [],
593+
errors: [],
594+
}
595+
",
596+
);
597+
}
598+
599+
#[test]
600+
fn load_matched_range() {
601+
let source = "
602+
# ruff: disable[foo]
603+
print('hello world')
604+
# ruff: enable[foo]
605+
";
606+
let ranges = vec![
607+
TextRange::at(1.into(), 20.into()),
608+
TextRange::at(43.into(), 19.into()),
609+
];
610+
let suppressions = Suppressions::load(source, &CommentRanges::new(ranges));
611+
assert_debug_snapshot!(
612+
suppressions,
613+
@r#"
614+
Suppressions {
615+
valid: [
616+
Suppression {
617+
code: "foo",
618+
range: 1..62,
619+
comments: [
620+
SuppressionComment {
621+
range: 1..21,
622+
action: Disable,
623+
codes: [
624+
17..20,
625+
],
626+
reason: 21..21,
627+
},
628+
SuppressionComment {
629+
range: 43..62,
630+
action: Enable,
631+
codes: [
632+
58..61,
633+
],
634+
reason: 62..62,
635+
},
636+
],
637+
},
638+
],
639+
invalid: [],
640+
errors: [],
641+
}
642+
"#,
643+
);
644+
}
645+
646+
#[test]
647+
fn load_unmatched_range() {
648+
let source = "
649+
# ruff: disable[foo]
650+
print('hello world')
651+
# unrelated comment
652+
";
653+
let ranges = vec![
654+
TextRange::at(1.into(), 20.into()),
655+
TextRange::at(43.into(), 19.into()),
656+
];
657+
let suppressions = Suppressions::load(source, &CommentRanges::new(ranges));
658+
assert_debug_snapshot!(
659+
suppressions,
660+
@r"
661+
Suppressions {
662+
valid: [],
663+
invalid: [
664+
SuppressionComment {
665+
range: 1..21,
666+
action: Disable,
667+
codes: [
668+
17..20,
669+
],
670+
reason: 21..21,
671+
},
672+
],
673+
errors: [],
674+
}
675+
",
676+
);
677+
}
678+
433679
#[test]
434680
fn comment_attributes() {
435681
let source = "# ruff: disable[foo, bar] hello world";

0 commit comments

Comments
 (0)