From 4496e2a06a91e5825f355ce730af802643e8018a Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Fri, 10 Jan 2020 18:40:45 +0100 Subject: Apply review suggestions --- crates/ra_hir_def/src/db.rs | 7 ------- crates/ra_hir_def/src/find_path.rs | 17 ++++++++--------- crates/ra_hir_def/src/item_scope.rs | 5 ++--- crates/ra_hir_def/src/path.rs | 14 ++++++-------- crates/ra_hir_def/src/resolver.rs | 14 +++++++------- 5 files changed, 23 insertions(+), 34 deletions(-) (limited to 'crates/ra_hir_def/src') 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; - - #[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 { 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 { 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 `::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 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 { 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 { - 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 { - let (def_map, local_id) = self.module()?; + pub fn module(&self) -> Option { + let (def_map, local_id) = self.module_scope()?; Some(ModuleId { krate: def_map.krate, local_id }) } pub fn krate(&self) -> Option { - self.module().map(|t| t.0.krate) + self.module_scope().map(|t| t.0.krate) } pub fn where_predicates_in_scope<'a>( -- cgit v1.2.3