language-servers/rust-analyzer
1
0
Fork 0
mirror of https://github.com/rust-lang/rust-analyzer.git synced 2025-09-29 05:15:04 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

fix: Fix general find-path inconsistencies

Browse source
This commit is contained in:
Lukas Wirth 2024-05-22 13:49:56 +02:00
parent 21ec8f5238
commit c88b421853
32 changed files with 242 additions and 193 deletions
Download patch file Download diff file Expand all files Collapse all files

281
crates/hir-def/src/find_path.rs
View file

@ -1,6 +1,9 @@
//! An algorithm to find a path to refer to a certain item.
use std::{cmp::Ordering, iter};
use std::{
cmp::Ordering,
iter::{self, once},
};
use hir_expand::{
name::{known, AsName, Name},
@ -14,7 +17,7 @@ use crate::{
nameres::DefMap,
path::{ModPath, PathKind},
visibility::{Visibility, VisibilityExplicitness},
CrateRootModuleId, ModuleDefId, ModuleId,
ModuleDefId, ModuleId,
};
/// Find a path that can be used to refer to a certain item. This can depend on
@ -23,26 +26,20 @@ pub fn find_path(
db: &dyn DefDatabase,
item: ItemInNs,
from: ModuleId,
prefix_kind: PrefixKind,
ignore_local_imports: bool,
prefer_no_std: bool,
prefer_prelude: bool,
) -> Option<ModPath> {
let _p = tracing::span!(tracing::Level::INFO, "find_path").entered();
find_path_inner(FindPathCtx { db, prefixed: None, prefer_no_std, prefer_prelude }, item, from)
}
/// Find a path that can be used to refer to a certain item. This can depend on
/// *from where* you're referring to the item, hence the `from` parameter.
pub fn find_path_prefixed(
db: &dyn DefDatabase,
item: ItemInNs,
from: ModuleId,
prefix_kind: PrefixKind,
prefer_no_std: bool,
prefer_prelude: bool,
) -> Option<ModPath> {
let _p = tracing::span!(tracing::Level::INFO, "find_path_prefixed").entered();
find_path_inner(
FindPathCtx { db, prefixed: Some(prefix_kind), prefer_no_std, prefer_prelude },
FindPathCtx {
db,
prefix: prefix_kind,
prefer_no_std,
prefer_prelude,
ignore_local_imports,
},
item,
from,
)
@ -70,7 +67,7 @@ pub enum PrefixKind {
/// This is the same as plain, just that paths will start with `self` prepended if the path
/// starts with an identifier that is not a crate.
BySelf,
/// Causes paths to ignore imports in the local module.
/// Causes paths to not use a self, super or crate prefix.
Plain,
/// Causes paths to start with `crate` where applicable, effectively forcing paths to be absolute.
ByCrate,
@ -78,37 +75,33 @@ pub enum PrefixKind {
impl PrefixKind {
#[inline]
fn prefix(self) -> PathKind {
fn path_kind(self) -> PathKind {
match self {
PrefixKind::BySelf => PathKind::Super(0),
PrefixKind::Plain => PathKind::Plain,
PrefixKind::ByCrate => PathKind::Crate,
}
}
#[inline]
fn is_absolute(&self) -> bool {
self == &PrefixKind::ByCrate
}
}
#[derive(Copy, Clone)]
struct FindPathCtx<'db> {
db: &'db dyn DefDatabase,
prefixed: Option<PrefixKind>,
prefix: PrefixKind,
prefer_no_std: bool,
prefer_prelude: bool,
ignore_local_imports: bool,
}
/// Attempts to find a path to refer to the given `item` visible from the `from` ModuleId
fn find_path_inner(ctx: FindPathCtx<'_>, item: ItemInNs, from: ModuleId) -> Option<ModPath> {
// - if the item is a builtin, it's in scope
if let ItemInNs::Types(ModuleDefId::BuiltinType(builtin)) = item {
return Some(ModPath::from_segments(PathKind::Plain, Some(builtin.as_name())));
return Some(ModPath::from_segments(PathKind::Plain, once(builtin.as_name())));
}
let def_map = from.def_map(ctx.db);
let crate_root = def_map.crate_root();
let crate_root = from.derive_crate_root();
// - if the item is a module, jump straight to module search
if let ItemInNs::Types(ModuleDefId::ModuleId(module_id)) = item {
let mut visited_modules = FxHashSet::default();
@ -119,7 +112,6 @@ fn find_path_inner(ctx: FindPathCtx<'_>, item: ItemInNs, from: ModuleId) -> Opti
},
&def_map,
&mut visited_modules,
crate_root,
from,
module_id,
MAX_PATH_LEN,
@ -127,11 +119,20 @@ fn find_path_inner(ctx: FindPathCtx<'_>, item: ItemInNs, from: ModuleId) -> Opti
.map(|(item, _)| item);
}
// - if the item is already in scope, return the name under which it is
let scope_name = find_in_scope(ctx.db, &def_map, from, item);
if ctx.prefixed.is_none() {
let prefix = if item.module(ctx.db).is_some_and(|it| it.is_within_block()) {
PrefixKind::Plain
} else {
ctx.prefix
};
let may_be_in_scope = match prefix {
PrefixKind::Plain | PrefixKind::BySelf => true,
PrefixKind::ByCrate => from.is_crate_root(),
};
if may_be_in_scope {
// - if the item is already in scope, return the name under which it is
let scope_name = find_in_scope(ctx.db, &def_map, from, item, ctx.ignore_local_imports);
if let Some(scope_name) = scope_name {
return Some(ModPath::from_segments(PathKind::Plain, Some(scope_name)));
return Some(ModPath::from_segments(prefix.path_kind(), Some(scope_name)));
}
}
@ -164,11 +165,9 @@ fn find_path_inner(ctx: FindPathCtx<'_>, item: ItemInNs, from: ModuleId) -> Opti
},
&def_map,
&mut visited_modules,
crate_root,
MAX_PATH_LEN,
item,
from,
scope_name,
)
.map(|(item, _)| item)
}
@ -178,7 +177,6 @@ fn find_path_for_module(
ctx: FindPathCtx<'_>,
def_map: &DefMap,
visited_modules: &mut FxHashSet<ModuleId>,
crate_root: CrateRootModuleId,
from: ModuleId,
module_id: ModuleId,
max_len: usize,
@ -187,38 +185,25 @@ fn find_path_for_module(
return None;
}
// Base cases:
// - if the item is already in scope, return the name under which it is
let scope_name = find_in_scope(ctx.db, def_map, from, ItemInNs::Types(module_id.into()));
if ctx.prefixed.is_none() {
if let Some(scope_name) = scope_name {
return Some((ModPath::from_segments(PathKind::Plain, Some(scope_name)), Stable));
}
}
let is_crate_root = module_id.as_crate_root();
// - if the item is the crate root, return `crate`
if module_id == crate_root {
if is_crate_root.is_some_and(|it| it == from.derive_crate_root()) {
return Some((ModPath::from_segments(PathKind::Crate, None), Stable));
}
// - if relative paths are fine, check if we are searching for a parent
if ctx.prefixed.filter(PrefixKind::is_absolute).is_none() {
if let modpath @ Some(_) = find_self_super(def_map, module_id, from) {
return modpath.zip(Some(Stable));
}
}
let root_def_map = from.derive_crate_root().def_map(ctx.db);
// - if the item is the crate root of a dependency crate, return the name from the extern prelude
let root_def_map = crate_root.def_map(ctx.db);
for (name, (def_id, _extern_crate)) in root_def_map.extern_prelude() {
if module_id == def_id {
let name = scope_name.unwrap_or_else(|| name.clone());
if let Some(crate_root) = is_crate_root {
// rev here so we prefer looking at renamed extern decls first
for (name, (def_id, _extern_crate)) in root_def_map.extern_prelude().rev() {
if crate_root != def_id {
continue;
}
let name_already_occupied_in_type_ns = def_map
.with_ancestor_maps(ctx.db, from.local_id, &mut |def_map, local_id| {
def_map[local_id]
.scope
.type_(&name)
.type_(name)
.filter(|&(id, _)| id != ModuleDefId::ModuleId(def_id.into()))
})
.is_some();
@ -228,24 +213,48 @@ fn find_path_for_module(
} else {
PathKind::Plain
};
return Some((ModPath::from_segments(kind, Some(name)), Stable));
return Some((ModPath::from_segments(kind, once(name.clone())), Stable));
}
}
let prefix = if module_id.is_within_block() { PrefixKind::Plain } else { ctx.prefix };
let may_be_in_scope = match prefix {
PrefixKind::Plain | PrefixKind::BySelf => true,
PrefixKind::ByCrate => from.is_crate_root(),
};
if may_be_in_scope {
let scope_name = find_in_scope(
ctx.db,
def_map,
from,
ItemInNs::Types(module_id.into()),
ctx.ignore_local_imports,
);
if let Some(scope_name) = scope_name {
// - if the item is already in scope, return the name under which it is
return Some((ModPath::from_segments(prefix.path_kind(), once(scope_name)), Stable));
}
}
if let value @ Some(_) =
// - if the module can be referenced as self, super or crate, do that
if let Some(mod_path) = is_kw_kind_relative_to_from(def_map, module_id, from) {
if ctx.prefix != PrefixKind::ByCrate || mod_path.kind == PathKind::Crate {
return Some((mod_path, Stable));
}
}
// - if the module is in the prelude, return it by that path
if let Some(mod_path) =
find_in_prelude(ctx.db, &root_def_map, def_map, ItemInNs::Types(module_id.into()), from)
{
return value.zip(Some(Stable));
return Some((mod_path, Stable));
}
calculate_best_path(
ctx,
def_map,
visited_modules,
crate_root,
max_len,
ItemInNs::Types(module_id.into()),
from,
scope_name,
)
}
@ -255,9 +264,13 @@ fn find_in_scope(
def_map: &DefMap,
from: ModuleId,
item: ItemInNs,
ignore_local_imports: bool,
) -> Option<Name> {
// FIXME: We could have multiple applicable names here, but we currently only return the first
def_map.with_ancestor_maps(db, from.local_id, &mut |def_map, local_id| {
def_map[local_id].scope.names_of(item, |name, _, _| Some(name.clone()))
def_map[local_id].scope.names_of(item, |name, _, declared| {
(declared || !ignore_local_imports).then(|| name.clone())
})
})
}
@ -292,21 +305,32 @@ fn find_in_prelude(
});
if found_and_same_def.unwrap_or(true) {
Some(ModPath::from_segments(PathKind::Plain, Some(name.clone())))
Some(ModPath::from_segments(PathKind::Plain, once(name.clone())))
} else {
None
}
}
fn find_self_super(def_map: &DefMap, item: ModuleId, from: ModuleId) -> Option<ModPath> {
fn is_kw_kind_relative_to_from(
def_map: &DefMap,
item: ModuleId,
from: ModuleId,
) -> Option<ModPath> {
if item.krate != from.krate || item.is_within_block() || from.is_within_block() {
return None;
}
let item = item.local_id;
let from = from.local_id;
if item == from {
// - if the item is the module we're in, use `self`
Some(ModPath::from_segments(PathKind::Super(0), None))
} else if let Some(parent_id) = def_map[from.local_id].parent {
// - if the item is the parent module, use `super` (this is not used recursively, since `super::super` is ugly)
let parent_id = def_map.module_id(parent_id);
} else if let Some(parent_id) = def_map[from].parent {
if item == parent_id {
Some(ModPath::from_segments(PathKind::Super(1), None))
// - if the item is the parent module, use `super` (this is not used recursively, since `super::super` is ugly)
Some(ModPath::from_segments(
if parent_id == DefMap::ROOT { PathKind::Crate } else { PathKind::Super(1) },
None,
))
} else {
None
}
@ -320,11 +344,9 @@ fn calculate_best_path(
ctx: FindPathCtx<'_>,
def_map: &DefMap,
visited_modules: &mut FxHashSet<ModuleId>,
crate_root: CrateRootModuleId,
max_len: usize,
item: ItemInNs,
from: ModuleId,
scope_name: Option<Name>,
) -> Option<(ModPath, Stability)> {
if max_len <= 1 {
return None;
@ -346,14 +368,12 @@ fn calculate_best_path(
// dependency in this case.
for (module_id, name) in find_local_import_locations(ctx.db, item, from) {
if !visited_modules.insert(module_id) {
cov_mark::hit!(recursive_imports);
continue;
}
if let Some(mut path) = find_path_for_module(
ctx,
def_map,
visited_modules,
crate_root,
from,
module_id,
best_path_len - 1,
@ -390,7 +410,6 @@ fn calculate_best_path(
ctx,
def_map,
visited_modules,
crate_root,
from,
info.container,
max_len - 1,
@ -418,19 +437,7 @@ fn calculate_best_path(
}
}
}
let mut prefixed = ctx.prefixed;
if let Some(module) = item.module(ctx.db) {
if module.containing_block().is_some() && ctx.prefixed.is_some() {
cov_mark::hit!(prefixed_in_block_expression);
prefixed = Some(PrefixKind::Plain);
}
}
match prefixed.map(PrefixKind::prefix) {
Some(prefix) => best_path.or_else(|| {
scope_name.map(|scope_name| (ModPath::from_segments(prefix, Some(scope_name)), Stable))
}),
None => best_path,
}
best_path
}
/// Select the best (most relevant) path between two paths.
@ -535,7 +542,6 @@ fn find_local_import_locations(
if !seen.insert(module) {
continue; // already processed this module
}
let ext_def_map;
let data = if module.krate == from.krate {
if module.block.is_some() {
@ -571,7 +577,7 @@ fn find_local_import_locations(
// what the user wants; and if this module can import
// the item and we're a submodule of it, so can we.
// Also this keeps the cached data smaller.
if is_pub_or_explicit || declared {
if declared || is_pub_or_explicit {
locations.push((module, name.clone()));
}
}
@ -605,8 +611,9 @@ mod tests {
fn check_found_path_(
ra_fixture: &str,
path: &str,
prefix_kind: Option<PrefixKind>,
prefix_kind: PrefixKind,
prefer_prelude: bool,
ignore_local_imports: bool,
) {
let (db, pos) = TestDB::with_position(ra_fixture);
let module = db.module_at_position(pos);
@ -628,43 +635,51 @@ mod tests {
crate::item_scope::BuiltinShadowMode::Module,
None,
)
.0
.0;
let resolved = resolved
.take_types()
.expect("path does not resolve to a type");
.map(ItemInNs::Types)
.or_else(|| resolved.take_values().map(ItemInNs::Values))
.expect("path does not resolve to a type or value");
let found_path = find_path_inner(
FindPathCtx { prefer_no_std: false, db: &db, prefixed: prefix_kind, prefer_prelude },
ItemInNs::Types(resolved),
FindPathCtx {
prefer_no_std: false,
db: &db,
prefix: prefix_kind,
prefer_prelude,
ignore_local_imports,
},
resolved,
module,
);
assert_eq!(found_path, Some(mod_path), "on kind: {prefix_kind:?}");
assert_eq!(found_path, Some(mod_path), "on kind: {prefix_kind:?} ({ignore_local_imports})");
}
#[track_caller]
fn check_found_path(
ra_fixture: &str,
unprefixed: &str,
prefixed: &str,
absolute: &str,
self_prefixed: &str,
plain_non_local: &str,
plain: &str,
by_crate: &str,
by_self: &str,
) {
check_found_path_(ra_fixture, unprefixed, None, false);
check_found_path_(ra_fixture, prefixed, Some(PrefixKind::Plain), false);
check_found_path_(ra_fixture, absolute, Some(PrefixKind::ByCrate), false);
check_found_path_(ra_fixture, self_prefixed, Some(PrefixKind::BySelf), false);
check_found_path_(ra_fixture, plain_non_local, PrefixKind::Plain, false, true);
check_found_path_(ra_fixture, plain, PrefixKind::Plain, false, false);
check_found_path_(ra_fixture, by_crate, PrefixKind::ByCrate, false, false);
check_found_path_(ra_fixture, by_self, PrefixKind::BySelf, false, false);
}
fn check_found_path_prelude(
ra_fixture: &str,
unprefixed: &str,
prefixed: &str,
absolute: &str,
self_prefixed: &str,
plain_non_local: &str,
plain: &str,
by_crate: &str,
by_self: &str,
) {
check_found_path_(ra_fixture, unprefixed, None, true);
check_found_path_(ra_fixture, prefixed, Some(PrefixKind::Plain), true);
check_found_path_(ra_fixture, absolute, Some(PrefixKind::ByCrate), true);
check_found_path_(ra_fixture, self_prefixed, Some(PrefixKind::BySelf), true);
check_found_path_(ra_fixture, plain_non_local, PrefixKind::Plain, true, true);
check_found_path_(ra_fixture, plain, PrefixKind::Plain, true, false);
check_found_path_(ra_fixture, by_crate, PrefixKind::ByCrate, true, false);
check_found_path_(ra_fixture, by_self, PrefixKind::BySelf, true, false);
}
#[test]
@ -831,10 +846,10 @@ pub mod ast {
}
}
"#,
"syntax::ast::ModuleItem",
"ast::ModuleItem",
"syntax::ast::ModuleItem",
"syntax::ast::ModuleItem",
"syntax::ast::ModuleItem",
"crate::ast::ModuleItem",
"self::ast::ModuleItem",
);
check_found_path(
@ -965,8 +980,8 @@ pub mod prelude {
"#,
"S",
"S",
"S",
"S",
"crate::S",
"self::S",
);
}
@ -1271,12 +1286,11 @@ fn main() {
#[test]
fn inner_items_from_inner_module() {
cov_mark::check!(prefixed_in_block_expression);
check_found_path(
r#"
fn main() {
mod module {
struct Struct {}
pub struct Struct {}
}
{
$0
@ -1303,11 +1317,10 @@ fn main() {
$0
}
"#,
// FIXME: these could use fewer/better prefixes
"module::CompleteMe",
"module::CompleteMe",
"crate::module::CompleteMe",
"crate::module::CompleteMe",
"crate::module::CompleteMe",
"self::module::CompleteMe",
)
}
@ -1358,7 +1371,6 @@ mod bar {
#[test]
fn recursive_pub_mod_reexport() {
cov_mark::check!(recursive_imports);
check_found_path(
r#"
fn main() {
@ -1587,4 +1599,25 @@ pub mod prelude {
"petgraph::graph::NodeIndex",
);
}
#[test]
fn regression_17271() {
check_found_path(
r#"
//- /lib.rs crate:main
mod foo;
//- /foo.rs
mod bar;
pub fn b() {$0}
//- /foo/bar.rs
pub fn c() {}
"#,
"bar::c",
"bar::c",
"crate::foo::bar::c",
"self::bar::c",
);
}
}