Remove mdtest Diagnostic trait, support tests with syntax errors

.pre-commit-config.yaml

@@ -51,6 +51,10 @@ repos:
       - id: blacken-docs
         args: ["--pyi", "--line-length", "130"]
         files: '^crates/.*/resources/mdtest/.*\.md'
+        exclude: |
+          (?x)^(
+            .*?invalid(_.+)_syntax.md
+          )$
         additional_dependencies:
           - black==24.10.0
 

Cargo.lock

@@ -2220,7 +2220,6 @@ dependencies = [
  "ruff_cache",
  "ruff_db",
  "ruff_python_ast",
- "ruff_text_size",
  "rustc-hash 2.0.0",
  "salsa",
  "tempfile",
@@ -2432,7 +2431,6 @@ dependencies = [
  "salsa",
  "serde",
  "tempfile",
- "thiserror",
  "tracing",
  "tracing-subscriber",
  "tracing-tree",

crates/red_knot/src/main.rs

@@ -5,6 +5,7 @@ use anyhow::{anyhow, Context};
 use clap::Parser;
 use colored::Colorize;
 use crossbeam::channel as crossbeam_channel;
+use ruff_db::diagnostic::CompileDiagnostic;
 use salsa::plumbing::ZalsaDatabase;
 
 use red_knot_python_semantic::SitePackages;
@@ -318,8 +319,9 @@ impl MainLoop {
                 } => {
                     let has_diagnostics = !result.is_empty();
                     if check_revision == revision {
+                        #[allow(clippy::print_stdout)]
                         for diagnostic in result {
-                            tracing::error!("{}", diagnostic);
+                            println!("{}", diagnostic.display(db));
                         }
                     } else {
                         tracing::debug!(
@@ -378,7 +380,10 @@ impl MainLoopCancellationToken {
 #[derive(Debug)]
 enum MainLoopMessage {
     CheckWorkspace,
-    CheckCompleted { result: Vec<String>, revision: u64 },
+    CheckCompleted {
+        result: Vec<CompileDiagnostic>,
+        revision: u64,
+    },
     ApplyChanges(Vec<watch::ChangeEvent>),
     Exit,
 }

crates/red_knot_python_semantic/resources/mdtest/exception/invalid_exception_syntax.md

@@ -0,0 +1,12 @@
+# Exception Handling
+
+## Invalid syntax
+
+```py
+from typing_extensions import reveal_type
+
+try:
+    print
+except as e:  # error: [invalid-syntax]
+    reveal_type(e)  # revealed: Unknown
+```

crates/red_knot_python_semantic/resources/mdtest/unpacking.md

@@ -145,13 +145,8 @@ reveal_type(f)  # revealed: Unknown
 
 ### Non-iterable unpacking
 
-TODO: Remove duplicate diagnostics. This is happening because for a sequence-like assignment target,
-multiple definitions are created and the inference engine runs on each of them which results in
-duplicate diagnostics.
-
 ```py
 # error: "Object of type `Literal[1]` is not iterable"
-# error: "Object of type `Literal[1]` is not iterable"
 a, b = 1
 reveal_type(a)  # revealed: Unknown
 reveal_type(b)  # revealed: Unknown

crates/red_knot_python_semantic/src/types.rs

@@ -1,8 +1,11 @@
+use diagnostic::{is_any_diagnostic_enabled, report_not_iterable, report_type_diagnostic};
 use mro::{ClassBase, Mro, MroError, MroIterator};
+use ruff_db::diagnostic::{CompileDiagnostic, Diagnostic, Severity};
 use ruff_db::files::File;
 use ruff_python_ast as ast;
 
 use itertools::Itertools;
+use ruff_text_size::{Ranged as _, TextRange};
 
 use crate::semantic_index::ast_ids::HasScopedAstId;
 use crate::semantic_index::definition::Definition;
@@ -15,12 +18,11 @@ use crate::stdlib::{
     builtins_symbol, types_symbol, typeshed_symbol, typing_extensions_symbol, typing_symbol,
 };
 use crate::symbol::{Boundness, Symbol};
-use crate::types::diagnostic::TypeCheckDiagnosticsBuilder;
 use crate::types::narrow::narrowing_constraint;
 use crate::{Db, FxOrderSet, HasTy, Module, SemanticModel};
 
 pub(crate) use self::builder::{IntersectionBuilder, UnionBuilder};
-pub use self::diagnostic::{TypeCheckDiagnostic, TypeCheckDiagnostics};
+pub use self::diagnostic::TypeCheckDiagnostic;
 pub(crate) use self::display::TypeArrayDisplay;
 pub(crate) use self::infer::{
     infer_deferred_types, infer_definition_types, infer_expression_types, infer_scope_types,
@@ -34,18 +36,15 @@ mod mro;
 mod narrow;
 mod unpacker;
 
-pub fn check_types(db: &dyn Db, file: File) -> TypeCheckDiagnostics {
+#[salsa::tracked]
+pub fn check_types(db: &dyn Db, file: File) {
     let _span = tracing::trace_span!("check_types", file=?file.path(db)).entered();
 
     let index = semantic_index(db, file);
-    let mut diagnostics = TypeCheckDiagnostics::new();
 
     for scope_id in index.scope_ids() {
-        let result = infer_scope_types(db, scope_id);
-        diagnostics.extend(result.diagnostics());
+        let _ = infer_scope_types(db, scope_id);
     }
-
-    diagnostics
 }
 
 /// Infer the public type of a symbol (its type as seen from outside its scope).
@@ -1598,19 +1597,21 @@ impl<'db> CallOutcome<'db> {
     }
 
     /// Get the return type of the call, emitting default diagnostics if needed.
-    fn unwrap_with_diagnostic<'a>(
+    fn unwrap_with_diagnostic(
         &self,
         db: &'db dyn Db,
         node: ast::AnyNodeRef,
-        diagnostics: &'a mut TypeCheckDiagnosticsBuilder<'db>,
+        file: File,
     ) -> Type<'db> {
-        match self.return_ty_result(db, node, diagnostics) {
+        match self.return_ty_result(db, node, file) {
             Ok(return_ty) => return_ty,
             Err(NotCallableError::Type {
                 not_callable_ty,
                 return_ty,
             }) => {
-                diagnostics.add(
+                report_type_diagnostic(
+                    db,
+                    file,
                     node,
                     "call-non-callable",
                     format_args!(
@@ -1625,7 +1626,9 @@ impl<'db> CallOutcome<'db> {
                 called_ty,
                 return_ty,
             }) => {
-                diagnostics.add(
+                report_type_diagnostic(
+                    db,
+                    file,
                     node,
                     "call-non-callable",
                     format_args!(
@@ -1641,7 +1644,9 @@ impl<'db> CallOutcome<'db> {
                 called_ty,
                 return_ty,
             }) => {
-                diagnostics.add(
+                report_type_diagnostic(
+                    db,
+                    file,
                     node,
                     "call-non-callable",
                     format_args!(
@@ -1656,11 +1661,11 @@ impl<'db> CallOutcome<'db> {
     }
 
     /// Get the return type of the call as a result.
-    fn return_ty_result<'a>(
+    fn return_ty_result(
         &self,
         db: &'db dyn Db,
         node: ast::AnyNodeRef,
-        diagnostics: &'a mut TypeCheckDiagnosticsBuilder<'db>,
+        file: File,
     ) -> Result<Type<'db>, NotCallableError<'db>> {
         match self {
             Self::Callable { return_ty } => Ok(*return_ty),
@@ -1668,11 +1673,17 @@ impl<'db> CallOutcome<'db> {
                 return_ty,
                 revealed_ty,
             } => {
-                diagnostics.add(
-                    node,
-                    "revealed-type",
-                    format_args!("Revealed type is `{}`", revealed_ty.display(db)),
-                );
+                if is_any_diagnostic_enabled(db, file) {
+                    CompileDiagnostic::report(
+                        db.upcast(),
+                        RevealTypeDiagnostic {
+                            file,
+                            range: node.range(),
+                            ty: revealed_ty.display(db).to_string(),
+                        },
+                    );
+                }
+
                 Ok(*return_ty)
             }
             Self::NotCallable { not_callable_ty } => Err(NotCallableError::Type {
@@ -1700,10 +1711,10 @@ impl<'db> CallOutcome<'db> {
                                 *return_ty
                             } else {
                                 revealed = true;
-                                outcome.unwrap_with_diagnostic(db, node, diagnostics)
+                                outcome.unwrap_with_diagnostic(db, node, file)
                             }
                         }
-                        _ => outcome.unwrap_with_diagnostic(db, node, diagnostics),
+                        _ => outcome.unwrap_with_diagnostic(db, node, file),
                     };
                     union_builder = union_builder.add(return_ty);
                 }
@@ -1785,13 +1796,14 @@ enum IterationOutcome<'db> {
 impl<'db> IterationOutcome<'db> {
     fn unwrap_with_diagnostic(
         self,
+        db: &dyn Db,
         iterable_node: ast::AnyNodeRef,
-        diagnostics: &mut TypeCheckDiagnosticsBuilder<'db>,
+        file: File,
     ) -> Type<'db> {
         match self {
             Self::Iterable { element_ty } => element_ty,
             Self::NotIterable { not_iterable_ty } => {
-                diagnostics.add_not_iterable(iterable_node, not_iterable_ty);
+                report_not_iterable(db, file, iterable_node, not_iterable_ty);
                 Type::Unknown
             }
         }
@@ -2228,6 +2240,35 @@ impl<'db> TupleType<'db> {
     }
 }
 
+#[derive(Debug)]
+pub struct RevealTypeDiagnostic {
+    range: TextRange,
+    ty: String,
+    file: File,
+}
+
+impl Diagnostic for RevealTypeDiagnostic {
+    fn rule(&self) -> &str {
+        "revealed-type"
+    }
+
+    fn message(&self) -> std::borrow::Cow<str> {
+        format!("Revealed type is `{}`", self.ty).into()
+    }
+
+    fn file(&self) -> File {
+        self.file
+    }
+
+    fn range(&self) -> Option<TextRange> {
+        Some(self.range)
+    }
+
+    fn severity(&self) -> Severity {
+        Severity::Info
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

crates/red_knot_python_semantic/src/types/diagnostic.rs

@@ -1,13 +1,198 @@
+use ruff_db::diagnostic::{CompileDiagnostic, Diagnostic, Severity};
 use ruff_db::files::File;
 use ruff_python_ast::{self as ast, AnyNodeRef};
 use ruff_text_size::{Ranged, TextRange};
-use std::fmt::Formatter;
-use std::ops::Deref;
-use std::sync::Arc;
+use std::borrow::Cow;
 
 use crate::types::{ClassLiteralType, Type};
 use crate::Db;
 
+/// Returns `true` if any diagnostic is enabled for this file.
+pub(crate) fn is_any_diagnostic_enabled(db: &dyn Db, file: File) -> bool {
+    db.is_file_open(file)
+}
+
+pub(crate) fn report_type_diagnostic(
+    db: &dyn Db,
+    file: File,
+    node: AnyNodeRef,
+    rule: &str,
+    message: std::fmt::Arguments,
+) {
+    if !is_any_diagnostic_enabled(db, file) {
+        return;
+    }
+
+    // TODO: Don't emit the diagnostic if:
+    // * The enclosing node contains any syntax errors
+    // * The rule is disabled for this file. We probably want to introduce a new query that
+    //   returns a rule selector for a given file that respects the package's settings,
+    //   any global pragma comments in the file, and any per-file-ignores.
+
+    CompileDiagnostic::report(
+        db.upcast(),
+        TypeCheckDiagnostic {
+            file,
+            rule: rule.to_string(),
+            message: message.to_string(),
+            range: node.range(),
+        },
+    );
+}
+
+/// Emit a diagnostic declaring that the object represented by `node` is not iterable
+pub(super) fn report_not_iterable(
+    db: &dyn Db,
+    file: File,
+    node: AnyNodeRef,
+    not_iterable_ty: Type,
+) {
+    report_type_diagnostic(
+        db,
+        file,
+        node,
+        "not-iterable",
+        format_args!(
+            "Object of type `{}` is not iterable",
+            not_iterable_ty.display(db)
+        ),
+    );
+}
+
+/// Emit a diagnostic declaring that an index is out of bounds for a tuple.
+pub(super) fn report_index_out_of_bounds(
+    db: &dyn Db,
+    file: File,
+    kind: &'static str,
+    node: AnyNodeRef,
+    tuple_ty: Type,
+    length: usize,
+    index: i64,
+) {
+    report_type_diagnostic(
+        db,
+        file,
+        node,
+        "index-out-of-bounds",
+        format_args!(
+            "Index {index} is out of bounds for {kind} `{}` with length {length}",
+            tuple_ty.display(db)
+        ),
+    );
+}
+
+/// Emit a diagnostic declaring that a type does not support subscripting.
+pub(super) fn report_non_subscriptable(
+    db: &dyn Db,
+    file: File,
+    node: AnyNodeRef,
+    non_subscriptable_ty: Type,
+    method: &str,
+) {
+    report_type_diagnostic(
+        db,
+        file,
+        node,
+        "non-subscriptable",
+        format_args!(
+            "Cannot subscript object of type `{}` with no `{method}` method",
+            non_subscriptable_ty.display(db)
+        ),
+    );
+}
+
+pub(super) fn report_unresolved_module<'a>(
+    db: &dyn Db,
+    file: File,
+    import_node: impl Into<AnyNodeRef<'a>>,
+    level: u32,
+    module: Option<&str>,
+) {
+    report_type_diagnostic(
+        db,
+        file,
+        import_node.into(),
+        "unresolved-import",
+        format_args!(
+            "Cannot resolve import `{}{}`",
+            ".".repeat(level as usize),
+            module.unwrap_or_default()
+        ),
+    );
+}
+
+pub(super) fn report_slice_step_size_zero(db: &dyn Db, file: File, node: AnyNodeRef) {
+    report_type_diagnostic(
+        db,
+        file,
+        node,
+        "zero-stepsize-in-slice",
+        format_args!("Slice step size can not be zero"),
+    );
+}
+
+pub(super) fn report_invalid_assignment(
+    db: &dyn Db,
+    file: File,
+    node: AnyNodeRef,
+    declared_ty: Type,
+    assigned_ty: Type,
+) {
+    match declared_ty {
+        Type::ClassLiteral(ClassLiteralType { class }) => {
+            report_type_diagnostic(db, file, node, "invalid-assignment", format_args!(
+                    "Implicit shadowing of class `{}`; annotate to make it explicit if this is intentional",
+                    class.name(db)));
+        }
+        Type::FunctionLiteral(function) => {
+            report_type_diagnostic(db, file, node, "invalid-assignment", format_args!(
+                    "Implicit shadowing of function `{}`; annotate to make it explicit if this is intentional",
+                    function.name(db)));
+        }
+        _ => {
+            report_type_diagnostic(
+                db,
+                file,
+                node,
+                "invalid-assignment",
+                format_args!(
+                    "Object of type `{}` is not assignable to `{}`",
+                    assigned_ty.display(db),
+                    declared_ty.display(db),
+                ),
+            );
+        }
+    }
+}
+
+pub(super) fn report_possibly_unresolved_reference(
+    db: &dyn Db,
+    file: File,
+    expr_name_node: &ast::ExprName,
+) {
+    let ast::ExprName { id, .. } = expr_name_node;
+
+    report_type_diagnostic(
+        db,
+        file,
+        expr_name_node.into(),
+        "possibly-unresolved-reference",
+        format_args!("Name `{id}` used when possibly not defined"),
+    );
+}
+
+pub(super) fn report_unresolved_reference(db: &dyn Db, file: File, expr_name_node: &ast::ExprName) {
+    let ast::ExprName { id, .. } = expr_name_node;
+
+    report_type_diagnostic(
+        db,
+        file,
+        expr_name_node.into(),
+        "unresolved-reference",
+        format_args!("Name `{id}` used when not defined"),
+    );
+}
+
 #[derive(Debug, Eq, PartialEq)]
 pub struct TypeCheckDiagnostic {
     // TODO: Don't use string keys for rules
@@ -22,265 +207,29 @@ impl TypeCheckDiagnostic {
         &self.rule
     }
 
-    pub fn message(&self) -> &str {
-        &self.message
-    }
-
-    pub fn file(&self) -> File {
-        self.file
-    }
-}
-
-impl Ranged for TypeCheckDiagnostic {
-    fn range(&self) -> TextRange {
+    pub fn range(&self) -> TextRange {
         self.range
     }
 }
 
-/// A collection of type check diagnostics.
-///
-/// The diagnostics are wrapped in an `Arc` because they need to be cloned multiple times
-/// when going from `infer_expression` to `check_file`. We could consider
-/// making [`TypeCheckDiagnostic`] a Salsa struct to have them Arena-allocated (once the Tables refactor is done).
-/// Using Salsa struct does have the downside that it leaks the Salsa dependency into diagnostics and
-/// each Salsa-struct comes with an overhead.
-#[derive(Default, Eq, PartialEq)]
-pub struct TypeCheckDiagnostics {
-    inner: Vec<std::sync::Arc<TypeCheckDiagnostic>>,
-}
-
-impl TypeCheckDiagnostics {
-    pub fn new() -> Self {
-        Self { inner: Vec::new() }
+impl Diagnostic for TypeCheckDiagnostic {
+    fn message(&self) -> std::borrow::Cow<str> {
+        Cow::Borrowed(&self.message)
     }
 
-    pub(super) fn push(&mut self, diagnostic: TypeCheckDiagnostic) {
-        self.inner.push(Arc::new(diagnostic));
+    fn file(&self) -> File {
+        self.file
     }
 
-    pub(crate) fn shrink_to_fit(&mut self) {
-        self.inner.shrink_to_fit();
-    }
-}
-
-impl Extend<TypeCheckDiagnostic> for TypeCheckDiagnostics {
-    fn extend<T: IntoIterator<Item = TypeCheckDiagnostic>>(&mut self, iter: T) {
-        self.inner.extend(iter.into_iter().map(std::sync::Arc::new));
-    }
-}
-
-impl Extend<std::sync::Arc<TypeCheckDiagnostic>> for TypeCheckDiagnostics {
-    fn extend<T: IntoIterator<Item = Arc<TypeCheckDiagnostic>>>(&mut self, iter: T) {
-        self.inner.extend(iter);
-    }
-}
-
-impl<'a> Extend<&'a std::sync::Arc<TypeCheckDiagnostic>> for TypeCheckDiagnostics {
-    fn extend<T: IntoIterator<Item = &'a Arc<TypeCheckDiagnostic>>>(&mut self, iter: T) {
-        self.inner
-            .extend(iter.into_iter().map(std::sync::Arc::clone));
-    }
-}
-
-impl std::fmt::Debug for TypeCheckDiagnostics {
-    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
-        self.inner.fmt(f)
-    }
-}
-
-impl Deref for TypeCheckDiagnostics {
-    type Target = [std::sync::Arc<TypeCheckDiagnostic>];
-
-    fn deref(&self) -> &Self::Target {
-        &self.inner
-    }
-}
-
-impl IntoIterator for TypeCheckDiagnostics {
-    type Item = Arc<TypeCheckDiagnostic>;
-    type IntoIter = std::vec::IntoIter<std::sync::Arc<TypeCheckDiagnostic>>;
-
-    fn into_iter(self) -> Self::IntoIter {
-        self.inner.into_iter()
-    }
-}
-
-impl<'a> IntoIterator for &'a TypeCheckDiagnostics {
-    type Item = &'a Arc<TypeCheckDiagnostic>;
-    type IntoIter = std::slice::Iter<'a, std::sync::Arc<TypeCheckDiagnostic>>;
-
-    fn into_iter(self) -> Self::IntoIter {
-        self.inner.iter()
-    }
-}
-
-pub(super) struct TypeCheckDiagnosticsBuilder<'db> {
-    db: &'db dyn Db,
-    file: File,
-    diagnostics: TypeCheckDiagnostics,
-}
-
-impl<'db> TypeCheckDiagnosticsBuilder<'db> {
-    pub(super) fn new(db: &'db dyn Db, file: File) -> Self {
-        Self {
-            db,
-            file,
-            diagnostics: TypeCheckDiagnostics::new(),
-        }
-    }
-
-    /// Emit a diagnostic declaring that the object represented by `node` is not iterable
-    pub(super) fn add_not_iterable(&mut self, node: AnyNodeRef, not_iterable_ty: Type<'db>) {
-        self.add(
-            node,
-            "not-iterable",
-            format_args!(
-                "Object of type `{}` is not iterable",
-                not_iterable_ty.display(self.db)
-            ),
-        );
-    }
-
-    /// Emit a diagnostic declaring that an index is out of bounds for a tuple.
-    pub(super) fn add_index_out_of_bounds(
-        &mut self,
-        kind: &'static str,
-        node: AnyNodeRef,
-        tuple_ty: Type<'db>,
-        length: usize,
-        index: i64,
-    ) {
-        self.add(
-            node,
-            "index-out-of-bounds",
-            format_args!(
-                "Index {index} is out of bounds for {kind} `{}` with length {length}",
-                tuple_ty.display(self.db)
-            ),
-        );
-    }
-
-    /// Emit a diagnostic declaring that a type does not support subscripting.
-    pub(super) fn add_non_subscriptable(
-        &mut self,
-        node: AnyNodeRef,
-        non_subscriptable_ty: Type<'db>,
-        method: &str,
-    ) {
-        self.add(
-            node,
-            "non-subscriptable",
-            format_args!(
-                "Cannot subscript object of type `{}` with no `{method}` method",
-                non_subscriptable_ty.display(self.db)
-            ),
-        );
-    }
-
-    pub(super) fn add_unresolved_module(
-        &mut self,
-        import_node: impl Into<AnyNodeRef<'db>>,
-        level: u32,
-        module: Option<&str>,
-    ) {
-        self.add(
-            import_node.into(),
-            "unresolved-import",
-            format_args!(
-                "Cannot resolve import `{}{}`",
-                ".".repeat(level as usize),
-                module.unwrap_or_default()
-            ),
-        );
-    }
-
-    pub(super) fn add_slice_step_size_zero(&mut self, node: AnyNodeRef) {
-        self.add(
-            node,
-            "zero-stepsize-in-slice",
-            format_args!("Slice step size can not be zero"),
-        );
-    }
-
-    pub(super) fn add_invalid_assignment(
-        &mut self,
-        node: AnyNodeRef,
-        declared_ty: Type<'db>,
-        assigned_ty: Type<'db>,
-    ) {
-        match declared_ty {
-            Type::ClassLiteral(ClassLiteralType { class }) => {
-                self.add(node, "invalid-assignment", format_args!(
-                        "Implicit shadowing of class `{}`; annotate to make it explicit if this is intentional",
-                        class.name(self.db)));
-            }
-            Type::FunctionLiteral(function) => {
-                self.add(node, "invalid-assignment", format_args!(
-                        "Implicit shadowing of function `{}`; annotate to make it explicit if this is intentional",
-                        function.name(self.db)));
-            }
-            _ => {
-                self.add(
-                    node,
-                    "invalid-assignment",
-                    format_args!(
-                        "Object of type `{}` is not assignable to `{}`",
-                        assigned_ty.display(self.db),
-                        declared_ty.display(self.db),
-                    ),
-                );
-            }
-        }
-    }
-
-    pub(super) fn add_possibly_unresolved_reference(&mut self, expr_name_node: &ast::ExprName) {
-        let ast::ExprName { id, .. } = expr_name_node;
-
-        self.add(
-            expr_name_node.into(),
-            "possibly-unresolved-reference",
-            format_args!("Name `{id}` used when possibly not defined"),
-        );
-    }
-
-    pub(super) fn add_unresolved_reference(&mut self, expr_name_node: &ast::ExprName) {
-        let ast::ExprName { id, .. } = expr_name_node;
-
-        self.add(
-            expr_name_node.into(),
-            "unresolved-reference",
-            format_args!("Name `{id}` used when not defined"),
-        );
-    }
-
-    /// Adds a new diagnostic.
-    ///
-    /// The diagnostic does not get added if the rule isn't enabled for this file.
-    pub(super) fn add(&mut self, node: AnyNodeRef, rule: &str, message: std::fmt::Arguments) {
-        if !self.db.is_file_open(self.file) {
-            return;
-        }
-
-        // TODO: Don't emit the diagnostic if:
-        // * The enclosing node contains any syntax errors
-        // * The rule is disabled for this file. We probably want to introduce a new query that
-        //   returns a rule selector for a given file that respects the package's settings,
-        //   any global pragma comments in the file, and any per-file-ignores.
-
-        self.diagnostics.push(TypeCheckDiagnostic {
-            file: self.file,
-            rule: rule.to_string(),
-            message: message.to_string(),
-            range: node.range(),
-        });
-    }
-
-    pub(super) fn extend(&mut self, diagnostics: &TypeCheckDiagnostics) {
-        self.diagnostics.extend(diagnostics);
-    }
-
-    pub(super) fn finish(mut self) -> TypeCheckDiagnostics {
-        self.diagnostics.shrink_to_fit();
-        self.diagnostics
+    fn range(&self) -> Option<TextRange> {
+        Some(self.range)
+    }
+
+    fn severity(&self) -> Severity {
+        Severity::Error
+    }
+
+    fn rule(&self) -> &str {
+        &self.rule
     }
 }

crates/red_knot_python_semantic/src/types/infer.rs

@@ -48,9 +48,6 @@ use crate::semantic_index::semantic_index;
 use crate::semantic_index::symbol::{NodeWithScopeKind, NodeWithScopeRef, ScopeId};
 use crate::semantic_index::SemanticIndex;
 use crate::stdlib::builtins_module_scope;
-use crate::types::diagnostic::{
-    TypeCheckDiagnostic, TypeCheckDiagnostics, TypeCheckDiagnosticsBuilder,
-};
 use crate::types::mro::MroErrorKind;
 use crate::types::unpacker::{UnpackResult, Unpacker};
 use crate::types::{
@@ -64,6 +61,12 @@ use crate::unpack::Unpack;
 use crate::util::subscript::{PyIndex, PySlice};
 use crate::Db;
 
+use super::diagnostic::{
+    is_any_diagnostic_enabled, report_index_out_of_bounds, report_invalid_assignment,
+    report_non_subscriptable, report_possibly_unresolved_reference, report_slice_step_size_zero,
+    report_type_diagnostic, report_unresolved_module, report_unresolved_reference,
+};
+
 /// Infer all types for a [`ScopeId`], including all definitions and expressions in that scope.
 /// Use when checking a scope, or needing to provide a type for an arbitrary expression in the
 /// scope.
@@ -212,9 +215,6 @@ pub(crate) struct TypeInference<'db> {
     /// The types of every declaration in this region.
     declarations: FxHashMap<Definition<'db>, Type<'db>>,
 
-    /// The diagnostics for this region.
-    diagnostics: TypeCheckDiagnostics,
-
     /// Are there deferred type expressions in this region?
     has_deferred: bool,
 
@@ -228,7 +228,6 @@ impl<'db> TypeInference<'db> {
             expressions: FxHashMap::default(),
             bindings: FxHashMap::default(),
             declarations: FxHashMap::default(),
-            diagnostics: TypeCheckDiagnostics::default(),
             has_deferred: false,
             scope,
         }
@@ -253,15 +252,10 @@ impl<'db> TypeInference<'db> {
         self.declarations[&definition]
     }
 
-    pub(crate) fn diagnostics(&self) -> &[std::sync::Arc<TypeCheckDiagnostic>] {
-        &self.diagnostics
-    }
-
     fn shrink_to_fit(&mut self) {
         self.expressions.shrink_to_fit();
         self.bindings.shrink_to_fit();
         self.declarations.shrink_to_fit();
-        self.diagnostics.shrink_to_fit();
     }
 }
 
@@ -321,8 +315,6 @@ pub(super) struct TypeInferenceBuilder<'db> {
 
     /// The type inference results
     types: TypeInference<'db>,
-
-    diagnostics: TypeCheckDiagnosticsBuilder<'db>,
 }
 
 impl<'db> TypeInferenceBuilder<'db> {
@@ -352,7 +344,6 @@ impl<'db> TypeInferenceBuilder<'db> {
             region,
             file,
             types: TypeInference::empty(scope),
-            diagnostics: TypeCheckDiagnosticsBuilder::new(db, file),
         }
     }
 
@@ -364,7 +355,6 @@ impl<'db> TypeInferenceBuilder<'db> {
             .declarations
             .extend(inference.declarations.iter());
         self.types.expressions.extend(inference.expressions.iter());
-        self.diagnostics.extend(&inference.diagnostics);
         self.types.has_deferred |= inference.has_deferred;
     }
 
@@ -439,13 +429,14 @@ impl<'db> TypeInferenceBuilder<'db> {
                 if infer_definition_types(self.db, *definition).has_deferred {
                     let deferred = infer_deferred_types(self.db, *definition);
                     self.types.expressions.extend(&deferred.expressions);
-                    self.diagnostics.extend(&deferred.diagnostics);
                 }
             }
         }
 
-        // TODO: Only call this function when diagnostics are enabled.
-        self.check_class_definitions();
+        // TODO: Test specifically for the check class definition rules
+        if is_any_diagnostic_enabled(self.db, self.file) {
+            self.check_class_definitions();
+        }
     }
 
     /// Iterate over all class definitions to check that Python will be able to create a
@@ -473,14 +464,18 @@ impl<'db> TypeInferenceBuilder<'db> {
                 MroErrorKind::DuplicateBases(duplicates) => {
                     let base_nodes = class.node(self.db).bases();
                     for (index, duplicate) in duplicates {
-                        self.diagnostics.add(
+                        report_type_diagnostic(
+                            self.db,
+                            self.file,
                             (&base_nodes[*index]).into(),
                              "duplicate-base",
                              format_args!("Duplicate base class `{}`", duplicate.name(self.db))
                         );
                     }
                 }
-                MroErrorKind::CyclicClassDefinition => self.diagnostics.add(
+                MroErrorKind::CyclicClassDefinition => report_type_diagnostic(
+                    self.db,
+                    self.file,
                     class.node(self.db).into(),
                     "cyclic-class-def",
                     format_args!(
@@ -492,7 +487,9 @@ impl<'db> TypeInferenceBuilder<'db> {
                 MroErrorKind::InvalidBases(bases) => {
                     let base_nodes = class.node(self.db).bases();
                     for (index, base_ty) in bases {
-                        self.diagnostics.add(
+                        report_type_diagnostic(
+                            self.db,
+                            self.file,
                             (&base_nodes[*index]).into(),
                             "invalid-base",
                             format_args!(
@@ -502,7 +499,9 @@ impl<'db> TypeInferenceBuilder<'db> {
                         );
                     }
                 },
-                MroErrorKind::UnresolvableMro{bases_list} => self.diagnostics.add(
+                MroErrorKind::UnresolvableMro{bases_list} => report_type_diagnostic(
+                    self.db,
+                    self.file,
                     class.node(self.db).into(),
                     "inconsistent-mro",
                     format_args!(
@@ -627,7 +626,9 @@ impl<'db> TypeInferenceBuilder<'db> {
             _ => return,
         };
 
-        self.diagnostics.add(
+        report_type_diagnostic(
+            self.db,
+            self.file,
             expr.into(),
             "division-by-zero",
             format_args!(
@@ -652,7 +653,9 @@ impl<'db> TypeInferenceBuilder<'db> {
                 // TODO point out the conflicting declarations in the diagnostic?
                 let symbol_table = self.index.symbol_table(binding.file_scope(self.db));
                 let symbol_name = symbol_table.symbol(binding.symbol(self.db)).name();
-                self.diagnostics.add(
+                report_type_diagnostic(
+                    self.db,
+                    self.file,
                     node,
                     "conflicting-declarations",
                     format_args!(
@@ -664,8 +667,7 @@ impl<'db> TypeInferenceBuilder<'db> {
             },
         );
         if !bound_ty.is_assignable_to(self.db, declared_ty) {
-            self.diagnostics
-                .add_invalid_assignment(node, declared_ty, bound_ty);
+            report_invalid_assignment(self.db, self.file, node, declared_ty, bound_ty);
             // allow declarations to override inference in case of invalid assignment
             bound_ty = declared_ty;
         };
@@ -682,7 +684,9 @@ impl<'db> TypeInferenceBuilder<'db> {
         let ty = if inferred_ty.is_assignable_to(self.db, ty) {
             ty
         } else {
-            self.diagnostics.add(
+            report_type_diagnostic(
+                self.db,
+                self.file,
                 node,
                 "invalid-declaration",
                 format_args!(
@@ -708,8 +712,7 @@ impl<'db> TypeInferenceBuilder<'db> {
         let inferred_ty = if inferred_ty.is_assignable_to(self.db, declared_ty) {
             inferred_ty
         } else {
-            self.diagnostics
-                .add_invalid_assignment(node, declared_ty, inferred_ty);
+            report_invalid_assignment(self.db, self.file, node, declared_ty, inferred_ty);
             // if the assignment is invalid, fall back to assuming the annotation is correct
             declared_ty
         };
@@ -1150,7 +1153,9 @@ impl<'db> TypeInferenceBuilder<'db> {
         // TODO: Make use of Protocols when we support it (the manager be assignable to `contextlib.AbstractContextManager`).
         match (enter, exit) {
             (Symbol::Unbound, Symbol::Unbound) => {
-                self.diagnostics.add(
+                report_type_diagnostic(
+                    self.db,
+                    self.file,
                     context_expression.into(),
                     "invalid-context-manager",
                     format_args!(
@@ -1161,7 +1166,9 @@ impl<'db> TypeInferenceBuilder<'db> {
                 Type::Unknown
             }
             (Symbol::Unbound, _) => {
-                self.diagnostics.add(
+                report_type_diagnostic(
+                    self.db,
+                    self.file,
                     context_expression.into(),
                     "invalid-context-manager",
                     format_args!(
@@ -1173,7 +1180,9 @@ impl<'db> TypeInferenceBuilder<'db> {
             }
             (Symbol::Type(enter_ty, enter_boundness), exit) => {
                 if enter_boundness == Boundness::MayBeUnbound {
-                    self.diagnostics.add(
+                    report_type_diagnostic(
+                        self.db,
+                        self.file,
                         context_expression.into(),
                         "invalid-context-manager",
                         format_args!(
@@ -1185,9 +1194,11 @@ impl<'db> TypeInferenceBuilder<'db> {
 
                 let target_ty = enter_ty
                     .call(self.db, &[context_expression_ty])
-                    .return_ty_result(self.db, context_expression.into(), &mut self.diagnostics)
+                    .return_ty_result(self.db, context_expression.into(), self.file)
                     .unwrap_or_else(|err| {
-                        self.diagnostics.add(
+                        report_type_diagnostic(
+                            self.db,
+                            self.file,
                             context_expression.into(),
                             "invalid-context-manager",
                             format_args!("
@@ -1201,7 +1212,9 @@ impl<'db> TypeInferenceBuilder<'db> {
 
                 match exit {
                     Symbol::Unbound => {
-                        self.diagnostics.add(
+                        report_type_diagnostic(
+                            self.db,
+                            self.file,
                             context_expression.into(),
                             "invalid-context-manager",
                             format_args!(
@@ -1214,7 +1227,9 @@ impl<'db> TypeInferenceBuilder<'db> {
                         // TODO: Use the `exit_ty` to determine if any raised exception is suppressed.
 
                         if exit_boundness == Boundness::MayBeUnbound {
-                            self.diagnostics.add(
+                            report_type_diagnostic(
+                                self.db,
+                                self.file,
                                 context_expression.into(),
                                 "invalid-context-manager",
                                 format_args!(
@@ -1234,14 +1249,12 @@ impl<'db> TypeInferenceBuilder<'db> {
                                     Type::none(self.db),
                                 ],
                             )
-                            .return_ty_result(
-                                self.db,
-                                context_expression.into(),
-                                &mut self.diagnostics,
-                            )
+                            .return_ty_result(self.db, context_expression.into(), self.file)
                             .is_err()
                         {
-                            self.diagnostics.add(
+                            report_type_diagnostic(
+                                self.db,
+                                self.file,
                                 context_expression.into(),
                                 "invalid-context-manager",
                                 format_args!(
@@ -1440,7 +1453,6 @@ impl<'db> TypeInferenceBuilder<'db> {
         let target_ty = match target {
             TargetKind::Sequence(unpack) => {
                 let unpacked = infer_unpack_types(self.db, unpack);
-                self.diagnostics.extend(unpacked.diagnostics());
                 unpacked.get(name_ast_id).unwrap_or(Type::Unknown)
             }
             TargetKind::Name => value_ty,
@@ -1545,11 +1557,13 @@ impl<'db> TypeInferenceBuilder<'db> {
                     let augmented_return_ty = match call.return_ty_result(
                         self.db,
                         AnyNodeRef::StmtAugAssign(assignment),
-                        &mut self.diagnostics,
+                        self.file,
                     ) {
                         Ok(t) => t,
                         Err(e) => {
-                            self.diagnostics.add(
+                            report_type_diagnostic(
+                                self.db,
+                                self.file,
                                 assignment.into(),
                                 "unsupported-operator",
                                 format_args!(
@@ -1570,7 +1584,9 @@ impl<'db> TypeInferenceBuilder<'db> {
 
                             let binary_return_ty = self.infer_binary_expression_type(left_ty, right_ty, op)
                                 .unwrap_or_else(|| {
-                                    self.diagnostics.add(
+                                    report_type_diagnostic(
+                                        self.db,
+                                        self.file,
                                         assignment.into(),
                                         "unsupported-operator",
                                         format_args!(
@@ -1599,7 +1615,9 @@ impl<'db> TypeInferenceBuilder<'db> {
 
         self.infer_binary_expression_type(left_ty, right_ty, op)
             .unwrap_or_else(|| {
-                self.diagnostics.add(
+                report_type_diagnostic(
+                    self.db,
+                    self.file,
                     assignment.into(),
                     "unsupported-operator",
                     format_args!(
@@ -1697,7 +1715,7 @@ impl<'db> TypeInferenceBuilder<'db> {
         } else {
             iterable_ty
                 .iterate(self.db)
-                .unwrap_with_diagnostic(iterable.into(), &mut self.diagnostics)
+                .unwrap_with_diagnostic(self.db, iterable.into(), self.file)
         };
 
         self.store_expression_type(target, loop_var_value_ty);
@@ -1736,7 +1754,7 @@ impl<'db> TypeInferenceBuilder<'db> {
             if let Some(module) = self.module_ty_from_name(&module_name) {
                 module
             } else {
-                self.diagnostics.add_unresolved_module(alias, 0, Some(name));
+                report_unresolved_module(self.db, self.file, alias, 0, Some(name));
                 Type::Unknown
             }
         } else {
@@ -1867,7 +1885,9 @@ impl<'db> TypeInferenceBuilder<'db> {
                         .member(self.db, &ast::name::Name::new(&name.id))
                         .as_type()
                         .unwrap_or_else(|| {
-                            self.diagnostics.add(
+                            report_type_diagnostic(
+                                self.db,
+                                self.file,
                                 AnyNodeRef::Alias(alias),
                                 "unresolved-import",
                                 format_args!("Module `{module_name}` has no member `{name}`",),
@@ -1876,8 +1896,7 @@ impl<'db> TypeInferenceBuilder<'db> {
                             Type::Unknown
                         })
                 } else {
-                    self.diagnostics
-                        .add_unresolved_module(import_from, *level, module);
+                    report_unresolved_module(self.db, self.file, import_from, *level, module);
                     Type::Unknown
                 }
             }
@@ -1891,8 +1910,7 @@ impl<'db> TypeInferenceBuilder<'db> {
                     "Relative module resolution `{}` failed: too many leading dots",
                     format_import_from_module(*level, module),
                 );
-                self.diagnostics
-                    .add_unresolved_module(import_from, *level, module);
+                report_unresolved_module(self.db, self.file, import_from, *level, module);
                 Type::Unknown
             }
             Err(ModuleNameResolutionError::UnknownCurrentModule) => {
@@ -1901,8 +1919,7 @@ impl<'db> TypeInferenceBuilder<'db> {
                     format_import_from_module(*level, module),
                     self.file.path(self.db)
                 );
-                self.diagnostics
-                    .add_unresolved_module(import_from, *level, module);
+                report_unresolved_module(self.db, self.file, import_from, *level, module);
                 Type::Unknown
             }
         };
@@ -2364,7 +2381,7 @@ impl<'db> TypeInferenceBuilder<'db> {
         } else {
             iterable_ty
                 .iterate(self.db)
-                .unwrap_with_diagnostic(iterable.into(), &mut self.diagnostics)
+                .unwrap_with_diagnostic(self.db, iterable.into(), self.file)
         };
 
         self.types
@@ -2456,7 +2473,7 @@ impl<'db> TypeInferenceBuilder<'db> {
         let function_type = self.infer_expression(func);
         function_type
             .call(self.db, arg_types.as_slice())
-            .unwrap_with_diagnostic(self.db, func.as_ref().into(), &mut self.diagnostics)
+            .unwrap_with_diagnostic(self.db, func.as_ref().into(), self.file)
     }
 
     fn infer_starred_expression(&mut self, starred: &ast::ExprStarred) -> Type<'db> {
@@ -2467,9 +2484,11 @@ impl<'db> TypeInferenceBuilder<'db> {
         } = starred;
 
         let iterable_ty = self.infer_expression(value);
-        iterable_ty
-            .iterate(self.db)
-            .unwrap_with_diagnostic(value.as_ref().into(), &mut self.diagnostics);
+        iterable_ty.iterate(self.db).unwrap_with_diagnostic(
+            self.db,
+            value.as_ref().into(),
+            self.file,
+        );
 
         // TODO
         Type::Todo
@@ -2488,9 +2507,11 @@ impl<'db> TypeInferenceBuilder<'db> {
         let ast::ExprYieldFrom { range: _, value } = yield_from;
 
         let iterable_ty = self.infer_expression(value);
-        iterable_ty
-            .iterate(self.db)
-            .unwrap_with_diagnostic(value.as_ref().into(), &mut self.diagnostics);
+        iterable_ty.iterate(self.db).unwrap_with_diagnostic(
+            self.db,
+            value.as_ref().into(),
+            self.file,
+        );
 
         // TODO get type from `ReturnType` of generator
         Type::Todo
@@ -2558,7 +2579,9 @@ impl<'db> TypeInferenceBuilder<'db> {
             {
                 let mut symbol = builtins_symbol(self.db, name);
                 if symbol.is_unbound() && name == "reveal_type" {
-                    self.diagnostics.add(
+                    report_type_diagnostic(
+                        self.db,
+                        self.file,
                         name_node.into(),
                         "undefined-reveal",
                         format_args!(
@@ -2611,7 +2634,7 @@ impl<'db> TypeInferenceBuilder<'db> {
             match self.lookup_name(name) {
                 Symbol::Type(looked_up_ty, looked_up_boundness) => {
                     if looked_up_boundness == Boundness::MayBeUnbound {
-                        self.diagnostics.add_possibly_unresolved_reference(name);
+                        report_possibly_unresolved_reference(self.db, self.file, name);
                     }
 
                     bindings_ty
@@ -2620,9 +2643,9 @@ impl<'db> TypeInferenceBuilder<'db> {
                 }
                 Symbol::Unbound => {
                     if bindings_ty.is_some() {
-                        self.diagnostics.add_possibly_unresolved_reference(name);
+                        report_possibly_unresolved_reference(self.db, self.file, name);
                     } else {
-                        self.diagnostics.add_unresolved_reference(name);
+                        report_unresolved_reference(self.db, self.file, name);
                     }
                     bindings_ty.unwrap_or(Type::Unknown)
                 }
@@ -2715,14 +2738,13 @@ impl<'db> TypeInferenceBuilder<'db> {
                 {
                     let call = class_member.call(self.db, &[operand_type]);
 
-                    match call.return_ty_result(
-                        self.db,
-                        AnyNodeRef::ExprUnaryOp(unary),
-                        &mut self.diagnostics,
-                    ) {
+                    match call.return_ty_result(self.db, AnyNodeRef::ExprUnaryOp(unary), self.file)
+                    {
                         Ok(t) => t,
                         Err(e) => {
-                            self.diagnostics.add(
+                            report_type_diagnostic(
+                                self.db,
+                                self.file,
                                 unary.into(),
                                 "unsupported-operator",
                                 format_args!(
@@ -2734,7 +2756,9 @@ impl<'db> TypeInferenceBuilder<'db> {
                         }
                     }
                 } else {
-                    self.diagnostics.add(
+                    report_type_diagnostic(
+                        self.db,
+                        self.file,
                         unary.into(),
                         "unsupported-operator",
                         format_args!(
@@ -2775,7 +2799,9 @@ impl<'db> TypeInferenceBuilder<'db> {
 
         self.infer_binary_expression_type(left_ty, right_ty, *op)
             .unwrap_or_else(|| {
-                self.diagnostics.add(
+                report_type_diagnostic(
+                    self.db,
+                    self.file,
                     binary.into(),
                     "unsupported-operator",
                     format_args!(
@@ -3105,7 +3131,9 @@ impl<'db> TypeInferenceBuilder<'db> {
                     self.infer_binary_type_comparison(left_ty, *op, right_ty)
                         .unwrap_or_else(|error| {
                             // Handle unsupported operators (diagnostic, `bool`/`Unknown` outcome)
-                            self.diagnostics.add(
+                            report_type_diagnostic(
+                                self.db,
+                                self.file,
                                 AnyNodeRef::ExprCompare(compare),
                                 "unsupported-operator",
                                 format_args!(
@@ -3495,7 +3523,9 @@ impl<'db> TypeInferenceBuilder<'db> {
                     .py_index(i32::try_from(int).expect("checked in branch arm"))
                     .copied()
                     .unwrap_or_else(|_| {
-                        self.diagnostics.add_index_out_of_bounds(
+                        report_index_out_of_bounds(
+                            self.db,
+                            self.file,
                             "tuple",
                             value_node.into(),
                             value_ty,
@@ -3514,7 +3544,7 @@ impl<'db> TypeInferenceBuilder<'db> {
                     let new_elements: Vec<_> = new_elements.copied().collect();
                     Type::Tuple(TupleType::new(self.db, new_elements.into_boxed_slice()))
                 } else {
-                    self.diagnostics.add_slice_step_size_zero(value_node.into());
+                    report_slice_step_size_zero(self.db, self.file, value_node.into());
                     Type::Unknown
                 }
             }
@@ -3533,7 +3563,9 @@ impl<'db> TypeInferenceBuilder<'db> {
                         ))
                     })
                     .unwrap_or_else(|_| {
-                        self.diagnostics.add_index_out_of_bounds(
+                        report_index_out_of_bounds(
+                            self.db,
+                            self.file,
                             "string",
                             value_node.into(),
                             value_ty,
@@ -3553,7 +3585,7 @@ impl<'db> TypeInferenceBuilder<'db> {
                     let literal: String = new_chars.collect();
                     Type::StringLiteral(StringLiteralType::new(self.db, literal.into_boxed_str()))
                 } else {
-                    self.diagnostics.add_slice_step_size_zero(value_node.into());
+                    report_slice_step_size_zero(self.db, self.file, value_node.into());
                     Type::Unknown
                 };
                 result
@@ -3570,7 +3602,9 @@ impl<'db> TypeInferenceBuilder<'db> {
                         Type::BytesLiteral(BytesLiteralType::new(self.db, [*byte].as_slice()))
                     })
                     .unwrap_or_else(|_| {
-                        self.diagnostics.add_index_out_of_bounds(
+                        report_index_out_of_bounds(
+                            self.db,
+                            self.file,
                             "bytes literal",
                             value_node.into(),
                             value_ty,
@@ -3589,7 +3623,7 @@ impl<'db> TypeInferenceBuilder<'db> {
                     let new_bytes: Vec<u8> = new_bytes.copied().collect();
                     Type::BytesLiteral(BytesLiteralType::new(self.db, new_bytes.into_boxed_slice()))
                 } else {
-                    self.diagnostics.add_slice_step_size_zero(value_node.into());
+                    report_slice_step_size_zero(self.db, self.file, value_node.into());
                     Type::Unknown
                 }
             }
@@ -3613,7 +3647,9 @@ impl<'db> TypeInferenceBuilder<'db> {
                     Symbol::Unbound => {}
                     Symbol::Type(dunder_getitem_method, boundness) => {
                         if boundness == Boundness::MayBeUnbound {
-                            self.diagnostics.add(
+                            report_type_diagnostic(
+                                self.db,
+                                self.file,
                                 value_node.into(),
                                 "call-possibly-unbound-method",
                                 format_args!(
@@ -3625,9 +3661,11 @@ impl<'db> TypeInferenceBuilder<'db> {
 
                         return dunder_getitem_method
                         .call(self.db, &[slice_ty])
-                        .return_ty_result(self.db, value_node.into(), &mut self.diagnostics)
+                        .return_ty_result(self.db, value_node.into(), self.file)
                         .unwrap_or_else(|err| {
-                            self.diagnostics.add(
+                            report_type_diagnostic(
+                                self.db,
+                                self.file,
                                 value_node.into(),
                                 "call-non-callable",
                                 format_args!(
@@ -3657,7 +3695,9 @@ impl<'db> TypeInferenceBuilder<'db> {
                         Symbol::Unbound => {}
                         Symbol::Type(ty, boundness) => {
                             if boundness == Boundness::MayBeUnbound {
-                                self.diagnostics.add(
+                                report_type_diagnostic(
+                                    self.db,
+                                    self.file,
                                     value_node.into(),
                                     "call-possibly-unbound-method",
                                     format_args!(
@@ -3669,9 +3709,11 @@ impl<'db> TypeInferenceBuilder<'db> {
 
                             return ty
                                 .call(self.db, &[slice_ty])
-                                .return_ty_result(self.db, value_node.into(), &mut self.diagnostics)
+                                .return_ty_result(self.db, value_node.into(), self.file)
                                 .unwrap_or_else(|err| {
-                                    self.diagnostics.add(
+                                    report_type_diagnostic(
+                                        self.db,
+                                        self.file,
                                         value_node.into(),
                                         "call-non-callable",
                                         format_args!(
@@ -3690,13 +3732,17 @@ impl<'db> TypeInferenceBuilder<'db> {
                         return KnownClass::GenericAlias.to_instance(self.db);
                     }
 
-                    self.diagnostics.add_non_subscriptable(
+                    report_non_subscriptable(
+                        self.db,
+                        self.file,
                         value_node.into(),
                         value_ty,
                         "__class_getitem__",
                     );
                 } else {
-                    self.diagnostics.add_non_subscriptable(
+                    report_non_subscriptable(
+                        self.db,
+                        self.file,
                         value_node.into(),
                         value_ty,
                         "__getitem__",
@@ -3791,7 +3837,6 @@ impl<'db> TypeInferenceBuilder<'db> {
 
     pub(super) fn finish(mut self) -> TypeInference<'db> {
         self.infer_region();
-        self.types.diagnostics = self.diagnostics.finish();
         self.types.shrink_to_fit();
         self.types
     }
@@ -4079,7 +4124,9 @@ impl<'db> TypeInferenceBuilder<'db> {
                 Ok(ty) => ty,
                 Err(nodes) => {
                     for node in nodes {
-                        self.diagnostics.add(
+                        report_type_diagnostic(
+                            self.db,
+                            self.file,
                             node.into(),
                             "invalid-literal-parameter",
                             format_args!(
@@ -4401,8 +4448,9 @@ fn perform_membership_test_comparison<'db>(
 
 #[cfg(test)]
 mod tests {
-
     use anyhow::Context;
+    use ruff_db::diagnostic::CompileDiagnostic;
+    use ruff_db::diagnostic::Diagnostic;
 
     use crate::db::tests::TestDb;
     use crate::program::{Program, SearchPathSettings};
@@ -4503,22 +4551,18 @@ mod tests {
     }
 
     #[track_caller]
-    fn assert_diagnostic_messages(diagnostics: &TypeCheckDiagnostics, expected: &[&str]) {
-        let messages: Vec<&str> = diagnostics
+    fn assert_file_diagnostics(db: &TestDb, filename: &str, expected: &[&str]) {
+        let file = system_path_to_file(db, filename).unwrap();
+
+        let diagnostics = check_types::accumulated::<CompileDiagnostic>(db, file);
+
+        let messages: Vec<_> = diagnostics
             .iter()
             .map(|diagnostic| diagnostic.message())
             .collect();
         assert_eq!(&messages, expected);
     }
 
-    #[track_caller]
-    fn assert_file_diagnostics(db: &TestDb, filename: &str, expected: &[&str]) {
-        let file = system_path_to_file(db, filename).unwrap();
-        let diagnostics = check_types(db, file);
-
-        assert_diagnostic_messages(&diagnostics, expected);
-    }
-
     #[test]
     fn from_import_with_no_module_name() -> anyhow::Result<()> {
         // This test checks that invalid syntax in a `StmtImportFrom` node
@@ -5061,27 +5105,6 @@ mod tests {
         Ok(())
     }
 
-    #[test]
-    fn exception_handler_with_invalid_syntax() -> anyhow::Result<()> {
-        let mut db = setup_db();
-
-        db.write_dedented(
-            "src/a.py",
-            "
-            from typing_extensions import reveal_type
-
-            try:
-                print
-            except as e:
-                reveal_type(e)
-            ",
-        )?;
-
-        assert_file_diagnostics(&db, "src/a.py", &["Revealed type is `Unknown`"]);
-
-        Ok(())
-    }
-
     #[test]
     fn basic_comprehension() -> anyhow::Result<()> {
         let mut db = setup_db();
@@ -5337,8 +5360,15 @@ mod tests {
 
         assert_scope_ty(&db, "src/a.py", &["foo", "<listcomp>"], "z", "Unknown");
 
-        // (There is a diagnostic for invalid syntax that's emitted, but it's not listed by `assert_file_diagnostics`)
-        assert_file_diagnostics(&db, "src/a.py", &["Name `z` used when not defined"]);
+        assert_file_diagnostics(
+            &db,
+            "src/a.py",
+            &[
+                "Expected an identifier, but found a keyword 'in' that cannot be used here",
+                "Expected 'in', found name",
+                "Name `z` used when not defined",
+            ],
+        );
 
         Ok(())
     }
@@ -5424,7 +5454,7 @@ mod tests {
                     return 42
 
             class Iterable:
-                def __iter__(self) -> Iterator:
+                def __iter__(self) -> Iterator: ...
 
             x = [*NotIterable()]
             y = [*Iterable()]

crates/red_knot_python_semantic/src/types/unpacker.rs

@@ -6,14 +6,14 @@ use rustc_hash::FxHashMap;
 
 use crate::semantic_index::ast_ids::{HasScopedAstId, ScopedExpressionId};
 use crate::semantic_index::symbol::ScopeId;
-use crate::types::{TupleType, Type, TypeCheckDiagnostics, TypeCheckDiagnosticsBuilder};
+use crate::types::{TupleType, Type};
 use crate::Db;
 
 /// Unpacks the value expression type to their respective targets.
 pub(crate) struct Unpacker<'db> {
     db: &'db dyn Db,
     targets: FxHashMap<ScopedExpressionId, Type<'db>>,
-    diagnostics: TypeCheckDiagnosticsBuilder<'db>,
+    file: File,
 }
 
 impl<'db> Unpacker<'db> {
@@ -21,7 +21,7 @@ impl<'db> Unpacker<'db> {
         Self {
             db,
             targets: FxHashMap::default(),
-            diagnostics: TypeCheckDiagnosticsBuilder::new(db, file),
+            file,
         }
     }
 
@@ -104,9 +104,11 @@ impl<'db> Unpacker<'db> {
                     let value_ty = if value_ty.is_literal_string() {
                         Type::LiteralString
                     } else {
-                        value_ty
-                            .iterate(self.db)
-                            .unwrap_with_diagnostic(AnyNodeRef::from(target), &mut self.diagnostics)
+                        value_ty.iterate(self.db).unwrap_with_diagnostic(
+                            self.db,
+                            AnyNodeRef::from(target),
+                            self.file,
+                        )
                     };
                     for element in elts {
                         self.unpack(element, value_ty, scope);
@@ -120,7 +122,6 @@ impl<'db> Unpacker<'db> {
     pub(crate) fn finish(mut self) -> UnpackResult<'db> {
         self.targets.shrink_to_fit();
         UnpackResult {
-            diagnostics: self.diagnostics.finish(),
             targets: self.targets,
         }
     }
@@ -129,15 +130,10 @@ impl<'db> Unpacker<'db> {
 #[derive(Debug, Default, PartialEq, Eq)]
 pub(crate) struct UnpackResult<'db> {
     targets: FxHashMap<ScopedExpressionId, Type<'db>>,
-    diagnostics: TypeCheckDiagnostics,
 }
 
 impl<'db> UnpackResult<'db> {
     pub(crate) fn get(&self, expr_id: ScopedExpressionId) -> Option<Type<'db>> {
         self.targets.get(&expr_id).copied()
     }
-
-    pub(crate) fn diagnostics(&self) -> &TypeCheckDiagnostics {
-        &self.diagnostics
-    }
 }

crates/red_knot_server/src/edit.rs

@@ -6,7 +6,7 @@ mod text_document;
 
 use lsp_types::{PositionEncodingKind, Url};
 pub use notebook::NotebookDocument;
-pub(crate) use range::RangeExt;
+pub(crate) use range::{RangeExt, ToRangeExt};
 pub(crate) use text_document::DocumentVersion;
 pub use text_document::TextDocument;
 

crates/red_knot_server/src/edit/range.rs

@@ -1,13 +1,32 @@
+use super::notebook;
 use super::PositionEncoding;
-use ruff_source_file::LineIndex;
+use lsp_types as types;
+use ruff_notebook::NotebookIndex;
 use ruff_source_file::OneIndexed;
+use ruff_source_file::{LineIndex, SourceLocation};
 use ruff_text_size::{TextRange, TextSize};
 
+pub(crate) struct NotebookRange {
+    pub(crate) cell: notebook::CellId,
+    pub(crate) range: types::Range,
+}
+
 pub(crate) trait RangeExt {
     fn to_text_range(&self, text: &str, index: &LineIndex, encoding: PositionEncoding)
         -> TextRange;
 }
 
+pub(crate) trait ToRangeExt {
+    fn to_range(&self, text: &str, index: &LineIndex, encoding: PositionEncoding) -> types::Range;
+    fn to_notebook_range(
+        &self,
+        text: &str,
+        source_index: &LineIndex,
+        notebook_index: &NotebookIndex,
+        encoding: PositionEncoding,
+    ) -> NotebookRange;
+}
+
 fn u32_index_to_usize(index: u32) -> usize {
     usize::try_from(index).expect("u32 fits in usize")
 }
@@ -75,6 +94,61 @@ impl RangeExt for lsp_types::Range {
     }
 }
 
+impl ToRangeExt for TextRange {
+    fn to_range(&self, text: &str, index: &LineIndex, encoding: PositionEncoding) -> types::Range {
+        types::Range {
+            start: source_location_to_position(&offset_to_source_location(
+                self.start(),
+                text,
+                index,
+                encoding,
+            )),
+            end: source_location_to_position(&offset_to_source_location(
+                self.end(),
+                text,
+                index,
+                encoding,
+            )),
+        }
+    }
+
+    fn to_notebook_range(
+        &self,
+        text: &str,
+        source_index: &LineIndex,
+        notebook_index: &NotebookIndex,
+        encoding: PositionEncoding,
+    ) -> NotebookRange {
+        let start = offset_to_source_location(self.start(), text, source_index, encoding);
+        let mut end = offset_to_source_location(self.end(), text, source_index, encoding);
+        let starting_cell = notebook_index.cell(start.row);
+
+        // weird edge case here - if the end of the range is where the newline after the cell got added (making it 'out of bounds')
+        // we need to move it one character back (which should place it at the end of the last line).
+        // we test this by checking if the ending offset is in a different (or nonexistent) cell compared to the cell of the starting offset.
+        if notebook_index.cell(end.row) != starting_cell {
+            end.row = end.row.saturating_sub(1);
+            end.column = offset_to_source_location(
+                self.end().checked_sub(1.into()).unwrap_or_default(),
+                text,
+                source_index,
+                encoding,
+            )
+            .column;
+        }
+
+        let start = source_location_to_position(&notebook_index.translate_location(&start));
+        let end = source_location_to_position(&notebook_index.translate_location(&end));
+
+        NotebookRange {
+            cell: starting_cell
+                .map(OneIndexed::to_zero_indexed)
+                .unwrap_or_default(),
+            range: types::Range { start, end },
+        }
+    }
+}
+
 /// Converts a UTF-16 code unit offset for a given line into a UTF-8 column number.
 fn utf8_column_offset(utf16_code_unit_offset: u32, line: &str) -> TextSize {
     let mut utf8_code_unit_offset = TextSize::new(0);
@@ -96,3 +170,46 @@ fn utf8_column_offset(utf16_code_unit_offset: u32, line: &str) -> TextSize {
 
     utf8_code_unit_offset
 }
+
+fn offset_to_source_location(
+    offset: TextSize,
+    text: &str,
+    index: &LineIndex,
+    encoding: PositionEncoding,
+) -> SourceLocation {
+    match encoding {
+        PositionEncoding::UTF8 => {
+            let row = index.line_index(offset);
+            let column = offset - index.line_start(row, text);
+
+            SourceLocation {
+                column: OneIndexed::from_zero_indexed(column.to_usize()),
+                row,
+            }
+        }
+        PositionEncoding::UTF16 => {
+            let row = index.line_index(offset);
+
+            let column = if index.is_ascii() {
+                (offset - index.line_start(row, text)).to_usize()
+            } else {
+                let up_to_line = &text[TextRange::new(index.line_start(row, text), offset)];
+                up_to_line.encode_utf16().count()
+            };
+
+            SourceLocation {
+                column: OneIndexed::from_zero_indexed(column),
+                row,
+            }
+        }
+        PositionEncoding::UTF32 => index.source_location(offset, text),
+    }
+}
+
+fn source_location_to_position(location: &SourceLocation) -> types::Position {
+    types::Position {
+        line: u32::try_from(location.row.to_zero_indexed()).expect("row usize fits in u32"),
+        character: u32::try_from(location.column.to_zero_indexed())
+            .expect("character usize fits in u32"),
+    }
+}

crates/red_knot_server/src/server/api/requests/diagnostic.rs

@@ -3,12 +3,15 @@ use std::borrow::Cow;
 use lsp_types::request::DocumentDiagnosticRequest;
 use lsp_types::{
     Diagnostic, DiagnosticSeverity, DocumentDiagnosticParams, DocumentDiagnosticReport,
-    DocumentDiagnosticReportResult, FullDocumentDiagnosticReport, Position, Range,
+    DocumentDiagnosticReportResult, FullDocumentDiagnosticReport, NumberOrString, Range,
     RelatedFullDocumentDiagnosticReport, Url,
 };
 
-use red_knot_workspace::db::RootDatabase;
+use red_knot_workspace::db::{Db, RootDatabase};
+use ruff_db::diagnostic::{CompileDiagnostic, Diagnostic as _, Severity};
+use ruff_db::source::{line_index, source_text};
 
+use crate::edit::ToRangeExt as _;
 use crate::server::api::traits::{BackgroundDocumentRequestHandler, RequestHandler};
 use crate::server::{client::Notifier, Result};
 use crate::session::DocumentSnapshot;
@@ -64,36 +67,37 @@ fn compute_diagnostics(snapshot: &DocumentSnapshot, db: &RootDatabase) -> Vec<Di
     diagnostics
         .as_slice()
         .iter()
-        .map(|message| to_lsp_diagnostic(message))
+        .map(|message| to_lsp_diagnostic(db, message, snapshot.encoding()))
         .collect()
 }
 
-fn to_lsp_diagnostic(message: &str) -> Diagnostic {
-    let words = message.split(':').collect::<Vec<_>>();
+fn to_lsp_diagnostic(
+    db: &dyn Db,
+    diagnostic: &CompileDiagnostic,
+    encoding: crate::PositionEncoding,
+) -> Diagnostic {
+    let range = if let Some(range) = diagnostic.range() {
+        let index = line_index(db.upcast(), diagnostic.file());
+        let source = source_text(db.upcast(), diagnostic.file());
 
-    let (range, message) = match words.as_slice() {
-        [_, _, line, column, message] | [_, line, column, message] => {
-            let line = line.parse::<u32>().unwrap_or_default().saturating_sub(1);
-            let column = column.parse::<u32>().unwrap_or_default();
-            (
-                Range::new(
-                    Position::new(line, column.saturating_sub(1)),
-                    Position::new(line, column),
-                ),
-                message.trim(),
-            )
-        }
-        _ => (Range::default(), message),
+        range.to_range(&source, &index, encoding)
+    } else {
+        Range::default()
+    };
+
+    let severity = match diagnostic.severity() {
+        Severity::Info => DiagnosticSeverity::INFORMATION,
+        Severity::Error => DiagnosticSeverity::ERROR,
     };
 
     Diagnostic {
         range,
-        severity: Some(DiagnosticSeverity::ERROR),
+        severity: Some(severity),
         tags: None,
-        code: None,
+        code: Some(NumberOrString::String(diagnostic.rule().to_string())),
         code_description: None,
         source: Some("red-knot".into()),
-        message: message.to_string(),
+        message: diagnostic.message().into_owned(),
         related_information: None,
         data: None,
     }

crates/red_knot_test/src/diagnostic.rs

@@ -2,8 +2,8 @@
 //!
 //! We don't assume that we will get the diagnostics in source order.
 
+use ruff_db::diagnostic::Diagnostic;
 use ruff_source_file::{LineIndex, OneIndexed};
-use ruff_text_size::Ranged;
 use std::ops::{Deref, Range};
 
 /// All diagnostics for one embedded Python file, sorted and grouped by start line number.
@@ -19,13 +19,16 @@ pub(crate) struct SortedDiagnostics<T> {
 
 impl<T> SortedDiagnostics<T>
 where
-    T: Ranged + Clone,
+    T: Diagnostic,
 {
     pub(crate) fn new(diagnostics: impl IntoIterator<Item = T>, line_index: &LineIndex) -> Self {
         let mut diagnostics: Vec<_> = diagnostics
             .into_iter()
             .map(|diagnostic| DiagnosticWithLine {
-                line_number: line_index.line_index(diagnostic.start()),
+                line_number: diagnostic
+                    .range()
+                    .map(|range| line_index.line_index(range.start()))
+                    .unwrap_or(OneIndexed::from_zero_indexed(0)),
                 diagnostic,
             })
             .collect();
@@ -94,7 +97,7 @@ pub(crate) struct LineDiagnosticsIterator<'a, T> {
 
 impl<'a, T> Iterator for LineDiagnosticsIterator<'a, T>
 where
-    T: Ranged + Clone,
+    T: Diagnostic,
 {
     type Item = LineDiagnostics<'a, T>;
 
@@ -110,7 +113,7 @@ where
     }
 }
 
-impl<T> std::iter::FusedIterator for LineDiagnosticsIterator<'_, T> where T: Clone + Ranged {}
+impl<T> std::iter::FusedIterator for LineDiagnosticsIterator<'_, T> where T: Diagnostic {}
 
 /// All diagnostics that start on a single line of source code in one embedded Python file.
 #[derive(Debug)]
@@ -139,11 +142,13 @@ struct DiagnosticWithLine<T> {
 #[cfg(test)]
 mod tests {
     use crate::db::Db;
-    use ruff_db::files::system_path_to_file;
+    use ruff_db::diagnostic::{Diagnostic, Severity};
+    use ruff_db::files::{system_path_to_file, File};
     use ruff_db::source::line_index;
     use ruff_db::system::{DbWithTestSystem, SystemPathBuf};
     use ruff_source_file::OneIndexed;
     use ruff_text_size::{TextRange, TextSize};
+    use std::fmt::Debug;
 
     #[test]
     fn sort_and_group() {
@@ -152,13 +157,18 @@ mod tests {
         let file = system_path_to_file(&db, "/src/test.py").unwrap();
         let lines = line_index(&db, file);
 
-        let ranges = vec![
+        let ranges = [
             TextRange::new(TextSize::new(0), TextSize::new(1)),
             TextRange::new(TextSize::new(5), TextSize::new(10)),
             TextRange::new(TextSize::new(1), TextSize::new(7)),
         ];
 
-        let sorted = super::SortedDiagnostics::new(&ranges, &lines);
+        let diagnostics: Vec<_> = ranges
+            .into_iter()
+            .map(|range| DummyDiagnostic { file, range })
+            .collect();
+
+        let sorted = super::SortedDiagnostics::new(diagnostics, &lines);
         let grouped = sorted.iter_lines().collect::<Vec<_>>();
 
         let [line1, line2] = &grouped[..] else {
@@ -170,4 +180,32 @@ mod tests {
         assert_eq!(line2.line_number, OneIndexed::from_zero_indexed(1));
         assert_eq!(line2.diagnostics.len(), 1);
     }
+
+    #[derive(Debug)]
+    struct DummyDiagnostic {
+        file: File,
+        range: TextRange,
+    }
+
+    impl Diagnostic for DummyDiagnostic {
+        fn rule(&self) -> &str {
+            "dummy"
+        }
+
+        fn message(&self) -> std::borrow::Cow<str> {
+            "Dummy error".into()
+        }
+
+        fn file(&self) -> File {
+            self.file
+        }
+
+        fn range(&self) -> Option<TextRange> {
+            Some(self.range)
+        }
+
+        fn severity(&self) -> Severity {
+            Severity::Error
+        }
+    }
 }

crates/red_knot_test/src/lib.rs

@@ -1,8 +1,8 @@
 use colored::Colorize;
 use parser as test_parser;
 use red_knot_python_semantic::types::check_types;
+use ruff_db::diagnostic::{CompileDiagnostic, Diagnostic as _};
 use ruff_db::files::{system_path_to_file, File, Files};
-use ruff_db::parsed::parsed_module;
 use ruff_db::system::{DbWithTestSystem, SystemPathBuf};
 use ruff_source_file::LineIndex;
 use ruff_text_size::TextSize;
@@ -85,18 +85,15 @@ fn run_test(db: &mut db::Db, test: &parser::MarkdownTest) -> Result<(), Failures
     let failures: Failures = test_files
         .into_iter()
         .filter_map(|test_file| {
-            let parsed = parsed_module(db, test_file.file);
+            let diagnostics: Vec<_> =
+                // The accumulator returns all diagnostics from all files. We're only interested in
+                // diagnostics from this file.
+                check_types::accumulated::<CompileDiagnostic>(db, test_file.file)
+                    .into_iter()
+                    .filter(|diagnostic| diagnostic.file() == test_file.file)
+                    .collect();
 
-            // TODO allow testing against code with syntax errors
-            assert!(
-                parsed.errors().is_empty(),
-                "Python syntax errors in {}, {}: {:?}",
-                test.name(),
-                test_file.file.path(db),
-                parsed.errors()
-            );
-
-            match matcher::match_file(db, test_file.file, check_types(db, test_file.file)) {
+            match matcher::match_file(db, test_file.file, diagnostics) {
                 Ok(()) => None,
                 Err(line_failures) => Some(FileFailures {
                     backtick_offset: test_file.backtick_offset,

crates/red_knot_test/src/matcher.rs

@@ -1,17 +1,15 @@
-//! Match [`TypeCheckDiagnostic`]s against [`Assertion`]s and produce test failure messages for any
+//! Match [`Diagnostic`]s against [`Assertion`]s and produce test failure messages for any
 //! mismatches.
 use crate::assertion::{Assertion, ErrorAssertion, InlineFileAssertions};
 use crate::db::Db;
 use crate::diagnostic::SortedDiagnostics;
 use colored::Colorize;
-use red_knot_python_semantic::types::TypeCheckDiagnostic;
+use ruff_db::diagnostic::Diagnostic;
 use ruff_db::files::File;
 use ruff_db::source::{line_index, source_text, SourceText};
 use ruff_source_file::{LineIndex, OneIndexed};
-use ruff_text_size::Ranged;
 use std::cmp::Ordering;
 use std::ops::Range;
-use std::sync::Arc;
 
 #[derive(Debug, Default)]
 pub(super) struct FailuresByLine {
@@ -55,7 +53,7 @@ pub(super) fn match_file<T>(
     diagnostics: impl IntoIterator<Item = T>,
 ) -> Result<(), FailuresByLine>
 where
-    T: Diagnostic + Clone,
+    T: Diagnostic,
 {
     // Parse assertions from comments in the file, and get diagnostics from the file; both
     // ordered by line number.
@@ -126,22 +124,6 @@ where
     }
 }
 
-pub(super) trait Diagnostic: Ranged {
-    fn rule(&self) -> &str;
-
-    fn message(&self) -> &str;
-}
-
-impl Diagnostic for Arc<TypeCheckDiagnostic> {
-    fn rule(&self) -> &str {
-        self.as_ref().rule()
-    }
-
-    fn message(&self) -> &str {
-        self.as_ref().message()
-    }
-}
-
 trait Unmatched {
     fn unmatched(&self) -> String;
 }
@@ -253,10 +235,15 @@ impl Matcher {
         }
     }
 
-    fn column<T: Ranged>(&self, ranged: &T) -> OneIndexed {
-        self.line_index
-            .source_location(ranged.start(), &self.source)
-            .column
+    fn column<T: Diagnostic>(&self, diagnostic: &T) -> OneIndexed {
+        diagnostic
+            .range()
+            .map(|range| {
+                self.line_index
+                    .source_location(range.start(), &self.source)
+                    .column
+            })
+            .unwrap_or(OneIndexed::from_zero_indexed(0))
     }
 
     /// Check if `assertion` matches any [`Diagnostic`]s in `unmatched`.
@@ -323,20 +310,22 @@ impl Matcher {
 #[cfg(test)]
 mod tests {
     use super::FailuresByLine;
-    use ruff_db::files::system_path_to_file;
+    use ruff_db::diagnostic::{Diagnostic, Severity};
+    use ruff_db::files::{system_path_to_file, File};
     use ruff_db::system::{DbWithTestSystem, SystemPathBuf};
     use ruff_python_trivia::textwrap::dedent;
     use ruff_source_file::OneIndexed;
-    use ruff_text_size::{Ranged, TextRange};
+    use ruff_text_size::TextRange;
+    use std::borrow::Cow;
 
     #[derive(Clone, Debug)]
-    struct TestDiagnostic {
+    struct ExpectedDiagnostic {
         rule: &'static str,
         message: &'static str,
         range: TextRange,
     }
 
-    impl TestDiagnostic {
+    impl ExpectedDiagnostic {
         fn new(rule: &'static str, message: &'static str, offset: usize) -> Self {
             let offset: u32 = offset.try_into().unwrap();
             Self {
@@ -345,34 +334,66 @@ mod tests {
                 range: TextRange::new(offset.into(), (offset + 1).into()),
             }
         }
+
+        fn into_diagnostic(self, file: File) -> TestDiagnostic {
+            TestDiagnostic {
+                file,
+                rule: self.rule,
+                message: self.message,
+                range: self.range,
+            }
+        }
     }
 
-    impl super::Diagnostic for TestDiagnostic {
+    #[derive(Debug)]
+    struct TestDiagnostic {
+        file: File,
+        rule: &'static str,
+        message: &'static str,
+        range: TextRange,
+    }
+
+    impl Diagnostic for TestDiagnostic {
         fn rule(&self) -> &str {
             self.rule
         }
 
-        fn message(&self) -> &str {
-            self.message
+        fn message(&self) -> Cow<str> {
+            Cow::Borrowed(self.message)
+        }
+
+        fn file(&self) -> File {
+            self.file
+        }
+
+        fn range(&self) -> Option<TextRange> {
+            Some(self.range)
+        }
+
+        fn severity(&self) -> Severity {
+            Severity::Error
         }
     }
 
-    impl Ranged for TestDiagnostic {
-        fn range(&self) -> ruff_text_size::TextRange {
-            self.range
-        }
-    }
-
-    fn get_result(source: &str, diagnostics: Vec<TestDiagnostic>) -> Result<(), FailuresByLine> {
+    fn get_result(
+        source: &str,
+        diagnostics: Vec<ExpectedDiagnostic>,
+    ) -> Result<(), FailuresByLine> {
         colored::control::set_override(false);
 
         let mut db = crate::db::Db::setup(SystemPathBuf::from("/src"));
         db.write_file("/src/test.py", source).unwrap();
         let file = system_path_to_file(&db, "/src/test.py").unwrap();
 
+        let diagnostics: Vec<_> = diagnostics
+            .into_iter()
+            .map(|diagnostic| diagnostic.into_diagnostic(file))
+            .collect();
+
         super::match_file(&db, file, diagnostics)
     }
 
+    #[track_caller]
     fn assert_fail(result: Result<(), FailuresByLine>, messages: &[(usize, &[&str])]) {
         let Err(failures) = result else {
             panic!("expected a failure");
@@ -395,6 +416,7 @@ mod tests {
         assert_eq!(failures, expected);
     }
 
+    #[track_caller]
     fn assert_ok(result: &Result<(), FailuresByLine>) {
         assert!(result.is_ok(), "{result:?}");
     }
@@ -403,7 +425,7 @@ mod tests {
     fn revealed_match() {
         let result = get_result(
             "x # revealed: Foo",
-            vec![TestDiagnostic::new(
+            vec![ExpectedDiagnostic::new(
                 "revealed-type",
                 "Revealed type is `Foo`",
                 0,
@@ -417,7 +439,7 @@ mod tests {
     fn revealed_wrong_rule() {
         let result = get_result(
             "x # revealed: Foo",
-            vec![TestDiagnostic::new(
+            vec![ExpectedDiagnostic::new(
                 "not-revealed-type",
                 "Revealed type is `Foo`",
                 0,
@@ -440,7 +462,11 @@ mod tests {
     fn revealed_wrong_message() {
         let result = get_result(
             "x # revealed: Foo",
-            vec![TestDiagnostic::new("revealed-type", "Something else", 0)],
+            vec![ExpectedDiagnostic::new(
+                "revealed-type",
+                "Something else",
+                0,
+            )],
         );
 
         assert_fail(
@@ -467,8 +493,8 @@ mod tests {
         let result = get_result(
             "x # revealed: Foo",
             vec![
-                TestDiagnostic::new("revealed-type", "Revealed type is `Foo`", 0),
-                TestDiagnostic::new("undefined-reveal", "Doesn't matter", 0),
+                ExpectedDiagnostic::new("revealed-type", "Revealed type is `Foo`", 0),
+                ExpectedDiagnostic::new("undefined-reveal", "Doesn't matter", 0),
             ],
         );
 
@@ -479,7 +505,11 @@ mod tests {
     fn revealed_match_with_only_undefined() {
         let result = get_result(
             "x # revealed: Foo",
-            vec![TestDiagnostic::new("undefined-reveal", "Doesn't matter", 0)],
+            vec![ExpectedDiagnostic::new(
+                "undefined-reveal",
+                "Doesn't matter",
+                0,
+            )],
         );
 
         assert_fail(result, &[(0, &["unmatched assertion: revealed: Foo"])]);
@@ -490,8 +520,8 @@ mod tests {
         let result = get_result(
             "x # revealed: Foo",
             vec![
-                TestDiagnostic::new("revealed-type", "Revealed type is `Bar`", 0),
-                TestDiagnostic::new("undefined-reveal", "Doesn't matter", 0),
+                ExpectedDiagnostic::new("revealed-type", "Revealed type is `Bar`", 0),
+                ExpectedDiagnostic::new("undefined-reveal", "Doesn't matter", 0),
             ],
         );
 
@@ -512,8 +542,8 @@ mod tests {
         let result = get_result(
             "reveal_type(1)",
             vec![
-                TestDiagnostic::new("undefined-reveal", "undefined reveal message", 0),
-                TestDiagnostic::new("revealed-type", "Revealed type is `Literal[1]`", 12),
+                ExpectedDiagnostic::new("undefined-reveal", "undefined reveal message", 0),
+                ExpectedDiagnostic::new("revealed-type", "Revealed type is `Literal[1]`", 12),
             ],
         );
 
@@ -535,8 +565,8 @@ mod tests {
         let result = get_result(
             "reveal_type(1) # error: [something-else]",
             vec![
-                TestDiagnostic::new("undefined-reveal", "undefined reveal message", 0),
-                TestDiagnostic::new("revealed-type", "Revealed type is `Literal[1]`", 12),
+                ExpectedDiagnostic::new("undefined-reveal", "undefined reveal message", 0),
+                ExpectedDiagnostic::new("revealed-type", "Revealed type is `Literal[1]`", 12),
             ],
         );
 
@@ -565,7 +595,7 @@ mod tests {
     fn error_match_rule() {
         let result = get_result(
             "x # error: [some-rule]",
-            vec![TestDiagnostic::new("some-rule", "Any message", 0)],
+            vec![ExpectedDiagnostic::new("some-rule", "Any message", 0)],
         );
 
         assert_ok(&result);
@@ -575,7 +605,7 @@ mod tests {
     fn error_wrong_rule() {
         let result = get_result(
             "x # error: [some-rule]",
-            vec![TestDiagnostic::new("anything", "Any message", 0)],
+            vec![ExpectedDiagnostic::new("anything", "Any message", 0)],
         );
 
         assert_fail(
@@ -594,7 +624,11 @@ mod tests {
     fn error_match_message() {
         let result = get_result(
             r#"x # error: "contains this""#,
-            vec![TestDiagnostic::new("anything", "message contains this", 0)],
+            vec![ExpectedDiagnostic::new(
+                "anything",
+                "message contains this",
+                0,
+            )],
         );
 
         assert_ok(&result);
@@ -604,7 +638,7 @@ mod tests {
     fn error_wrong_message() {
         let result = get_result(
             r#"x # error: "contains this""#,
-            vec![TestDiagnostic::new("anything", "Any message", 0)],
+            vec![ExpectedDiagnostic::new("anything", "Any message", 0)],
         );
 
         assert_fail(
@@ -623,7 +657,7 @@ mod tests {
     fn error_match_column_and_rule() {
         let result = get_result(
             "x # error: 1 [some-rule]",
-            vec![TestDiagnostic::new("some-rule", "Any message", 0)],
+            vec![ExpectedDiagnostic::new("some-rule", "Any message", 0)],
         );
 
         assert_ok(&result);
@@ -633,7 +667,7 @@ mod tests {
     fn error_wrong_column() {
         let result = get_result(
             "x # error: 2 [rule]",
-            vec![TestDiagnostic::new("rule", "Any message", 0)],
+            vec![ExpectedDiagnostic::new("rule", "Any message", 0)],
         );
 
         assert_fail(
@@ -652,7 +686,11 @@ mod tests {
     fn error_match_column_and_message() {
         let result = get_result(
             r#"x # error: 1 "contains this""#,
-            vec![TestDiagnostic::new("anything", "message contains this", 0)],
+            vec![ExpectedDiagnostic::new(
+                "anything",
+                "message contains this",
+                0,
+            )],
         );
 
         assert_ok(&result);
@@ -662,7 +700,11 @@ mod tests {
     fn error_match_rule_and_message() {
         let result = get_result(
             r#"x # error: [a-rule] "contains this""#,
-            vec![TestDiagnostic::new("a-rule", "message contains this", 0)],
+            vec![ExpectedDiagnostic::new(
+                "a-rule",
+                "message contains this",
+                0,
+            )],
         );
 
         assert_ok(&result);
@@ -672,7 +714,11 @@ mod tests {
     fn error_match_all() {
         let result = get_result(
             r#"x # error: 1 [a-rule] "contains this""#,
-            vec![TestDiagnostic::new("a-rule", "message contains this", 0)],
+            vec![ExpectedDiagnostic::new(
+                "a-rule",
+                "message contains this",
+                0,
+            )],
         );
 
         assert_ok(&result);
@@ -682,7 +728,11 @@ mod tests {
     fn error_match_all_wrong_column() {
         let result = get_result(
             r#"x # error: 2 [some-rule] "contains this""#,
-            vec![TestDiagnostic::new("some-rule", "message contains this", 0)],
+            vec![ExpectedDiagnostic::new(
+                "some-rule",
+                "message contains this",
+                0,
+            )],
         );
 
         assert_fail(
@@ -701,7 +751,7 @@ mod tests {
     fn error_match_all_wrong_rule() {
         let result = get_result(
             r#"x # error: 1 [some-rule] "contains this""#,
-            vec![TestDiagnostic::new(
+            vec![ExpectedDiagnostic::new(
                 "other-rule",
                 "message contains this",
                 0,
@@ -724,7 +774,7 @@ mod tests {
     fn error_match_all_wrong_message() {
         let result = get_result(
             r#"x # error: 1 [some-rule] "contains this""#,
-            vec![TestDiagnostic::new("some-rule", "Any message", 0)],
+            vec![ExpectedDiagnostic::new("some-rule", "Any message", 0)],
         );
 
         assert_fail(
@@ -757,9 +807,9 @@ mod tests {
         let result = get_result(
             &source,
             vec![
-                TestDiagnostic::new("line-two", "msg", two),
-                TestDiagnostic::new("line-three", "msg", three),
-                TestDiagnostic::new("line-five", "msg", five),
+                ExpectedDiagnostic::new("line-two", "msg", two),
+                ExpectedDiagnostic::new("line-three", "msg", three),
+                ExpectedDiagnostic::new("line-five", "msg", five),
             ],
         );
 
@@ -788,8 +838,8 @@ mod tests {
         let result = get_result(
             &source,
             vec![
-                TestDiagnostic::new("line-one", "msg", one),
-                TestDiagnostic::new("line-two", "msg", two),
+                ExpectedDiagnostic::new("line-one", "msg", one),
+                ExpectedDiagnostic::new("line-two", "msg", two),
             ],
         );
 
@@ -809,8 +859,8 @@ mod tests {
         let result = get_result(
             &source,
             vec![
-                TestDiagnostic::new("one-rule", "msg", x),
-                TestDiagnostic::new("other-rule", "msg", x),
+                ExpectedDiagnostic::new("one-rule", "msg", x),
+                ExpectedDiagnostic::new("other-rule", "msg", x),
             ],
         );
 
@@ -830,8 +880,8 @@ mod tests {
         let result = get_result(
             &source,
             vec![
-                TestDiagnostic::new("one-rule", "msg", x),
-                TestDiagnostic::new("one-rule", "msg", x),
+                ExpectedDiagnostic::new("one-rule", "msg", x),
+                ExpectedDiagnostic::new("one-rule", "msg", x),
             ],
         );
 
@@ -851,9 +901,9 @@ mod tests {
         let result = get_result(
             &source,
             vec![
-                TestDiagnostic::new("one-rule", "msg", x),
-                TestDiagnostic::new("other-rule", "msg", x),
-                TestDiagnostic::new("third-rule", "msg", x),
+                ExpectedDiagnostic::new("one-rule", "msg", x),
+                ExpectedDiagnostic::new("other-rule", "msg", x),
+                ExpectedDiagnostic::new("third-rule", "msg", x),
             ],
         );
 
@@ -877,8 +927,8 @@ mod tests {
         let result = get_result(
             &source,
             vec![
-                TestDiagnostic::new("undefined-reveal", "msg", reveal),
-                TestDiagnostic::new("revealed-type", "Revealed type is `Literal[5]`", reveal),
+                ExpectedDiagnostic::new("undefined-reveal", "msg", reveal),
+                ExpectedDiagnostic::new("revealed-type", "Revealed type is `Literal[5]`", reveal),
             ],
         );
 
@@ -891,7 +941,7 @@ mod tests {
         let x = source.find('x').unwrap();
         let result = get_result(
             source,
-            vec![TestDiagnostic::new("some-rule", "some message", x)],
+            vec![ExpectedDiagnostic::new("some-rule", "some message", x)],
         );
 
         assert_fail(
@@ -912,7 +962,7 @@ mod tests {
         let x = source.find('x').unwrap();
         let result = get_result(
             source,
-            vec![TestDiagnostic::new("some-rule", "some message", x)],
+            vec![ExpectedDiagnostic::new("some-rule", "some message", x)],
         );
 
         assert_fail(

crates/red_knot_wasm/src/lib.rs

@@ -1,9 +1,10 @@
 use std::any::Any;
 
 use js_sys::Error;
+use ruff_db::diagnostic::CompileDiagnostic;
 use wasm_bindgen::prelude::*;
 
-use red_knot_workspace::db::RootDatabase;
+use red_knot_workspace::db::{Db, RootDatabase};
 use red_knot_workspace::workspace::settings::Configuration;
 use red_knot_workspace::workspace::WorkspaceMetadata;
 use ruff_db::files::{system_path_to_file, File};
@@ -110,14 +111,14 @@ impl Workspace {
     pub fn check_file(&self, file_id: &FileHandle) -> Result<Vec<String>, Error> {
         let result = self.db.check_file(file_id.file).map_err(into_error)?;
 
-        Ok(result)
+        Ok(map_diagnostics(&self.db, result))
     }
 
     /// Checks all open files
     pub fn check(&self) -> Result<Vec<String>, Error> {
         let result = self.db.check().map_err(into_error)?;
 
-        Ok(result)
+        Ok(map_diagnostics(&self.db, result))
     }
 
     /// Returns the parsed AST for `path`
@@ -142,6 +143,13 @@ impl Workspace {
     }
 }
 
+fn map_diagnostics(db: &dyn Db, diagnostics: Vec<CompileDiagnostic>) -> Vec<String> {
+    diagnostics
+        .into_iter()
+        .map(|diagnostic| diagnostic.display(db.upcast()).to_string())
+        .collect()
+}
+
 pub(crate) fn into_error<E: std::fmt::Display>(err: E) -> Error {
     Error::new(&err.to_string())
 }

crates/red_knot_wasm/tests/api.rs

@@ -19,6 +19,6 @@ fn check() {
 
     assert_eq!(
         result,
-        vec!["/test.py:1:8: Cannot resolve import `random22`"]
+        vec!["error[unresolved-import] /test.py:1:8 Cannot resolve import `random22`"]
     );
 }

crates/red_knot_workspace/Cargo.toml

@@ -17,7 +17,6 @@ red_knot_python_semantic = { workspace = true }
 ruff_cache = { workspace = true }
 ruff_db = { workspace = true, features = ["os", "cache"] }
 ruff_python_ast = { workspace = true }
-ruff_text_size = { workspace = true }
 red_knot_vendored = { workspace = true }
 
 anyhow = { workspace = true }

crates/red_knot_workspace/src/db.rs

@@ -1,6 +1,7 @@
 use std::panic::RefUnwindSafe;
 use std::sync::Arc;
 
+use ruff_db::diagnostic::CompileDiagnostic;
 use salsa::plumbing::ZalsaDatabase;
 use salsa::{Cancelled, Event};
 
@@ -10,7 +11,7 @@ use ruff_db::system::System;
 use ruff_db::vendored::VendoredFileSystem;
 use ruff_db::{Db as SourceDb, Upcast};
 
-use crate::workspace::{check_file, Workspace, WorkspaceMetadata};
+use crate::workspace::{Workspace, WorkspaceMetadata};
 
 mod changes;
 
@@ -51,14 +52,14 @@ impl RootDatabase {
     }
 
     /// Checks all open files in the workspace and its dependencies.
-    pub fn check(&self) -> Result<Vec<String>, Cancelled> {
+    pub fn check(&self) -> Result<Vec<CompileDiagnostic>, Cancelled> {
         self.with_db(|db| db.workspace().check(db))
     }
 
-    pub fn check_file(&self, file: File) -> Result<Vec<String>, Cancelled> {
+    pub fn check_file(&self, file: File) -> Result<Vec<CompileDiagnostic>, Cancelled> {
         let _span = tracing::debug_span!("check_file", file=%file.path(self)).entered();
 
-        self.with_db(|db| check_file(db, file))
+        self.with_db(|db| db.workspace().check_file(db, file))
     }
 
     /// Returns a mutable reference to the system.

crates/red_knot_workspace/src/workspace.rs

@@ -1,19 +1,18 @@
 use std::{collections::BTreeMap, sync::Arc};
 
+use ruff_db::diagnostic::{CompileDiagnostic, Diagnostic};
 use rustc_hash::{FxBuildHasher, FxHashSet};
 use salsa::{Durability, Setter as _};
 
 pub use metadata::{PackageMetadata, WorkspaceMetadata};
 use red_knot_python_semantic::types::check_types;
 use red_knot_python_semantic::SearchPathSettings;
-use ruff_db::parsed::parsed_module;
-use ruff_db::source::{line_index, source_text, SourceDiagnostic};
+use ruff_db::source::source_text;
 use ruff_db::{
     files::{system_path_to_file, File},
     system::{walk_directory::WalkState, SystemPath, SystemPathBuf},
 };
 use ruff_python_ast::{name::Name, PySourceType};
-use ruff_text_size::Ranged;
 
 use crate::db::Db;
 use crate::db::RootDatabase;
@@ -188,7 +187,7 @@ impl Workspace {
     }
 
     /// Checks all open files in the workspace and its dependencies.
-    pub fn check(self, db: &RootDatabase) -> Vec<String> {
+    pub fn check(self, db: &RootDatabase) -> Vec<CompileDiagnostic> {
         let workspace_span = tracing::debug_span!("check_workspace");
         let _span = workspace_span.enter();
 
@@ -200,6 +199,13 @@ impl Workspace {
         let db = db.snapshot();
         let workspace_span = workspace_span.clone();
 
+        // TODO: Checking per-file and then filtering out the diagnostics from other files
+        // isn't the ideal solution but doing it "properly" requires support for
+        // ["parallel Salsa"](https://github.com/salsa-rs/salsa/pull/568).
+        //
+        // The solution with parallel Salsa is:
+        // * Create a new `check_workspace_impl` query similar to `check_file_impl`
+        // * Collect the workspace diagnostics using `check_workspace_impl::accumulated::<CompileDiagnostic>(&db, file)`
         rayon::scope(move |scope| {
             for file in &files {
                 let result = inner_result.clone();
@@ -210,8 +216,17 @@ impl Workspace {
                     let check_file_span = tracing::debug_span!(parent: &workspace_span, "check_file", file=%file.path(&db));
                     let _entered = check_file_span.entered();
 
-                    let file_diagnostics = check_file(&db, file);
-                    result.lock().unwrap().extend(file_diagnostics);
+                    // Filter out the diagnostics from other files to avoid duplicates.
+                    // This should no longer be necessary with parallel-salsa where
+                    // it's possible to call the query for the entire workspace.
+                    let mut file_diagnostics =
+                        check_file_impl::accumulated::<CompileDiagnostic>(&db, file);
+
+                    file_diagnostics.sort_unstable_by_key(|a| {
+                        a.range().unwrap_or_default().start()
+                    });
+
+                    result.lock().unwrap().extend(file_diagnostics.into_iter().filter(|diagnostic| diagnostic.file() == file));
                 });
             }
         });
@@ -219,6 +234,20 @@ impl Workspace {
         Arc::into_inner(result).unwrap().into_inner().unwrap()
     }
 
+    pub fn check_file(self, db: &dyn Db, file: File) -> Vec<CompileDiagnostic> {
+        let diagnostics = check_file_impl::accumulated::<CompileDiagnostic>(db, file);
+
+        let mut file_diagnostics: Vec<_> = diagnostics
+            .into_iter()
+            .filter(|diagnostic| diagnostic.file() == file)
+            .collect();
+
+        file_diagnostics
+            .sort_unstable_by_key(|diagnostic| diagnostic.range().unwrap_or_default().start());
+
+        file_diagnostics
+    }
+
     /// Opens a file in the workspace.
     ///
     /// This changes the behavior of `check` to only check the open files rather than all files in the workspace.
@@ -378,49 +407,28 @@ impl Package {
     }
 }
 
+/// Checks a single file
+///
+/// This is a Salsa query so that [`Workspace::check_file`] can retrieve the accumulated diagnostics.
 #[salsa::tracked]
-pub(super) fn check_file(db: &dyn Db, file: File) -> Vec<String> {
+pub(super) fn check_file_impl(db: &dyn Db, file: File) {
     tracing::debug!("Checking file '{path}'", path = file.path(db));
 
-    let mut diagnostics = Vec::new();
-
-    let source_diagnostics = source_text::accumulated::<SourceDiagnostic>(db.upcast(), file);
-    // TODO(micha): Consider using a single accumulator for all diagnostics
-    diagnostics.extend(
-        source_diagnostics
-            .iter()
-            .map(std::string::ToString::to_string),
-    );
-
     // Abort checking if there are IO errors.
-    let source = source_text(db.upcast(), file);
-
-    if source.has_read_error() {
-        return diagnostics;
+    if source_text(db.upcast(), file).has_read_error() {
+        return;
     }
 
-    let parsed = parsed_module(db.upcast(), file);
+    check_types(db.upcast(), file);
+}
 
-    if !parsed.errors().is_empty() {
-        let path = file.path(db);
-        let line_index = line_index(db.upcast(), file);
-        diagnostics.extend(parsed.errors().iter().map(|err| {
-            let source_location = line_index.source_location(err.location.start(), source.as_str());
-            format!("{path}:{source_location}: {message}", message = err.error)
-        }));
+#[salsa::tracked]
+fn check_workspace_sync(db: &dyn Db, workspace: Workspace) {
+    let files = WorkspaceFiles::new(db, workspace);
+
+    for file in &files {
+        check_file_impl(db, file);
     }
-
-    for diagnostic in check_types(db.upcast(), file) {
-        let index = line_index(db.upcast(), diagnostic.file());
-        let location = index.source_location(diagnostic.start(), source.as_str());
-        diagnostics.push(format!(
-            "{path}:{location}: {message}",
-            path = file.path(db),
-            message = diagnostic.message()
-        ));
-    }
-
-    diagnostics
 }
 
 fn discover_package_files(db: &dyn Db, path: &SystemPath) -> FxHashSet<File> {
@@ -536,17 +544,43 @@ impl Iterator for WorkspaceFilesIter<'_> {
 #[cfg(test)]
 mod tests {
     use red_knot_python_semantic::types::check_types;
+    use red_knot_python_semantic::{
+        ProgramSettings, PythonVersion, SearchPathSettings, SitePackages,
+    };
+    use ruff_db::diagnostic::Diagnostic;
     use ruff_db::files::system_path_to_file;
     use ruff_db::source::source_text;
-    use ruff_db::system::{DbWithTestSystem, SystemPath};
+    use ruff_db::system::{DbWithTestSystem, SystemPath, SystemPathBuf};
     use ruff_db::testing::assert_function_query_was_not_run;
 
     use crate::db::tests::TestDb;
-    use crate::workspace::check_file;
+
+    use crate::workspace::settings::WorkspaceSettings;
+    use crate::workspace::{Workspace, WorkspaceMetadata};
 
     #[test]
     fn check_file_skips_type_checking_when_file_cant_be_read() -> ruff_db::system::Result<()> {
         let mut db = TestDb::new();
+        let root = SystemPathBuf::from("src");
+        let workspace = Workspace::from_metadata(
+            &db,
+            WorkspaceMetadata {
+                root: root.clone(),
+                packages: vec![],
+                settings: WorkspaceSettings {
+                    program: ProgramSettings {
+                        target_version: PythonVersion::default(),
+                        search_paths: SearchPathSettings {
+                            extra_paths: vec![],
+                            src_root: root,
+                            custom_typeshed: None,
+                            site_packages: SitePackages::Known(vec![]),
+                        },
+                    },
+                },
+            },
+        );
+
         let path = SystemPath::new("test.py");
 
         db.write_file(path, "x = 10")?;
@@ -556,9 +590,16 @@ mod tests {
         db.memory_file_system().remove_file(path)?;
         file.sync(&mut db);
 
+        let diagnostics: Vec<_> = workspace
+            .check_file(&db, file)
+            .into_iter()
+            .map(|diagnostic| diagnostic.message().to_string())
+            .collect();
+
         assert_eq!(source_text(&db, file).as_str(), "");
+
         assert_eq!(
-            check_file(&db, file),
+            diagnostics,
             vec!["Failed to read file: No such file or directory".to_string()]
         );
 
@@ -569,8 +610,14 @@ mod tests {
         // content returned by `source_text` remains unchanged, but the diagnostics should get updated.
         db.write_file(path, "").unwrap();
 
+        let diagnostics: Vec<_> = workspace
+            .check_file(&db, file)
+            .into_iter()
+            .map(|diagnostic| diagnostic.message().to_string())
+            .collect();
+
         assert_eq!(source_text(&db, file).as_str(), "");
-        assert_eq!(check_file(&db, file), vec![] as Vec<String>);
+        assert_eq!(diagnostics, vec![] as Vec<String>);
 
         Ok(())
     }

crates/ruff_benchmark/benches/red_knot.rs

@@ -24,32 +24,32 @@ const TOMLLIB_312_URL: &str = "https://raw.githubusercontent.com/python/cpython/
 
 static EXPECTED_DIAGNOSTICS: &[&str] = &[
     // We don't support `*` imports yet:
-    "/src/tomllib/_parser.py:7:29: Module `collections.abc` has no member `Iterable`",
+    "error[unresolved-import] /src/tomllib/_parser.py:7:29 Module `collections.abc` has no member `Iterable`",
     // We don't support terminal statements in control flow yet:
-    "/src/tomllib/_parser.py:246:15: Method `__class_getitem__` of type `Literal[frozenset]` is possibly unbound",
-    "/src/tomllib/_parser.py:692:8354: Invalid class base with type `GenericAlias` (all bases must be a class, `Any`, `Unknown` or `Todo`)",
-    "/src/tomllib/_parser.py:66:18: Name `s` used when possibly not defined",
-    "/src/tomllib/_parser.py:98:12: Name `char` used when possibly not defined",
-    "/src/tomllib/_parser.py:101:12: Name `char` used when possibly not defined",
-    "/src/tomllib/_parser.py:104:14: Name `char` used when possibly not defined",
-    "/src/tomllib/_parser.py:108:17: Conflicting declared types for `second_char`: Unknown, str | None",
-    "/src/tomllib/_parser.py:115:14: Name `char` used when possibly not defined",
-    "/src/tomllib/_parser.py:126:12: Name `char` used when possibly not defined",
-    "/src/tomllib/_parser.py:267:9: Conflicting declared types for `char`: Unknown, str | None",
-    "/src/tomllib/_parser.py:348:20: Name `nest` used when possibly not defined",
-    "/src/tomllib/_parser.py:353:5: Name `nest` used when possibly not defined",
-    "/src/tomllib/_parser.py:364:9: Conflicting declared types for `char`: Unknown, str | None",
-    "/src/tomllib/_parser.py:381:13: Conflicting declared types for `char`: Unknown, str | None",
-    "/src/tomllib/_parser.py:395:9: Conflicting declared types for `char`: Unknown, str | None",
-    "/src/tomllib/_parser.py:453:24: Name `nest` used when possibly not defined",
-    "/src/tomllib/_parser.py:455:9: Name `nest` used when possibly not defined",
-    "/src/tomllib/_parser.py:482:16: Name `char` used when possibly not defined",
-    "/src/tomllib/_parser.py:566:12: Name `char` used when possibly not defined",
-    "/src/tomllib/_parser.py:573:12: Name `char` used when possibly not defined",
-    "/src/tomllib/_parser.py:579:12: Name `char` used when possibly not defined",
-    "/src/tomllib/_parser.py:580:63: Name `char` used when possibly not defined",
-    "/src/tomllib/_parser.py:590:9: Conflicting declared types for `char`: Unknown, str | None",
-    "/src/tomllib/_parser.py:629:38: Name `datetime_obj` used when possibly not defined",
+    "error[possibly-unresolved-reference] /src/tomllib/_parser.py:66:18 Name `s` used when possibly not defined",
+    "error[possibly-unresolved-reference] /src/tomllib/_parser.py:98:12 Name `char` used when possibly not defined",
+    "error[possibly-unresolved-reference] /src/tomllib/_parser.py:101:12 Name `char` used when possibly not defined",
+    "error[possibly-unresolved-reference] /src/tomllib/_parser.py:104:14 Name `char` used when possibly not defined",
+    "error[conflicting-declarations] /src/tomllib/_parser.py:108:17 Conflicting declared types for `second_char`: Unknown, str | None",
+    "error[possibly-unresolved-reference] /src/tomllib/_parser.py:115:14 Name `char` used when possibly not defined",
+    "error[possibly-unresolved-reference] /src/tomllib/_parser.py:126:12 Name `char` used when possibly not defined",
+    "error[call-possibly-unbound-method] /src/tomllib/_parser.py:246:15 Method `__class_getitem__` of type `Literal[frozenset]` is possibly unbound",
+    "error[conflicting-declarations] /src/tomllib/_parser.py:267:9 Conflicting declared types for `char`: Unknown, str | None",
+    "error[possibly-unresolved-reference] /src/tomllib/_parser.py:348:20 Name `nest` used when possibly not defined",
+    "error[possibly-unresolved-reference] /src/tomllib/_parser.py:353:5 Name `nest` used when possibly not defined",
+    "error[conflicting-declarations] /src/tomllib/_parser.py:364:9 Conflicting declared types for `char`: Unknown, str | None",
+    "error[conflicting-declarations] /src/tomllib/_parser.py:381:13 Conflicting declared types for `char`: Unknown, str | None",
+    "error[conflicting-declarations] /src/tomllib/_parser.py:395:9 Conflicting declared types for `char`: Unknown, str | None",
+    "error[possibly-unresolved-reference] /src/tomllib/_parser.py:453:24 Name `nest` used when possibly not defined",
+    "error[possibly-unresolved-reference] /src/tomllib/_parser.py:455:9 Name `nest` used when possibly not defined",
+    "error[possibly-unresolved-reference] /src/tomllib/_parser.py:482:16 Name `char` used when possibly not defined",
+    "error[possibly-unresolved-reference] /src/tomllib/_parser.py:566:12 Name `char` used when possibly not defined",
+    "error[possibly-unresolved-reference] /src/tomllib/_parser.py:573:12 Name `char` used when possibly not defined",
+    "error[possibly-unresolved-reference] /src/tomllib/_parser.py:579:12 Name `char` used when possibly not defined",
+    "error[possibly-unresolved-reference] /src/tomllib/_parser.py:580:63 Name `char` used when possibly not defined",
+    "error[conflicting-declarations] /src/tomllib/_parser.py:590:9 Conflicting declared types for `char`: Unknown, str | None",
+    "error[possibly-unresolved-reference] /src/tomllib/_parser.py:629:38 Name `datetime_obj` used when possibly not defined",
+    "error[invalid-base] /src/tomllib/_parser.py:692:8354 Invalid class base with type `GenericAlias` (all bases must be a class, `Any`, `Unknown` or `Todo`)",
 ];
 
 fn get_test_file(name: &str) -> TestFile {
@@ -123,7 +123,13 @@ fn setup_rayon() {
 fn benchmark_incremental(criterion: &mut Criterion) {
     fn setup() -> Case {
         let case = setup_case();
-        let result = case.db.check().unwrap();
+        let result: Vec<_> = case
+            .db
+            .check()
+            .unwrap()
+            .into_iter()
+            .map(|diagnostic| diagnostic.display(&case.db).to_string())
+            .collect();
 
         assert_eq!(result, EXPECTED_DIAGNOSTICS);
 
@@ -150,7 +156,7 @@ fn benchmark_incremental(criterion: &mut Criterion) {
 
         let result = db.check().unwrap();
 
-        assert_eq!(result, EXPECTED_DIAGNOSTICS);
+        assert_eq!(result.len(), EXPECTED_DIAGNOSTICS.len());
     }
 
     setup_rayon();
@@ -168,7 +174,12 @@ fn benchmark_cold(criterion: &mut Criterion) {
             setup_case,
             |case| {
                 let Case { db, .. } = case;
-                let result = db.check().unwrap();
+                let result: Vec<_> = db
+                    .check()
+                    .unwrap()
+                    .into_iter()
+                    .map(|diagnostic| diagnostic.display(db).to_string())
+                    .collect();
 
                 assert_eq!(result, EXPECTED_DIAGNOSTICS);
             },

crates/ruff_db/Cargo.toml

@@ -28,7 +28,6 @@ matchit = { workspace = true }
 salsa = { workspace = true }
 serde = { workspace = true, optional = true }
 path-slash = { workspace = true }
-thiserror = { workspace = true }
 tracing = { workspace = true }
 tracing-subscriber = { workspace = true, optional = true }
 tracing-tree = { workspace = true, optional = true }

crates/ruff_db/src/diagnostic.rs

@@ -0,0 +1,155 @@
+use ruff_text_size::TextRange;
+use salsa::Accumulator as _;
+
+use crate::{
+    files::File,
+    source::{line_index, source_text},
+    Db,
+};
+
+pub trait Diagnostic: Send + Sync + std::fmt::Debug {
+    fn rule(&self) -> &str;
+
+    fn message(&self) -> std::borrow::Cow<str>;
+
+    fn file(&self) -> File;
+
+    fn range(&self) -> Option<TextRange>;
+
+    fn severity(&self) -> Severity;
+}
+
+#[derive(Debug, Clone, Copy)]
+pub enum Severity {
+    Info,
+    Error,
+}
+
+#[salsa::accumulator]
+pub struct CompileDiagnostic(std::sync::Arc<dyn Diagnostic>);
+
+impl CompileDiagnostic {
+    pub fn report<T>(db: &dyn Db, diagnostic: T)
+    where
+        T: Diagnostic + 'static,
+    {
+        Self(std::sync::Arc::new(diagnostic)).accumulate(db);
+    }
+
+    pub fn display<'a>(&'a self, db: &'a dyn Db) -> DisplayDiagnostic<'a> {
+        DisplayDiagnostic {
+            db,
+            diagnostic: &*self.0,
+        }
+    }
+}
+
+impl Diagnostic for CompileDiagnostic {
+    fn rule(&self) -> &str {
+        self.0.rule()
+    }
+
+    fn message(&self) -> std::borrow::Cow<str> {
+        self.0.message()
+    }
+
+    fn file(&self) -> File {
+        self.0.file()
+    }
+
+    fn range(&self) -> Option<TextRange> {
+        self.0.range()
+    }
+
+    fn severity(&self) -> Severity {
+        self.0.severity()
+    }
+}
+
+pub struct DisplayDiagnostic<'db> {
+    db: &'db dyn Db,
+    diagnostic: &'db dyn Diagnostic,
+}
+
+impl<'db> DisplayDiagnostic<'db> {
+    pub fn new(db: &'db dyn Db, diagnostic: &'db dyn Diagnostic) -> Self {
+        Self { db, diagnostic }
+    }
+}
+
+impl std::fmt::Display for DisplayDiagnostic<'_> {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self.diagnostic.severity() {
+            Severity::Info => f.write_str("info")?,
+            Severity::Error => f.write_str("error")?,
+        }
+
+        write!(
+            f,
+            "[{rule}] {path}",
+            rule = self.diagnostic.rule(),
+            path = self.diagnostic.file().path(self.db)
+        )?;
+
+        if let Some(range) = self.diagnostic.range() {
+            let index = line_index(self.db, self.diagnostic.file());
+            let source = source_text(self.db, self.diagnostic.file());
+
+            let start = index.source_location(range.start(), &source);
+
+            write!(f, ":{line}:{col}", line = start.row, col = start.column)?;
+        }
+
+        write!(f, " {message}", message = self.diagnostic.message())
+    }
+}
+
+impl<T> Diagnostic for Box<T>
+where
+    T: Diagnostic,
+{
+    fn rule(&self) -> &str {
+        (**self).rule()
+    }
+
+    fn message(&self) -> std::borrow::Cow<str> {
+        (**self).message()
+    }
+
+    fn file(&self) -> File {
+        (**self).file()
+    }
+
+    fn range(&self) -> Option<TextRange> {
+        (**self).range()
+    }
+
+    fn severity(&self) -> Severity {
+        (**self).severity()
+    }
+}
+
+impl<T> Diagnostic for std::sync::Arc<T>
+where
+    T: Diagnostic,
+{
+    fn rule(&self) -> &str {
+        (**self).rule()
+    }
+
+    fn message(&self) -> std::borrow::Cow<str> {
+        (**self).message()
+    }
+
+    fn file(&self) -> File {
+        (**self).file()
+    }
+
+    fn range(&self) -> Option<TextRange> {
+        (**self).range()
+    }
+
+    fn severity(&self) -> Severity {
+        (**self).severity()
+    }
+}

crates/ruff_db/src/lib.rs

@@ -6,6 +6,7 @@ use crate::files::Files;
 use crate::system::System;
 use crate::vendored::VendoredFileSystem;
 
+pub mod diagnostic;
 pub mod display;
 pub mod file_revision;
 pub mod files;

crates/ruff_db/src/parsed.rs

@@ -1,10 +1,13 @@
+use std::borrow::Cow;
 use std::fmt::Formatter;
 use std::ops::Deref;
 use std::sync::Arc;
 
 use ruff_python_ast::{ModModule, PySourceType};
 use ruff_python_parser::{parse_unchecked_source, Parsed};
+use ruff_text_size::TextRange;
 
+use crate::diagnostic::{CompileDiagnostic, Diagnostic};
 use crate::files::{File, FilePath};
 use crate::source::source_text;
 use crate::Db;
@@ -37,7 +40,20 @@ pub fn parsed_module(db: &dyn Db, file: File) -> ParsedModule {
             .map_or(PySourceType::Python, PySourceType::from_extension),
     };
 
-    ParsedModule::new(parse_unchecked_source(&source, ty))
+    let parsed = parse_unchecked_source(&source, ty);
+
+    for error in parsed.errors() {
+        CompileDiagnostic::report(
+            db,
+            ParseDiagnostic {
+                error: error.error.to_string(),
+                range: error.location,
+                file,
+            },
+        );
+    }
+
+    ParsedModule::new(parsed)
 }
 
 /// Cheap cloneable wrapper around the parsed module.
@@ -73,6 +89,35 @@ impl std::fmt::Debug for ParsedModule {
     }
 }
 
+#[derive(Debug)]
+pub struct ParseDiagnostic {
+    error: String,
+    range: TextRange,
+    file: File,
+}
+
+impl Diagnostic for ParseDiagnostic {
+    fn rule(&self) -> &str {
+        "invalid-syntax"
+    }
+
+    fn message(&self) -> std::borrow::Cow<str> {
+        Cow::Borrowed(&self.error)
+    }
+
+    fn file(&self) -> File {
+        self.file
+    }
+
+    fn range(&self) -> Option<ruff_text_size::TextRange> {
+        Some(self.range)
+    }
+
+    fn severity(&self) -> crate::diagnostic::Severity {
+        crate::diagnostic::Severity::Error
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use crate::files::{system_path_to_file, vendored_path_to_file};

crates/ruff_db/src/source.rs

@@ -1,14 +1,13 @@
-use std::fmt::Formatter;
 use std::ops::Deref;
 use std::sync::Arc;
 
 use countme::Count;
-use salsa::Accumulator;
 
 use ruff_notebook::Notebook;
 use ruff_python_ast::PySourceType;
 use ruff_source_file::LineIndex;
 
+use crate::diagnostic::{CompileDiagnostic, Diagnostic};
 use crate::files::{File, FilePath};
 use crate::Db;
 
@@ -25,8 +24,14 @@ pub fn source_text(db: &dyn Db, file: File) -> SourceText {
                 tracing::debug!("Failed to read notebook '{path}': {error}");
 
                 has_read_error = true;
-                SourceDiagnostic(Arc::new(SourceTextError::FailedToReadNotebook(error)))
-                    .accumulate(db);
+                CompileDiagnostic::report(
+                    db,
+                    SourceTextDiagnostic {
+                        error: SourceTextError::FailedToReadNotebook(error),
+                        file,
+                    },
+                );
+
                 Notebook::empty()
             })
             .into()
@@ -36,7 +41,14 @@ pub fn source_text(db: &dyn Db, file: File) -> SourceText {
                 tracing::debug!("Failed to read file '{path}': {error}");
 
                 has_read_error = true;
-                SourceDiagnostic(Arc::new(SourceTextError::FailedToReadFile(error))).accumulate(db);
+                CompileDiagnostic::report(
+                    db,
+                    SourceTextDiagnostic {
+                        error: SourceTextError::FailedToReadFile(error),
+                        file,
+                    },
+                );
+
                 String::new()
             })
             .into()
@@ -153,21 +165,45 @@ impl From<Notebook> for SourceTextKind {
     }
 }
 
-#[salsa::accumulator]
-pub struct SourceDiagnostic(Arc<SourceTextError>);
+#[derive(Debug)]
+pub struct SourceTextDiagnostic {
+    error: SourceTextError,
+    file: File,
+}
 
-impl std::fmt::Display for SourceDiagnostic {
-    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
-        std::fmt::Display::fmt(&self.0, f)
+impl Diagnostic for SourceTextDiagnostic {
+    fn message(&self) -> std::borrow::Cow<str> {
+        match &self.error {
+            SourceTextError::FailedToReadNotebook(notebook_error) => {
+                format!("Failed to read notebook: {notebook_error}").into()
+            }
+            SourceTextError::FailedToReadFile(error) => {
+                format!("Failed to read file: {error}").into()
+            }
+        }
+    }
+
+    fn rule(&self) -> &str {
+        "io-error"
+    }
+
+    fn file(&self) -> File {
+        self.file
+    }
+
+    fn severity(&self) -> crate::diagnostic::Severity {
+        crate::diagnostic::Severity::Error
+    }
+
+    fn range(&self) -> Option<ruff_text_size::TextRange> {
+        None
     }
 }
 
-#[derive(Debug, thiserror::Error)]
+#[derive(Debug)]
 pub enum SourceTextError {
-    #[error("Failed to read notebook: {0}`")]
-    FailedToReadNotebook(#[from] ruff_notebook::NotebookError),
-    #[error("Failed to read file: {0}")]
-    FailedToReadFile(#[from] std::io::Error),
+    FailedToReadNotebook(ruff_notebook::NotebookError),
+    FailedToReadFile(std::io::Error),
 }
 
 /// Computes the [`LineIndex`] for `file`.