From 5c9f31d4c28478b4373e6cf5ec155745c840ee3f Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sun, 23 May 2021 23:31:59 +0300 Subject: internal: move diagnostics to hir The idea here is to eventually get rid of `dyn Diagnostic` and `DiagnosticSink` infrastructure altogether, and just have a `enum hir::Diagnostic` instead. The problem with `dyn Diagnostic` is that it is defined in the lowest level of the stack (hir_expand), but is used by the highest level (ide). As a first step, we free hir_expand and hir_def from `dyn Diagnostic` and kick the can up to `hir_ty`, as an intermediate state. The plan is then to move DiagnosticSink similarly to the hir crate, and, as final third step, remove its usage from the ide. One currently unsolved problem is testing. You can notice that the test which checks precise diagnostic ranges, unresolved_import_in_use_tree, was moved to the ide layer. Logically, only IDE should have the infra to render a specific range. At the same time, the range is determined with the data produced in hir_def and hir crates, so this layering is rather unfortunate. Working on hir_def shouldn't require compiling `ide` for testing. --- crates/hir_expand/src/db.rs | 2 +- crates/hir_expand/src/diagnostics.rs | 110 ----------------------------------- crates/hir_expand/src/lib.rs | 7 +-- 3 files changed, 4 insertions(+), 115 deletions(-) delete mode 100644 crates/hir_expand/src/diagnostics.rs (limited to 'crates/hir_expand') diff --git a/crates/hir_expand/src/db.rs b/crates/hir_expand/src/db.rs index 625c26f0a..e8f4af309 100644 --- a/crates/hir_expand/src/db.rs +++ b/crates/hir_expand/src/db.rs @@ -186,7 +186,7 @@ fn parse_macro_expansion( // The final goal we would like to make all parse_macro success, // such that the following log will not call anyway. let loc: MacroCallLoc = db.lookup_intern_macro(macro_file.macro_call_id); - let node = loc.kind.node(db); + let node = loc.kind.to_node(db); // collect parent information for warning log let parents = diff --git a/crates/hir_expand/src/diagnostics.rs b/crates/hir_expand/src/diagnostics.rs deleted file mode 100644 index bf0b85ce9..000000000 --- a/crates/hir_expand/src/diagnostics.rs +++ /dev/null @@ -1,110 +0,0 @@ -//! Semantic errors and warnings. -//! -//! The `Diagnostic` trait defines a trait object which can represent any -//! diagnostic. -//! -//! `DiagnosticSink` struct is used as an emitter for diagnostic. When creating -//! a `DiagnosticSink`, you supply a callback which can react to a `dyn -//! Diagnostic` or to any concrete diagnostic (downcasting is used internally). -//! -//! Because diagnostics store file offsets, it's a bad idea to store them -//! directly in salsa. For this reason, every hir subsytem defines it's own -//! strongly-typed closed set of diagnostics which use hir ids internally, are -//! stored in salsa and do *not* implement the `Diagnostic` trait. Instead, a -//! subsystem provides a separate, non-query-based API which can walk all stored -//! values and transform them into instances of `Diagnostic`. - -use std::{any::Any, fmt}; - -use syntax::SyntaxNodePtr; - -use crate::InFile; - -#[derive(Copy, Clone, Debug, PartialEq)] -pub struct DiagnosticCode(pub &'static str); - -impl DiagnosticCode { - pub fn as_str(&self) -> &str { - self.0 - } -} - -pub trait Diagnostic: Any + Send + Sync + fmt::Debug + 'static { - fn code(&self) -> DiagnosticCode; - fn message(&self) -> String; - /// Source element that triggered the diagnostics. - /// - /// Note that this should reflect "semantics", rather than specific span we - /// want to highlight. When rendering the diagnostics into an error message, - /// the IDE will fetch the `SyntaxNode` and will narrow the span - /// appropriately. - fn display_source(&self) -> InFile; - fn as_any(&self) -> &(dyn Any + Send + 'static); - fn is_experimental(&self) -> bool { - false - } -} - -pub struct DiagnosticSink<'a> { - callbacks: Vec Result<(), ()> + 'a>>, - filters: Vec bool + 'a>>, - default_callback: Box, -} - -impl<'a> DiagnosticSink<'a> { - pub fn push(&mut self, d: impl Diagnostic) { - let d: &dyn Diagnostic = &d; - self._push(d); - } - - fn _push(&mut self, d: &dyn Diagnostic) { - for filter in &mut self.filters { - if !filter(d) { - return; - } - } - for cb in &mut self.callbacks { - match cb(d) { - Ok(()) => return, - Err(()) => (), - } - } - (self.default_callback)(d) - } -} - -pub struct DiagnosticSinkBuilder<'a> { - callbacks: Vec Result<(), ()> + 'a>>, - filters: Vec bool + 'a>>, -} - -impl<'a> DiagnosticSinkBuilder<'a> { - pub fn new() -> Self { - Self { callbacks: Vec::new(), filters: Vec::new() } - } - - pub fn filter bool + 'a>(mut self, cb: F) -> Self { - self.filters.push(Box::new(cb)); - self - } - - pub fn on(mut self, mut cb: F) -> Self { - let cb = move |diag: &dyn Diagnostic| match diag.as_any().downcast_ref::() { - Some(d) => { - cb(d); - Ok(()) - } - None => Err(()), - }; - self.callbacks.push(Box::new(cb)); - self - } - - pub fn build(self, default_callback: F) -> DiagnosticSink<'a> { - DiagnosticSink { - callbacks: self.callbacks, - filters: self.filters, - default_callback: Box::new(default_callback), - } - } -} diff --git a/crates/hir_expand/src/lib.rs b/crates/hir_expand/src/lib.rs index 6be4516a3..10d37234e 100644 --- a/crates/hir_expand/src/lib.rs +++ b/crates/hir_expand/src/lib.rs @@ -8,7 +8,6 @@ pub mod db; pub mod ast_id_map; pub mod name; pub mod hygiene; -pub mod diagnostics; pub mod builtin_derive; pub mod builtin_macro; pub mod proc_macro; @@ -108,7 +107,7 @@ impl HirFileId { HirFileIdRepr::FileId(_) => None, HirFileIdRepr::MacroFile(macro_file) => { let loc: MacroCallLoc = db.lookup_intern_macro(macro_file.macro_call_id); - Some(loc.kind.node(db)) + Some(loc.kind.to_node(db)) } } } @@ -153,7 +152,7 @@ impl HirFileId { HirFileIdRepr::MacroFile(macro_file) => { let loc: MacroCallLoc = db.lookup_intern_macro(macro_file.macro_call_id); let item = match loc.def.kind { - MacroDefKind::BuiltInDerive(..) => loc.kind.node(db), + MacroDefKind::BuiltInDerive(..) => loc.kind.to_node(db), _ => return None, }; Some(item.with_value(ast::Item::cast(item.value.clone())?)) @@ -269,7 +268,7 @@ impl MacroCallKind { } } - fn node(&self, db: &dyn db::AstDatabase) -> InFile { + pub fn to_node(&self, db: &dyn db::AstDatabase) -> InFile { match self { MacroCallKind::FnLike { ast_id, .. } => { ast_id.with_value(ast_id.to_node(db).syntax().clone()) -- cgit v1.2.3