Merge #429

429: Reorganize hir public API in terms of code model r=matklad a=matklad

Recently, I've been thinking about introducing "object orient code model" API for rust: a set of APIs with types like `Function`, `Module`, etc, with methods like `get_containing_declaration()`, `get_type()`, etc. 

Here's how a similar API might look like in .Net land:

https://docs.microsoft.com/en-us/dotnet/api/microsoft.codeanalysis.semanticmodel?view=roslyn-dotnet

https://docs.microsoft.com/en-us/dotnet/api/microsoft.codeanalysis.imethodsymbol?view=roslyn-dotnet

The main feature of such API is that it can be powered by different backends. For example, one can imagine a backend based on salsa, and a backend which reads all the data from a specially prepared JSON file. The "OO" bit is interesting mostly in this "can swap implementations via dynamic dispatch" aspect, the actual API could have a more database/ECS flavored feeling.

It's not clear at this moment how exactly should we implement such a dynamically (or if we even need dynamism in the first pace) swapable API in Rust, but I'd love to experiment with this a bit. 

For starters, I propose creating a `code_model_api` which contains various definition types and their public methods (mandatory implemented as one-liners, so that the API has a header-file feel). Specifically, I propose that each type is a wrapper around some integer ID, and that all methods of it accept a `&db` argument.  The actual impl goes elsewhere: into the db queries or, absent a better place, into the `code_model_api_impl`. In the first commit, I've moved the simplest type, `Crate`, over to this pattern.

I *think* that we, at least initially, will be used types from `code_model_api` *inside* `hir` as well, but this is not required: we might pick a different implementation down the line, while preserving the API. 

Long term I'd love to replace the `db: &impl HirDatabase` argument by a `mp: &dyn ModelProvider`, implement `ModelProvider` for `T: HirDatabase`, and move `code_model_api` into the separate crate, which does not depend on `hir`. 

@flodiebold you've recently done some `Def`s work, would do you think of this plan? Could it become a good API in the future, or is it just a useless boilerplate duplicating method signatures between `code_model_api` and `code_model_impl`?


Co-authored-by: Aleksey Kladov <[email protected]>

1 files changed, 17 insertions, 15 deletions

diff --git a/crates/ra_hir/src/impl_block.rs b/crates/ra_hir/src/impl_block.rs
index 01afa84c4..7ce8d17e6 100644
--- a/crates/ra_hir/src/impl_block.rs
+++ b/crates/ra_hir/src/impl_block.rs
@@ -7,12 +7,14 @@ use ra_db::{LocationIntener, Cancelable, SourceRootId};
 
 use crate::{
     DefId, DefLoc, DefKind, SourceItemId, SourceFileItems,
-    Module, Function,
+    Function,
     db::HirDatabase,
     type_ref::TypeRef,
-    module::{ModuleSourceNode, ModuleId},
+    module_tree::ModuleId,
 };
 
+use crate::code_model_api::{Module, ModuleSource};
+
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct ImplBlock {
     module_impl_blocks: Arc<ModuleImplBlocks>,
@@ -64,7 +66,7 @@ impl ImplData {
     ) -> Self {
         let target_trait = node.target_type().map(TypeRef::from_ast);
         let target_type = TypeRef::from_ast_opt(node.target_type());
-        let file_id = module.source().file_id();
+        let module_loc = module.def_id.loc(db);
         let items = if let Some(item_list) = node.item_list() {
             item_list
                 .impl_items()
@@ -75,14 +77,14 @@ impl ImplData {
                         ast::ImplItem::TypeDef(..) => DefKind::Item,
                     };
                     let item_id = file_items.id_of_unchecked(item_node.syntax());
+                    let source_item_id = SourceItemId {
+                        file_id: module_loc.source_item_id.file_id,
+                        item_id: Some(item_id),
+                    };
                     let def_loc = DefLoc {
                         kind,
-                        source_root_id: module.source_root_id,
-                        module_id: module.module_id,
-                        source_item_id: SourceItemId {
-                            file_id,
-                            item_id: Some(item_id),
-                        },
+                        source_item_id,
+                        ..module_loc
                     };
                     let def_id = def_loc.id(db);
                     match item_node {
@@ -148,13 +150,13 @@ impl ModuleImplBlocks {
     }
 
     fn collect(&mut self, db: &impl HirDatabase, module: Module) -> Cancelable<()> {
-        let module_source_node = module.source().resolve(db);
-        let node = match &module_source_node {
-            ModuleSourceNode::SourceFile(node) => node.borrowed().syntax(),
-            ModuleSourceNode::Module(node) => node.borrowed().syntax(),
+        let (file_id, module_source) = module.defenition_source(db)?;
+        let node = match &module_source {
+            ModuleSource::SourceFile(node) => node.borrowed().syntax(),
+            ModuleSource::Module(node) => node.borrowed().syntax(),
         };
 
-        let source_file_items = db.file_items(module.source().file_id());
+        let source_file_items = db.file_items(file_id.into());
 
         for impl_block_ast in node.children().filter_map(ast::ImplBlock::cast) {
             let impl_block = ImplData::from_ast(db, &source_file_items, &module, impl_block_ast);
@@ -174,7 +176,7 @@ pub(crate) fn impls_in_module(
     module_id: ModuleId,
 ) -> Cancelable<Arc<ModuleImplBlocks>> {
     let mut result = ModuleImplBlocks::new();
-    let module = Module::new(db, source_root_id, module_id)?;
+    let module = Module::from_module_id(db, source_root_id, module_id)?;
     result.collect(db, module)?;
     Ok(Arc::new(result))
 }