Unnamed repository; edit this file 'description' to name the repository.
Prefer stable paths over unstable ones in import path calculation
| -rw-r--r-- | crates/hir-def/src/find_path.rs | 103 | ||||
| -rw-r--r-- | crates/hir-def/src/import_map.rs | 58 |
2 files changed, 128 insertions, 33 deletions
diff --git a/crates/hir-def/src/find_path.rs b/crates/hir-def/src/find_path.rs index b60e790910..b9c5ff7279 100644 --- a/crates/hir-def/src/find_path.rs +++ b/crates/hir-def/src/find_path.rs @@ -37,6 +37,20 @@ pub fn find_path_prefixed( find_path_inner(db, item, from, Some(prefix_kind), prefer_no_std) } +#[derive(Copy, Clone, Debug)] +enum Stability { + Unstable, + Stable, +} +use Stability::*; + +fn zip_stability(a: Stability, b: Stability) -> Stability { + match (a, b) { + (Stable, Stable) => Stable, + _ => Unstable, + } +} + const MAX_PATH_LEN: usize = 15; #[derive(Copy, Clone, Debug, PartialEq, Eq)] @@ -95,7 +109,8 @@ fn find_path_inner( MAX_PATH_LEN, prefixed, prefer_no_std || db.crate_supports_no_std(crate_root.krate), - ); + ) + .map(|(item, _)| item); } // - if the item is already in scope, return the name under which it is @@ -143,6 +158,7 @@ fn find_path_inner( prefer_no_std || db.crate_supports_no_std(crate_root.krate), scope_name, ) + .map(|(item, _)| item) } fn find_path_for_module( @@ -155,7 +171,7 @@ fn find_path_for_module( max_len: usize, prefixed: Option<PrefixKind>, prefer_no_std: bool, -) -> Option<ModPath> { +) -> Option<(ModPath, Stability)> { if max_len == 0 { return None; } @@ -165,19 +181,19 @@ fn find_path_for_module( let scope_name = find_in_scope(db, def_map, from, ItemInNs::Types(module_id.into())); if prefixed.is_none() { if let Some(scope_name) = scope_name { - return Some(ModPath::from_segments(PathKind::Plain, Some(scope_name))); + return Some((ModPath::from_segments(PathKind::Plain, Some(scope_name)), Stable)); } } // - if the item is the crate root, return `crate` if module_id == crate_root { - return Some(ModPath::from_segments(PathKind::Crate, None)); + return Some((ModPath::from_segments(PathKind::Crate, None), Stable)); } // - if relative paths are fine, check if we are searching for a parent if prefixed.filter(PrefixKind::is_absolute).is_none() { if let modpath @ Some(_) = find_self_super(def_map, module_id, from) { - return modpath; + return modpath.zip(Some(Stable)); } } @@ -201,14 +217,14 @@ fn find_path_for_module( } else { PathKind::Plain }; - return Some(ModPath::from_segments(kind, Some(name))); + return Some((ModPath::from_segments(kind, Some(name)), Stable)); } } if let value @ Some(_) = find_in_prelude(db, &root_def_map, &def_map, ItemInNs::Types(module_id.into()), from) { - return value; + return value.zip(Some(Stable)); } calculate_best_path( db, @@ -301,11 +317,19 @@ fn calculate_best_path( mut prefixed: Option<PrefixKind>, prefer_no_std: bool, scope_name: Option<Name>, -) -> Option<ModPath> { +) -> Option<(ModPath, Stability)> { if max_len <= 1 { return None; } let mut best_path = None; + let update_best_path = + |best_path: &mut Option<_>, new_path: (ModPath, Stability)| match best_path { + Some((old_path, old_stability)) => { + *old_path = new_path.0; + *old_stability = zip_stability(*old_stability, new_path.1); + } + None => *best_path = Some(new_path), + }; // Recursive case: // - otherwise, look for modules containing (reexporting) it and import it from one of those if item.krate(db) == Some(from.krate) { @@ -328,14 +352,14 @@ fn calculate_best_path( prefixed, prefer_no_std, ) { - path.push_segment(name); + path.0.push_segment(name); - let new_path = match best_path { + let new_path = match best_path.take() { Some(best_path) => select_best_path(best_path, path, prefer_no_std), None => path, }; - best_path_len = new_path.len(); - best_path = Some(new_path); + best_path_len = new_path.0.len(); + update_best_path(&mut best_path, new_path); } } } else { @@ -354,7 +378,7 @@ fn calculate_best_path( // Determine best path for containing module and append last segment from `info`. // FIXME: we should guide this to look up the path locally, or from the same crate again? - let mut path = find_path_for_module( + let (mut path, path_stability) = find_path_for_module( db, def_map, visited_modules, @@ -367,16 +391,19 @@ fn calculate_best_path( )?; cov_mark::hit!(partially_imported); path.push_segment(info.name.clone()); - Some(path) + Some(( + path, + zip_stability(path_stability, if info.is_unstable { Unstable } else { Stable }), + )) }) }); for path in extern_paths { - let new_path = match best_path { + let new_path = match best_path.take() { Some(best_path) => select_best_path(best_path, path, prefer_no_std), None => path, }; - best_path = Some(new_path); + update_best_path(&mut best_path, new_path); } } if let Some(module) = item.module(db) { @@ -387,15 +414,24 @@ fn calculate_best_path( } match prefixed.map(PrefixKind::prefix) { Some(prefix) => best_path.or_else(|| { - scope_name.map(|scope_name| ModPath::from_segments(prefix, Some(scope_name))) + scope_name.map(|scope_name| (ModPath::from_segments(prefix, Some(scope_name)), Stable)) }), None => best_path, } } -fn select_best_path(old_path: ModPath, new_path: ModPath, prefer_no_std: bool) -> ModPath { +fn select_best_path( + old_path: (ModPath, Stability), + new_path: (ModPath, Stability), + prefer_no_std: bool, +) -> (ModPath, Stability) { + match (old_path.1, new_path.1) { + (Stable, Unstable) => return old_path, + (Unstable, Stable) => return new_path, + _ => {} + } const STD_CRATES: [Name; 3] = [known::std, known::core, known::alloc]; - match (old_path.segments().first(), new_path.segments().first()) { + match (old_path.0.segments().first(), new_path.0.segments().first()) { (Some(old), Some(new)) if STD_CRATES.contains(old) && STD_CRATES.contains(new) => { let rank = match prefer_no_std { false => |name: &Name| match name { @@ -416,7 +452,7 @@ fn select_best_path(old_path: ModPath, new_path: ModPath, prefer_no_std: bool) - match nrank.cmp(&orank) { Ordering::Less => old_path, Ordering::Equal => { - if new_path.len() < old_path.len() { + if new_path.0.len() < old_path.0.len() { new_path } else { old_path @@ -426,7 +462,7 @@ fn select_best_path(old_path: ModPath, new_path: ModPath, prefer_no_std: bool) - } } _ => { - if new_path.len() < old_path.len() { + if new_path.0.len() < old_path.0.len() { new_path } else { old_path @@ -1360,4 +1396,29 @@ pub mod ops { "std::ops::Deref", ); } + + #[test] + fn respect_unstable_modules() { + check_found_path( + r#" +//- /main.rs crate:main deps:std,core +#![no_std] +extern crate std; +$0 +//- /longer.rs crate:std deps:core +pub mod error { + pub use core::error::Error; +} +//- /core.rs crate:core +pub mod error { + #![unstable(feature = "error_in_core", issue = "103765")] + pub trait Error {} +} +"#, + "std::error::Error", + "std::error::Error", + "std::error::Error", + "std::error::Error", + ); + } } diff --git a/crates/hir-def/src/import_map.rs b/crates/hir-def/src/import_map.rs index 721da29ce8..eb32c76b06 100644 --- a/crates/hir-def/src/import_map.rs +++ b/crates/hir-def/src/import_map.rs @@ -32,6 +32,8 @@ pub struct ImportInfo { pub is_trait_assoc_item: bool, /// Whether this item is annotated with `#[doc(hidden)]`. pub is_doc_hidden: bool, + /// Whether this item is annotated with `#[unstable(..)]`. + pub is_unstable: bool, } /// A map from publicly exported items to its name. @@ -117,7 +119,6 @@ fn collect_import_map(db: &dyn DefDatabase, krate: CrateId) -> FxIndexMap<ItemIn for (name, per_ns) in visible_items { for (item, import) in per_ns.iter_items() { - // FIXME: Not yet used, but will be once we handle doc(hidden) import sources let attr_id = if let Some(import) = import { match import { ImportOrExternCrate::ExternCrate(id) => Some(id.into()), @@ -129,28 +130,59 @@ fn collect_import_map(db: &dyn DefDatabase, krate: CrateId) -> FxIndexMap<ItemIn ItemInNs::Macros(id) => Some(id.into()), } }; - let is_doc_hidden = - attr_id.map_or(false, |attr_id| db.attrs(attr_id).has_doc_hidden()); + let status @ (is_doc_hidden, is_unstable) = + attr_id.map_or((false, false), |attr_id| { + let attrs = db.attrs(attr_id); + (attrs.has_doc_hidden(), attrs.is_unstable()) + }); let import_info = ImportInfo { name: name.clone(), container: module, is_trait_assoc_item: false, is_doc_hidden, + is_unstable, }; match depth_map.entry(item) { - Entry::Vacant(entry) => _ = entry.insert((depth, is_doc_hidden)), + Entry::Vacant(entry) => _ = entry.insert((depth, status)), Entry::Occupied(mut entry) => { - let &(occ_depth, occ_is_doc_hidden) = entry.get(); - // Prefer the one that is not doc(hidden), - // Otherwise, if both have the same doc(hidden)-ness and the new path is shorter, prefer that one. - let overwrite_entry = occ_is_doc_hidden && !is_doc_hidden - || occ_is_doc_hidden == is_doc_hidden && depth < occ_depth; - if !overwrite_entry { + let &(occ_depth, (occ_is_doc_hidden, occ_is_unstable)) = entry.get(); + (depth, occ_depth); + let overwrite = match ( + is_doc_hidden, + occ_is_doc_hidden, + is_unstable, + occ_is_unstable, + ) { + // no change of hiddeness or unstableness + (true, true, true, true) + | (true, true, false, false) + | (false, false, true, true) + | (false, false, false, false) => depth < occ_depth, + + // either less hidden or less unstable, accept + (true, true, false, true) + | (false, true, true, true) + | (false, true, false, true) + | (false, true, false, false) + | (false, false, false, true) => true, + // more hidden or unstable, discard + (true, true, true, false) + | (true, false, true, true) + | (true, false, true, false) + | (true, false, false, false) + | (false, false, true, false) => false, + + // exchanges doc(hidden) for unstable (and vice-versa), + (true, false, false, true) | (false, true, true, false) => { + depth < occ_depth + } + }; + if !overwrite { continue; } - entry.insert((depth, is_doc_hidden)); + entry.insert((depth, status)); } } @@ -204,11 +236,13 @@ fn collect_trait_assoc_items( ItemInNs::Values(module_def_id) }; + let attrs = &db.attrs(item.into()); let assoc_item_info = ImportInfo { container: trait_import_info.container, name: assoc_item_name.clone(), is_trait_assoc_item: true, - is_doc_hidden: db.attrs(item.into()).has_doc_hidden(), + is_doc_hidden: attrs.has_doc_hidden(), + is_unstable: attrs.is_unstable(), }; map.insert(assoc_item, assoc_item_info); } |