Merge #2474

2474: Move `ModuleSource`, create `ModuleOrigin` r=matklad a=ice1000

As title. This comes right after #2473 

Co-authored-by: ice1000 <[email protected]>

10 files changed, 149 insertions, 136 deletions

diff --git a/crates/ra_hir/src/code_model.rs b/crates/ra_hir/src/code_model.rs
index 5877afefa..7706399ae 100644
--- a/crates/ra_hir/src/code_model.rs
+++ b/crates/ra_hir/src/code_model.rs
@@ -11,6 +11,7 @@ use hir_def::{
     builtin_type::BuiltinType,
     docs::Documentation,
     expr::{BindingAnnotation, Pat, PatId},
+    nameres::ModuleSource,
     per_ns::PerNs,
     resolver::HasResolver,
     type_ref::{Mutability, TypeRef},
@@ -21,11 +22,11 @@ use hir_def::{
 use hir_expand::{
     diagnostics::DiagnosticSink,
     name::{self, AsName},
-    AstId, MacroDefId,
+    MacroDefId,
 };
 use hir_ty::expr::ExprValidator;
-use ra_db::{CrateId, Edition, FileId, FilePosition};
-use ra_syntax::{ast, AstNode, SyntaxNode};
+use ra_db::{CrateId, Edition};
+use ra_syntax::ast;
 
 use crate::{
     db::{DefDatabase, HirDatabase},
@@ -79,64 +80,6 @@ impl Crate {
     }
 }
 
-pub enum ModuleSource {
-    SourceFile(ast::SourceFile),
-    Module(ast::Module),
-}
-
-impl ModuleSource {
-    pub fn new(
-        db: &impl DefDatabase,
-        file_id: Option<FileId>,
-        decl_id: Option<AstId<ast::Module>>,
-    ) -> ModuleSource {
-        match (file_id, decl_id) {
-            (Some(file_id), _) => {
-                let source_file = db.parse(file_id).tree();
-                ModuleSource::SourceFile(source_file)
-            }
-            (None, Some(item_id)) => {
-                let module = item_id.to_node(db);
-                assert!(module.item_list().is_some(), "expected inline module");
-                ModuleSource::Module(module)
-            }
-            (None, None) => panic!(),
-        }
-    }
-
-    // FIXME: this methods do not belong here
-    pub fn from_position(db: &impl DefDatabase, position: FilePosition) -> ModuleSource {
-        let parse = db.parse(position.file_id);
-        match &ra_syntax::algo::find_node_at_offset::<ast::Module>(
-            parse.tree().syntax(),
-            position.offset,
-        ) {
-            Some(m) if !m.has_semi() => ModuleSource::Module(m.clone()),
-            _ => {
-                let source_file = parse.tree();
-                ModuleSource::SourceFile(source_file)
-            }
-        }
-    }
-
-    pub fn from_child_node(db: &impl DefDatabase, child: InFile<&SyntaxNode>) -> ModuleSource {
-        if let Some(m) =
-            child.value.ancestors().filter_map(ast::Module::cast).find(|it| !it.has_semi())
-        {
-            ModuleSource::Module(m)
-        } else {
-            let file_id = child.file_id.original_file(db);
-            let source_file = db.parse(file_id).tree();
-            ModuleSource::SourceFile(source_file)
-        }
-    }
-
-    pub fn from_file_id(db: &impl DefDatabase, file_id: FileId) -> ModuleSource {
-        let source_file = db.parse(file_id).tree();
-        ModuleSource::SourceFile(source_file)
-    }
-}
-
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 pub struct Module {
     pub(crate) id: ModuleId,
diff --git a/crates/ra_hir/src/code_model/src.rs b/crates/ra_hir/src/code_model/src.rs
index 36cfbc8f1..d9bccd902 100644
--- a/crates/ra_hir/src/code_model/src.rs
+++ b/crates/ra_hir/src/code_model/src.rs
@@ -2,6 +2,7 @@
 
 use either::Either;
 use hir_def::{
+    nameres::ModuleSource,
     src::{HasChildSource, HasSource as _},
     AstItemDef, Lookup, VariantId,
 };
@@ -9,7 +10,7 @@ use ra_syntax::ast;
 
 use crate::{
     db::DefDatabase, Const, Enum, EnumVariant, FieldSource, Function, ImplBlock, Import, MacroDef,
-    Module, ModuleSource, Static, Struct, StructField, Trait, TypeAlias, Union,
+    Module, Static, Struct, StructField, Trait, TypeAlias, Union,
 };
 
 pub use hir_expand::InFile;
@@ -25,11 +26,7 @@ impl Module {
     /// Returns a node which defines this module. That is, a file or a `mod foo {}` with items.
     pub fn definition_source(self, db: &impl DefDatabase) -> InFile<ModuleSource> {
         let def_map = db.crate_def_map(self.id.krate);
-        let src = def_map[self.id.local_id].definition_source(db);
-        src.map(|it| match it {
-            Either::Left(it) => ModuleSource::SourceFile(it),
-            Either::Right(it) => ModuleSource::Module(it),
-        })
+        def_map[self.id.local_id].definition_source(db)
     }
 
     /// Returns a node which declares this module, either a `mod foo;` or a `mod foo {}`.
diff --git a/crates/ra_hir/src/from_source.rs b/crates/ra_hir/src/from_source.rs
index 82bf641dc..18d87f6d7 100644
--- a/crates/ra_hir/src/from_source.rs
+++ b/crates/ra_hir/src/from_source.rs
@@ -1,6 +1,6 @@
 //! FIXME: write short doc here
 
-use hir_def::{AstItemDef, LocationCtx, ModuleId};
+use hir_def::{nameres::ModuleSource, AstItemDef, LocationCtx, ModuleId};
 use hir_expand::{name::AsName, AstId, MacroDefId, MacroDefKind};
 use ra_syntax::{
     ast::{self, AstNode, NameOwner},
@@ -10,8 +10,8 @@ use ra_syntax::{
 use crate::{
     db::{AstDatabase, DefDatabase, HirDatabase},
     AssocItem, Const, DefWithBody, Enum, EnumVariant, FieldSource, Function, HasSource, ImplBlock,
-    InFile, Local, MacroDef, Module, ModuleDef, ModuleSource, Static, Struct, StructField, Trait,
-    TypeAlias, Union, VariantDef,
+    InFile, Local, MacroDef, Module, ModuleDef, Static, Struct, StructField, Trait, TypeAlias,
+    Union, VariantDef,
 };
 
 pub trait FromSource: Sized {
@@ -235,11 +235,10 @@ impl Module {
                 let src_parent = InFile { file_id: src.file_id, value: parent_declaration };
                 Module::from_declaration(db, src_parent)
             }
-            _ => {
-                let src_parent = InFile {
-                    file_id: src.file_id,
-                    value: ModuleSource::new(db, Some(src.file_id.original_file(db)), None),
-                };
+            None => {
+                let source_file = db.parse(src.file_id.original_file(db)).tree();
+                let src_parent =
+                    InFile { file_id: src.file_id, value: ModuleSource::SourceFile(source_file) };
                 Module::from_definition(db, src_parent)
             }
         }?;
diff --git a/crates/ra_hir/src/lib.rs b/crates/ra_hir/src/lib.rs
index 853760cb1..f12e4ca3f 100644
--- a/crates/ra_hir/src/lib.rs
+++ b/crates/ra_hir/src/lib.rs
@@ -43,8 +43,8 @@ pub use crate::{
     code_model::{
         src::HasSource, Adt, AssocItem, AttrDef, Const, Container, Crate, CrateDependency,
         DefWithBody, Docs, Enum, EnumVariant, FieldSource, Function, GenericDef, GenericParam,
-        HasAttrs, ImplBlock, Import, Local, MacroDef, Module, ModuleDef, ModuleSource, ScopeDef,
-        Static, Struct, StructField, Trait, Type, TypeAlias, Union, VariantDef,
+        HasAttrs, ImplBlock, Import, Local, MacroDef, Module, ModuleDef, ScopeDef, Static, Struct,
+        StructField, Trait, Type, TypeAlias, Union, VariantDef,
     },
     from_source::FromSource,
     source_binder::{PathResolution, ScopeEntryWithSyntax, SourceAnalyzer},
@@ -59,6 +59,7 @@ pub use hir_def::{
     body::scope::ExprScopes,
     builtin_type::BuiltinType,
     docs::Documentation,
+    nameres::ModuleSource,
     path::{Path, PathKind},
     type_ref::Mutability,
 };
diff --git a/crates/ra_hir/src/source_binder.rs b/crates/ra_hir/src/source_binder.rs
index 28d41b647..db0451059 100644
--- a/crates/ra_hir/src/source_binder.rs
+++ b/crates/ra_hir/src/source_binder.rs
@@ -14,6 +14,7 @@ use hir_def::{
         BodySourceMap,
     },
     expr::{ExprId, PatId},
+    nameres::ModuleSource,
     path::known,
     resolver::{self, resolver_for_scope, HasResolver, Resolver, TypeNs, ValueNs},
     AssocItemId, DefWithBodyId,
@@ -46,7 +47,7 @@ fn try_get_resolver_for_node(db: &impl HirDatabase, node: InFile<&SyntaxNode>) -
                 Some(crate::Module::from_declaration(db, src)?.id.resolver(db))
             },
              ast::SourceFile(it) => {
-                let src = node.with_value(crate::ModuleSource::SourceFile(it));
+                let src = node.with_value(ModuleSource::SourceFile(it));
                 Some(crate::Module::from_definition(db, src)?.id.resolver(db))
             },
             ast::StructDef(it) => {
diff --git a/crates/ra_hir_def/src/nameres.rs b/crates/ra_hir_def/src/nameres.rs
index faf3566f4..bd237a7b3 100644
--- a/crates/ra_hir_def/src/nameres.rs
+++ b/crates/ra_hir_def/src/nameres.rs
@@ -57,15 +57,17 @@ mod tests;
 
 use std::sync::Arc;
 
-use either::Either;
 use hir_expand::{
     ast_id_map::FileAstId, diagnostics::DiagnosticSink, name::Name, InFile, MacroDefId,
 };
 use once_cell::sync::Lazy;
 use ra_arena::Arena;
-use ra_db::{CrateId, Edition, FileId};
+use ra_db::{CrateId, Edition, FileId, FilePosition};
 use ra_prof::profile;
-use ra_syntax::ast;
+use ra_syntax::{
+    ast::{self, AstNode},
+    SyntaxNode,
+};
 use rustc_hash::FxHashMap;
 
 use crate::{
@@ -100,19 +102,76 @@ impl std::ops::Index<LocalModuleId> for CrateDefMap {
     }
 }
 
+#[derive(Debug, PartialEq, Eq, Clone, Copy, Hash)]
+pub enum ModuleOrigin {
+    CrateRoot {
+        definition: FileId,
+    },
+    /// Note that non-inline modules, by definition, live inside non-macro file.
+    File {
+        declaration: AstId<ast::Module>,
+        definition: FileId,
+    },
+    Inline {
+        definition: AstId<ast::Module>,
+    },
+}
+
+impl Default for ModuleOrigin {
+    fn default() -> Self {
+        ModuleOrigin::CrateRoot { definition: FileId(0) }
+    }
+}
+
+impl ModuleOrigin {
+    pub(crate) fn not_sure_file(file: Option<FileId>, declaration: AstId<ast::Module>) -> Self {
+        match file {
+            None => ModuleOrigin::Inline { definition: declaration },
+            Some(definition) => ModuleOrigin::File { declaration, definition },
+        }
+    }
+
+    fn declaration(&self) -> Option<AstId<ast::Module>> {
+        match self {
+            ModuleOrigin::File { declaration: module, .. }
+            | ModuleOrigin::Inline { definition: module, .. } => Some(*module),
+            ModuleOrigin::CrateRoot { .. } => None,
+        }
+    }
+
+    pub fn file_id(&self) -> Option<FileId> {
+        match self {
+            ModuleOrigin::File { definition, .. } | ModuleOrigin::CrateRoot { definition } => {
+                Some(*definition)
+            }
+            _ => None,
+        }
+    }
+
+    /// Returns a node which defines this module.
+    /// That is, a file or a `mod foo {}` with items.
+    fn definition_source(&self, db: &impl DefDatabase) -> InFile<ModuleSource> {
+        match self {
+            ModuleOrigin::File { definition, .. } | ModuleOrigin::CrateRoot { definition } => {
+                let file_id = *definition;
+                let sf = db.parse(file_id).tree();
+                return InFile::new(file_id.into(), ModuleSource::SourceFile(sf));
+            }
+            ModuleOrigin::Inline { definition } => {
+                InFile::new(definition.file_id, ModuleSource::Module(definition.to_node(db)))
+            }
+        }
+    }
+}
+
 #[derive(Default, Debug, PartialEq, Eq)]
 pub struct ModuleData {
     pub parent: Option<LocalModuleId>,
     pub children: FxHashMap<Name, LocalModuleId>,
     pub scope: ModuleScope,
 
-    //  FIXME: these can't be both null, we need a three-state enum here.
-    /// None for root
-    pub declaration: Option<AstId<ast::Module>>,
-    /// None for inline modules.
-    ///
-    /// Note that non-inline modules, by definition, live inside non-macro file.
-    pub definition: Option<FileId>,
+    /// Where does this module come from?
+    pub origin: ModuleOrigin,
 
     pub impls: Vec<ImplId>,
 }
@@ -262,7 +321,7 @@ impl CrateDefMap {
     pub fn modules_for_file(&self, file_id: FileId) -> impl Iterator<Item = LocalModuleId> + '_ {
         self.modules
             .iter()
-            .filter(move |(_id, data)| data.definition == Some(file_id))
+            .filter(move |(_id, data)| data.origin.file_id() == Some(file_id))
             .map(|(id, _data)| id)
     }
 
@@ -281,27 +340,54 @@ impl CrateDefMap {
 
 impl ModuleData {
     /// Returns a node which defines this module. That is, a file or a `mod foo {}` with items.
-    pub fn definition_source(
-        &self,
-        db: &impl DefDatabase,
-    ) -> InFile<Either<ast::SourceFile, ast::Module>> {
-        if let Some(file_id) = self.definition {
-            let sf = db.parse(file_id).tree();
-            return InFile::new(file_id.into(), Either::Left(sf));
-        }
-        let decl = self.declaration.unwrap();
-        InFile::new(decl.file_id, Either::Right(decl.to_node(db)))
+    pub fn definition_source(&self, db: &impl DefDatabase) -> InFile<ModuleSource> {
+        self.origin.definition_source(db)
     }
 
     /// Returns a node which declares this module, either a `mod foo;` or a `mod foo {}`.
-    /// `None` for the crate root.
+    /// `None` for the crate root or block.
     pub fn declaration_source(&self, db: &impl DefDatabase) -> Option<InFile<ast::Module>> {
-        let decl = self.declaration?;
+        let decl = self.origin.declaration()?;
         let value = decl.to_node(db);
         Some(InFile { file_id: decl.file_id, value })
     }
 }
 
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub enum ModuleSource {
+    SourceFile(ast::SourceFile),
+    Module(ast::Module),
+}
+
+impl ModuleSource {
+    // FIXME: this methods do not belong here
+    pub fn from_position(db: &impl DefDatabase, position: FilePosition) -> ModuleSource {
+        let parse = db.parse(position.file_id);
+        match &ra_syntax::algo::find_node_at_offset::<ast::Module>(
+            parse.tree().syntax(),
+            position.offset,
+        ) {
+            Some(m) if !m.has_semi() => ModuleSource::Module(m.clone()),
+            _ => {
+                let source_file = parse.tree();
+                ModuleSource::SourceFile(source_file)
+            }
+        }
+    }
+
+    pub fn from_child_node(db: &impl DefDatabase, child: InFile<&SyntaxNode>) -> ModuleSource {
+        if let Some(m) =
+            child.value.ancestors().filter_map(ast::Module::cast).find(|it| !it.has_semi())
+        {
+            ModuleSource::Module(m)
+        } else {
+            let file_id = child.file_id.original_file(db);
+            let source_file = db.parse(file_id).tree();
+            ModuleSource::SourceFile(source_file)
+        }
+    }
+}
+
 mod diagnostics {
     use hir_expand::diagnostics::DiagnosticSink;
     use ra_db::RelativePathBuf;
diff --git a/crates/ra_hir_def/src/nameres/collector.rs b/crates/ra_hir_def/src/nameres/collector.rs
index d4bfcae1d..9d948d4f4 100644
--- a/crates/ra_hir_def/src/nameres/collector.rs
+++ b/crates/ra_hir_def/src/nameres/collector.rs
@@ -19,7 +19,7 @@ use crate::{
     db::DefDatabase,
     nameres::{
         diagnostics::DefDiagnostic, mod_resolution::ModDir, path_resolution::ReachedFixedPoint,
-        raw, BuiltinShadowMode, CrateDefMap, ModuleData, Resolution, ResolveMode,
+        raw, BuiltinShadowMode, CrateDefMap, ModuleData, ModuleOrigin, Resolution, ResolveMode,
     },
     path::{Path, PathKind},
     per_ns::PerNs,
@@ -131,7 +131,7 @@ where
         let file_id = crate_graph.crate_root(self.def_map.krate);
         let raw_items = self.db.raw_items(file_id.into());
         let module_id = self.def_map.root;
-        self.def_map.modules[module_id].definition = Some(file_id);
+        self.def_map.modules[module_id].origin = ModuleOrigin::CrateRoot { definition: file_id };
         ModCollector {
             def_collector: &mut *self,
             module_id,
@@ -669,8 +669,7 @@ where
         let modules = &mut self.def_collector.def_map.modules;
         let res = modules.alloc(ModuleData::default());
         modules[res].parent = Some(self.module_id);
-        modules[res].declaration = Some(declaration);
-        modules[res].definition = definition;
+        modules[res].origin = ModuleOrigin::not_sure_file(definition, declaration);
         modules[res].scope.legacy_macros = modules[self.module_id].scope.legacy_macros.clone();
         modules[self.module_id].children.insert(name.clone(), res);
         let resolution = Resolution {
diff --git a/crates/ra_hir_ty/src/test_db.rs b/crates/ra_hir_ty/src/test_db.rs
index 1dc9793f9..476f6df52 100644
--- a/crates/ra_hir_ty/src/test_db.rs
+++ b/crates/ra_hir_ty/src/test_db.rs
@@ -74,7 +74,7 @@ impl TestDB {
         for &krate in self.relevant_crates(file_id).iter() {
             let crate_def_map = self.crate_def_map(krate);
             for (local_id, data) in crate_def_map.modules.iter() {
-                if data.definition == Some(file_id) {
+                if data.origin.file_id() == Some(file_id) {
                     return ModuleId { krate, local_id };
                 }
             }
diff --git a/crates/ra_ide/src/display/navigation_target.rs b/crates/ra_ide/src/display/navigation_target.rs
index f920d3db6..add11fbc3 100644
--- a/crates/ra_ide/src/display/navigation_target.rs
+++ b/crates/ra_ide/src/display/navigation_target.rs
@@ -231,34 +231,20 @@ impl ToNav for hir::Module {
     fn to_nav(&self, db: &RootDatabase) -> NavigationTarget {
         let src = self.definition_source(db);
         let name = self.name(db).map(|it| it.to_string().into()).unwrap_or_default();
-        match &src.value {
-            ModuleSource::SourceFile(node) => {
-                let frange = original_range(db, src.with_value(node.syntax()));
-
-                NavigationTarget::from_syntax(
-                    frange.file_id,
-                    name,
-                    None,
-                    frange.range,
-                    node.syntax().kind(),
-                    None,
-                    None,
-                )
-            }
-            ModuleSource::Module(node) => {
-                let frange = original_range(db, src.with_value(node.syntax()));
-
-                NavigationTarget::from_syntax(
-                    frange.file_id,
-                    name,
-                    None,
-                    frange.range,
-                    node.syntax().kind(),
-                    node.doc_comment_text(),
-                    node.short_label(),
-                )
-            }
-        }
+        let syntax = match &src.value {
+            ModuleSource::SourceFile(node) => node.syntax(),
+            ModuleSource::Module(node) => node.syntax(),
+        };
+        let frange = original_range(db, src.with_value(syntax));
+        NavigationTarget::from_syntax(
+            frange.file_id,
+            name,
+            None,
+            frange.range,
+            syntax.kind(),
+            None,
+            None,
+        )
     }
 }
 
diff --git a/crates/ra_ide/src/parent_module.rs b/crates/ra_ide/src/parent_module.rs
index 616d69fce..aef3fa3df 100644
--- a/crates/ra_ide/src/parent_module.rs
+++ b/crates/ra_ide/src/parent_module.rs
@@ -1,6 +1,6 @@
 //! FIXME: write short doc here
 
-use ra_db::{CrateId, FileId, FilePosition};
+use ra_db::{CrateId, FileId, FilePosition, SourceDatabase};
 
 use crate::{db::RootDatabase, NavigationTarget};
 
@@ -21,7 +21,8 @@ pub(crate) fn parent_module(db: &RootDatabase, position: FilePosition) -> Vec<Na
 
 /// Returns `Vec` for the same reason as `parent_module`
 pub(crate) fn crate_for(db: &RootDatabase, file_id: FileId) -> Vec<CrateId> {
-    let src = hir::ModuleSource::from_file_id(db, file_id);
+    let source_file = db.parse(file_id).tree();
+    let src = hir::ModuleSource::SourceFile(source_file);
     let module =
         match hir::Module::from_definition(db, hir::InFile { file_id: file_id.into(), value: src })
         {