Skip to content

Commit e1d77de

Browse files
committed
Support suppression comments on the start or end of the range
1 parent 15eb044 commit e1d77de

2 files changed

Lines changed: 25 additions & 49 deletions

File tree

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

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,8 @@ a = 4 + test # type: ignore
1010

1111
## Multiline ranges
1212

13-
A diagnostic with a multiline range can be suppressed
14-
by a comment on the same line as the diagnostic's start or end.
15-
This is the same behavior as Mypy's.
13+
A diagnostic with a multiline range can be suppressed by a comment on the same line as the
14+
diagnostic's start or end. This is the same behavior as Mypy's.
1615

1716
```py
1817
# fmt: off
@@ -31,16 +30,16 @@ y = (
3130
)
3231
```
3332

34-
This is different from Pyright where Pyright uses the intersection between
35-
the diagnostic range and any suppression comment. This can be problematic for nested expressions
36-
because a suppression in a child expression now suppresses errors in the outer expression.
33+
Pyright diverges from this behavior and instead applies a suppression if its range intersects with
34+
the diagnostic range. This can be problematic for nested expressions because a suppression in a
35+
child expression now suppresses errors in the outer expression.
3736

38-
For example, the `type: ignore` comment in this example
39-
suppresses the error of adding `2` to `"test"` and
40-
adding `"other"` to the result of the cast.
37+
For example, the `type: ignore` comment in this example suppresses the error of adding `2` to
38+
`"test"` and adding `"other"` to the result of the cast.
4139

4240
```py path=nested.py
43-
from typing import cast
41+
# fmt: off
42+
from typing import cast
4443

4544
y = (
4645
cast(int, "test" +
@@ -87,8 +86,8 @@ a = test \
8786

8887
## Codes
8988

90-
Mypy supports `type: ignore[code]`. Red Knot doesn't understand mypy's
91-
rule names. Therefore, ignore the codes and suppress all errors.
89+
Mypy supports `type: ignore[code]`. Red Knot doesn't understand mypy's rule names. Therefore, ignore
90+
the codes and suppress all errors.
9291

9392
```py
9493
a = test # type: ignore[name-defined]

crates/red_knot_python_semantic/src/suppression.rs

Lines changed: 14 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
use ruff_python_parser::TokenKind;
22
use ruff_source_file::LineRanges;
33
use ruff_text_size::{Ranged, TextRange};
4-
use std::ops::RangeBounds;
54

65
use ruff_db::{files::File, parsed::parsed_module, source::source_text};
76

@@ -12,6 +11,8 @@ pub(crate) fn suppressions(db: &dyn Db, file: File) -> Suppressions {
1211
let source = source_text(db.upcast(), file);
1312
let parsed = parsed_module(db.upcast(), file);
1413

14+
// TODO: Support `type: ignore` comments at the
15+
// [start of the file](https://typing.readthedocs.io/en/latest/spec/directives.html#type-ignore-comments).
1516
let mut suppressions = Vec::default();
1617
let mut line_start = source.bom_start_offset();
1718

@@ -50,7 +51,9 @@ pub(crate) struct Suppressions {
5051
impl Suppressions {
5152
/// Finds a suppression for the specified lint.
5253
///
53-
/// Returns the first matching suppression if more than one suppression apply for the current line.
54+
/// Returns the first matching suppression if more than one suppression apply to `range` and `id`.
55+
///
56+
/// Returns `None` if the lint isn't suppressed.
5457
pub(crate) fn find_suppression(&self, range: TextRange, _id: LintId) -> Option<&Suppression> {
5558
// TODO(micha):
5659
// * Test if the suppression suppresses the passed lint
@@ -63,48 +66,22 @@ impl Suppressions {
6366
/// start or end offset. This means the suppression is on the same line
6467
/// as the diagnostic's start or end.
6568
fn for_range(&self, range: TextRange) -> impl Iterator<Item = &Suppression> + '_ {
66-
// First find the index of the suppression that starts closest to the range's start.
67-
let start_offset = self
69+
// First find the index of the suppression comment that ends right before the range
70+
// starts. This allows us to skip suppressions that are not relevant for the range.
71+
let end_offset = self
6872
.suppressions
6973
.binary_search_by_key(&range.start(), |suppression| {
70-
suppression.suppressed_range.start()
74+
suppression.suppressed_range.end()
7175
})
7276
.unwrap_or_else(|index| index);
7377

74-
// Search backward for suppressions that start before the range's start
75-
// but overlap with `range`.
76-
//
77-
// ```python
78-
// y = (
79-
// 4 / 0 # type: ignore
80-
// ^----^ range
81-
// ^--- suppression range --^
82-
// )
83-
// ```
84-
self.suppressions[..start_offset]
78+
// From here, search the remaining suppression comments for one that
79+
// contains the range's start or end offset. Stop the search
80+
// as soon as the suppression's range and the range no longer overlap.
81+
self.suppressions[end_offset..]
8582
.iter()
86-
.rev()
83+
// Stop searching if the suppression starts after the range we're looking for.
8784
.take_while(move |suppression| range.end() >= suppression.suppressed_range.start())
88-
.chain(
89-
// Search forward for suppressions that start at or after the range's start
90-
// but overlap with `range`.
91-
//
92-
// ```python
93-
// y = (
94-
// 4 /
95-
// ^--- range start
96-
// ^------ suppression start
97-
// 0 # type: ignore
98-
// ^- range end ^---suppression end
99-
// )
100-
// ```
101-
//
102-
self.suppressions[start_offset..]
103-
.iter()
104-
.take_while(move |suppression| {
105-
range.end() >= suppression.suppressed_range.start()
106-
}),
107-
)
10885
.filter(move |suppression| {
10986
// Don't use intersect to avoid that suppressions on inner-expression
11087
// ignore errors for outer expressions

0 commit comments

Comments
 (0)