Basic support for type: ignore comments

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

@@ -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]
+)
+```

crates/red_knot_python_semantic/src/lib.rs

@@ -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;

crates/red_knot_python_semantic/src/suppression.rs

@@ -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();
-
-    for range in comments {
-        let text = &source[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,
-            });
+    // 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 token in parsed.tokens() {
+        match token.kind() {
+            TokenKind::Comment => {
+                let text = &source[token.range()];
+
+                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;
-
-#[derive(Clone, Debug, Eq, PartialEq, Hash)]
-pub(crate) struct Suppression {
-    target: Option<LintId>,
-    kind: SuppressionKind,
+/// 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>,
 }
 
-#[derive(Copy, Clone, Debug, Eq, PartialEq, Hash)]
-pub(crate) enum SuppressionKind {
-    /// A `type: ignore` comment
-    TypeIgnore,
+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()
+    }
 
-    /// A `knot: ignore` comment
-    KnotIgnore,
+    /// 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 {
+    /// 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,
 }

crates/red_knot_python_semantic/src/types/context.rs

@@ -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);
     }
 

crates/red_knot_test/src/lib.rs

@@ -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();