mirror of
https://github.com/astral-sh/ruff.git
synced 2025-08-03 10:22:24 +00:00
Basic support for type: ignore
comments (#15046)
## Summary This PR adds initial support for `type: ignore`. It doesn't do anything fancy yet like: * Detecting invalid type ignore comments * Detecting type ignore comments that are part of another suppression comment: `# fmt: skip # type: ignore` * Suppressing specific lints `type: ignore [code]` * Detecting unsused type ignore comments * ... The goal is to add this functionality in separate PRs. ## Test Plan --------- Co-authored-by: Carl Meyer <carl@astral.sh> Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
This commit is contained in:
parent
6195c026ff
commit
913bce3cd5
5 changed files with 232 additions and 35 deletions
|
@ -0,0 +1,128 @@
|
|||
# Suppressing errors with `type: ignore`
|
||||
|
||||
Type check errors can be suppressed by a `type: ignore` comment on the same line as the violation.
|
||||
|
||||
## Simple `type: ignore`
|
||||
|
||||
```py
|
||||
a = 4 + test # type: ignore
|
||||
```
|
||||
|
||||
## Multiline ranges
|
||||
|
||||
A diagnostic with a multiline range can be suppressed by a comment on the same line as the
|
||||
diagnostic's start or end. This is the same behavior as Mypy's.
|
||||
|
||||
```py
|
||||
# fmt: off
|
||||
y = (
|
||||
4 / 0 # type: ignore
|
||||
)
|
||||
|
||||
y = (
|
||||
4 / # type: ignore
|
||||
0
|
||||
)
|
||||
|
||||
y = (
|
||||
4 /
|
||||
0 # type: ignore
|
||||
)
|
||||
```
|
||||
|
||||
Pyright diverges from this behavior and instead applies a suppression if its range intersects with
|
||||
the diagnostic range. This can be problematic for nested expressions because a suppression in a
|
||||
child expression now suppresses errors in the outer expression.
|
||||
|
||||
For example, the `type: ignore` comment in this example suppresses the error of adding `2` to
|
||||
`"test"` and adding `"other"` to the result of the cast.
|
||||
|
||||
```py path=nested.py
|
||||
# fmt: off
|
||||
from typing import cast
|
||||
|
||||
y = (
|
||||
cast(int, "test" +
|
||||
2 # type: ignore
|
||||
)
|
||||
+ "other" # TODO: expected-error[invalid-operator]
|
||||
)
|
||||
```
|
||||
|
||||
Mypy flags the second usage.
|
||||
|
||||
## Before opening parenthesis
|
||||
|
||||
A suppression that applies to all errors before the opening parenthesis.
|
||||
|
||||
```py
|
||||
a: Test = ( # type: ignore
|
||||
Test() # error: [unresolved-reference]
|
||||
) # fmt: skip
|
||||
```
|
||||
|
||||
## Multiline string
|
||||
|
||||
```py
|
||||
a: int = 4
|
||||
a = """
|
||||
This is a multiline string and the suppression is at its end
|
||||
""" # type: ignore
|
||||
```
|
||||
|
||||
## Line continuations
|
||||
|
||||
Suppressions after a line continuation apply to all previous lines.
|
||||
|
||||
```py
|
||||
# fmt: off
|
||||
a = test \
|
||||
+ 2 # type: ignore
|
||||
|
||||
a = test \
|
||||
+ a \
|
||||
+ 2 # type: ignore
|
||||
```
|
||||
|
||||
## Codes
|
||||
|
||||
Mypy supports `type: ignore[code]`. Red Knot doesn't understand mypy's rule names. Therefore, ignore
|
||||
the codes and suppress all errors.
|
||||
|
||||
```py
|
||||
a = test # type: ignore[name-defined]
|
||||
```
|
||||
|
||||
## Nested comments
|
||||
|
||||
TODO: We should support this for better interopability with other suppression comments.
|
||||
|
||||
```py
|
||||
# fmt: off
|
||||
# TODO this error should be suppressed
|
||||
# error: [unresolved-reference]
|
||||
a = test \
|
||||
+ 2 # fmt: skip # type: ignore
|
||||
|
||||
a = test \
|
||||
+ 2 # type: ignore # fmt: skip
|
||||
```
|
||||
|
||||
## Misspelled `type: ignore`
|
||||
|
||||
```py
|
||||
# error: [unresolved-reference]
|
||||
a = test + 2 # type: ignoree
|
||||
```
|
||||
|
||||
## Invalid - ignore on opening parentheses
|
||||
|
||||
`type: ignore` comments after an opening parentheses suppress any type errors inside the parentheses
|
||||
in Pyright. Neither Ruff, nor mypy support this and neither does Red Knot.
|
||||
|
||||
```py
|
||||
# fmt: off
|
||||
a = ( # type: ignore
|
||||
test + 4 # error: [unresolved-reference]
|
||||
)
|
||||
```
|
|
@ -22,6 +22,7 @@ pub mod semantic_index;
|
|||
mod semantic_model;
|
||||
pub(crate) mod site_packages;
|
||||
mod stdlib;
|
||||
mod suppression;
|
||||
pub(crate) mod symbol;
|
||||
pub mod types;
|
||||
mod unpack;
|
||||
|
|
|
@ -1,50 +1,104 @@
|
|||
use salsa;
|
||||
use ruff_python_parser::TokenKind;
|
||||
use ruff_source_file::LineRanges;
|
||||
use ruff_text_size::{Ranged, TextRange};
|
||||
|
||||
use ruff_db::{files::File, parsed::comment_ranges, source::source_text};
|
||||
use ruff_index::{newtype_index, IndexVec};
|
||||
use ruff_db::{files::File, parsed::parsed_module, source::source_text};
|
||||
|
||||
use crate::{lint::LintId, Db};
|
||||
|
||||
#[salsa::tracked(return_ref)]
|
||||
pub(crate) fn suppressions(db: &dyn Db, file: File) -> IndexVec<SuppressionIndex, Suppression> {
|
||||
let comments = comment_ranges(db.upcast(), file);
|
||||
pub(crate) fn suppressions(db: &dyn Db, file: File) -> Suppressions {
|
||||
let source = source_text(db.upcast(), file);
|
||||
let parsed = parsed_module(db.upcast(), file);
|
||||
|
||||
let mut suppressions = IndexVec::default();
|
||||
// TODO: Support `type: ignore` comments at the
|
||||
// [start of the file](https://typing.readthedocs.io/en/latest/spec/directives.html#type-ignore-comments).
|
||||
let mut suppressions = Vec::default();
|
||||
let mut line_start = source.bom_start_offset();
|
||||
|
||||
for range in comments {
|
||||
let text = &source[range];
|
||||
for token in parsed.tokens() {
|
||||
match token.kind() {
|
||||
TokenKind::Comment => {
|
||||
let text = &source[token.range()];
|
||||
|
||||
if text.starts_with("# type: ignore") {
|
||||
suppressions.push(Suppression {
|
||||
target: None,
|
||||
kind: SuppressionKind::TypeIgnore,
|
||||
});
|
||||
} else if text.starts_with("# knot: ignore") {
|
||||
suppressions.push(Suppression {
|
||||
target: None,
|
||||
kind: SuppressionKind::KnotIgnore,
|
||||
});
|
||||
let suppressed_range = TextRange::new(line_start, token.end());
|
||||
|
||||
if text.strip_prefix("# type: ignore").is_some_and(|suffix| {
|
||||
suffix.is_empty()
|
||||
|| suffix.starts_with(char::is_whitespace)
|
||||
|| suffix.starts_with('[')
|
||||
}) {
|
||||
suppressions.push(Suppression { suppressed_range });
|
||||
}
|
||||
}
|
||||
TokenKind::Newline | TokenKind::NonLogicalNewline => {
|
||||
line_start = token.end();
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
|
||||
suppressions
|
||||
Suppressions { suppressions }
|
||||
}
|
||||
|
||||
#[newtype_index]
|
||||
pub(crate) struct SuppressionIndex;
|
||||
/// The suppression comments of a single file.
|
||||
#[derive(Clone, Debug, Eq, PartialEq)]
|
||||
pub(crate) struct Suppressions {
|
||||
/// The suppressions sorted by the suppressed range.
|
||||
suppressions: Vec<Suppression>,
|
||||
}
|
||||
|
||||
impl Suppressions {
|
||||
/// Finds a suppression for the specified lint.
|
||||
///
|
||||
/// Returns the first matching suppression if more than one suppression apply to `range` and `id`.
|
||||
///
|
||||
/// Returns `None` if the lint isn't suppressed.
|
||||
pub(crate) fn find_suppression(&self, range: TextRange, _id: LintId) -> Option<&Suppression> {
|
||||
// TODO(micha):
|
||||
// * Test if the suppression suppresses the passed lint
|
||||
self.for_range(range).next()
|
||||
}
|
||||
|
||||
/// Returns all suppression comments that apply for `range`.
|
||||
///
|
||||
/// A suppression applies for the given range if it contains the range's
|
||||
/// start or end offset. This means the suppression is on the same line
|
||||
/// as the diagnostic's start or end.
|
||||
fn for_range(&self, range: TextRange) -> impl Iterator<Item = &Suppression> + '_ {
|
||||
// First find the index of the suppression comment that ends right before the range
|
||||
// starts. This allows us to skip suppressions that are not relevant for the range.
|
||||
let end_offset = self
|
||||
.suppressions
|
||||
.binary_search_by_key(&range.start(), |suppression| {
|
||||
suppression.suppressed_range.end()
|
||||
})
|
||||
.unwrap_or_else(|index| index);
|
||||
|
||||
// From here, search the remaining suppression comments for one that
|
||||
// contains the range's start or end offset. Stop the search
|
||||
// as soon as the suppression's range and the range no longer overlap.
|
||||
self.suppressions[end_offset..]
|
||||
.iter()
|
||||
// Stop searching if the suppression starts after the range we're looking for.
|
||||
.take_while(move |suppression| range.end() >= suppression.suppressed_range.start())
|
||||
.filter(move |suppression| {
|
||||
// Don't use intersect to avoid that suppressions on inner-expression
|
||||
// ignore errors for outer expressions
|
||||
suppression.suppressed_range.contains(range.start())
|
||||
|| suppression.suppressed_range.contains(range.end())
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
/// A `type: ignore` or `knot: ignore` suppression comment.
|
||||
#[derive(Clone, Debug, Eq, PartialEq, Hash)]
|
||||
pub(crate) struct Suppression {
|
||||
target: Option<LintId>,
|
||||
kind: SuppressionKind,
|
||||
}
|
||||
|
||||
#[derive(Copy, Clone, Debug, Eq, PartialEq, Hash)]
|
||||
pub(crate) enum SuppressionKind {
|
||||
/// A `type: ignore` comment
|
||||
TypeIgnore,
|
||||
|
||||
/// A `knot: ignore` comment
|
||||
KnotIgnore,
|
||||
/// The range for which this suppression applies.
|
||||
/// Most of the time, this is the range of the comment's line.
|
||||
/// However, there are few cases where the range gets expanded to
|
||||
/// cover multiple lines:
|
||||
/// * multiline strings: `expr + """multiline\nstring""" # type: ignore`
|
||||
/// * line continuations: `expr \ + "test" # type: ignore`
|
||||
suppressed_range: TextRange,
|
||||
}
|
||||
|
|
|
@ -10,6 +10,7 @@ use ruff_text_size::Ranged;
|
|||
|
||||
use crate::{
|
||||
lint::{LintId, LintMetadata},
|
||||
suppression::suppressions,
|
||||
Db,
|
||||
};
|
||||
|
||||
|
@ -74,6 +75,15 @@ impl<'db> InferContext<'db> {
|
|||
return;
|
||||
};
|
||||
|
||||
let suppressions = suppressions(self.db, self.file);
|
||||
|
||||
if suppressions
|
||||
.find_suppression(node.range(), LintId::of(lint))
|
||||
.is_some()
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
self.report_diagnostic(node, DiagnosticId::Lint(lint.name()), severity, message);
|
||||
}
|
||||
|
||||
|
|
|
@ -97,7 +97,11 @@ fn run_test(db: &mut db::Db, test: &parser::MarkdownTest) -> Result<(), Failures
|
|||
|
||||
let test_files: Vec<_> = test
|
||||
.files()
|
||||
.map(|embedded| {
|
||||
.filter_map(|embedded| {
|
||||
if embedded.lang == "ignore" {
|
||||
return None;
|
||||
}
|
||||
|
||||
assert!(
|
||||
matches!(embedded.lang, "py" | "pyi"),
|
||||
"Non-Python files not supported yet."
|
||||
|
@ -106,10 +110,10 @@ fn run_test(db: &mut db::Db, test: &parser::MarkdownTest) -> Result<(), Failures
|
|||
db.write_file(&full_path, embedded.code).unwrap();
|
||||
let file = system_path_to_file(db, full_path).unwrap();
|
||||
|
||||
TestFile {
|
||||
Some(TestFile {
|
||||
file,
|
||||
backtick_offset: embedded.md_offset,
|
||||
}
|
||||
})
|
||||
})
|
||||
.collect();
|
||||
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue