8 files changed, 107 insertions, 89 deletions

diff --git a/crates/ra_hir/src/code_model.rs b/crates/ra_hir/src/code_model.rs
index c97ea18a2..5b78bdfef 100644
--- a/crates/ra_hir/src/code_model.rs
+++ b/crates/ra_hir/src/code_model.rs
@@ -11,14 +11,16 @@ use hir_def::{
     type_ref::{Mutability, TypeRef},
     CrateModuleId, LocalEnumVariantId, LocalStructFieldId, ModuleId,
 };
-use hir_expand::name::{self, AsName};
+use hir_expand::{
+    diagnostics::DiagnosticSink,
+    name::{self, AsName},
+};
 use ra_db::{CrateId, Edition};
 use ra_syntax::ast::{self, NameOwner, TypeAscriptionOwner};
 
 use crate::{
     adt::VariantDef,
     db::{AstDatabase, DefDatabase, HirDatabase},
-    diagnostics::DiagnosticSink,
     expr::{validation::ExprValidator, Body, BodySourceMap},
     generics::HasGenericParams,
     ids::{
diff --git a/crates/ra_hir/src/diagnostics.rs b/crates/ra_hir/src/diagnostics.rs
index 9acdaf8ed..a33af8f46 100644
--- a/crates/ra_hir/src/diagnostics.rs
+++ b/crates/ra_hir/src/diagnostics.rs
@@ -1,82 +1,13 @@
 //! FIXME: write short doc here
 
-use std::{any::Any, fmt};
+use std::any::Any;
 
-use ra_syntax::{ast, AstNode, AstPtr, SyntaxNode, SyntaxNodePtr, TextRange};
+use ra_syntax::{ast, AstNode, AstPtr, SyntaxNodePtr};
 use relative_path::RelativePathBuf;
 
-use crate::{db::HirDatabase, HirFileId, Name, Source};
-
-/// Diagnostic defines hir API for errors and warnings.
-///
-/// It is used as a `dyn` object, which you can downcast to a concrete
-/// diagnostic. DiagnosticSink are structured, meaning that they include rich
-/// information which can be used by IDE to create fixes. DiagnosticSink are
-/// expressed in terms of macro-expanded syntax tree nodes (so, it's a bad idea
-/// to diagnostic in a salsa value).
-///
-/// Internally, various subsystems of hir produce diagnostics specific to a
-/// subsystem (typically, an `enum`), which are safe to store in salsa but do not
-/// include source locations. Such internal diagnostic are transformed into an
-/// instance of `Diagnostic` on demand.
-pub trait Diagnostic: Any + Send + Sync + fmt::Debug + 'static {
-    fn message(&self) -> String;
-    fn source(&self) -> Source<SyntaxNodePtr>;
-    fn highlight_range(&self) -> TextRange {
-        self.source().ast.range()
-    }
-    fn as_any(&self) -> &(dyn Any + Send + 'static);
-}
-
-pub trait AstDiagnostic {
-    type AST;
-    fn ast(&self, db: &impl HirDatabase) -> Self::AST;
-}
-
-impl dyn Diagnostic {
-    pub fn syntax_node(&self, db: &impl HirDatabase) -> SyntaxNode {
-        let node = db.parse_or_expand(self.source().file_id).unwrap();
-        self.source().ast.to_node(&node)
-    }
-
-    pub fn downcast_ref<D: Diagnostic>(&self) -> Option<&D> {
-        self.as_any().downcast_ref()
-    }
-}
+use crate::{db::AstDatabase, HirFileId, Name, Source};
 
-pub struct DiagnosticSink<'a> {
-    callbacks: Vec<Box<dyn FnMut(&dyn Diagnostic) -> Result<(), ()> + 'a>>,
-    default_callback: Box<dyn FnMut(&dyn Diagnostic) + 'a>,
-}
-
-impl<'a> DiagnosticSink<'a> {
-    pub fn new(cb: impl FnMut(&dyn Diagnostic) + 'a) -> DiagnosticSink<'a> {
-        DiagnosticSink { callbacks: Vec::new(), default_callback: Box::new(cb) }
-    }
-
-    pub fn on<D: Diagnostic, F: FnMut(&D) + 'a>(mut self, mut cb: F) -> DiagnosticSink<'a> {
-        let cb = move |diag: &dyn Diagnostic| match diag.downcast_ref::<D>() {
-            Some(d) => {
-                cb(d);
-                Ok(())
-            }
-            None => Err(()),
-        };
-        self.callbacks.push(Box::new(cb));
-        self
-    }
-
-    pub(crate) fn push(&mut self, d: impl Diagnostic) {
-        let d: &dyn Diagnostic = &d;
-        for cb in self.callbacks.iter_mut() {
-            match cb(d) {
-                Ok(()) => return,
-                Err(()) => (),
-            }
-        }
-        (self.default_callback)(d)
-    }
-}
+pub use hir_expand::diagnostics::{AstDiagnostic, Diagnostic, DiagnosticSink};
 
 #[derive(Debug)]
 pub struct NoSuchField {
@@ -139,7 +70,7 @@ impl Diagnostic for MissingFields {
 impl AstDiagnostic for MissingFields {
     type AST = ast::RecordFieldList;
 
-    fn ast(&self, db: &impl HirDatabase) -> Self::AST {
+    fn ast(&self, db: &impl AstDatabase) -> Self::AST {
         let root = db.parse_or_expand(self.source().file_id).unwrap();
         let node = self.source().ast.to_node(&root);
         ast::RecordFieldList::cast(node).unwrap()
@@ -167,7 +98,7 @@ impl Diagnostic for MissingOkInTailExpr {
 impl AstDiagnostic for MissingOkInTailExpr {
     type AST = ast::Expr;
 
-    fn ast(&self, db: &impl HirDatabase) -> Self::AST {
+    fn ast(&self, db: &impl AstDatabase) -> Self::AST {
         let root = db.parse_or_expand(self.file).unwrap();
         let node = self.source().ast.to_node(&root);
         ast::Expr::cast(node).unwrap()
diff --git a/crates/ra_hir/src/expr/validation.rs b/crates/ra_hir/src/expr/validation.rs
index c685edda1..3054f1dce 100644
--- a/crates/ra_hir/src/expr/validation.rs
+++ b/crates/ra_hir/src/expr/validation.rs
@@ -3,12 +3,13 @@
 use std::sync::Arc;
 
 use hir_def::path::known;
+use hir_expand::diagnostics::DiagnosticSink;
 use ra_syntax::ast;
 use rustc_hash::FxHashSet;
 
 use crate::{
     db::HirDatabase,
-    diagnostics::{DiagnosticSink, MissingFields, MissingOkInTailExpr},
+    diagnostics::{MissingFields, MissingOkInTailExpr},
     expr::AstPtr,
     ty::{ApplicationTy, InferenceResult, Ty, TypeCtor},
     Adt, Function, Name, Path,
diff --git a/crates/ra_hir/src/mock.rs b/crates/ra_hir/src/mock.rs
index 35dfaf3ba..4c89c8d38 100644
--- a/crates/ra_hir/src/mock.rs
+++ b/crates/ra_hir/src/mock.rs
@@ -2,6 +2,7 @@
 
 use std::{panic, sync::Arc};
 
+use hir_expand::diagnostics::DiagnosticSink;
 use parking_lot::Mutex;
 use ra_cfg::CfgOptions;
 use ra_db::{
@@ -12,7 +13,7 @@ use relative_path::{RelativePath, RelativePathBuf};
 use rustc_hash::FxHashMap;
 use test_utils::{extract_offset, parse_fixture, CURSOR_MARKER};
 
-use crate::{db, debug::HirDebugHelper, diagnostics::DiagnosticSink};
+use crate::{db, debug::HirDebugHelper};
 
 pub const WORKSPACE: SourceRootId = SourceRootId(0);
 
diff --git a/crates/ra_hir/src/nameres.rs b/crates/ra_hir/src/nameres.rs
index 7ba031827..32a6ab474 100644
--- a/crates/ra_hir/src/nameres.rs
+++ b/crates/ra_hir/src/nameres.rs
@@ -55,6 +55,7 @@ mod tests;
 use std::sync::Arc;
 
 use hir_def::{builtin_type::BuiltinType, CrateModuleId};
+use hir_expand::diagnostics::DiagnosticSink;
 use once_cell::sync::Lazy;
 use ra_arena::Arena;
 use ra_db::{Edition, FileId};
@@ -65,7 +66,6 @@ use test_utils::tested_by;
 
 use crate::{
     db::{AstDatabase, DefDatabase},
-    diagnostics::DiagnosticSink,
     ids::MacroDefId,
     nameres::diagnostics::DefDiagnostic,
     Adt, AstId, Crate, HirFileId, MacroDef, Module, ModuleDef, Name, Path, PathKind, Trait,
@@ -513,12 +513,13 @@ impl CrateDefMap {
 }
 
 mod diagnostics {
+    use hir_expand::diagnostics::DiagnosticSink;
     use ra_syntax::{ast, AstPtr};
     use relative_path::RelativePathBuf;
 
     use crate::{
         db::{AstDatabase, DefDatabase},
-        diagnostics::{DiagnosticSink, UnresolvedModule},
+        diagnostics::UnresolvedModule,
         nameres::CrateModuleId,
         AstId,
     };
diff --git a/crates/ra_hir/src/ty/infer.rs b/crates/ra_hir/src/ty/infer.rs
index 6694467a3..2370e8d4f 100644
--- a/crates/ra_hir/src/ty/infer.rs
+++ b/crates/ra_hir/src/ty/infer.rs
@@ -25,7 +25,7 @@ use hir_def::{
     path::known,
     type_ref::{Mutability, TypeRef},
 };
-use hir_expand::name;
+use hir_expand::{diagnostics::DiagnosticSink, name};
 use ra_arena::map::ArenaMap;
 use ra_prof::profile;
 use test_utils::tested_by;
@@ -40,7 +40,6 @@ use crate::{
     adt::VariantDef,
     code_model::TypeAlias,
     db::HirDatabase,
-    diagnostics::DiagnosticSink,
     expr::{BindingAnnotation, Body, ExprId, PatId},
     resolve::{Resolver, TypeNs},
     ty::infer::diagnostics::InferenceDiagnostic,
@@ -719,12 +718,9 @@ impl Expectation {
 }
 
 mod diagnostics {
-    use crate::{
-        db::HirDatabase,
-        diagnostics::{DiagnosticSink, NoSuchField},
-        expr::ExprId,
-        Function, HasSource,
-    };
+    use hir_expand::diagnostics::DiagnosticSink;
+
+    use crate::{db::HirDatabase, diagnostics::NoSuchField, expr::ExprId, Function, HasSource};
 
     #[derive(Debug, PartialEq, Eq, Clone)]
     pub(super) enum InferenceDiagnostic {
diff --git a/crates/ra_hir_expand/src/diagnostics.rs b/crates/ra_hir_expand/src/diagnostics.rs
new file mode 100644
index 000000000..201884b95
--- /dev/null
+++ b/crates/ra_hir_expand/src/diagnostics.rs
@@ -0,0 +1,85 @@
+//! 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 sued 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 ra_syntax::{SyntaxNode, SyntaxNodePtr, TextRange};
+
+use crate::{db::AstDatabase, Source};
+
+pub trait Diagnostic: Any + Send + Sync + fmt::Debug + 'static {
+    fn message(&self) -> String;
+    fn source(&self) -> Source<SyntaxNodePtr>;
+    fn highlight_range(&self) -> TextRange {
+        self.source().ast.range()
+    }
+    fn as_any(&self) -> &(dyn Any + Send + 'static);
+}
+
+pub trait AstDiagnostic {
+    type AST;
+    fn ast(&self, db: &impl AstDatabase) -> Self::AST;
+}
+
+impl dyn Diagnostic {
+    pub fn syntax_node(&self, db: &impl AstDatabase) -> SyntaxNode {
+        let node = db.parse_or_expand(self.source().file_id).unwrap();
+        self.source().ast.to_node(&node)
+    }
+
+    pub fn downcast_ref<D: Diagnostic>(&self) -> Option<&D> {
+        self.as_any().downcast_ref()
+    }
+}
+
+pub struct DiagnosticSink<'a> {
+    callbacks: Vec<Box<dyn FnMut(&dyn Diagnostic) -> Result<(), ()> + 'a>>,
+    default_callback: Box<dyn FnMut(&dyn Diagnostic) + 'a>,
+}
+
+impl<'a> DiagnosticSink<'a> {
+    /// FIXME: split `new` and `on` into a separate builder type
+    pub fn new(cb: impl FnMut(&dyn Diagnostic) + 'a) -> DiagnosticSink<'a> {
+        DiagnosticSink { callbacks: Vec::new(), default_callback: Box::new(cb) }
+    }
+
+    pub fn on<D: Diagnostic, F: FnMut(&D) + 'a>(mut self, mut cb: F) -> DiagnosticSink<'a> {
+        let cb = move |diag: &dyn Diagnostic| match diag.downcast_ref::<D>() {
+            Some(d) => {
+                cb(d);
+                Ok(())
+            }
+            None => Err(()),
+        };
+        self.callbacks.push(Box::new(cb));
+        self
+    }
+
+    pub fn push(&mut self, d: impl Diagnostic) {
+        let d: &dyn Diagnostic = &d;
+        self._push(d);
+    }
+
+    fn _push(&mut self, d: &dyn Diagnostic) {
+        for cb in self.callbacks.iter_mut() {
+            match cb(d) {
+                Ok(()) => return,
+                Err(()) => (),
+            }
+        }
+        (self.default_callback)(d)
+    }
+}
diff --git a/crates/ra_hir_expand/src/lib.rs b/crates/ra_hir_expand/src/lib.rs
index 85c2b22ac..dd07a16b4 100644
--- a/crates/ra_hir_expand/src/lib.rs
+++ b/crates/ra_hir_expand/src/lib.rs
@@ -9,6 +9,7 @@ pub mod ast_id_map;
 pub mod either;
 pub mod name;
 pub mod hygiene;
+pub mod diagnostics;
 
 use std::hash::{Hash, Hasher};