Apply review suggestions

5 files changed, 23 insertions, 34 deletions

diff --git a/crates/ra_hir_def/src/db.rs b/crates/ra_hir_def/src/db.rs
index 0c00627b5..da273eb11 100644
--- a/crates/ra_hir_def/src/db.rs
+++ b/crates/ra_hir_def/src/db.rs
@@ -107,13 +107,6 @@ pub trait DefDatabase: InternDatabase + AstDatabase {
     // Remove this query completely, in favor of `Attrs::docs` method
     #[salsa::invoke(Documentation::documentation_query)]
     fn documentation(&self, def: AttrDefId) -> Option<Documentation>;
-
-    #[salsa::invoke(crate::find_path::importable_locations_in_crate_query)]
-    fn importable_locations_in_crate(
-        &self,
-        item: crate::item_scope::ItemInNs,
-        krate: CrateId,
-    ) -> Arc<[(ModuleId, hir_expand::name::Name, crate::visibility::Visibility)]>;
 }
 
 fn crate_def_map(db: &impl DefDatabase, krate: CrateId) -> Arc<CrateDefMap> {
diff --git a/crates/ra_hir_def/src/find_path.rs b/crates/ra_hir_def/src/find_path.rs
index 69cdfc943..f7dc8acb7 100644
--- a/crates/ra_hir_def/src/find_path.rs
+++ b/crates/ra_hir_def/src/find_path.rs
@@ -34,7 +34,7 @@ fn find_path_inner(
     // - if the item is already in scope, return the name under which it is
     let def_map = db.crate_def_map(from.krate);
     let from_scope: &crate::item_scope::ItemScope = &def_map.modules[from.local_id].scope;
-    if let Some((name, _)) = from_scope.reverse_get(item) {
+    if let Some((name, _)) = from_scope.name_of(item) {
         return Some(ModPath::from_simple_segments(PathKind::Plain, vec![name.clone()]));
     }
 
@@ -77,7 +77,7 @@ fn find_path_inner(
         let prelude_def_map = db.crate_def_map(prelude_module.krate);
         let prelude_scope: &crate::item_scope::ItemScope =
             &prelude_def_map.modules[prelude_module.local_id].scope;
-        if let Some((name, vis)) = prelude_scope.reverse_get(item) {
+        if let Some((name, vis)) = prelude_scope.name_of(item) {
             if vis.is_visible_from(db, from) {
                 return Some(ModPath::from_simple_segments(PathKind::Plain, vec![name.clone()]));
             }
@@ -146,7 +146,7 @@ fn find_importable_locations(
         .chain(crate_graph.dependencies(from.krate).map(|dep| dep.crate_id))
     {
         result.extend(
-            db.importable_locations_in_crate(item, krate)
+            importable_locations_in_crate(db, item, krate)
                 .iter()
                 .filter(|(_, _, vis)| vis.is_visible_from(db, from))
                 .map(|(m, n, _)| (*m, n.clone())),
@@ -160,17 +160,16 @@ fn find_importable_locations(
 /// non-private `use`s.
 ///
 /// Note that the crate doesn't need to be the one in which the item is defined;
-/// it might be re-exported in other crates. We cache this as a query since we
-/// need to walk the whole def map for it.
-pub(crate) fn importable_locations_in_crate_query(
+/// it might be re-exported in other crates.
+fn importable_locations_in_crate(
     db: &impl DefDatabase,
     item: ItemInNs,
     krate: CrateId,
-) -> std::sync::Arc<[(ModuleId, Name, Visibility)]> {
+) -> Vec<(ModuleId, Name, Visibility)> {
     let def_map = db.crate_def_map(krate);
     let mut result = Vec::new();
     for (local_id, data) in def_map.modules.iter() {
-        if let Some((name, vis)) = data.scope.reverse_get(item) {
+        if let Some((name, vis)) = data.scope.name_of(item) {
             let is_private = if let Visibility::Module(private_to) = vis {
                 private_to.local_id == local_id
             } else {
@@ -192,7 +191,7 @@ pub(crate) fn importable_locations_in_crate_query(
             result.push((ModuleId { krate, local_id }, name.clone(), vis));
         }
     }
-    result.into()
+    result
 }
 
 #[cfg(test)]
diff --git a/crates/ra_hir_def/src/item_scope.rs b/crates/ra_hir_def/src/item_scope.rs
index 87c50b34f..d74a1cef2 100644
--- a/crates/ra_hir_def/src/item_scope.rs
+++ b/crates/ra_hir_def/src/item_scope.rs
@@ -104,7 +104,7 @@ impl ItemScope {
         }
     }
 
-    pub(crate) fn reverse_get(&self, item: ItemInNs) -> Option<(&Name, Visibility)> {
+    pub(crate) fn name_of(&self, item: ItemInNs) -> Option<(&Name, Visibility)> {
         for (name, per_ns) in &self.visible {
             if let Some(vis) = item.match_with(*per_ns) {
                 return Some((name, vis));
@@ -207,8 +207,7 @@ impl ItemInNs {
 
     pub fn as_module_def_id(self) -> Option<ModuleDefId> {
         match self {
-            ItemInNs::Types(t) => Some(t),
-            ItemInNs::Values(v) => Some(v),
+            ItemInNs::Types(id) | ItemInNs::Values(id) => Some(id),
             ItemInNs::Macros(_) => None,
         }
     }
diff --git a/crates/ra_hir_def/src/path.rs b/crates/ra_hir_def/src/path.rs
index 7dd1939b9..9f93a5424 100644
--- a/crates/ra_hir_def/src/path.rs
+++ b/crates/ra_hir_def/src/path.rs
@@ -1,7 +1,11 @@
 //! A desugared representation of paths like `crate::foo` or `<Type as Trait>::bar`.
 mod lower;
 
-use std::{fmt::Display, iter, sync::Arc};
+use std::{
+    fmt::{self, Display},
+    iter,
+    sync::Arc,
+};
 
 use hir_expand::{
     hygiene::Hygiene,
@@ -78,12 +82,6 @@ impl ModPath {
         }
         self.segments.first()
     }
-
-    pub fn to_ast(&self) -> ast::Path {
-        use ast::AstNode;
-        let parse = ast::SourceFile::parse(&self.to_string());
-        parse.tree().syntax().descendants().find_map(ast::Path::cast).unwrap()
-    }
 }
 
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
@@ -255,7 +253,7 @@ impl From<Name> for ModPath {
 }
 
 impl Display for ModPath {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         let mut first_segment = true;
         let mut add_segment = |s| {
             if !first_segment {
diff --git a/crates/ra_hir_def/src/resolver.rs b/crates/ra_hir_def/src/resolver.rs
index 40d0cb588..f7bac5801 100644
--- a/crates/ra_hir_def/src/resolver.rs
+++ b/crates/ra_hir_def/src/resolver.rs
@@ -128,7 +128,7 @@ impl Resolver {
         path: &ModPath,
         shadow: BuiltinShadowMode,
     ) -> PerNs {
-        let (item_map, module) = match self.module() {
+        let (item_map, module) = match self.module_scope() {
             Some(it) => it,
             None => return PerNs::none(),
         };
@@ -239,7 +239,7 @@ impl Resolver {
     ) -> Option<Visibility> {
         match visibility {
             RawVisibility::Module(_) => {
-                let (item_map, module) = match self.module() {
+                let (item_map, module) = match self.module_scope() {
                     Some(it) => it,
                     None => return None,
                 };
@@ -379,7 +379,7 @@ impl Resolver {
         db: &impl DefDatabase,
         path: &ModPath,
     ) -> Option<MacroDefId> {
-        let (item_map, module) = self.module()?;
+        let (item_map, module) = self.module_scope()?;
         item_map.resolve_path(db, module, &path, BuiltinShadowMode::Other).0.take_macros()
     }
 
@@ -403,7 +403,7 @@ impl Resolver {
         traits
     }
 
-    fn module(&self) -> Option<(&CrateDefMap, LocalModuleId)> {
+    fn module_scope(&self) -> Option<(&CrateDefMap, LocalModuleId)> {
         self.scopes.iter().rev().find_map(|scope| match scope {
             Scope::ModuleScope(m) => Some((&*m.crate_def_map, m.module_id)),
 
@@ -411,13 +411,13 @@ impl Resolver {
         })
     }
 
-    pub fn module_id(&self) -> Option<ModuleId> {
-        let (def_map, local_id) = self.module()?;
+    pub fn module(&self) -> Option<ModuleId> {
+        let (def_map, local_id) = self.module_scope()?;
         Some(ModuleId { krate: def_map.krate, local_id })
     }
 
     pub fn krate(&self) -> Option<CrateId> {
-        self.module().map(|t| t.0.krate)
+        self.module_scope().map(|t| t.0.krate)
     }
 
     pub fn where_predicates_in_scope<'a>(