internal: kill diagnostic sink

crates/hir/src/diagnostics.rs

@@ -9,10 +9,6 @@ use hir_def::path::ModPath;
 use hir_expand::{name::Name, HirFileId, InFile};
 use syntax::{ast, AstPtr, SyntaxNodePtr, TextRange};
 
-pub use crate::diagnostics_sink::{
-    Diagnostic, DiagnosticCode, DiagnosticSink, DiagnosticSinkBuilder,
-};
-
 macro_rules! diagnostics {
     ($($diag:ident,)*) => {
         pub enum AnyDiagnostic {$(

crates/hir/src/diagnostics_sink.rs

@@ -1,109 +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 hir_expand::InFile;
-use syntax::SyntaxNodePtr;
-
-#[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<SyntaxNodePtr>;
-    fn as_any(&self) -> &(dyn Any + Send + 'static);
-    fn is_experimental(&self) -> bool {
-        false
-    }
-}
-
-pub struct DiagnosticSink<'a> {
-    callbacks: Vec<Box<dyn FnMut(&dyn Diagnostic) -> Result<(), ()> + 'a>>,
-    filters: Vec<Box<dyn FnMut(&dyn Diagnostic) -> bool + 'a>>,
-    default_callback: Box<dyn FnMut(&dyn Diagnostic) + 'a>,
-}
-
-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<Box<dyn FnMut(&dyn Diagnostic) -> Result<(), ()> + 'a>>,
-    filters: Vec<Box<dyn FnMut(&dyn Diagnostic) -> bool + 'a>>,
-}
-
-impl<'a> DiagnosticSinkBuilder<'a> {
-    pub fn new() -> Self {
-        Self { callbacks: Vec::new(), filters: Vec::new() }
-    }
-
-    pub fn filter<F: FnMut(&dyn Diagnostic) -> bool + 'a>(mut self, cb: F) -> Self {
-        self.filters.push(Box::new(cb));
-        self
-    }
-
-    pub fn on<D: Diagnostic, F: FnMut(&D) + 'a>(mut self, mut cb: F) -> Self {
-        let cb = move |diag: &dyn Diagnostic| match diag.as_any().downcast_ref::<D>() {
-            Some(d) => {
-                cb(d);
-                Ok(())
-            }
-            None => Err(()),
-        };
-        self.callbacks.push(Box::new(cb));
-        self
-    }
-
-    pub fn build<F: FnMut(&dyn Diagnostic) + 'a>(self, default_callback: F) -> DiagnosticSink<'a> {
-        DiagnosticSink {
-            callbacks: self.callbacks,
-            filters: self.filters,
-            default_callback: Box::new(default_callback),
-        }
-    }
-}

crates/hir/src/lib.rs

@@ -27,7 +27,6 @@ mod attrs;
 mod has_source;
 
 pub mod diagnostics;
-pub mod diagnostics_sink;
 pub mod db;
 
 mod display;
@@ -78,10 +77,7 @@ use syntax::{
 };
 use tt::{Ident, Leaf, Literal, TokenTree};
 
