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]>

5 files changed, 26 insertions, 24 deletions

diff --git a/crates/ra_analysis/src/completion/complete_scope.rs b/crates/ra_analysis/src/completion/complete_scope.rs
index 21d77aa97..ee9052d3d 100644
--- a/crates/ra_analysis/src/completion/complete_scope.rs
+++ b/crates/ra_analysis/src/completion/complete_scope.rs
@@ -20,14 +20,17 @@ pub(super) fn complete_scope(acc: &mut Completions, ctx: &CompletionContext) ->
     }
 
     let module_scope = module.scope(ctx.db)?;
+    let (file_id, _) = module.defenition_source(ctx.db)?;
     module_scope
         .entries()
         .filter(|(_name, res)| {
             // Don't expose this item
+            // FIXME: this penetrates through all kinds of abstractions,
+            // we need to figura out the way to do it less ugly.
             match res.import {
                 None => true,
                 Some(import) => {
-                    let range = import.range(ctx.db, module.file_id());
+                    let range = import.range(ctx.db, file_id);
                     !range.is_subrange(&ctx.leaf.range())
                 }
             }
diff --git a/crates/ra_analysis/src/goto_defenition.rs b/crates/ra_analysis/src/goto_defenition.rs
index 68b6ac3ba..aa0616e3b 100644
--- a/crates/ra_analysis/src/goto_defenition.rs
+++ b/crates/ra_analysis/src/goto_defenition.rs
@@ -60,8 +60,8 @@ fn name_defenition(
             if let Some(child_module) =
                 hir::source_binder::module_from_declaration(db, file_id, module)?
             {
-                let file_id = child_module.file_id();
-                let name = match child_module.name() {
+                let (file_id, _) = child_module.defenition_source(db)?;
+                let name = match child_module.name(db)? {
                     Some(name) => name.to_string().into(),
                     None => "".into(),
                 };
diff --git a/crates/ra_analysis/src/imp.rs b/crates/ra_analysis/src/imp.rs
index 5988fb779..07a966290 100644
--- a/crates/ra_analysis/src/imp.rs
+++ b/crates/ra_analysis/src/imp.rs
@@ -105,39 +105,35 @@ impl db::RootDatabase {
         &self,
         position: FilePosition,
     ) -> Cancelable<Vec<NavigationTarget>> {
-        let descr = match source_binder::module_from_position(self, position)? {
+        let module = match source_binder::module_from_position(self, position)? {
             None => return Ok(Vec::new()),
             Some(it) => it,
         };
-        let (file_id, decl) = match descr.parent_link_source(self) {
+        let (file_id, ast_module) = match module.declaration_source(self)? {
             None => return Ok(Vec::new()),
             Some(it) => it,
         };
-        let decl = decl.borrowed();
-        let decl_name = decl.name().unwrap();
+        let ast_module = ast_module.borrowed();
+        let name = ast_module.name().unwrap();
         Ok(vec![NavigationTarget {
             file_id,
-            name: decl_name.text(),
-            range: decl_name.syntax().range(),
+            name: name.text(),
+            range: name.syntax().range(),
             kind: MODULE,
             ptr: None,
         }])
     }
     /// Returns `Vec` for the same reason as `parent_module`
     pub(crate) fn crate_for(&self, file_id: FileId) -> Cancelable<Vec<CrateId>> {
-        let descr = match source_binder::module_from_file_id(self, file_id)? {
+        let module = match source_binder::module_from_file_id(self, file_id)? {
+            Some(it) => it,
             None => return Ok(Vec::new()),
+        };
+        let krate = match module.krate(self)? {
             Some(it) => it,
+            None => return Ok(Vec::new()),
         };
-        let root = descr.crate_root();
-        let file_id = root.file_id();
-
-        let crate_graph = self.crate_graph();
-        let crate_id = crate_graph.crate_id_for_crate_root(file_id);
-        Ok(crate_id.into_iter().collect())
-    }
-    pub(crate) fn crate_root(&self, crate_id: CrateId) -> FileId {
-        self.crate_graph().crate_root(crate_id)
+        Ok(vec![krate.crate_id()])
     }
     pub(crate) fn find_all_refs(
         &self,
@@ -209,7 +205,7 @@ impl db::RootDatabase {
             })
             .collect::<Vec<_>>();
         if let Some(m) = source_binder::module_from_file_id(self, file_id)? {
-            for (name_node, problem) in m.problems(self) {
+            for (name_node, problem) in m.problems(self)? {
                 let source_root = self.file_source_root(file_id);
                 let diag = match problem {
                     Problem::UnresolvedModule { candidate } => {
diff --git a/crates/ra_analysis/src/lib.rs b/crates/ra_analysis/src/lib.rs
index 390c31c3f..77f77e9a8 100644
--- a/crates/ra_analysis/src/lib.rs
+++ b/crates/ra_analysis/src/lib.rs
@@ -397,7 +397,7 @@ impl Analysis {
     }
     /// Returns the root file of the given crate.
     pub fn crate_root(&self, crate_id: CrateId) -> Cancelable<FileId> {
-        Ok(self.db.crate_root(crate_id))
+        Ok(self.db.crate_graph().crate_root(crate_id))
     }
     /// Returns the set of possible targets to run for the current file.
     pub fn runnables(&self, file_id: FileId) -> Cancelable<Vec<Runnable>> {
diff --git a/crates/ra_analysis/src/runnables.rs b/crates/ra_analysis/src/runnables.rs
index 474267605..216209098 100644
--- a/crates/ra_analysis/src/runnables.rs
+++ b/crates/ra_analysis/src/runnables.rs
@@ -72,12 +72,15 @@ fn runnable_mod(db: &RootDatabase, file_id: FileId, module: ast::Module) -> Opti
     let range = module.syntax().range();
     let module =
         hir::source_binder::module_from_child_node(db, file_id, module.syntax()).ok()??;
+
+    // FIXME: thread cancellation instead of `.ok`ing
     let path = module
-        .path_to_root()
+        .path_to_root(db)
+        .ok()?
         .into_iter()
         .rev()
-        .into_iter()
-        .filter_map(|it| it.name().map(Clone::clone))
+        .filter_map(|it| it.name(db).ok())
+        .filter_map(|it| it)
         .join("::");
     Some(Runnable {
         range,