Merge #551

551: remove Cancelable from Module API, part 2 r=matklad a=matklad



Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>

crates/ra_hir/src/code_model_api.rs

@@ -33,10 +33,10 @@ impl Crate {
     pub fn crate_id(&self) -> CrateId {
         self.crate_id
     }
-    pub fn dependencies(&self, db: &impl HirDatabase) -> Cancelable<Vec<CrateDependency>> {
+    pub fn dependencies(&self, db: &impl HirDatabase) -> Vec<CrateDependency> {
-        Ok(self.dependencies_impl(db))
+        self.dependencies_impl(db)
     }
-    pub fn root_module(&self, db: &impl HirDatabase) -> Cancelable<Option<Module>> {
+    pub fn root_module(&self, db: &impl HirDatabase) -> Option<Module> {
         self.root_module_impl(db)
     }
 }
@@ -78,12 +78,12 @@ pub enum Problem {
 
 impl Module {
     /// Name of this module.
-    pub fn name(&self, db: &impl HirDatabase) -> Cancelable<Option<Name>> {
+    pub fn name(&self, db: &impl HirDatabase) -> Option<Name> {
         self.name_impl(db)
     }
 
     /// Returns a node which defines this module. That is, a file or a `mod foo {}` with items.
-    pub fn definition_source(&self, db: &impl HirDatabase) -> Cancelable<(FileId, ModuleSource)> {
+    pub fn definition_source(&self, db: &impl HirDatabase) -> (FileId, ModuleSource) {
         self.definition_source_impl(db)
     }
 
@@ -92,19 +92,19 @@ impl Module {
     pub fn declaration_source(
         &self,
         db: &impl HirDatabase,
-    ) -> Cancelable<Option<(FileId, TreeArc<ast::Module>)>> {
+    ) -> Option<(FileId, TreeArc<ast::Module>)> {
         self.declaration_source_impl(db)
     }
 
     /// Returns the crate this module is part of.
-    pub fn krate(&self, db: &impl HirDatabase) -> Cancelable<Option<Crate>> {
+    pub fn krate(&self, db: &impl HirDatabase) -> Option<Crate> {
         self.krate_impl(db)
     }
 
     /// Topmost parent of this module. Every module has a `crate_root`, but some
     /// might be missing `krate`. This can happen if a module's file is not included
     /// in the module tree of any target in Cargo.toml.
-    pub fn crate_root(&self, db: &impl HirDatabase) -> Cancelable<Module> {
+    pub fn crate_root(&self, db: &impl HirDatabase) -> Module {
         self.crate_root_impl(db)
     }
 
@@ -114,23 +114,23 @@ impl Module {
     }
 
     /// Iterates over all child modules.
-    pub fn children(&self, db: &impl HirDatabase) -> Cancelable<impl Iterator<Item = Module>> {
+    pub fn children(&self, db: &impl HirDatabase) -> impl Iterator<Item = Module> {
         self.children_impl(db)
     }
 
     /// Finds a parent module.
-    pub fn parent(&self, db: &impl HirDatabase) -> Cancelable<Option<Module>> {
+    pub fn parent(&self, db: &impl HirDatabase) -> Option<Module> {
         self.parent_impl(db)
     }
 
-    pub fn path_to_root(&self, db: &impl HirDatabase) -> Cancelable<Vec<Module>> {
+    pub fn path_to_root(&self, db: &impl HirDatabase) -> Vec<Module> {
         let mut res = vec![self.clone()];
         let mut curr = self.clone();
-        while let Some(next) = curr.parent(db)? {
+        while let Some(next) = curr.parent(db) {
             res.push(next.clone());
             curr = next
         }
-        Ok(res)
+        res
     }
 
     /// Returns a `ModuleScope`: a set of items, visible in this module.

crates/ra_hir/src/code_model_impl/krate.rs

@@ -1,4 +1,4 @@
-use ra_db::{CrateId, Cancelable};
+use ra_db::CrateId;
 
 use crate::{
     HirFileId, Crate, CrateDependency, AsName, DefLoc, DefKind, Module, SourceItemId,
@@ -20,7 +20,7 @@ impl Crate {
             })
             .collect()
     }
-    pub(crate) fn root_module_impl(&self, db: &impl HirDatabase) -> Cancelable<Option<Module>> {
+    pub(crate) fn root_module_impl(&self, db: &impl HirDatabase) -> Option<Module> {
         let crate_graph = db.crate_graph();
         let file_id = crate_graph.crate_root(self.crate_id);
         let source_root_id = db.file_source_root(file_id);
@@ -31,7 +31,7 @@ impl Crate {
             file_id,
             item_id: None,
         };
-        let module_id = ctry!(module_tree.find_module_by_source(source));
+        let module_id = module_tree.find_module_by_source(source)?;
 
         let def_loc = DefLoc {
             kind: DefKind::Module,
@@ -42,6 +42,6 @@ impl Crate {
         let def_id = def_loc.id(db);
 
         let module = Module::new(def_id);
-        Ok(Some(module))
+        Some(module)
     }
 }

crates/ra_hir/src/code_model_impl/module.rs

@@ -30,17 +30,14 @@ impl Module {
         Module::new(def_id)
     }
 
-    pub(crate) fn name_impl(&self, db: &impl HirDatabase) -> Cancelable<Option<Name>> {
+    pub(crate) fn name_impl(&self, db: &impl HirDatabase) -> Option<Name> {
         let loc = self.def_id.loc(db);
         let module_tree = db.module_tree(loc.source_root_id);
-        let link = ctry!(loc.module_id.parent_link(&module_tree));
+        let link = loc.module_id.parent_link(&module_tree)?;
-        Ok(Some(link.name(&module_tree).clone()))
+        Some(link.name(&module_tree).clone())
     }
 
-    pub fn definition_source_impl(
+    pub fn definition_source_impl(&self, db: &impl HirDatabase) -> (FileId, ModuleSource) {
-        &self,
-        db: &impl HirDatabase,
-    ) -> Cancelable<(FileId, ModuleSource)> {
         let loc = self.def_id.loc(db);
         let file_id = loc.source_item_id.file_id.as_original_file();
         let syntax_node = db.file_item(loc.source_item_id);
@@ -50,40 +47,40 @@ impl Module {
             let module = ast::Module::cast(&syntax_node).unwrap();
             ModuleSource::Module(module.to_owned())
         };
-        Ok((file_id, module_source))
+        (file_id, module_source)
     }
 
     pub fn declaration_source_impl(
         &self,
         db: &impl HirDatabase,
-    ) -> Cancelable<Option<(FileId, TreeArc<ast::Module>)>> {
+    ) -> Option<(FileId, TreeArc<ast::Module>)> {
         let loc = self.def_id.loc(db);
         let module_tree = db.module_tree(loc.source_root_id);
-        let link = ctry!(loc.module_id.parent_link(&module_tree));
+        let link = loc.module_id.parent_link(&module_tree)?;
         let file_id = link
             .owner(&module_tree)
             .source(&module_tree)
             .file_id
             .as_original_file();
         let src = link.source(&module_tree, db);
-        Ok(Some((file_id, src)))
+        Some((file_id, src))
     }
 
-    pub(crate) fn krate_impl(&self, db: &impl HirDatabase) -> Cancelable<Option<Crate>> {
+    pub(crate) fn krate_impl(&self, db: &impl HirDatabase) -> Option<Crate> {
-        let root = self.crate_root(db)?;
+        let root = self.crate_root(db);
         let loc = root.def_id.loc(db);
         let file_id = loc.source_item_id.file_id.as_original_file();
 
         let crate_graph = db.crate_graph();
-        let crate_id = ctry!(crate_graph.crate_id_for_crate_root(file_id));
+        let crate_id = crate_graph.crate_id_for_crate_root(file_id)?;
-        Ok(Some(Crate::new(crate_id)))
+        Some(Crate::new(crate_id))
     }
 
-    pub(crate) fn crate_root_impl(&self, db: &impl HirDatabase) -> Cancelable<Module> {
+    pub(crate) fn crate_root_impl(&self, db: &impl HirDatabase) -> Module {
         let loc = self.def_id.loc(db);
         let module_tree = db.module_tree(loc.source_root_id);
         let module_id = loc.module_id.crate_root(&module_tree);
-        Ok(Module::from_module_id(db, loc.source_root_id, module_id))
+        Module::from_module_id(db, loc.source_root_id, module_id)
     }
 
     /// Finds a child module with the specified name.
@@ -95,7 +92,7 @@ impl Module {
     }
 
     /// Iterates over all child modules.
-    pub fn children_impl(&self, db: &impl HirDatabase) -> Cancelable<impl Iterator<Item = Module>> {
+    pub fn children_impl(&self, db: &impl HirDatabase) -> impl Iterator<Item = Module> {
         // FIXME this should be implementable without collecting into a vec, but
         // it's kind of hard since the iterator needs to keep a reference to the
         // module tree.
@@ -106,18 +103,14 @@ impl Module {
             .children(&module_tree)
             .map(|(_, module_id)| Module::from_module_id(db, loc.source_root_id, module_id))
             .collect::<Vec<_>>();
-        Ok(children.into_iter())
+        children.into_iter()
     }
 
-    pub fn parent_impl(&self, db: &impl HirDatabase) -> Cancelable<Option<Module>> {
+    pub fn parent_impl(&self, db: &impl HirDatabase) -> Option<Module> {
         let loc = self.def_id.loc(db);
         let module_tree = db.module_tree(loc.source_root_id);
-        let parent_id = ctry!(loc.module_id.parent(&module_tree));
+        let parent_id = loc.module_id.parent(&module_tree)?;
-        Ok(Some(Module::from_module_id(
+        Some(Module::from_module_id(db, loc.source_root_id, parent_id))
-            db,
-            loc.source_root_id,
-            parent_id,
-        )))
     }
 
     /// Returns a `ModuleScope`: a set of items, visible in this module.
@@ -135,10 +128,10 @@ impl Module {
     ) -> Cancelable<PerNs<DefId>> {
         let mut curr_per_ns = PerNs::types(
             match path.kind {
-                PathKind::Crate => self.crate_root(db)?,
+                PathKind::Crate => self.crate_root(db),
                 PathKind::Self_ | PathKind::Plain => self.clone(),
                 PathKind::Super => {
-                    if let Some(p) = self.parent(db)? {
+                    if let Some(p) = self.parent(db) {
                         p
                     } else {
                         return Ok(PerNs::none());

crates/ra_hir/src/ids.rs

@@ -217,7 +217,7 @@ impl DefId {
 
     /// Returns the containing crate.
     pub fn krate(&self, db: &impl HirDatabase) -> Cancelable<Option<Crate>> {
-        Ok(self.module(db)?.krate(db)?)
+        Ok(self.module(db)?.krate(db))
     }
 
     /// Returns the containing impl block, if this is an impl item.

crates/ra_hir/src/impl_block.rs

@@ -167,7 +167,7 @@ impl ModuleImplBlocks {
     }
 
     fn collect(&mut self, db: &impl HirDatabase, module: Module) -> Cancelable<()> {
-        let (file_id, module_source) = module.definition_source(db)?;
+        let (file_id, module_source) = module.definition_source(db);
         let node = match &module_source {
             ModuleSource::SourceFile(node) => node.syntax(),
             ModuleSource::Module(node) => node

crates/ra_hir/src/nameres.rs

@@ -353,8 +353,8 @@ where
             if let Some(crate_id) = crate_graph.crate_id_for_crate_root(file_id.as_original_file())
             {
                 let krate = Crate::new(crate_id);
-                for dep in krate.dependencies(self.db)? {
+                for dep in krate.dependencies(self.db) {
-                    if let Some(module) = dep.krate.root_module(self.db)? {
+                    if let Some(module) = dep.krate.root_module(self.db) {
                         let def_id = module.def_id;
                         self.add_module_item(
                             &mut module_items,

crates/ra_hir/src/ty/method_resolution.rs

@@ -75,7 +75,7 @@ impl CrateImplBlocks {
             }
         }
 
-        for child in module.children(db)? {
+        for child in module.children(db) {
             self.collect_recursive(db, child)?;
         }
 
@@ -93,7 +93,7 @@ impl CrateImplBlocks {
             source_root_id,
             impls: FxHashMap::default(),
         };
-        if let Some(module) = krate.root_module(db)? {
+        if let Some(module) = krate.root_module(db) {
             crate_impl_blocks.collect_recursive(db, module)?;
         }
         Ok(Arc::new(crate_impl_blocks))

crates/ra_ide_api/src/completion/complete_scope.rs

@@ -20,7 +20,7 @@ pub(super) fn complete_scope(acc: &mut Completions, ctx: &CompletionContext) ->
     }
 
     let module_scope = module.scope(ctx.db)?;
-    let (file_id, _) = module.definition_source(ctx.db)?;
+    let (file_id, _) = module.definition_source(ctx.db);
     module_scope
         .entries()
         .filter(|(_name, res)| {

crates/ra_ide_api/src/imp.rs

@@ -99,16 +99,16 @@ impl db::RootDatabase {
 
 impl db::RootDatabase {
     /// Returns `Vec` for the same reason as `parent_module`
-    pub(crate) fn crate_for(&self, file_id: FileId) -> Cancelable<Vec<CrateId>> {
+    pub(crate) fn crate_for(&self, file_id: FileId) -> Vec<CrateId> {
         let module = match source_binder::module_from_file_id(self, file_id) {
             Some(it) => it,
-            None => return Ok(Vec::new()),
+            None => return Vec::new(),
         };
-        let krate = match module.krate(self)? {
+        let krate = match module.krate(self) {
             Some(it) => it,
-            None => return Ok(Vec::new()),
+            None => return Vec::new(),
         };
-        Ok(vec![krate.crate_id()])
+        vec![krate.crate_id()]
     }
     pub(crate) fn find_all_refs(
         &self,

crates/ra_ide_api/src/lib.rs

@@ -419,7 +419,7 @@ impl Analysis {
 
     /// Returns crates this file belongs too.
     pub fn crate_for(&self, file_id: FileId) -> Cancelable<Vec<CrateId>> {
-        self.with_db(|db| db.crate_for(file_id))?
+        self.with_db(|db| db.crate_for(file_id))
     }
 
     /// Returns the root file of the given crate.

crates/ra_ide_api/src/navigation_target.rs

@@ -73,9 +73,9 @@ impl NavigationTarget {
         db: &RootDatabase,
         module: hir::Module,
     ) -> Cancelable<NavigationTarget> {
-        let (file_id, source) = module.definition_source(db)?;
+        let (file_id, source) = module.definition_source(db);
         let name = module
-            .name(db)?
+            .name(db)
             .map(|it| it.to_string().into())
             .unwrap_or_default();
         let res = match source {
@@ -94,10 +94,10 @@ impl NavigationTarget {
         module: hir::Module,
     ) -> Cancelable<NavigationTarget> {
         let name = module
-            .name(db)?
+            .name(db)
             .map(|it| it.to_string().into())
             .unwrap_or_default();
-        if let Some((file_id, source)) = module.declaration_source(db)? {
+        if let Some((file_id, source)) = module.declaration_source(db) {
             return Ok(NavigationTarget::from_syntax(
                 file_id,
                 name,

crates/ra_ide_api/src/runnables.rs

@@ -80,11 +80,9 @@ fn runnable_mod(db: &RootDatabase, file_id: FileId, module: &ast::Module) -> Opt
     // FIXME: thread cancellation instead of `.ok`ing
     let path = module
         .path_to_root(db)
-        .ok()?
         .into_iter()
         .rev()
-        .filter_map(|it| it.name(db).ok())
+        .filter_map(|it| it.name(db))
-        .filter_map(|it| it)
         .join("::");
     Some(Runnable {
         range,