-use crate::{
+use crate::db::{DefDatabase, HirDatabase};
-    db::{DefDatabase, HirDatabase},
-    diagnostics_sink::DiagnosticSink,
-};
 
 pub use crate::{
     attrs::{HasAttrs, Namespace},
@@ -457,15 +453,10 @@ impl Module {
         self.id.def_map(db.upcast())[self.id.local_id].scope.visibility_of((*def).into())
     }
 
-    pub fn diagnostics(
+    pub fn diagnostics(self, db: &dyn HirDatabase, acc: &mut Vec<AnyDiagnostic>) {
-        self,
-        db: &dyn HirDatabase,
-        sink: &mut DiagnosticSink,
-    ) -> Vec<AnyDiagnostic> {
         let _p = profile::span("Module::diagnostics").detail(|| {
             format!("{:?}", self.name(db).map_or("<unknown>".into(), |name| name.to_string()))
         });
-        let mut acc: Vec<AnyDiagnostic> = Vec::new();
         let def_map = self.id.def_map(db.upcast());
         for diag in def_map.diagnostics() {
             if diag.in_module != self.id.local_id {
@@ -618,11 +609,11 @@ impl Module {
         }
         for decl in self.declarations(db) {
             match decl {
-                ModuleDef::Function(f) => acc.extend(f.diagnostics(db, sink)),
+                ModuleDef::Function(f) => f.diagnostics(db, acc),
                 ModuleDef::Module(m) => {
                     // Only add diagnostics from inline modules
                     if def_map[m.id.local_id].origin.is_inline() {
-                        acc.extend(m.diagnostics(db, sink))
+                        m.diagnostics(db, acc)
                     }
                 }
                 _ => acc.extend(decl.diagnostics(db)),
@@ -632,11 +623,10 @@ impl Module {
         for impl_def in self.impl_defs(db) {
             for item in impl_def.items(db) {
                 if let AssocItem::Function(f) = item {
-                    acc.extend(f.diagnostics(db, sink));
+                    f.diagnostics(db, acc);
                 }
             }
         }
-        acc
     }
 
     pub fn declarations(self, db: &dyn HirDatabase) -> Vec<ModuleDef> {
@@ -1035,12 +1025,7 @@ impl Function {
         db.function_data(self.id).is_async()
     }
 
-    pub fn diagnostics(
+    pub fn diagnostics(self, db: &dyn HirDatabase, acc: &mut Vec<AnyDiagnostic>) {
-        self,
-        db: &dyn HirDatabase,
-        sink: &mut DiagnosticSink,
-    ) -> Vec<AnyDiagnostic> {
-        let mut acc: Vec<AnyDiagnostic> = Vec::new();
         let krate = self.module(db).id.krate();
 
         let source_map = db.body_with_source_map(self.id.into()).1;
@@ -1225,7 +1210,6 @@ impl Function {
         for diag in hir_ty::diagnostics::validate_module_item(db, krate, self.id.into()) {
             acc.push(diag.into())
         }
-        acc
     }
 
     /// Whether this function declaration has a definition.

crates/ide/src/diagnostics.rs

@@ -26,12 +26,7 @@ mod unresolved_proc_macro;
 
 mod field_shorthand;
 
-use std::cell::RefCell;
+use hir::{diagnostics::AnyDiagnostic, Semantics};
-
-use hir::{
-    diagnostics::{AnyDiagnostic, DiagnosticCode, DiagnosticSinkBuilder},
-    Semantics,
-};
 use ide_assists::AssistResolveStrategy;
 use ide_db::{base_db::SourceDatabase, RootDatabase};
 use itertools::Itertools;
@@ -45,6 +40,15 @@ use unlinked_file::UnlinkedFile;
 
 use crate::{Assist, AssistId, AssistKind, FileId, Label, SourceChange};
 
+#[derive(Copy, Clone, Debug, PartialEq)]
+pub struct DiagnosticCode(pub &'static str);
+
+impl DiagnosticCode {
+    pub fn as_str(&self) -> &str {
+        self.0
+    }
+}
+
 #[derive(Debug)]
 pub struct Diagnostic {
     // pub name: Option<String>,
@@ -113,10 +117,6 @@ impl Diagnostic {
     fn with_unused(self, unused: bool) -> Self {
         Self { unused, ..self }
     }
-
-    fn with_code(self, code: Option<DiagnosticCode>) -> Self {
-        Self { code, ..self }
-    }
 }
 
 #[derive(Debug, Copy, Clone)]
@@ -161,35 +161,13 @@ pub(crate) fn diagnostics(
         check_unnecessary_braces_in_use_statement(&mut res, file_id, &node);
         field_shorthand::check(&mut res, file_id, &node);
     }
-    let res = RefCell::new(res);
-    let sink_builder = DiagnosticSinkBuilder::new()
-        // Only collect experimental diagnostics when they're enabled.
-        .filter(|diag| !(diag.is_experimental() && config.disable_experimental))
-        .filter(|diag| !config.disabled.contains(diag.code().as_str()));
-
-    // Finalize the `DiagnosticSink` building process.
-    let mut sink = sink_builder
-        // Diagnostics not handled above get no fix and default treatment.
-        .build(|d| {
-            res.borrow_mut().push(
-                Diagnostic::error(
-                    sema.diagnostics_display_range(d.display_source()).range,
-                    d.message(),
-                )
-                .with_code(Some(d.code())),
-            );
-        });
 
     let mut diags = Vec::new();
     let module = sema.to_module_def(file_id);
     if let Some(m) = module {
-        diags = m.diagnostics(db, &mut sink)
+        m.diagnostics(db, &mut diags)
     }
 
-    drop(sink);
-
-    let mut res = res.into_inner();
-
     let ctx = DiagnosticsContext { config, sema, resolve };
     if module.is_none() {
         let d = UnlinkedFile { file: file_id };
@@ -350,8 +328,8 @@ mod tests {
             )
             .unwrap()
             .pop()
-            .unwrap();
+            .expect("no diagnostics");
-        let fix = &diagnostic.fixes.unwrap()[nth];
+        let fix = &diagnostic.fixes.expect("diagnostic misses fixes")[nth];
         let actual = {
             let source_change = fix.source_change.as_ref().unwrap();
             let file_id = *source_change.source_file_edits.keys().next().unwrap();