8 files changed, 156 insertions, 135 deletions

diff --git a/crates/ra_hir/src/code_model_api.rs b/crates/ra_hir/src/code_model_api.rs
index a37d960a1..03b1acef3 100644
--- a/crates/ra_hir/src/code_model_api.rs
+++ b/crates/ra_hir/src/code_model_api.rs
@@ -1,8 +1,7 @@
 use std::sync::Arc;
 
-use relative_path::RelativePathBuf;
 use ra_db::{CrateId, SourceRootId, Edition};
-use ra_syntax::{ast::self, TreeArc, SyntaxNode};
+use ra_syntax::{ast::self, TreeArc};
 
 use crate::{
     Name, ScopesWithSourceMap, Ty, HirFileId,
@@ -96,11 +95,6 @@ pub enum ModuleSource {
     Module(TreeArc<ast::Module>),
 }
 
-#[derive(Clone, Debug, Hash, PartialEq, Eq)]
-pub enum Problem {
-    UnresolvedModule { candidate: RelativePathBuf },
-}
-
 impl Module {
     /// Name of this module.
     pub fn name(&self, db: &impl HirDatabase) -> Option<Name> {
@@ -172,8 +166,8 @@ impl Module {
         db.crate_def_map(self.krate)[self.module_id].scope.clone()
     }
 
-    pub fn problems(&self, db: &impl HirDatabase) -> Vec<(TreeArc<SyntaxNode>, Problem)> {
-        self.problems_impl(db)
+    pub fn diagnostics(&self, db: &impl HirDatabase, sink: &mut Diagnostics) {
+        db.crate_def_map(self.krate).add_diagnostics(db, self.module_id, sink);
     }
 
     pub fn resolver(&self, db: &impl HirDatabase) -> Resolver {
@@ -521,10 +515,8 @@ impl Function {
         r
     }
 
-    pub fn diagnostics(&self, db: &impl HirDatabase) -> Diagnostics {
-        let mut res = Diagnostics::default();
-        self.infer(db).add_diagnostics(db, *self, &mut res);
-        res
+    pub fn diagnostics(&self, db: &impl HirDatabase, sink: &mut Diagnostics) {
+        self.infer(db).add_diagnostics(db, *self, sink);
     }
 }
 
diff --git a/crates/ra_hir/src/code_model_impl/module.rs b/crates/ra_hir/src/code_model_impl/module.rs
index 52a33e981..14237060c 100644
--- a/crates/ra_hir/src/code_model_impl/module.rs
+++ b/crates/ra_hir/src/code_model_impl/module.rs
@@ -1,8 +1,8 @@
 use ra_db::FileId;
-use ra_syntax::{ast, SyntaxNode, TreeArc, AstNode};
+use ra_syntax::{ast, TreeArc, AstNode};
 
 use crate::{
-    Module, ModuleSource, Problem, Name,
+    Module, ModuleSource, Name,
     nameres::{CrateModuleId, ImportId},
     HirDatabase, DefDatabase,
     HirFileId, SourceItemId,
@@ -108,19 +108,4 @@ impl Module {
         let parent_id = def_map[self.module_id].parent?;
         Some(self.with_module_id(parent_id))
     }
-
-    pub(crate) fn problems_impl(
-        &self,
-        db: &impl HirDatabase,
-    ) -> Vec<(TreeArc<SyntaxNode>, Problem)> {
-        let def_map = db.crate_def_map(self.krate);
-        let (my_file_id, _) = self.definition_source(db);
-        // FIXME: not entirely corret filterint by module
-        def_map
-            .problems()
-            .iter()
-            .filter(|(source_item_id, _problem)| my_file_id == source_item_id.file_id)
-            .map(|(source_item_id, problem)| (db.file_item(*source_item_id), problem.clone()))
-            .collect()
-    }
 }
diff --git a/crates/ra_hir/src/diagnostics.rs b/crates/ra_hir/src/diagnostics.rs
index 46a3fdd47..e8bb1ed92 100644
--- a/crates/ra_hir/src/diagnostics.rs
+++ b/crates/ra_hir/src/diagnostics.rs
@@ -3,7 +3,20 @@ use std::{fmt, any::Any};
 use ra_syntax::{SyntaxNodePtr, AstPtr, ast};
 
 use crate::HirFileId;
+use relative_path::RelativePathBuf;
 
+/// Diagnostic defines hir API for errors and warnings.
+///
+/// It is used as a `dyn` object, which you can downcast to a concrete
+/// diagnostic. Diagnostics are structured, meaning that they include rich
+/// information which can be used by IDE to create fixes. Diagnostics 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
+/// subsytem (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 file(&self) -> HirFileId;
     fn syntax_node(&self) -> SyntaxNodePtr;
@@ -34,14 +47,8 @@ impl Diagnostics {
 
 #[derive(Debug)]
 pub struct NoSuchField {
-    pub(crate) file: HirFileId,
-    pub(crate) field: AstPtr<ast::NamedField>,
-}
-
-impl NoSuchField {
-    pub fn field(&self) -> AstPtr<ast::NamedField> {
-        self.field
-    }
+    pub file: HirFileId,
+    pub field: AstPtr<ast::NamedField>,
 }
 
 impl Diagnostic for NoSuchField {
@@ -58,3 +65,25 @@ impl Diagnostic for NoSuchField {
         self
     }
 }
+
+#[derive(Debug)]
+pub struct UnresolvedModule {
+    pub file: HirFileId,
+    pub decl: AstPtr<ast::Module>,
+    pub candidate: RelativePathBuf,
+}
+
+impl Diagnostic for UnresolvedModule {
+    fn file(&self) -> HirFileId {
+        self.file
+    }
+    fn syntax_node(&self) -> SyntaxNodePtr {
+        self.decl.into()
+    }
+    fn message(&self) -> String {
+        "unresolved module".to_string()
+    }
+    fn as_any(&self) -> &(Any + Send + 'static) {
+        self
+    }
+}
diff --git a/crates/ra_hir/src/lib.rs b/crates/ra_hir/src/lib.rs
index 390aef0a9..ce54d7608 100644
--- a/crates/ra_hir/src/lib.rs
+++ b/crates/ra_hir/src/lib.rs
@@ -64,7 +64,7 @@ pub use self::{
 
 pub use self::code_model_api::{
     Crate, CrateDependency,
-    Module, ModuleDef, ModuleSource, Problem,
+    Module, ModuleDef, ModuleSource,
     Struct, Enum, EnumVariant,
     Function, FnSignature,
     StructField, FieldSource,
diff --git a/crates/ra_hir/src/nameres.rs b/crates/ra_hir/src/nameres.rs
index d361cf9e6..416f114b4 100644
--- a/crates/ra_hir/src/nameres.rs
+++ b/crates/ra_hir/src/nameres.rs
@@ -56,14 +56,17 @@ mod tests;
 use std::sync::Arc;
 
 use rustc_hash::FxHashMap;
+use relative_path::RelativePathBuf;
 use ra_arena::{Arena, RawId, impl_arena_id};
 use ra_db::{FileId, Edition};
+use ra_syntax::{AstNode, AstPtr, ast};
 use test_utils::tested_by;
 
 use crate::{
-    ModuleDef, Name, Crate, Module, Problem,
+    ModuleDef, Name, Crate, Module,
     DefDatabase, Path, PathKind, HirFileId,
     ids::{SourceItemId, SourceFileItemId, MacroCallId},
+    diagnostics::{Diagnostics, UnresolvedModule},
 };
 
 pub(crate) use self::raw::{RawItems, ImportId, ImportSourceMap};
@@ -85,7 +88,7 @@ pub struct CrateDefMap {
     macros: Arena<CrateMacroId, mbe::MacroRules>,
     public_macros: FxHashMap<Name, CrateMacroId>,
     macro_resolutions: FxHashMap<MacroCallId, (Crate, CrateMacroId)>,
-    problems: CrateDefMapProblems,
+    diagnostics: Vec<DefDiagnostic>,
 }
 
 impl std::ops::Index<CrateModuleId> for CrateDefMap {
@@ -125,21 +128,6 @@ pub(crate) struct ModuleData {
     pub(crate) definition: Option<FileId>,
 }
 
-#[derive(Default, Debug, PartialEq, Eq)]
-pub(crate) struct CrateDefMapProblems {
-    problems: Vec<(SourceItemId, Problem)>,
-}
-
-impl CrateDefMapProblems {
-    fn add(&mut self, source_item_id: SourceItemId, problem: Problem) {
-        self.problems.push((source_item_id, problem))
-    }
-
-    pub(crate) fn iter<'a>(&'a self) -> impl Iterator<Item = (&'a SourceItemId, &'a Problem)> + 'a {
-        self.problems.iter().map(|(s, p)| (s, p))
-    }
-}
-
 #[derive(Debug, Default, PartialEq, Eq, Clone)]
 pub struct ModuleScope {
     items: FxHashMap<Name, Resolution>,
@@ -212,7 +200,7 @@ impl CrateDefMap {
                 macros: Arena::default(),
                 public_macros: FxHashMap::default(),
                 macro_resolutions: FxHashMap::default(),
-                problems: CrateDefMapProblems::default(),
+                diagnostics: Vec::new(),
             }
         };
         let def_map = collector::collect_defs(db, def_map);
@@ -224,10 +212,6 @@ impl CrateDefMap {
         self.root
     }
 
-    pub(crate) fn problems(&self) -> &CrateDefMapProblems {
-        &self.problems
-    }
-
     pub(crate) fn mk_module(&self, module_id: CrateModuleId) -> Module {
         Module { krate: self.krate, module_id }
     }
@@ -240,6 +224,15 @@ impl CrateDefMap {
         &self.extern_prelude
     }
 
+    pub(crate) fn add_diagnostics(
+        &self,
+        db: &impl DefDatabase,
+        module: CrateModuleId,
+        sink: &mut Diagnostics,
+    ) {
+        self.diagnostics.iter().for_each(|it| it.add_to(db, module, sink))
+    }
+
     pub(crate) fn resolve_macro(
         &self,
         macro_call_id: MacroCallId,
@@ -452,3 +445,31 @@ impl CrateDefMap {
         }
     }
 }
+
+#[derive(Debug, PartialEq, Eq)]
+enum DefDiagnostic {
+    UnresolvedModule {
+        module: CrateModuleId,
+        declaration: SourceItemId,
+        candidate: RelativePathBuf,
+    },
+}
+
+impl DefDiagnostic {
+    fn add_to(&self, db: &impl DefDatabase, target_module: CrateModuleId, sink: &mut Diagnostics) {
+        match self {
+            DefDiagnostic::UnresolvedModule { module, declaration, candidate } => {
+                if *module != target_module {
+                    return;
+                }
+                let syntax = db.file_item(*declaration);
+                let decl = ast::Module::cast(&syntax).unwrap();
+                sink.push(UnresolvedModule {
+                    file: declaration.file_id,
+                    decl: AstPtr::new(&decl),
+                    candidate: candidate.clone(),
+                })
+            }
+        }
+    }
+}
diff --git a/crates/ra_hir/src/nameres/collector.rs b/crates/ra_hir/src/nameres/collector.rs
index c5b73cfbe..bc6bc5e7f 100644
--- a/crates/ra_hir/src/nameres/collector.rs
+++ b/crates/ra_hir/src/nameres/collector.rs
@@ -6,9 +6,9 @@ use ra_db::FileId;
 
 use crate::{
     Function, Module, Struct, Enum, Const, Static, Trait, TypeAlias,
-    DefDatabase, HirFileId, Name, Path, Problem, Crate,
+    DefDatabase, HirFileId, Name, Path, Crate,
     KnownName,
-    nameres::{Resolution, PerNs, ModuleDef, ReachedFixedPoint, ResolveMode, raw},
+    nameres::{Resolution, PerNs, ModuleDef, ReachedFixedPoint, ResolveMode, raw, DefDiagnostic},
     ids::{AstItemDef, LocationCtx, MacroCallLoc, SourceItemId, MacroCallId},
 };
 
@@ -405,25 +405,27 @@ where
             raw::ModuleData::Declaration { name, source_item_id } => {
                 let source_item_id = source_item_id.with_file_id(self.file_id);
                 let is_root = self.def_collector.def_map.modules[self.module_id].parent.is_none();
-                let (file_ids, problem) =
-                    resolve_submodule(self.def_collector.db, self.file_id, name, is_root);
-
-                if let Some(problem) = problem {
-                    self.def_collector.def_map.problems.add(source_item_id, problem)
-                }
-
-                if let Some(&file_id) = file_ids.first() {
-                    let module_id =
-                        self.push_child_module(name.clone(), source_item_id, Some(file_id));
-                    let raw_items = self.def_collector.db.raw_items(file_id);
-                    ModCollector {
-                        def_collector: &mut *self.def_collector,
-                        module_id,
-                        file_id: file_id.into(),
-                        raw_items: &raw_items,
+                match resolve_submodule(self.def_collector.db, self.file_id, name, is_root) {
+                    Ok(file_id) => {
+                        let module_id =
+                            self.push_child_module(name.clone(), source_item_id, Some(file_id));
+                        let raw_items = self.def_collector.db.raw_items(file_id);
+                        ModCollector {
+                            def_collector: &mut *self.def_collector,
+                            module_id,
+                            file_id: file_id.into(),
+                            raw_items: &raw_items,
+                        }
+                        .collect(raw_items.items())
                     }
-                    .collect(raw_items.items())
-                }
+                    Err(candidate) => self.def_collector.def_map.diagnostics.push(
+                        DefDiagnostic::UnresolvedModule {
+                            module: self.module_id,
+                            declaration: source_item_id,
+                            candidate,
+                        },
+                    ),
+                };
             }
         }
     }
@@ -524,7 +526,7 @@ fn resolve_submodule(
     file_id: HirFileId,
     name: &Name,
     is_root: bool,
-) -> (Vec<FileId>, Option<Problem>) {
+) -> Result<FileId, RelativePathBuf> {
     // FIXME: handle submodules of inline modules properly
     let file_id = file_id.original_file(db);
     let source_root_id = db.file_source_root(file_id);
@@ -545,17 +547,10 @@ fn resolve_submodule(
         candidates.push(file_dir_mod.clone());
     };
     let sr = db.source_root(source_root_id);
-    let points_to = candidates
-        .into_iter()
-        .filter_map(|path| sr.files.get(&path))
-        .map(|&it| it)
-        .collect::<Vec<_>>();
-    let problem = if points_to.is_empty() {
-        Some(Problem::UnresolvedModule {
-            candidate: if is_dir_owner { file_mod } else { file_dir_mod },
-        })
-    } else {
-        None
-    };
-    (points_to, problem)
+    let mut points_to = candidates.into_iter().filter_map(|path| sr.files.get(&path)).map(|&it| it);
+    // FIXME: handle ambiguity
+    match points_to.next() {
+        Some(file_id) => Ok(file_id),
+        None => Err(if is_dir_owner { file_mod } else { file_dir_mod }),
+    }
 }
diff --git a/crates/ra_hir/src/ty/infer.rs b/crates/ra_hir/src/ty/infer.rs
index 02708ba0f..6dc3edc7a 100644
--- a/crates/ra_hir/src/ty/infer.rs
+++ b/crates/ra_hir/src/ty/infer.rs
@@ -120,9 +120,9 @@ impl InferenceResult {
         &self,
         db: &impl HirDatabase,
         owner: Function,
-        diagnostics: &mut Diagnostics,
+        sink: &mut Diagnostics,
     ) {
-        self.diagnostics.iter().for_each(|it| it.add_to(db, owner, diagnostics))
+        self.diagnostics.iter().for_each(|it| it.add_to(db, owner, sink))
     }
 }
 
@@ -1277,12 +1277,17 @@ mod diagnostics {
     }
 
     impl InferenceDiagnostic {
-        pub(super) fn add_to(&self, db: &impl HirDatabase, owner: Function, acc: &mut Diagnostics) {
+        pub(super) fn add_to(
+            &self,
+            db: &impl HirDatabase,
+            owner: Function,
+            sink: &mut Diagnostics,
+        ) {
             match self {
                 InferenceDiagnostic::NoSuchField { expr, field } => {
                     let (file, _) = owner.source(db);
                     let field = owner.body_source_map(db).field_syntax(*expr, *field);
-                    acc.push(NoSuchField { file, field })
+                    sink.push(NoSuchField { file, field })
                 }
             }
         }
diff --git a/crates/ra_ide_api/src/diagnostics.rs b/crates/ra_ide_api/src/diagnostics.rs
index 943fd2f53..254342c0a 100644
--- a/crates/ra_ide_api/src/diagnostics.rs
+++ b/crates/ra_ide_api/src/diagnostics.rs
@@ -1,5 +1,5 @@
 use itertools::Itertools;
-use hir::{Problem, source_binder};
+use hir::{source_binder, diagnostics::Diagnostic as _};
 use ra_db::SourceDatabase;
 use ra_syntax::{
     Location, SourceFile, SyntaxKind, TextRange, SyntaxNode,
@@ -28,7 +28,7 @@ pub(crate) fn diagnostics(db: &RootDatabase, file_id: FileId) -> Vec<Diagnostic>
     }
 
     if let Some(m) = source_binder::module_from_file_id(db, file_id) {
-        check_module(&mut res, db, file_id, m);
+        check_module(&mut res, db, m);
     };
     res
 }
@@ -128,46 +128,40 @@ fn check_struct_shorthand_initialization(
     Some(())
 }
 
-fn check_module(
-    acc: &mut Vec<Diagnostic>,
-    db: &RootDatabase,
-    file_id: FileId,
-    module: hir::Module,
-) {
+fn check_module(acc: &mut Vec<Diagnostic>, db: &RootDatabase, module: hir::Module) {
+    let mut diagnostics = hir::diagnostics::Diagnostics::default();
+    module.diagnostics(db, &mut diagnostics);
     for decl in module.declarations(db) {
         match decl {
-            hir::ModuleDef::Function(f) => check_function(acc, db, f),
+            hir::ModuleDef::Function(f) => f.diagnostics(db, &mut diagnostics),
             _ => (),
         }
     }
 
-    let source_root = db.file_source_root(file_id);
-    for (name_node, problem) in module.problems(db) {
-        let diag = match problem {
-            Problem::UnresolvedModule { candidate } => {
-                let create_file =
-                    FileSystemEdit::CreateFile { source_root, path: candidate.clone() };
-                let fix = SourceChange::file_system_edit("create module", create_file);
-                Diagnostic {
-                    range: name_node.range(),
-                    message: "unresolved module".to_string(),
-                    severity: Severity::Error,
-                    fix: Some(fix),
-                }
-            }
-        };
-        acc.push(diag)
-    }
-}
-
-fn check_function(acc: &mut Vec<Diagnostic>, db: &RootDatabase, function: hir::Function) {
-    for d in function.diagnostics(db).iter() {
-        acc.push(Diagnostic {
-            message: d.message(),
-            range: d.syntax_node().range(),
-            severity: Severity::Error,
-            fix: None,
-        })
+    for d in diagnostics.iter() {
+        if let Some(d) = d.downcast_ref::<hir::diagnostics::UnresolvedModule>() {
+            let source_root = db.file_source_root(d.file().original_file(db));
+            let create_file = FileSystemEdit::CreateFile { source_root, path: d.candidate.clone() };
+            let fix = SourceChange {
+                label: "create module".to_string(),
+                source_file_edits: Vec::new(),
+                file_system_edits: vec![create_file],
+                cursor_position: None,
+            };
+            acc.push(Diagnostic {
+                range: d.syntax_node().range(),
+                message: d.message(),
+                severity: Severity::Error,
+                fix: Some(fix),
+            })
+        } else {
+            acc.push(Diagnostic {
+                message: d.message(),
+                range: d.syntax_node().range(),
+                severity: Severity::Error,
+                fix: None,
+            })
+        }
     }
 }