red_knot: use Diagnostic inside of red knot

This replaces things like `TypeCheckDiagnostic` with the new Diagnostic`
type.

This is a "surgical" replacement where we retain the existing API of
of diagnostic reporting such that _most_ of Red Knot doesn't need to be
changed to support this update. But it will enable us to start using the
new diagnostic renderer and to delete the old renderer. It also paves
the path for exposing the new `Diagnostic` data model to the broader Red
Knot codebase.
This commit is contained in:
Andrew Gallant 2025-03-21 14:01:34 -04:00 committed by Andrew Gallant
parent 883b8e3870
commit 4e169e5f6c
18 changed files with 263 additions and 339 deletions

View file

@ -15,7 +15,7 @@ use red_knot_project::watch::ProjectWatcher;
use red_knot_project::{watch, Db}; use red_knot_project::{watch, Db};
use red_knot_project::{ProjectDatabase, ProjectMetadata}; use red_knot_project::{ProjectDatabase, ProjectMetadata};
use red_knot_server::run_server; use red_knot_server::run_server;
use ruff_db::diagnostic::{DisplayDiagnosticConfig, OldDiagnosticTrait, Severity}; use ruff_db::diagnostic::{Diagnostic, DisplayDiagnosticConfig, Severity};
use ruff_db::system::{OsSystem, SystemPath, SystemPathBuf}; use ruff_db::system::{OsSystem, SystemPath, SystemPathBuf};
use salsa::plumbing::ZalsaDatabase; use salsa::plumbing::ZalsaDatabase;
@ -288,7 +288,7 @@ impl MainLoop {
let diagnostics_count = result.len(); let diagnostics_count = result.len();
for diagnostic in result { for diagnostic in result {
writeln!(stdout, "{}", diagnostic.display(db, &display_config))?; diagnostic.print(db, &display_config, &mut stdout)?;
failed |= diagnostic.severity() >= min_error_severity; failed |= diagnostic.severity() >= min_error_severity;
} }
@ -359,7 +359,7 @@ enum MainLoopMessage {
CheckWorkspace, CheckWorkspace,
CheckCompleted { CheckCompleted {
/// The diagnostics that were found during the check. /// The diagnostics that were found during the check.
result: Vec<Box<dyn OldDiagnosticTrait>>, result: Vec<Diagnostic>,
revision: u64, revision: u64,
}, },
ApplyChanges(Vec<watch::ChangeEvent>), ApplyChanges(Vec<watch::ChangeEvent>),

View file

@ -1125,11 +1125,11 @@ print(sys.last_exc, os.getegid())
assert_eq!(diagnostics.len(), 2); assert_eq!(diagnostics.len(), 2);
assert_eq!( assert_eq!(
diagnostics[0].message(), diagnostics[0].primary_message(),
"Type `<module 'sys'>` has no attribute `last_exc`" "Type `<module 'sys'>` has no attribute `last_exc`"
); );
assert_eq!( assert_eq!(
diagnostics[1].message(), diagnostics[1].primary_message(),
"Type `<module 'os'>` has no attribute `getegid`" "Type `<module 'os'>` has no attribute `getegid`"
); );

View file

@ -6,7 +6,7 @@ use crate::{Project, ProjectMetadata};
use red_knot_ide::Db as IdeDb; use red_knot_ide::Db as IdeDb;
use red_knot_python_semantic::lint::{LintRegistry, RuleSelection}; use red_knot_python_semantic::lint::{LintRegistry, RuleSelection};
use red_knot_python_semantic::{Db as SemanticDb, Program}; use red_knot_python_semantic::{Db as SemanticDb, Program};
use ruff_db::diagnostic::OldDiagnosticTrait; use ruff_db::diagnostic::Diagnostic;
use ruff_db::files::{File, Files}; use ruff_db::files::{File, Files};
use ruff_db::system::System; use ruff_db::system::System;
use ruff_db::vendored::VendoredFileSystem; use ruff_db::vendored::VendoredFileSystem;
@ -56,12 +56,12 @@ impl ProjectDatabase {
} }
/// Checks all open files in the project and its dependencies. /// Checks all open files in the project and its dependencies.
pub fn check(&self) -> Result<Vec<Box<dyn OldDiagnosticTrait>>, Cancelled> { pub fn check(&self) -> Result<Vec<Diagnostic>, Cancelled> {
self.with_db(|db| db.project().check(db)) self.with_db(|db| db.project().check(db))
} }
#[tracing::instrument(level = "debug", skip(self))] #[tracing::instrument(level = "debug", skip(self))]
pub fn check_file(&self, file: File) -> Result<Vec<Box<dyn OldDiagnosticTrait>>, Cancelled> { pub fn check_file(&self, file: File) -> Result<Vec<Diagnostic>, Cancelled> {
self.with_db(|db| self.project().check_file(db, file)) self.with_db(|db| self.project().check_file(db, file))
} }

View file

@ -9,7 +9,9 @@ pub use metadata::{ProjectDiscoveryError, ProjectMetadata};
use red_knot_python_semantic::lint::{LintRegistry, LintRegistryBuilder, RuleSelection}; use red_knot_python_semantic::lint::{LintRegistry, LintRegistryBuilder, RuleSelection};
use red_knot_python_semantic::register_lints; use red_knot_python_semantic::register_lints;
use red_knot_python_semantic::types::check_types; use red_knot_python_semantic::types::check_types;
use ruff_db::diagnostic::{DiagnosticId, OldDiagnosticTrait, OldParseDiagnostic, Severity, Span}; use ruff_db::diagnostic::{
create_parse_diagnostic, Annotation, Diagnostic, DiagnosticId, Severity, Span,
};
use ruff_db::files::File; use ruff_db::files::File;
use ruff_db::parsed::parsed_module; use ruff_db::parsed::parsed_module;
use ruff_db::source::{source_text, SourceTextError}; use ruff_db::source::{source_text, SourceTextError};
@ -17,7 +19,6 @@ use ruff_db::system::{SystemPath, SystemPathBuf};
use rustc_hash::FxHashSet; use rustc_hash::FxHashSet;
use salsa::Durability; use salsa::Durability;
use salsa::Setter; use salsa::Setter;
use std::borrow::Cow;
use std::sync::Arc; use std::sync::Arc;
use thiserror::Error; use thiserror::Error;
@ -163,24 +164,27 @@ impl Project {
} }
/// Checks all open files in the project and its dependencies. /// Checks all open files in the project and its dependencies.
pub(crate) fn check(self, db: &ProjectDatabase) -> Vec<Box<dyn OldDiagnosticTrait>> { pub(crate) fn check(self, db: &ProjectDatabase) -> Vec<Diagnostic> {
let project_span = tracing::debug_span!("Project::check"); let project_span = tracing::debug_span!("Project::check");
let _span = project_span.enter(); let _span = project_span.enter();
tracing::debug!("Checking project '{name}'", name = self.name(db)); tracing::debug!("Checking project '{name}'", name = self.name(db));
let mut diagnostics: Vec<Box<dyn OldDiagnosticTrait>> = Vec::new(); let mut diagnostics: Vec<Diagnostic> = Vec::new();
diagnostics.extend(self.settings_diagnostics(db).iter().map(|diagnostic| { diagnostics.extend(
let diagnostic: Box<dyn OldDiagnosticTrait> = Box::new(diagnostic.clone()); self.settings_diagnostics(db)
diagnostic .iter()
})); .map(OptionDiagnostic::to_diagnostic),
);
let files = ProjectFiles::new(db, self); let files = ProjectFiles::new(db, self);
diagnostics.extend(files.diagnostics().iter().cloned().map(|diagnostic| { diagnostics.extend(
let diagnostic: Box<dyn OldDiagnosticTrait> = Box::new(diagnostic); files
diagnostic .diagnostics()
})); .iter()
.map(IOErrorDiagnostic::to_diagnostic),
);
let result = Arc::new(std::sync::Mutex::new(diagnostics)); let result = Arc::new(std::sync::Mutex::new(diagnostics));
let inner_result = Arc::clone(&result); let inner_result = Arc::clone(&result);
@ -208,14 +212,11 @@ impl Project {
Arc::into_inner(result).unwrap().into_inner().unwrap() Arc::into_inner(result).unwrap().into_inner().unwrap()
} }
pub(crate) fn check_file(self, db: &dyn Db, file: File) -> Vec<Box<dyn OldDiagnosticTrait>> { pub(crate) fn check_file(self, db: &dyn Db, file: File) -> Vec<Diagnostic> {
let mut file_diagnostics: Vec<_> = self let mut file_diagnostics: Vec<_> = self
.settings_diagnostics(db) .settings_diagnostics(db)
.iter() .iter()
.map(|diagnostic| { .map(OptionDiagnostic::to_diagnostic)
let diagnostic: Box<dyn OldDiagnosticTrait> = Box::new(diagnostic.clone());
diagnostic
})
.collect(); .collect();
let check_diagnostics = check_file_impl(db, file); let check_diagnostics = check_file_impl(db, file);
@ -398,35 +399,36 @@ impl Project {
} }
} }
fn check_file_impl(db: &dyn Db, file: File) -> Vec<Box<dyn OldDiagnosticTrait>> { fn check_file_impl(db: &dyn Db, file: File) -> Vec<Diagnostic> {
let mut diagnostics: Vec<Box<dyn OldDiagnosticTrait>> = Vec::new(); let mut diagnostics: Vec<Diagnostic> = Vec::new();
// Abort checking if there are IO errors. // Abort checking if there are IO errors.
let source = source_text(db.upcast(), file); let source = source_text(db.upcast(), file);
if let Some(read_error) = source.read_error() { if let Some(read_error) = source.read_error() {
diagnostics.push(Box::new(IOErrorDiagnostic { diagnostics.push(
file: Some(file), IOErrorDiagnostic {
error: read_error.clone().into(), file: Some(file),
})); error: read_error.clone().into(),
}
.to_diagnostic(),
);
return diagnostics; return diagnostics;
} }
let parsed = parsed_module(db.upcast(), file); let parsed = parsed_module(db.upcast(), file);
diagnostics.extend(parsed.errors().iter().map(|error| { diagnostics.extend(
let diagnostic: Box<dyn OldDiagnosticTrait> = parsed
Box::new(OldParseDiagnostic::new(file, error.clone())); .errors()
diagnostic .iter()
})); .map(|error| create_parse_diagnostic(file, error)),
);
diagnostics.extend(check_types(db.upcast(), file).iter().map(|diagnostic| { diagnostics.extend(check_types(db.upcast(), file).into_iter().cloned());
let boxed: Box<dyn OldDiagnosticTrait> = Box::new(diagnostic.clone());
boxed
}));
diagnostics.sort_unstable_by_key(|diagnostic| { diagnostics.sort_unstable_by_key(|diagnostic| {
diagnostic diagnostic
.span() .primary_span()
.and_then(|span| span.range()) .and_then(|span| span.range())
.unwrap_or_default() .unwrap_or_default()
.start() .start()
@ -494,21 +496,13 @@ pub struct IOErrorDiagnostic {
error: IOErrorKind, error: IOErrorKind,
} }
impl OldDiagnosticTrait for IOErrorDiagnostic { impl IOErrorDiagnostic {
fn id(&self) -> DiagnosticId { fn to_diagnostic(&self) -> Diagnostic {
DiagnosticId::Io let mut diag = Diagnostic::new(DiagnosticId::Io, Severity::Error, &self.error);
} if let Some(file) = self.file {
diag.annotate(Annotation::primary(Span::from(file)));
fn message(&self) -> Cow<str> { }
self.error.to_string().into() diag
}
fn span(&self) -> Option<Span> {
self.file.map(Span::from)
}
fn severity(&self) -> Severity {
Severity::Error
} }
} }
@ -526,7 +520,6 @@ mod tests {
use crate::db::tests::TestDb; use crate::db::tests::TestDb;
use crate::{check_file_impl, ProjectMetadata}; use crate::{check_file_impl, ProjectMetadata};
use red_knot_python_semantic::types::check_types; use red_knot_python_semantic::types::check_types;
use ruff_db::diagnostic::OldDiagnosticTrait;
use ruff_db::files::system_path_to_file; use ruff_db::files::system_path_to_file;
use ruff_db::source::source_text; use ruff_db::source::source_text;
use ruff_db::system::{DbWithTestSystem, DbWithWritableSystem as _, SystemPath, SystemPathBuf}; use ruff_db::system::{DbWithTestSystem, DbWithWritableSystem as _, SystemPath, SystemPathBuf};
@ -550,7 +543,7 @@ mod tests {
assert_eq!( assert_eq!(
check_file_impl(&db, file) check_file_impl(&db, file)
.into_iter() .into_iter()
.map(|diagnostic| diagnostic.message().into_owned()) .map(|diagnostic| diagnostic.primary_message().to_string())
.collect::<Vec<_>>(), .collect::<Vec<_>>(),
vec!["Failed to read file: No such file or directory".to_string()] vec!["Failed to read file: No such file or directory".to_string()]
); );
@ -566,7 +559,7 @@ mod tests {
assert_eq!( assert_eq!(
check_file_impl(&db, file) check_file_impl(&db, file)
.into_iter() .into_iter()
.map(|diagnostic| diagnostic.message().into_owned()) .map(|diagnostic| diagnostic.primary_message().to_string())
.collect::<Vec<_>>(), .collect::<Vec<_>>(),
vec![] as Vec<String> vec![] as Vec<String>
); );

View file

@ -2,14 +2,13 @@ use crate::metadata::value::{RangedValue, RelativePathBuf, ValueSource, ValueSou
use crate::Db; use crate::Db;
use red_knot_python_semantic::lint::{GetLintError, Level, LintSource, RuleSelection}; use red_knot_python_semantic::lint::{GetLintError, Level, LintSource, RuleSelection};
use red_knot_python_semantic::{ProgramSettings, PythonPath, PythonPlatform, SearchPathSettings}; use red_knot_python_semantic::{ProgramSettings, PythonPath, PythonPlatform, SearchPathSettings};
use ruff_db::diagnostic::{DiagnosticFormat, DiagnosticId, OldDiagnosticTrait, Severity, Span}; use ruff_db::diagnostic::{Annotation, Diagnostic, DiagnosticFormat, DiagnosticId, Severity, Span};
use ruff_db::files::system_path_to_file; use ruff_db::files::system_path_to_file;
use ruff_db::system::{System, SystemPath}; use ruff_db::system::{System, SystemPath};
use ruff_macros::Combine; use ruff_macros::Combine;
use ruff_python_ast::PythonVersion; use ruff_python_ast::PythonVersion;
use rustc_hash::FxHashMap; use rustc_hash::FxHashMap;
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
use std::borrow::Cow;
use std::fmt::Debug; use std::fmt::Debug;
use thiserror::Error; use thiserror::Error;
@ -397,22 +396,14 @@ impl OptionDiagnostic {
fn with_span(self, span: Option<Span>) -> Self { fn with_span(self, span: Option<Span>) -> Self {
OptionDiagnostic { span, ..self } OptionDiagnostic { span, ..self }
} }
}
impl OldDiagnosticTrait for OptionDiagnostic { pub(crate) fn to_diagnostic(&self) -> Diagnostic {
fn id(&self) -> DiagnosticId { if let Some(ref span) = self.span {
self.id let mut diag = Diagnostic::new(self.id, self.severity, "");
} diag.annotate(Annotation::primary(span.clone()).message(&self.message));
diag
fn message(&self) -> Cow<str> { } else {
Cow::Borrowed(&self.message) Diagnostic::new(self.id, self.severity, &self.message)
} }
fn span(&self) -> Option<Span> {
self.span.clone()
}
fn severity(&self) -> Severity {
self.severity
} }
} }

View file

@ -1,7 +1,7 @@
use crate::lint::{GetLintError, Level, LintMetadata, LintRegistry, LintStatus}; use crate::lint::{GetLintError, Level, LintMetadata, LintRegistry, LintStatus};
use crate::types::{TypeCheckDiagnostic, TypeCheckDiagnostics}; use crate::types::TypeCheckDiagnostics;
use crate::{declare_lint, lint::LintId, Db}; use crate::{declare_lint, lint::LintId, Db};
use ruff_db::diagnostic::DiagnosticId; use ruff_db::diagnostic::{Annotation, Diagnostic, DiagnosticId, Span};
use ruff_db::{files::File, parsed::parsed_module, source::source_text}; use ruff_db::{files::File, parsed::parsed_module, source::source_text};
use ruff_python_parser::TokenKind; use ruff_python_parser::TokenKind;
use ruff_python_trivia::Cursor; use ruff_python_trivia::Cursor;
@ -319,14 +319,11 @@ impl<'a> CheckSuppressionsContext<'a> {
return; return;
}; };
self.diagnostics.push(TypeCheckDiagnostic { let id = DiagnosticId::Lint(lint.name());
id: DiagnosticId::Lint(lint.name()), let mut diag = Diagnostic::new(id, severity, "");
message: message.to_string(), let span = Span::from(self.file).with_range(range);
range, diag.annotate(Annotation::primary(span).message(message));
severity, self.diagnostics.push(diag);
file: self.file,
secondary_messages: vec![],
});
} }
} }

View file

@ -15,7 +15,7 @@ use type_ordering::union_or_intersection_elements_ordering;
pub(crate) use self::builder::{IntersectionBuilder, UnionBuilder}; pub(crate) use self::builder::{IntersectionBuilder, UnionBuilder};
pub(crate) use self::diagnostic::register_lints; pub(crate) use self::diagnostic::register_lints;
pub use self::diagnostic::{TypeCheckDiagnostic, TypeCheckDiagnostics}; pub use self::diagnostic::TypeCheckDiagnostics;
pub(crate) use self::display::TypeArrayDisplay; pub(crate) use self::display::TypeArrayDisplay;
pub(crate) use self::infer::{ pub(crate) use self::infer::{
infer_deferred_types, infer_definition_types, infer_expression_type, infer_expression_types, infer_deferred_types, infer_definition_types, infer_expression_type, infer_expression_types,

View file

@ -2,12 +2,14 @@ use std::fmt;
use drop_bomb::DebugDropBomb; use drop_bomb::DebugDropBomb;
use ruff_db::{ use ruff_db::{
diagnostic::{DiagnosticId, OldSecondaryDiagnosticMessage, Severity}, diagnostic::{
Annotation, Diagnostic, DiagnosticId, OldSecondaryDiagnosticMessage, Severity, Span,
},
files::File, files::File,
}; };
use ruff_text_size::{Ranged, TextRange}; use ruff_text_size::{Ranged, TextRange};
use super::{binding_type, Type, TypeCheckDiagnostic, TypeCheckDiagnostics}; use super::{binding_type, Type, TypeCheckDiagnostics};
use crate::semantic_index::symbol::ScopeId; use crate::semantic_index::symbol::ScopeId;
use crate::{ use crate::{
@ -147,14 +149,13 @@ impl<'db> InferContext<'db> {
// returns a rule selector for a given file that respects the package's settings, // 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. // any global pragma comments in the file, and any per-file-ignores.
self.diagnostics.borrow_mut().push(TypeCheckDiagnostic { let mut diag = Diagnostic::new(id, severity, "");
file: self.file, for secondary_msg in &secondary_messages {
id, diag.sub(secondary_msg.to_sub_diagnostic());
message: message.to_string(), }
range: ranged.range(), let span = Span::from(self.file).with_range(ranged.range());
severity, diag.annotate(Annotation::primary(span).message(message));
secondary_messages, self.diagnostics.borrow_mut().push(diag);
});
} }
pub(super) fn set_in_no_type_check(&mut self, no_type_check: InNoTypeCheck) { pub(super) fn set_in_no_type_check(&mut self, no_type_check: InNoTypeCheck) {

View file

@ -8,16 +8,11 @@ use crate::types::string_annotation::{
RAW_STRING_TYPE_ANNOTATION, RAW_STRING_TYPE_ANNOTATION,
}; };
use crate::types::{ClassLiteralType, KnownInstanceType, Type}; use crate::types::{ClassLiteralType, KnownInstanceType, Type};
use ruff_db::diagnostic::{ use ruff_db::diagnostic::{Diagnostic, OldSecondaryDiagnosticMessage, Span};
DiagnosticId, OldDiagnosticTrait, OldSecondaryDiagnosticMessage, Severity, Span,
};
use ruff_db::files::File;
use ruff_python_ast::{self as ast, AnyNodeRef}; use ruff_python_ast::{self as ast, AnyNodeRef};
use ruff_text_size::{Ranged, TextRange}; use ruff_text_size::Ranged;
use rustc_hash::FxHashSet; use rustc_hash::FxHashSet;
use std::borrow::Cow;
use std::fmt::Formatter; use std::fmt::Formatter;
use std::sync::Arc;
/// Registers all known type check lints. /// Registers all known type check lints.
pub(crate) fn register_lints(registry: &mut LintRegistryBuilder) { pub(crate) fn register_lints(registry: &mut LintRegistryBuilder) {
@ -899,52 +894,6 @@ declare_lint! {
} }
} }
#[derive(Debug, Eq, PartialEq, Clone)]
pub struct TypeCheckDiagnostic {
pub(crate) id: DiagnosticId,
pub(crate) message: String,
pub(crate) range: TextRange,
pub(crate) severity: Severity,
pub(crate) file: File,
pub(crate) secondary_messages: Vec<OldSecondaryDiagnosticMessage>,
}
impl TypeCheckDiagnostic {
pub fn id(&self) -> DiagnosticId {
self.id
}
pub fn message(&self) -> &str {
&self.message
}
pub fn file(&self) -> File {
self.file
}
}
impl OldDiagnosticTrait for TypeCheckDiagnostic {
fn id(&self) -> DiagnosticId {
self.id
}
fn message(&self) -> Cow<str> {
TypeCheckDiagnostic::message(self).into()
}
fn span(&self) -> Option<Span> {
Some(Span::from(self.file).with_range(self.range))
}
fn secondary_messages(&self) -> &[OldSecondaryDiagnosticMessage] {
&self.secondary_messages
}
fn severity(&self) -> Severity {
self.severity
}
}
/// A collection of type check diagnostics. /// A collection of type check diagnostics.
/// ///
/// The diagnostics are wrapped in an `Arc` because they need to be cloned multiple times /// The diagnostics are wrapped in an `Arc` because they need to be cloned multiple times
@ -954,13 +903,13 @@ impl OldDiagnosticTrait for TypeCheckDiagnostic {
/// each Salsa-struct comes with an overhead. /// each Salsa-struct comes with an overhead.
#[derive(Default, Eq, PartialEq)] #[derive(Default, Eq, PartialEq)]
pub struct TypeCheckDiagnostics { pub struct TypeCheckDiagnostics {
diagnostics: Vec<Arc<TypeCheckDiagnostic>>, diagnostics: Vec<Diagnostic>,
used_suppressions: FxHashSet<FileSuppressionId>, used_suppressions: FxHashSet<FileSuppressionId>,
} }
impl TypeCheckDiagnostics { impl TypeCheckDiagnostics {
pub(crate) fn push(&mut self, diagnostic: TypeCheckDiagnostic) { pub(crate) fn push(&mut self, diagnostic: Diagnostic) {
self.diagnostics.push(Arc::new(diagnostic)); self.diagnostics.push(diagnostic);
} }
pub(super) fn extend(&mut self, other: &TypeCheckDiagnostics) { pub(super) fn extend(&mut self, other: &TypeCheckDiagnostics) {
@ -985,7 +934,7 @@ impl TypeCheckDiagnostics {
self.diagnostics.shrink_to_fit(); self.diagnostics.shrink_to_fit();
} }
pub fn iter(&self) -> std::slice::Iter<'_, Arc<TypeCheckDiagnostic>> { pub fn iter(&self) -> std::slice::Iter<'_, Diagnostic> {
self.diagnostics.iter() self.diagnostics.iter()
} }
} }
@ -997,8 +946,8 @@ impl std::fmt::Debug for TypeCheckDiagnostics {
} }
impl IntoIterator for TypeCheckDiagnostics { impl IntoIterator for TypeCheckDiagnostics {
type Item = Arc<TypeCheckDiagnostic>; type Item = Diagnostic;
type IntoIter = std::vec::IntoIter<std::sync::Arc<TypeCheckDiagnostic>>; type IntoIter = std::vec::IntoIter<Diagnostic>;
fn into_iter(self) -> Self::IntoIter { fn into_iter(self) -> Self::IntoIter {
self.diagnostics.into_iter() self.diagnostics.into_iter()
@ -1006,8 +955,8 @@ impl IntoIterator for TypeCheckDiagnostics {
} }
impl<'a> IntoIterator for &'a TypeCheckDiagnostics { impl<'a> IntoIterator for &'a TypeCheckDiagnostics {
type Item = &'a Arc<TypeCheckDiagnostic>; type Item = &'a Diagnostic;
type IntoIter = std::slice::Iter<'a, std::sync::Arc<TypeCheckDiagnostic>>; type IntoIter = std::slice::Iter<'a, Diagnostic>;
fn into_iter(self) -> Self::IntoIter { fn into_iter(self) -> Self::IntoIter {
self.diagnostics.iter() self.diagnostics.iter()

View file

@ -7343,6 +7343,7 @@ mod tests {
use crate::semantic_index::{global_scope, semantic_index, symbol_table, use_def_map}; use crate::semantic_index::{global_scope, semantic_index, symbol_table, use_def_map};
use crate::symbol::global_symbol; use crate::symbol::global_symbol;
use crate::types::check_types; use crate::types::check_types;
use ruff_db::diagnostic::Diagnostic;
use ruff_db::files::{system_path_to_file, File}; use ruff_db::files::{system_path_to_file, File};
use ruff_db::system::DbWithWritableSystem as _; use ruff_db::system::DbWithWritableSystem as _;
use ruff_db::testing::{assert_function_query_was_not_run, assert_function_query_was_run}; use ruff_db::testing::{assert_function_query_was_not_run, assert_function_query_was_run};
@ -7377,7 +7378,7 @@ mod tests {
fn assert_diagnostic_messages(diagnostics: &TypeCheckDiagnostics, expected: &[&str]) { fn assert_diagnostic_messages(diagnostics: &TypeCheckDiagnostics, expected: &[&str]) {
let messages: Vec<&str> = diagnostics let messages: Vec<&str> = diagnostics
.iter() .iter()
.map(|diagnostic| diagnostic.message()) .map(Diagnostic::primary_message)
.collect(); .collect();
assert_eq!(&messages, expected); assert_eq!(&messages, expected);
} }

View file

@ -72,10 +72,10 @@ fn compute_diagnostics(snapshot: &DocumentSnapshot, db: &ProjectDatabase) -> Vec
fn to_lsp_diagnostic( fn to_lsp_diagnostic(
db: &dyn Db, db: &dyn Db,
diagnostic: &dyn ruff_db::diagnostic::OldDiagnosticTrait, diagnostic: &ruff_db::diagnostic::Diagnostic,
encoding: crate::PositionEncoding, encoding: crate::PositionEncoding,
) -> Diagnostic { ) -> Diagnostic {
let range = if let Some(span) = diagnostic.span() { let range = if let Some(span) = diagnostic.primary_span() {
let index = line_index(db.upcast(), span.file()); let index = line_index(db.upcast(), span.file());
let source = source_text(db.upcast(), span.file()); let source = source_text(db.upcast(), span.file());
@ -99,7 +99,7 @@ fn to_lsp_diagnostic(
code: Some(NumberOrString::String(diagnostic.id().to_string())), code: Some(NumberOrString::String(diagnostic.id().to_string())),
code_description: None, code_description: None,
source: Some("red-knot".into()), source: Some("red-knot".into()),
message: diagnostic.message().into_owned(), message: diagnostic.primary_message().to_string(),
related_information: None, related_information: None,
data: None, data: None,
} }

View file

@ -13,7 +13,7 @@ license.workspace = true
[dependencies] [dependencies]
red_knot_python_semantic = { workspace = true, features = ["serde"] } red_knot_python_semantic = { workspace = true, features = ["serde"] }
red_knot_vendored = { workspace = true } red_knot_vendored = { workspace = true }
ruff_db = { workspace = true, features = ["testing"] } ruff_db = { workspace = true, features = ["os", "testing"] }
ruff_index = { workspace = true } ruff_index = { workspace = true }
ruff_notebook = { workspace = true } ruff_notebook = { workspace = true }
ruff_python_trivia = { workspace = true } ruff_python_trivia = { workspace = true }

View file

@ -2,7 +2,7 @@
//! //!
//! We don't assume that we will get the diagnostics in source order. //! We don't assume that we will get the diagnostics in source order.
use ruff_db::diagnostic::OldDiagnosticTrait; use ruff_db::diagnostic::Diagnostic;
use ruff_source_file::{LineIndex, OneIndexed}; use ruff_source_file::{LineIndex, OneIndexed};
use std::ops::{Deref, Range}; use std::ops::{Deref, Range};
@ -12,21 +12,21 @@ use std::ops::{Deref, Range};
/// [`LineDiagnosticRange`] has one entry for each contiguous slice of the diagnostics vector /// [`LineDiagnosticRange`] has one entry for each contiguous slice of the diagnostics vector
/// containing diagnostics which all start on the same line. /// containing diagnostics which all start on the same line.
#[derive(Debug)] #[derive(Debug)]
pub(crate) struct SortedDiagnostics<T> { pub(crate) struct SortedDiagnostics<'a> {
diagnostics: Vec<T>, diagnostics: Vec<&'a Diagnostic>,
line_ranges: Vec<LineDiagnosticRange>, line_ranges: Vec<LineDiagnosticRange>,
} }
impl<T> SortedDiagnostics<T> impl<'a> SortedDiagnostics<'a> {
where pub(crate) fn new(
T: OldDiagnosticTrait, diagnostics: impl IntoIterator<Item = &'a Diagnostic>,
{ line_index: &LineIndex,
pub(crate) fn new(diagnostics: impl IntoIterator<Item = T>, line_index: &LineIndex) -> Self { ) -> Self {
let mut diagnostics: Vec<_> = diagnostics let mut diagnostics: Vec<_> = diagnostics
.into_iter() .into_iter()
.map(|diagnostic| DiagnosticWithLine { .map(|diagnostic| DiagnosticWithLine {
line_number: diagnostic line_number: diagnostic
.span() .primary_span()
.and_then(|span| span.range()) .and_then(|span| span.range())
.map_or(OneIndexed::from_zero_indexed(0), |range| { .map_or(OneIndexed::from_zero_indexed(0), |range| {
line_index.line_index(range.start()) line_index.line_index(range.start())
@ -76,7 +76,7 @@ where
diags diags
} }
pub(crate) fn iter_lines(&self) -> LineDiagnosticsIterator<T> { pub(crate) fn iter_lines(&self) -> LineDiagnosticsIterator<'_> {
LineDiagnosticsIterator { LineDiagnosticsIterator {
diagnostics: self.diagnostics.as_slice(), diagnostics: self.diagnostics.as_slice(),
inner: self.line_ranges.iter(), inner: self.line_ranges.iter(),
@ -92,16 +92,13 @@ struct LineDiagnosticRange {
} }
/// Iterator to group sorted diagnostics by line. /// Iterator to group sorted diagnostics by line.
pub(crate) struct LineDiagnosticsIterator<'a, T> { pub(crate) struct LineDiagnosticsIterator<'a> {
diagnostics: &'a [T], diagnostics: &'a [&'a Diagnostic],
inner: std::slice::Iter<'a, LineDiagnosticRange>, inner: std::slice::Iter<'a, LineDiagnosticRange>,
} }
impl<'a, T> Iterator for LineDiagnosticsIterator<'a, T> impl<'a> Iterator for LineDiagnosticsIterator<'a> {
where type Item = LineDiagnostics<'a>;
T: OldDiagnosticTrait,
{
type Item = LineDiagnostics<'a, T>;
fn next(&mut self) -> Option<Self::Item> { fn next(&mut self) -> Option<Self::Item> {
let LineDiagnosticRange { let LineDiagnosticRange {
@ -115,20 +112,20 @@ where
} }
} }
impl<T> std::iter::FusedIterator for LineDiagnosticsIterator<'_, T> where T: OldDiagnosticTrait {} impl std::iter::FusedIterator for LineDiagnosticsIterator<'_> {}
/// All diagnostics that start on a single line of source code in one embedded Python file. /// All diagnostics that start on a single line of source code in one embedded Python file.
#[derive(Debug)] #[derive(Debug)]
pub(crate) struct LineDiagnostics<'a, T> { pub(crate) struct LineDiagnostics<'a> {
/// Line number on which these diagnostics start. /// Line number on which these diagnostics start.
pub(crate) line_number: OneIndexed, pub(crate) line_number: OneIndexed,
/// Diagnostics starting on this line. /// Diagnostics starting on this line.
pub(crate) diagnostics: &'a [T], pub(crate) diagnostics: &'a [&'a Diagnostic],
} }
impl<T> Deref for LineDiagnostics<'_, T> { impl<'a> Deref for LineDiagnostics<'a> {
type Target = [T]; type Target = [&'a Diagnostic];
fn deref(&self) -> &Self::Target { fn deref(&self) -> &Self::Target {
self.diagnostics self.diagnostics
@ -136,22 +133,20 @@ impl<T> Deref for LineDiagnostics<'_, T> {
} }
#[derive(Debug)] #[derive(Debug)]
struct DiagnosticWithLine<T> { struct DiagnosticWithLine<'a> {
line_number: OneIndexed, line_number: OneIndexed,
diagnostic: T, diagnostic: &'a Diagnostic,
} }
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use crate::db::Db; use crate::db::Db;
use crate::diagnostic::OldDiagnosticTrait; use ruff_db::diagnostic::{Annotation, Diagnostic, DiagnosticId, LintName, Severity, Span};
use ruff_db::diagnostic::{DiagnosticId, LintName, Severity, Span}; use ruff_db::files::system_path_to_file;
use ruff_db::files::{system_path_to_file, File};
use ruff_db::source::line_index; use ruff_db::source::line_index;
use ruff_db::system::DbWithWritableSystem as _; use ruff_db::system::DbWithWritableSystem as _;
use ruff_source_file::OneIndexed; use ruff_source_file::OneIndexed;
use ruff_text_size::{TextRange, TextSize}; use ruff_text_size::{TextRange, TextSize};
use std::borrow::Cow;
#[test] #[test]
fn sort_and_group() { fn sort_and_group() {
@ -168,10 +163,19 @@ mod tests {
let diagnostics: Vec<_> = ranges let diagnostics: Vec<_> = ranges
.into_iter() .into_iter()
.map(|range| DummyDiagnostic { range, file }) .map(|range| {
let mut diag = Diagnostic::new(
DiagnosticId::Lint(LintName::of("dummy")),
Severity::Error,
"dummy",
);
let span = Span::from(file).with_range(range);
diag.annotate(Annotation::primary(span));
diag
})
.collect(); .collect();
let sorted = super::SortedDiagnostics::new(diagnostics, &lines); let sorted = super::SortedDiagnostics::new(diagnostics.iter(), &lines);
let grouped = sorted.iter_lines().collect::<Vec<_>>(); let grouped = sorted.iter_lines().collect::<Vec<_>>();
let [line1, line2] = &grouped[..] else { let [line1, line2] = &grouped[..] else {
@ -183,28 +187,4 @@ mod tests {
assert_eq!(line2.line_number, OneIndexed::from_zero_indexed(1)); assert_eq!(line2.line_number, OneIndexed::from_zero_indexed(1));
assert_eq!(line2.diagnostics.len(), 1); assert_eq!(line2.diagnostics.len(), 1);
} }
#[derive(Debug)]
struct DummyDiagnostic {
range: TextRange,
file: File,
}
impl OldDiagnosticTrait for DummyDiagnostic {
fn id(&self) -> DiagnosticId {
DiagnosticId::Lint(LintName::of("dummy"))
}
fn message(&self) -> Cow<str> {
"dummy".into()
}
fn span(&self) -> Option<Span> {
Some(Span::from(self.file).with_range(self.range))
}
fn severity(&self) -> Severity {
Severity::Error
}
}
} }

View file

@ -6,14 +6,15 @@ use config::SystemKind;
use parser as test_parser; use parser as test_parser;
use red_knot_python_semantic::types::check_types; use red_knot_python_semantic::types::check_types;
use red_knot_python_semantic::{Program, ProgramSettings, PythonPath, SearchPathSettings}; use red_knot_python_semantic::{Program, ProgramSettings, PythonPath, SearchPathSettings};
use ruff_db::diagnostic::{DisplayDiagnosticConfig, OldDiagnosticTrait, OldParseDiagnostic}; use ruff_db::diagnostic::{create_parse_diagnostic, Diagnostic, DisplayDiagnosticConfig};
use ruff_db::files::{system_path_to_file, File}; use ruff_db::files::{system_path_to_file, File};
use ruff_db::panic::catch_unwind; use ruff_db::panic::catch_unwind;
use ruff_db::parsed::parsed_module; use ruff_db::parsed::parsed_module;
use ruff_db::system::{DbWithWritableSystem as _, SystemPath, SystemPathBuf}; use ruff_db::system::{DbWithWritableSystem as _, SystemPath, SystemPathBuf};
use ruff_db::testing::{setup_logging, setup_logging_with_filter}; use ruff_db::testing::{setup_logging, setup_logging_with_filter};
use ruff_source_file::{LineIndex, OneIndexed}; use ruff_source_file::{LineIndex, OneIndexed};
use std::fmt::Write; use std::fmt::Write as _;
use std::io::Write as _;
mod assertion; mod assertion;
mod config; mod config;
@ -220,15 +221,10 @@ fn run_test(
.filter_map(|test_file| { .filter_map(|test_file| {
let parsed = parsed_module(db, test_file.file); let parsed = parsed_module(db, test_file.file);
let mut diagnostics: Vec<Box<_>> = parsed let mut diagnostics: Vec<Diagnostic> = parsed
.errors() .errors()
.iter() .iter()
.cloned() .map(|error| create_parse_diagnostic(test_file.file, error))
.map(|error| {
let diagnostic: Box<dyn OldDiagnosticTrait> =
Box::new(OldParseDiagnostic::new(test_file.file, error));
diagnostic
})
.collect(); .collect();
let type_diagnostics = match catch_unwind(|| check_types(db, test_file.file)) { let type_diagnostics = match catch_unwind(|| check_types(db, test_file.file)) {
@ -258,19 +254,15 @@ fn run_test(
}); });
} }
}; };
diagnostics.extend(type_diagnostics.into_iter().map(|diagnostic| { diagnostics.extend(type_diagnostics.into_iter().cloned());
let diagnostic: Box<dyn OldDiagnosticTrait> = Box::new((*diagnostic).clone());
diagnostic
}));
let failure = let failure = match matcher::match_file(db, test_file.file, &diagnostics) {
match matcher::match_file(db, test_file.file, diagnostics.iter().map(|d| &**d)) { Ok(()) => None,
Ok(()) => None, Err(line_failures) => Some(FileFailures {
Err(line_failures) => Some(FileFailures { backtick_offsets: test_file.backtick_offsets,
backtick_offsets: test_file.backtick_offsets, by_line: line_failures,
by_line: line_failures, }),
}), };
};
if test.should_snapshot_diagnostics() { if test.should_snapshot_diagnostics() {
snapshot_diagnostics.extend(diagnostics); snapshot_diagnostics.extend(diagnostics);
} }
@ -324,15 +316,15 @@ struct TestFile {
backtick_offsets: Vec<BacktickOffsets>, backtick_offsets: Vec<BacktickOffsets>,
} }
fn create_diagnostic_snapshot<D: OldDiagnosticTrait>( fn create_diagnostic_snapshot(
db: &mut db::Db, db: &mut db::Db,
relative_fixture_path: &Utf8Path, relative_fixture_path: &Utf8Path,
test: &parser::MarkdownTest, test: &parser::MarkdownTest,
diagnostics: impl IntoIterator<Item = D>, diagnostics: impl IntoIterator<Item = Diagnostic>,
) -> String { ) -> String {
let display_config = DisplayDiagnosticConfig::default().color(false); let display_config = DisplayDiagnosticConfig::default().color(false);
let mut snapshot = String::new(); let mut snapshot = Vec::new();
writeln!(snapshot).unwrap(); writeln!(snapshot).unwrap();
writeln!(snapshot, "---").unwrap(); writeln!(snapshot, "---").unwrap();
writeln!(snapshot, "mdtest name: {}", test.name()).unwrap(); writeln!(snapshot, "mdtest name: {}", test.name()).unwrap();
@ -368,8 +360,8 @@ fn create_diagnostic_snapshot<D: OldDiagnosticTrait>(
writeln!(snapshot).unwrap(); writeln!(snapshot).unwrap();
} }
writeln!(snapshot, "```").unwrap(); writeln!(snapshot, "```").unwrap();
writeln!(snapshot, "{}", diag.display(db, &display_config)).unwrap(); diag.print(db, &display_config, &mut snapshot).unwrap();
writeln!(snapshot, "```").unwrap(); writeln!(snapshot, "```").unwrap();
} }
snapshot String::from_utf8(snapshot).unwrap()
} }

View file

@ -1,10 +1,10 @@
//! Match [`OldDiagnosticTrait`]s against assertions and produce test failure //! Match [`Diagnostic`]s against assertions and produce test failure
//! messages for any mismatches. //! messages for any mismatches.
use crate::assertion::{InlineFileAssertions, ParsedAssertion, UnparsedAssertion}; use crate::assertion::{InlineFileAssertions, ParsedAssertion, UnparsedAssertion};
use crate::db::Db; use crate::db::Db;
use crate::diagnostic::SortedDiagnostics; use crate::diagnostic::SortedDiagnostics;
use colored::Colorize; use colored::Colorize;
use ruff_db::diagnostic::{DiagnosticAsStrError, DiagnosticId, OldDiagnosticTrait}; use ruff_db::diagnostic::{Diagnostic, DiagnosticAsStrError, DiagnosticId};
use ruff_db::files::File; use ruff_db::files::File;
use ruff_db::source::{line_index, source_text, SourceText}; use ruff_db::source::{line_index, source_text, SourceText};
use ruff_source_file::{LineIndex, OneIndexed}; use ruff_source_file::{LineIndex, OneIndexed};
@ -47,14 +47,11 @@ struct LineFailures {
range: Range<usize>, range: Range<usize>,
} }
pub(super) fn match_file<T>( pub(super) fn match_file(
db: &Db, db: &Db,
file: File, file: File,
diagnostics: impl IntoIterator<Item = T>, diagnostics: &[Diagnostic],
) -> Result<(), FailuresByLine> ) -> Result<(), FailuresByLine> {
where
T: OldDiagnosticTrait,
{
// Parse assertions from comments in the file, and get diagnostics from the file; both // Parse assertions from comments in the file, and get diagnostics from the file; both
// ordered by line number. // ordered by line number.
let assertions = InlineFileAssertions::from_file(db, file); let assertions = InlineFileAssertions::from_file(db, file);
@ -155,8 +152,8 @@ impl Unmatched for ParsedAssertion<'_> {
} }
} }
fn maybe_add_undefined_reveal_clarification<T: OldDiagnosticTrait>( fn maybe_add_undefined_reveal_clarification(
diagnostic: &T, diagnostic: &Diagnostic,
original: std::fmt::Arguments, original: std::fmt::Arguments,
) -> String { ) -> String {
if diagnostic.id().is_lint_named("undefined-reveal") { if diagnostic.id().is_lint_named("undefined-reveal") {
@ -169,10 +166,7 @@ fn maybe_add_undefined_reveal_clarification<T: OldDiagnosticTrait>(
} }
} }
impl<T> Unmatched for T impl Unmatched for &Diagnostic {
where
T: OldDiagnosticTrait,
{
fn unmatched(&self) -> String { fn unmatched(&self) -> String {
let id = self.id(); let id = self.id();
let id = id.as_str().unwrap_or_else(|error| match error { let id = id.as_str().unwrap_or_else(|error| match error {
@ -181,15 +175,12 @@ where
maybe_add_undefined_reveal_clarification( maybe_add_undefined_reveal_clarification(
self, self,
format_args!(r#"[{id}] "{message}""#, message = self.message()), format_args!(r#"[{id}] "{message}""#, message = self.primary_message()),
) )
} }
} }
impl<T> UnmatchedWithColumn for T impl UnmatchedWithColumn for &Diagnostic {
where
T: OldDiagnosticTrait,
{
fn unmatched_with_column(&self, column: OneIndexed) -> String { fn unmatched_with_column(&self, column: OneIndexed) -> String {
let id = self.id(); let id = self.id();
let id = id.as_str().unwrap_or_else(|error| match error { let id = id.as_str().unwrap_or_else(|error| match error {
@ -198,7 +189,10 @@ where
maybe_add_undefined_reveal_clarification( maybe_add_undefined_reveal_clarification(
self, self,
format_args!(r#"{column} [{id}] "{message}""#, message = self.message()), format_args!(
r#"{column} [{id}] "{message}""#,
message = self.primary_message()
),
) )
} }
} }
@ -226,21 +220,21 @@ impl Matcher {
} }
} }
/// Check a slice of [`OldDiagnosticTrait`]s against a slice of /// Check a slice of [`Diagnostic`]s against a slice of
/// [`UnparsedAssertion`]s. /// [`UnparsedAssertion`]s.
/// ///
/// Return vector of [`Unmatched`] for any unmatched diagnostics or /// Return vector of [`Unmatched`] for any unmatched diagnostics or
/// assertions. /// assertions.
fn match_line<'a, 'b, T: OldDiagnosticTrait + 'a>( fn match_line<'a, 'b>(
&self, &self,
diagnostics: &'a [T], diagnostics: &'a [&'a Diagnostic],
assertions: &'a [UnparsedAssertion<'b>], assertions: &'a [UnparsedAssertion<'b>],
) -> Result<(), Vec<String>> ) -> Result<(), Vec<String>>
where where
'b: 'a, 'b: 'a,
{ {
let mut failures = vec![]; let mut failures = vec![];
let mut unmatched: Vec<_> = diagnostics.iter().collect(); let mut unmatched = diagnostics.to_vec();
for assertion in assertions { for assertion in assertions {
match assertion.parse() { match assertion.parse() {
Ok(assertion) => { Ok(assertion) => {
@ -263,9 +257,9 @@ impl Matcher {
} }
} }
fn column<T: OldDiagnosticTrait>(&self, diagnostic: &T) -> OneIndexed { fn column(&self, diagnostic: &Diagnostic) -> OneIndexed {
diagnostic diagnostic
.span() .primary_span()
.and_then(|span| span.range()) .and_then(|span| span.range())
.map(|range| { .map(|range| {
self.line_index self.line_index
@ -275,7 +269,7 @@ impl Matcher {
.unwrap_or(OneIndexed::from_zero_indexed(0)) .unwrap_or(OneIndexed::from_zero_indexed(0))
} }
/// Check if `assertion` matches any [`OldDiagnosticTrait`]s in `unmatched`. /// Check if `assertion` matches any [`Diagnostic`]s in `unmatched`.
/// ///
/// If so, return `true` and remove the matched diagnostics from `unmatched`. Otherwise, return /// If so, return `true` and remove the matched diagnostics from `unmatched`. Otherwise, return
/// `false`. /// `false`.
@ -285,11 +279,7 @@ impl Matcher {
/// ///
/// A `Revealed` assertion must match a revealed-type diagnostic, and may also match an /// A `Revealed` assertion must match a revealed-type diagnostic, and may also match an
/// undefined-reveal diagnostic, if present. /// undefined-reveal diagnostic, if present.
fn matches<T: OldDiagnosticTrait>( fn matches(&self, assertion: &ParsedAssertion, unmatched: &mut Vec<&Diagnostic>) -> bool {
&self,
assertion: &ParsedAssertion,
unmatched: &mut Vec<&T>,
) -> bool {
match assertion { match assertion {
ParsedAssertion::Error(error) => { ParsedAssertion::Error(error) => {
let position = unmatched.iter().position(|diagnostic| { let position = unmatched.iter().position(|diagnostic| {
@ -297,10 +287,10 @@ impl Matcher {
!(diagnostic.id().is_lint_named(rule) || diagnostic.id().matches(rule)) !(diagnostic.id().is_lint_named(rule) || diagnostic.id().matches(rule))
}) && error }) && error
.column .column
.is_none_or(|col| col == self.column(*diagnostic)) .is_none_or(|col| col == self.column(diagnostic))
&& error && error
.message_contains .message_contains
.is_none_or(|needle| diagnostic.message().contains(needle)) .is_none_or(|needle| diagnostic.primary_message().contains(needle))
}); });
if let Some(position) = position { if let Some(position) = position {
unmatched.swap_remove(position); unmatched.swap_remove(position);
@ -319,7 +309,7 @@ impl Matcher {
for (index, diagnostic) in unmatched.iter().enumerate() { for (index, diagnostic) in unmatched.iter().enumerate() {
if matched_revealed_type.is_none() if matched_revealed_type.is_none()
&& diagnostic.id() == DiagnosticId::RevealedType && diagnostic.id() == DiagnosticId::RevealedType
&& diagnostic.message() == expected_reveal_type_message && diagnostic.primary_message() == expected_reveal_type_message
{ {
matched_revealed_type = Some(index); matched_revealed_type = Some(index);
} else if matched_undefined_reveal.is_none() } else if matched_undefined_reveal.is_none()
@ -347,13 +337,12 @@ impl Matcher {
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use super::FailuresByLine; use super::FailuresByLine;
use ruff_db::diagnostic::{DiagnosticId, OldDiagnosticTrait, Severity, Span}; use ruff_db::diagnostic::{Annotation, Diagnostic, DiagnosticId, Severity, Span};
use ruff_db::files::{system_path_to_file, File}; use ruff_db::files::{system_path_to_file, File};
use ruff_db::system::DbWithWritableSystem as _; use ruff_db::system::DbWithWritableSystem as _;
use ruff_python_trivia::textwrap::dedent; use ruff_python_trivia::textwrap::dedent;
use ruff_source_file::OneIndexed; use ruff_source_file::OneIndexed;
use ruff_text_size::TextRange; use ruff_text_size::TextRange;
use std::borrow::Cow;
struct ExpectedDiagnostic { struct ExpectedDiagnostic {
id: DiagnosticId, id: DiagnosticId,
@ -371,45 +360,17 @@ mod tests {
} }
} }
fn into_diagnostic(self, file: File) -> TestDiagnostic { fn into_diagnostic(self, file: File) -> Diagnostic {
TestDiagnostic { let mut diag = Diagnostic::new(self.id, Severity::Error, "");
id: self.id, let span = Span::from(file).with_range(self.range);
message: self.message, diag.annotate(Annotation::primary(span).message(self.message));
range: self.range, diag
file,
}
}
}
#[derive(Debug)]
struct TestDiagnostic {
id: DiagnosticId,
message: &'static str,
range: TextRange,
file: File,
}
impl OldDiagnosticTrait for TestDiagnostic {
fn id(&self) -> DiagnosticId {
self.id
}
fn message(&self) -> Cow<str> {
self.message.into()
}
fn span(&self) -> Option<Span> {
Some(Span::from(self.file).with_range(self.range))
}
fn severity(&self) -> Severity {
Severity::Error
} }
} }
fn get_result( fn get_result(
source: &str, source: &str,
diagnostics: Vec<ExpectedDiagnostic>, expected_diagnostics: Vec<ExpectedDiagnostic>,
) -> Result<(), FailuresByLine> { ) -> Result<(), FailuresByLine> {
colored::control::set_override(false); colored::control::set_override(false);
@ -417,13 +378,11 @@ mod tests {
db.write_file("/src/test.py", source).unwrap(); db.write_file("/src/test.py", source).unwrap();
let file = system_path_to_file(&db, "/src/test.py").unwrap(); let file = system_path_to_file(&db, "/src/test.py").unwrap();
super::match_file( let diagnostics: Vec<Diagnostic> = expected_diagnostics
&db, .into_iter()
file, .map(|diagnostic| diagnostic.into_diagnostic(file))
diagnostics .collect();
.into_iter() super::match_file(&db, file, &diagnostics)
.map(|diagnostic| diagnostic.into_diagnostic(file)),
)
} }
fn assert_fail(result: Result<(), FailuresByLine>, messages: &[(usize, &[&str])]) { fn assert_fail(result: Result<(), FailuresByLine>, messages: &[(usize, &[&str])]) {

View file

@ -7,7 +7,7 @@ use red_knot_project::watch::{ChangeEvent, ChangedKind, CreatedKind, DeletedKind
use red_knot_project::ProjectMetadata; use red_knot_project::ProjectMetadata;
use red_knot_project::{Db, ProjectDatabase}; use red_knot_project::{Db, ProjectDatabase};
use red_knot_python_semantic::Program; use red_knot_python_semantic::Program;
use ruff_db::diagnostic::{DisplayDiagnosticConfig, OldDiagnosticTrait}; use ruff_db::diagnostic::{self, DisplayDiagnosticConfig};
use ruff_db::files::{system_path_to_file, File}; use ruff_db::files::{system_path_to_file, File};
use ruff_db::source::{line_index, source_text}; use ruff_db::source::{line_index, source_text};
use ruff_db::system::walk_directory::WalkDirectoryBuilder; use ruff_db::system::walk_directory::WalkDirectoryBuilder;
@ -223,18 +223,18 @@ impl FileHandle {
#[wasm_bindgen] #[wasm_bindgen]
pub struct Diagnostic { pub struct Diagnostic {
#[wasm_bindgen(readonly)] #[wasm_bindgen(readonly)]
inner: Box<dyn OldDiagnosticTrait>, inner: diagnostic::Diagnostic,
} }
#[wasm_bindgen] #[wasm_bindgen]
impl Diagnostic { impl Diagnostic {
fn wrap(diagnostic: Box<dyn OldDiagnosticTrait>) -> Self { fn wrap(diagnostic: diagnostic::Diagnostic) -> Self {
Self { inner: diagnostic } Self { inner: diagnostic }
} }
#[wasm_bindgen] #[wasm_bindgen]
pub fn message(&self) -> JsString { pub fn message(&self) -> JsString {
JsString::from(&*self.inner.message()) JsString::from(self.inner.primary_message())
} }
#[wasm_bindgen] #[wasm_bindgen]
@ -250,13 +250,13 @@ impl Diagnostic {
#[wasm_bindgen(js_name = "textRange")] #[wasm_bindgen(js_name = "textRange")]
pub fn text_range(&self) -> Option<TextRange> { pub fn text_range(&self) -> Option<TextRange> {
self.inner self.inner
.span() .primary_span()
.and_then(|span| Some(TextRange::from(span.range()?))) .and_then(|span| Some(TextRange::from(span.range()?)))
} }
#[wasm_bindgen(js_name = "toRange")] #[wasm_bindgen(js_name = "toRange")]
pub fn to_range(&self, workspace: &Workspace) -> Option<Range> { pub fn to_range(&self, workspace: &Workspace) -> Option<Range> {
self.inner.span().and_then(|span| { self.inner.primary_span().and_then(|span| {
let line_index = line_index(workspace.db.upcast(), span.file()); let line_index = line_index(workspace.db.upcast(), span.file());
let source = source_text(workspace.db.upcast(), span.file()); let source = source_text(workspace.db.upcast(), span.file());
let text_range = span.range()?; let text_range = span.range()?;
@ -273,10 +273,11 @@ impl Diagnostic {
#[wasm_bindgen] #[wasm_bindgen]
pub fn display(&self, workspace: &Workspace) -> JsString { pub fn display(&self, workspace: &Workspace) -> JsString {
let config = DisplayDiagnosticConfig::default().color(false); let config = DisplayDiagnosticConfig::default().color(false);
let mut buf = vec![];
self.inner self.inner
.display(workspace.db.upcast(), &config) .print(workspace.db.upcast(), &config, &mut buf)
.to_string() .unwrap();
.into() String::from_utf8(buf).unwrap().into()
} }
} }

View file

@ -1,7 +1,6 @@
#![allow(clippy::disallowed_names)] #![allow(clippy::disallowed_names)]
use ruff_benchmark::criterion; use ruff_benchmark::criterion;
use std::borrow::Cow;
use std::ops::Range; use std::ops::Range;
use criterion::{criterion_group, criterion_main, BatchSize, Criterion}; use criterion::{criterion_group, criterion_main, BatchSize, Criterion};
@ -13,7 +12,7 @@ use red_knot_project::metadata::value::RangedValue;
use red_knot_project::watch::{ChangeEvent, ChangedKind}; use red_knot_project::watch::{ChangeEvent, ChangedKind};
use red_knot_project::{Db, ProjectDatabase, ProjectMetadata}; use red_knot_project::{Db, ProjectDatabase, ProjectMetadata};
use ruff_benchmark::TestFile; use ruff_benchmark::TestFile;
use ruff_db::diagnostic::{DiagnosticId, OldDiagnosticTrait, Severity}; use ruff_db::diagnostic::{Diagnostic, DiagnosticId, Severity};
use ruff_db::files::{system_path_to_file, File}; use ruff_db::files::{system_path_to_file, File};
use ruff_db::source::source_text; use ruff_db::source::source_text;
use ruff_db::system::{MemoryFileSystem, SystemPath, SystemPathBuf, TestSystem}; use ruff_db::system::{MemoryFileSystem, SystemPath, SystemPathBuf, TestSystem};
@ -56,7 +55,7 @@ type KeyDiagnosticFields = (
DiagnosticId, DiagnosticId,
Option<&'static str>, Option<&'static str>,
Option<Range<usize>>, Option<Range<usize>>,
Cow<'static, str>, &'static str,
Severity, Severity,
); );
@ -64,7 +63,7 @@ static EXPECTED_DIAGNOSTICS: &[KeyDiagnosticFields] = &[(
DiagnosticId::lint("unused-ignore-comment"), DiagnosticId::lint("unused-ignore-comment"),
Some("/src/tomllib/_parser.py"), Some("/src/tomllib/_parser.py"),
Some(22299..22333), Some(22299..22333),
Cow::Borrowed("Unused blanket `type: ignore` directive"), "Unused blanket `type: ignore` directive",
Severity::Warning, Severity::Warning,
)]; )];
@ -193,21 +192,21 @@ fn benchmark_cold(criterion: &mut Criterion) {
} }
#[track_caller] #[track_caller]
fn assert_diagnostics(db: &dyn Db, diagnostics: &[Box<dyn OldDiagnosticTrait>]) { fn assert_diagnostics(db: &dyn Db, diagnostics: &[Diagnostic]) {
let normalized: Vec<_> = diagnostics let normalized: Vec<_> = diagnostics
.iter() .iter()
.map(|diagnostic| { .map(|diagnostic| {
( (
diagnostic.id(), diagnostic.id(),
diagnostic diagnostic
.span() .primary_span()
.map(|span| span.file()) .map(|span| span.file())
.map(|file| file.path(db).as_str()), .map(|file| file.path(db).as_str()),
diagnostic diagnostic
.span() .primary_span()
.and_then(|span| span.range()) .and_then(|span| span.range())
.map(Range::<usize>::from), .map(Range::<usize>::from),
diagnostic.message(), diagnostic.primary_message(),
diagnostic.severity(), diagnostic.severity(),
) )
}) })

View file

@ -119,6 +119,54 @@ impl Diagnostic {
let display = DisplayDiagnostic::new(&resolver, config, self); let display = DisplayDiagnostic::new(&resolver, config, self);
writeln!(wtr, "{display}") writeln!(wtr, "{display}")
} }
/// Returns the identifier for this diagnostic.
pub fn id(&self) -> DiagnosticId {
self.inner.id
}
/// Returns the primary message for this diagnostic.
///
/// A diagnostic always has a message, but it may be empty.
pub fn primary_message(&self) -> &str {
if !self.inner.message.is_empty() {
return &self.inner.message;
}
// FIXME: As a special case, while we're migrating Red Knot
// to the new diagnostic data model, we'll look for a primary
// message from the primary annotation. This is because most
// Red Knot diagnostics are created with an empty diagnostic
// message and instead attach the message to the annotation.
// Fixing this will require touching basically every diagnostic
// in Red Knot, so we do it this way for now to match the old
// semantics. ---AG
self.primary_annotation()
.and_then(|ann| ann.message.as_deref())
.unwrap_or_default()
}
/// Returns the severity of this diagnostic.
///
/// Note that this may be different than the severity of sub-diagnostics.
pub fn severity(&self) -> Severity {
self.inner.severity
}
/// Returns the "primary" annotation of this diagnostic if one exists.
///
/// When there are multiple primary annotation, then the first one that was
/// added to this diagnostic is returned.
pub fn primary_annotation(&self) -> Option<&Annotation> {
self.inner.annotations.iter().find(|ann| ann.is_primary)
}
/// Returns the "primary" span of this diagnostic if one exists.
///
/// When there are multiple primary spans, then the first one that was
/// added to this diagnostic is returned.
pub fn primary_span(&self) -> Option<Span> {
self.primary_annotation().map(|ann| ann.span.clone())
}
} }
#[derive(Debug, Clone, Eq, PartialEq)] #[derive(Debug, Clone, Eq, PartialEq)]
@ -564,3 +612,16 @@ pub enum DiagnosticFormat {
/// This may use color when printing to a `tty`. /// This may use color when printing to a `tty`.
Concise, Concise,
} }
/// Creates a `Diagnostic` from a parse error.
///
/// This should _probably_ be a method on `ruff_python_parser::ParseError`, but
/// at time of writing, `ruff_db` depends on `ruff_python_parser` instead of
/// the other way around. And since we want to do this conversion in a couple
/// places, it makes sense to centralize it _somewhere_. So it's here for now.
pub fn create_parse_diagnostic(file: File, err: &ruff_python_parser::ParseError) -> Diagnostic {
let mut diag = Diagnostic::new(DiagnosticId::InvalidSyntax, Severity::Error, "");
let span = Span::from(file).with_range(err.location);
diag.annotate(Annotation::primary(span).message(&err.error));
diag
}