Skip to content

Commit 788bfd2

Browse files
committed
Support file-level type: ignore comments
1 parent 2f85749 commit 788bfd2

File tree

3 files changed

+187
-69
lines changed

3 files changed

+187
-69
lines changed

crates/red_knot_python_semantic/resources/mdtest/suppressions/knot-ignore.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,3 +118,15 @@ An empty codes array suppresses no-diagnostics and is always useless
118118
# error: [division-by-zero]
119119
a = 4 / 0 # knot: ignore[]
120120
```
121+
122+
## File-level suppression comments
123+
124+
File level suppression comments are currently intentionally unsupported because we've yet to decide
125+
if they should use a different syntax that also supports enabling rules or changing the rule's
126+
severity: `knot: possibly-undefined-reference=error`
127+
128+
```py
129+
# knot: ignore[division-by-zero]
130+
131+
a = 4 / 0 # error: [division-by-zero]
132+
```

crates/red_knot_python_semantic/resources/mdtest/suppressions/type-ignore.md

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,3 +122,36 @@ a = ( # type: ignore
122122
test + 4 # error: [unresolved-reference]
123123
)
124124
```
125+
126+
## File level suppression
127+
128+
```py
129+
# type: ignore
130+
131+
a = 10 / 0
132+
b = a / 0
133+
```
134+
135+
## File level suppression with leading shebang
136+
137+
```py
138+
#!/usr/bin/env/python
139+
# type: ignore
140+
141+
a = 10 / 0
142+
b = a / 0
143+
```
144+
145+
## Invalid own-line suppression
146+
147+
```py
148+
"""
149+
File level suppressions must come before any non-trivia token,
150+
including module docstrings.
151+
"""
152+
153+
# type: ignore
154+
155+
a = 10 / 0 # error: [division-by-zero]
156+
b = a / 0 # error: [division-by-zero]
157+
```

crates/red_knot_python_semantic/src/suppression.rs

Lines changed: 142 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -1,80 +1,31 @@
11
use ruff_db::{files::File, parsed::parsed_module, source::source_text};
22
use ruff_python_parser::TokenKind;
33
use ruff_python_trivia::Cursor;
4-
use ruff_source_file::LineRanges;
5-
use ruff_text_size::{Ranged, TextRange, TextSize};
4+
use ruff_text_size::{Ranged, TextLen, TextRange, TextSize};
65
use smallvec::{smallvec, SmallVec};
76

7+
use crate::lint::LintRegistry;
88
use crate::{lint::LintId, Db};
99

