From 7a2e449424227c7f6cebe0581513287023297999 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 28 Jan 2020 00:09:56 +0200 Subject: Prefer imports starting with std --- crates/ra_hir_def/src/find_path.rs | 37 +++++++++++++++++++++++++++++++------ 1 file changed, 31 insertions(+), 6 deletions(-) (limited to 'crates/ra_hir_def/src/find_path.rs') diff --git a/crates/ra_hir_def/src/find_path.rs b/crates/ra_hir_def/src/find_path.rs index 8cc2fb160..d9e67607f 100644 --- a/crates/ra_hir_def/src/find_path.rs +++ b/crates/ra_hir_def/src/find_path.rs @@ -7,7 +7,7 @@ use crate::{ visibility::Visibility, CrateId, ModuleDefId, ModuleId, }; -use hir_expand::name::Name; +use hir_expand::name::{known, Name}; const MAX_PATH_LEN: usize = 15; @@ -102,7 +102,7 @@ fn find_path_inner( let mut best_path = None; let mut best_path_len = max_len; for (module_id, name) in importable_locations { - let mut path = match find_path_inner( + let mut new_path = match find_path_inner( db, ItemInNs::Types(ModuleDefId::ModuleId(module_id)), from, @@ -111,15 +111,40 @@ fn find_path_inner( None => continue, Some(path) => path, }; - path.segments.push(name); - if path_len(&path) < best_path_len { - best_path_len = path_len(&path); - best_path = Some(path); + new_path.segments.push(name); + + if prefer_new_path(best_path_len, best_path.as_ref(), &new_path) { + best_path_len = path_len(&new_path); + best_path = Some(new_path); } } best_path } +fn prefer_new_path(old_path_len: usize, old_path: Option<&ModPath>, new_path: &ModPath) -> bool { + match (old_path.and_then(|mod_path| mod_path.segments.first()), new_path.segments.first()) { + (Some(old_path_start), Some(new_path_start)) + if old_path_start == &known::std && use_std_instead(new_path_start) => + { + false + } + (Some(old_path_start), Some(new_path_start)) + if new_path_start == &known::std && use_std_instead(old_path_start) => + { + true + } + (None, Some(_)) => true, + (Some(_), None) => false, + _ => path_len(new_path) < old_path_len, + } +} + +// When std library is present, paths starting with `std::` +// should be preferred over paths starting with `core::` and `alloc::` +fn use_std_instead(name: &Name) -> bool { + name == &known::core || name == &known::alloc +} + fn path_len(path: &ModPath) -> usize { path.segments.len() + match path.kind { -- cgit v1.2.3 From 762d2935818bbd2639a19b1994b2ddfeac5529b2 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 28 Jan 2020 17:19:41 +0200 Subject: Apply the proposed refactoring --- crates/ra_hir_def/src/find_path.rs | 81 ++++++++++++++++++++------------------ 1 file changed, 43 insertions(+), 38 deletions(-) (limited to 'crates/ra_hir_def/src/find_path.rs') diff --git a/crates/ra_hir_def/src/find_path.rs b/crates/ra_hir_def/src/find_path.rs index d9e67607f..ebfd6e211 100644 --- a/crates/ra_hir_def/src/find_path.rs +++ b/crates/ra_hir_def/src/find_path.rs @@ -11,6 +11,34 @@ use hir_expand::name::{known, Name}; const MAX_PATH_LEN: usize = 15; +impl ModPath { + fn starts_with_std(&self) -> bool { + self.segments.first().filter(|&first_segment| first_segment == &known::std).is_some() + } + + // When std library is present, paths starting with `std::` + // should be preferred over paths starting with `core::` and `alloc::` + fn should_start_with_std(&self) -> bool { + self.segments + .first() + .filter(|&first_segment| { + first_segment == &known::alloc || first_segment == &known::core + }) + .is_some() + } + + fn len(&self) -> usize { + self.segments.len() + + match self.kind { + PathKind::Plain => 0, + PathKind::Super(i) => i as usize, + PathKind::Crate => 1, + PathKind::Abs => 0, + PathKind::DollarCrate(_) => 1, + } + } +} + // FIXME: handle local items /// Find a path that can be used to refer to a certain item. This can depend on @@ -102,7 +130,7 @@ fn find_path_inner( let mut best_path = None; let mut best_path_len = max_len; for (module_id, name) in importable_locations { - let mut new_path = match find_path_inner( + let mut path = match find_path_inner( db, ItemInNs::Types(ModuleDefId::ModuleId(module_id)), from, @@ -111,51 +139,28 @@ fn find_path_inner( None => continue, Some(path) => path, }; - new_path.segments.push(name); + path.segments.push(name); - if prefer_new_path(best_path_len, best_path.as_ref(), &new_path) { - best_path_len = path_len(&new_path); - best_path = Some(new_path); - } + let new_path = + if let Some(best_path) = best_path { select_best_path(best_path, path) } else { path }; + best_path_len = new_path.len(); + best_path = Some(new_path); } best_path } -fn prefer_new_path(old_path_len: usize, old_path: Option<&ModPath>, new_path: &ModPath) -> bool { - match (old_path.and_then(|mod_path| mod_path.segments.first()), new_path.segments.first()) { - (Some(old_path_start), Some(new_path_start)) - if old_path_start == &known::std && use_std_instead(new_path_start) => - { - false - } - (Some(old_path_start), Some(new_path_start)) - if new_path_start == &known::std && use_std_instead(old_path_start) => - { - true - } - (None, Some(_)) => true, - (Some(_), None) => false, - _ => path_len(new_path) < old_path_len, +fn select_best_path(old_path: ModPath, new_path: ModPath) -> ModPath { + if old_path.starts_with_std() && new_path.should_start_with_std() { + old_path + } else if new_path.starts_with_std() && old_path.should_start_with_std() { + new_path + } else if new_path.len() < old_path.len() { + new_path + } else { + old_path } } -// When std library is present, paths starting with `std::` -// should be preferred over paths starting with `core::` and `alloc::` -fn use_std_instead(name: &Name) -> bool { - name == &known::core || name == &known::alloc -} - -fn path_len(path: &ModPath) -> usize { - path.segments.len() - + match path.kind { - PathKind::Plain => 0, - PathKind::Super(i) => i as usize, - PathKind::Crate => 1, - PathKind::Abs => 0, - PathKind::DollarCrate(_) => 1, - } -} - fn find_importable_locations( db: &impl DefDatabase, item: ItemInNs, -- cgit v1.2.3 From 713870ee0c51a11dc0d2a5b8eafbc2624d42c359 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 28 Jan 2020 18:03:24 +0200 Subject: Add the tests --- crates/ra_hir_def/src/find_path.rs | 41 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) (limited to 'crates/ra_hir_def/src/find_path.rs') diff --git a/crates/ra_hir_def/src/find_path.rs b/crates/ra_hir_def/src/find_path.rs index ebfd6e211..43b9b124a 100644 --- a/crates/ra_hir_def/src/find_path.rs +++ b/crates/ra_hir_def/src/find_path.rs @@ -8,6 +8,7 @@ use crate::{ CrateId, ModuleDefId, ModuleId, }; use hir_expand::name::{known, Name}; +use test_utils::tested_by; const MAX_PATH_LEN: usize = 15; @@ -151,8 +152,10 @@ fn find_path_inner( fn select_best_path(old_path: ModPath, new_path: ModPath) -> ModPath { if old_path.starts_with_std() && new_path.should_start_with_std() { + tested_by!(prefer_std_paths); old_path } else if new_path.starts_with_std() && old_path.should_start_with_std() { + tested_by!(prefer_std_paths); new_path } else if new_path.len() < old_path.len() { new_path @@ -231,6 +234,7 @@ mod tests { use hir_expand::hygiene::Hygiene; use ra_db::fixture::WithFixture; use ra_syntax::ast::AstNode; + use test_utils::covers; /// `code` needs to contain a cursor marker; checks that `find_path` for the /// item the `path` refers to returns that same path when called from the @@ -482,4 +486,41 @@ mod tests { "#; check_found_path(code, "crate::foo::S"); } + + #[test] + fn prefer_std_paths_over_alloc() { + covers!(prefer_std_paths); + let code = r#" + //- /main.rs crate:main deps:alloc,std + <|> + + //- /std.rs crate:std deps:alloc + pub mod sync { + pub use alloc::sync::Arc; + } + + //- /zzz.rs crate:alloc + pub mod sync { + pub struct Arc; + } + "#; + check_found_path(code, "std::sync::Arc"); + } + + #[test] + fn prefer_shorter_paths_if_not_alloc() { + let code = r#" + //- /main.rs crate:main deps:megaalloc,std + <|> + + //- /std.rs crate:std deps:megaalloc + pub mod sync { + pub use megaalloc::sync::Arc; + } + + //- /zzz.rs crate:megaalloc + pub struct Arc; + "#; + check_found_path(code, "megaalloc::Arc"); + } } -- cgit v1.2.3