1010
#[salsa::tracked(return_ref)]
1111
pub(crate) fn suppressions(db: &dyn Db, file: File) -> Suppressions {
12-
let source = source_text(db.upcast(), file);
1312
let parsed = parsed_module(db.upcast(), file);
13+
let source = source_text(db.upcast(), file);
1414

15-
let lints = db.lint_registry();
16-
17-
// TODO: Support `type: ignore` comments at the
18-
// [start of the file](https://typing.readthedocs.io/en/latest/spec/directives.html#type-ignore-comments).
19-
let mut suppressions = Vec::default();
20-
let mut line_start = source.bom_start_offset();
15+
let mut builder = SuppressionsBuilder::new(&source, db.lint_registry());
16+
let mut line_start = TextSize::default();
2117

2218
for token in parsed.tokens() {
19+
if !token.kind().is_trivia() {
20+
builder.set_seen_non_trivia_token();
21+
}
22+
2323
match token.kind() {
2424
TokenKind::Comment => {
2525
let parser = SuppressionParser::new(&source, token.range());
26-
let suppressed_range = TextRange::new(line_start, token.range().end());
2726

2827
for comment in parser {
29-
match comment.codes {
30-
// `type: ignore`
31-
None => {
32-
suppressions.push(Suppression {
33-
target: SuppressionTarget::All,
34-
comment_range: comment.range,
35-
range: comment.range,
36-
suppressed_range,
37-
});
38-
}
39-
40-
// `type: ignore[..]`
41-
// The suppression applies to all lints if it is a `type: ignore`
42-
// comment. `type: ignore` apply to all lints for better mypy compatibility.
43-
Some(_) if comment.kind.is_type_ignore() => {
44-
suppressions.push(Suppression {
45-
target: SuppressionTarget::All,
46-
comment_range: comment.range,
47-
range: comment.range,
48-
suppressed_range,
49-
});
50-
}
51-
52-
// `knot: ignore[a, b]`
53-
Some(codes) => {
54-
for code in &codes {
55-
match lints.get(&source[*code]) {
56-
Ok(lint) => {
57-
let range = if codes.len() == 1 {
58-
comment.range
59-
} else {
60-
*code
61-
};
62-
63-
suppressions.push(Suppression {
64-
target: SuppressionTarget::Lint(lint),
65-
range,
66-
comment_range: comment.range,
67-
suppressed_range,
68-
});
69-
}
70-
Err(error) => {
71-
tracing::debug!("Invalid suppression: {error}");
72-
// TODO(micha): Handle invalid lint codes
73-
}
74-
}
75-
}
76-
}
77-
}
28+
builder.add_comment(comment, line_start);
7829
}
7930
}
8031
TokenKind::Newline | TokenKind::NonLogicalNewline => {
@@ -84,34 +35,46 @@ pub(crate) fn suppressions(db: &dyn Db, file: File) -> Suppressions {
8435
}
8536
}
8637

87-
Suppressions { suppressions }
38+
builder.finish()
8839
}
8940

90-
/// The suppression of a single file.
41+
/// The suppressions of a single file.
9142
#[derive(Clone, Debug, Eq, PartialEq)]
9243
pub(crate) struct Suppressions {
93-
/// The suppressions sorted by the suppressed range.
44+
/// Suppressions that apply to the entire file.
45+
///
46+
/// The suppressions are sorted by [`Suppression::comment_range`] and the [`Suppression::suppressed_range`]
47+
/// spans the entire file.
48+
///
49+
/// For now, this is limited to `type: ignore` comments.
50+
file: Vec<Suppression>,
51+
52+
/// Suppressions that apply to a specific line (or lines).
9453
///
95-
/// It's possible that multiple suppressions apply for the same range.
96-
suppressions: Vec<Suppression>,
54+
/// Comments with multiple codes create multiple [`Suppression`]s that all share the same [`Suppression::comment_range`].
55+
///
56+
/// The suppressions are sorted by [`Suppression::range`] (which implies [`Suppression::comment_range`]).
57+
line: Vec<Suppression>,
9758
}
9859

9960
impl Suppressions {
10061
pub(crate) fn find_suppression(&self, range: TextRange, id: LintId) -> Option<&Suppression> {
101-
self.for_range(range)
62+
self.file
63+
.iter()
64+
.chain(self.line_suppressions(range))
10265
.find(|suppression| suppression.matches(id))
10366
}
10467

105-
/// Returns all suppression comments that apply for `range`.
68+
/// Returns the line-level suppressions that apply for `range`.
10669
///
10770
/// A suppression applies for the given range if it contains the range's
10871
/// start or end offset. This means the suppression is on the same line
10972
/// as the diagnostic's start or end.
110-
fn for_range(&self, range: TextRange) -> impl Iterator<Item = &Suppression> + '_ {
73+
fn line_suppressions(&self, range: TextRange) -> impl Iterator<Item = &Suppression> + '_ {
11174
// First find the index of the suppression comment that ends right before the range
11275
// starts. This allows us to skip suppressions that are not relevant for the range.
11376
let end_offset = self
114-
.suppressions
77+
.line
11578
.binary_search_by_key(&range.start(), |suppression| {
11679
suppression.suppressed_range.end()
11780
})
@@ -120,7 +83,7 @@ impl Suppressions {
12083
// From here, search the remaining suppression comments for one that
12184
// contains the range's start or end offset. Stop the search
12285
// as soon as the suppression's range and the range no longer overlap.
123-
self.suppressions[end_offset..]
86+
self.line[end_offset..]
12487
.iter()
12588
// Stop searching if the suppression starts after the range we're looking for.
12689
.take_while(move |suppression| range.end() >= suppression.suppressed_range.start())
@@ -177,6 +140,116 @@ enum SuppressionTarget {
177140
Lint(LintId),
178141
}
179142

143+
struct SuppressionsBuilder<'a> {
144+
lint_registry: &'a LintRegistry,
145+
source: &'a str,
146+
147+
/// `type: ignore` comments at the top of the file before any non-trivia code apply to the entire file.
148+
/// This boolean tracks if there has been any non trivia token.
149+
seen_non_trivia_token: bool,
150+
151+
line: Vec<Suppression>,
152+
file: Vec<Suppression>,
153+
}
154+
155+
impl<'a> SuppressionsBuilder<'a> {
156+
fn new(source: &'a str, lint_registry: &'a LintRegistry) -> Self {
157+
Self {
158+
source,
159+
lint_registry,
160+
seen_non_trivia_token: false,
161+
line: Vec::new(),
162+
file: Vec::new(),
163+
}
164+
}
165+
166+
fn set_seen_non_trivia_token(&mut self) {
167+
self.seen_non_trivia_token = true;
168+
}
169+
170+
fn finish(mut self) -> Suppressions {
171+
self.line.shrink_to_fit();
172+
self.file.shrink_to_fit();
173+
174+
Suppressions {
175+
file: self.file,
176+
line: self.line,
177+
}
178+
}
179+
180+
fn add_comment(&mut self, comment: SuppressionComment, line_start: TextSize) {
181+
let (suppressions, suppressed_range) =
182+
// `type: ignore` comments at the start of the file apply to the entire range.
183+
// > A # type: ignore comment on a line by itself at the top of a file, before any docstrings,
184+
// > imports, or other executable code, silences all errors in the file.
185+
// > Blank lines and other comments, such as shebang lines and coding cookies,
186+
// > may precede the # type: ignore comment.
187+
// > https://typing.readthedocs.io/en/latest/spec/directives.html#type-ignore-comments
188+
if comment.kind.is_type_ignore() && !self.seen_non_trivia_token {
189+
(
190+
&mut self.file,
191+
TextRange::new(0.into(), self.source.text_len()),
192+
)
193+
} else {
194+
(
195+
&mut self.line,
196+
TextRange::new(line_start, comment.range.end()),
197+
)
198+
};
199+
200+
match comment.codes {
201+
// `type: ignore`
202+
None => {
203+
suppressions.push(Suppression {
204+
target: SuppressionTarget::All,
205+
comment_range: comment.range,
206+
range: comment.range,
207+
suppressed_range,
208+
});
209+
}
210+
211+
// `type: ignore[..]`
212+
// The suppression applies to all lints if it is a `type: ignore`
213+
// comment. `type: ignore` apply to all lints for better mypy compatibility.
214+
Some(_) if comment.kind.is_type_ignore() => {
215+
suppressions.push(Suppression {
216+
target: SuppressionTarget::All,
217+
comment_range: comment.range,
218+
range: comment.range,
219+
suppressed_range,
220+
});
221+
}
222+
223+
// `knot: ignore[a, b]`
224+
Some(codes) => {
225+
for code_range in &codes {
226+
let code = &self.source[*code_range];
227+
match self.lint_registry.get(code) {
228+
Ok(lint) => {
229+
let range = if codes.len() == 1 {
230+
comment.range
231+
} else {
232+
*code_range
233+
};
234+
235+
suppressions.push(Suppression {
236+
target: SuppressionTarget::Lint(lint),
237+
range,
238+
comment_range: comment.range,
239+
suppressed_range,
240+
});
241+
}
242+
Err(error) => {
243+
tracing::debug!("Invalid suppression: {error}");
244+
// TODO(micha): Handle invalid lint codes
245+
}
246+
}
247+
}
248+
}
249+
}
250+
}
251+
}
252+
180253
struct SuppressionParser<'src> {
181254
cursor: Cursor<'src>,
182255
range: TextRange,

0 commit comments

Comments
 (0)