Skip to content

Commit

Permalink
Add config for preferring / ignoring prelude modules in find_path
Browse files Browse the repository at this point in the history
  • Loading branch information
Veykril committed Nov 11, 2023
1 parent 801a887 commit ba61766
Show file tree
Hide file tree
Showing 36 changed files with 261 additions and 55 deletions.
140 changes: 110 additions & 30 deletions crates/hir-def/src/find_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,10 @@ pub fn find_path(
item: ItemInNs,
from: ModuleId,
prefer_no_std: bool,
prefer_prelude: bool,
) -> Option<ModPath> {
let _p = profile::span("find_path");
find_path_inner(db, item, from, None, prefer_no_std)
find_path_inner(db, item, from, None, prefer_no_std, prefer_prelude)
}

pub fn find_path_prefixed(
Expand All @@ -32,9 +33,10 @@ pub fn find_path_prefixed(
from: ModuleId,
prefix_kind: PrefixKind,
prefer_no_std: bool,
prefer_prelude: bool,
) -> Option<ModPath> {
let _p = profile::span("find_path_prefixed");
find_path_inner(db, item, from, Some(prefix_kind), prefer_no_std)
find_path_inner(db, item, from, Some(prefix_kind), prefer_no_std, prefer_prelude)
}

#[derive(Copy, Clone, Debug)]
Expand Down Expand Up @@ -88,6 +90,7 @@ fn find_path_inner(
from: ModuleId,
prefixed: Option<PrefixKind>,
prefer_no_std: bool,
prefer_prelude: bool,
) -> Option<ModPath> {
// - if the item is a builtin, it's in scope
if let ItemInNs::Types(ModuleDefId::BuiltinType(builtin)) = item {
Expand All @@ -109,6 +112,7 @@ fn find_path_inner(
MAX_PATH_LEN,
prefixed,
prefer_no_std || db.crate_supports_no_std(crate_root.krate),
prefer_prelude,
)
.map(|(item, _)| item);
}
Expand All @@ -134,6 +138,7 @@ fn find_path_inner(
from,
prefixed,
prefer_no_std,
prefer_prelude,
) {
let data = db.enum_data(variant.parent);
path.push_segment(data.variants[variant.local_id].name.clone());
Expand All @@ -156,6 +161,7 @@ fn find_path_inner(
from,
prefixed,
prefer_no_std || db.crate_supports_no_std(crate_root.krate),
prefer_prelude,
scope_name,
)
.map(|(item, _)| item)
Expand All @@ -171,6 +177,7 @@ fn find_path_for_module(
max_len: usize,
prefixed: Option<PrefixKind>,
prefer_no_std: bool,
prefer_prelude: bool,
) -> Option<(ModPath, Stability)> {
if max_len == 0 {
return None;
Expand Down Expand Up @@ -236,6 +243,7 @@ fn find_path_for_module(
from,
prefixed,
prefer_no_std,
prefer_prelude,
scope_name,
)
}
Expand Down Expand Up @@ -316,6 +324,7 @@ fn calculate_best_path(
from: ModuleId,
mut prefixed: Option<PrefixKind>,
prefer_no_std: bool,
prefer_prelude: bool,
scope_name: Option<Name>,
) -> Option<(ModPath, Stability)> {
if max_len <= 1 {
Expand Down Expand Up @@ -351,11 +360,14 @@ fn calculate_best_path(
best_path_len - 1,
prefixed,
prefer_no_std,
prefer_prelude,
) {
path.0.push_segment(name);

let new_path = match best_path.take() {
Some(best_path) => select_best_path(best_path, path, prefer_no_std),
Some(best_path) => {
select_best_path(best_path, path, prefer_no_std, prefer_prelude)
}
None => path,
};
best_path_len = new_path.0.len();
Expand Down Expand Up @@ -388,6 +400,7 @@ fn calculate_best_path(
max_len - 1,
prefixed,
prefer_no_std,
prefer_prelude,
) else {
continue;
};
Expand All @@ -400,7 +413,9 @@ fn calculate_best_path(
);

let new_path_with_stab = match best_path.take() {
Some(best_path) => select_best_path(best_path, path_with_stab, prefer_no_std),
Some(best_path) => {
select_best_path(best_path, path_with_stab, prefer_no_std, prefer_prelude)
}
None => path_with_stab,
};
update_best_path(&mut best_path, new_path_with_stab);
Expand All @@ -421,17 +436,39 @@ fn calculate_best_path(
}
}

/// Select the best (most relevant) path between two paths.
/// This accounts for stability, path length whether std should be chosen over alloc/core paths as
/// well as ignoring prelude like paths or not.
fn select_best_path(
old_path: (ModPath, Stability),
new_path: (ModPath, Stability),
old_path @ (_, old_stability): (ModPath, Stability),
new_path @ (_, new_stability): (ModPath, Stability),
prefer_no_std: bool,
prefer_prelude: bool,
) -> (ModPath, Stability) {
match (old_path.1, new_path.1) {
match (old_stability, new_stability) {
(Stable, Unstable) => return old_path,
(Unstable, Stable) => return new_path,
_ => {}
}
const STD_CRATES: [Name; 3] = [known::std, known::core, known::alloc];

let choose = |new_path: (ModPath, _), old_path: (ModPath, _)| {
let new_has_prelude = new_path.0.segments().iter().any(|seg| seg == &known::prelude);
let old_has_prelude = old_path.0.segments().iter().any(|seg| seg == &known::prelude);
match (new_has_prelude, old_has_prelude, prefer_prelude) {
(true, false, true) | (false, true, false) => new_path,
(true, false, false) | (false, true, true) => old_path,
// no prelude difference in the paths, so pick the smaller one
(true, true, _) | (false, false, _) => {
if new_path.0.len() < old_path.0.len() {
new_path
} else {
old_path
}
}
}
};

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 {
Expand All @@ -452,23 +489,11 @@ fn select_best_path(
let orank = rank(old);
match nrank.cmp(&orank) {
Ordering::Less => old_path,
Ordering::Equal => {
if new_path.0.len() < old_path.0.len() {
new_path
} else {
old_path
}
}
Ordering::Equal => choose(new_path, old_path),
Ordering::Greater => new_path,
}
}
_ => {
if new_path.0.len() < old_path.0.len() {
new_path
} else {
old_path
}
}
_ => choose(new_path, old_path),
}
}

Expand Down Expand Up @@ -571,7 +596,13 @@ mod tests {
/// `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
/// module the cursor is in.
fn check_found_path_(ra_fixture: &str, path: &str, prefix_kind: Option<PrefixKind>) {
#[track_caller]
fn check_found_path_(
ra_fixture: &str,
path: &str,
prefix_kind: Option<PrefixKind>,
prefer_prelude: bool,
) {
let (db, pos) = TestDB::with_position(ra_fixture);
let module = db.module_at_position(pos);
let parsed_path_file = syntax::SourceFile::parse(&format!("use {path};"));
Expand All @@ -590,10 +621,16 @@ mod tests {
)
.0
.take_types()
.unwrap();

let found_path =
find_path_inner(&db, ItemInNs::Types(resolved), module, prefix_kind, false);
.expect("path does not resolve to a type");

let found_path = find_path_inner(
&db,
ItemInNs::Types(resolved),
module,
prefix_kind,
false,
prefer_prelude,
);
assert_eq!(found_path, Some(mod_path), "on kind: {prefix_kind:?}");
}

Expand All @@ -604,10 +641,23 @@ mod tests {
absolute: &str,
self_prefixed: &str,
) {
check_found_path_(ra_fixture, unprefixed, None);
check_found_path_(ra_fixture, prefixed, Some(PrefixKind::Plain));
check_found_path_(ra_fixture, absolute, Some(PrefixKind::ByCrate));
check_found_path_(ra_fixture, self_prefixed, Some(PrefixKind::BySelf));
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);
}

fn check_found_path_prelude(
ra_fixture: &str,
unprefixed: &str,
prefixed: &str,
absolute: &str,
self_prefixed: &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);
}

#[test]
Expand Down Expand Up @@ -1422,4 +1472,34 @@ pub mod error {
"std::error::Error",
);
}

#[test]
fn respects_prelude_setting() {
let ra_fixture = r#"
//- /main.rs crate:main deps:krate
$0
//- /krate.rs crate:krate
pub mod prelude {
pub use crate::foo::*;
}
pub mod foo {
pub struct Foo;
}
"#;
check_found_path(
ra_fixture,
"krate::foo::Foo",
"krate::foo::Foo",
"krate::foo::Foo",
"krate::foo::Foo",
);
check_found_path_prelude(
ra_fixture,
"krate::prelude::Foo",
"krate::prelude::Foo",
"krate::prelude::Foo",
"krate::prelude::Foo",
);
}
}
1 change: 1 addition & 0 deletions crates/hir-ty/src/display.rs
Original file line number Diff line number Diff line change
Expand Up @@ -945,6 +945,7 @@ impl HirDisplay for Ty {
ItemInNs::Types((*def_id).into()),
module_id,
false,
true,
) {
write!(f, "{}", path.display(f.db.upcast()))?;
} else {
Expand Down
11 changes: 10 additions & 1 deletion crates/hir/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -664,8 +664,15 @@ impl Module {
db: &dyn DefDatabase,
item: impl Into<ItemInNs>,
prefer_no_std: bool,
prefer_prelude: bool,
) -> Option<ModPath> {
hir_def::find_path::find_path(db, item.into().into(), self.into(), prefer_no_std)
hir_def::find_path::find_path(
db,
item.into().into(),
self.into(),
prefer_no_std,
prefer_prelude,
)
}

/// Finds a path that can be used to refer to the given item from within
Expand All @@ -676,13 +683,15 @@ impl Module {
item: impl Into<ItemInNs>,
prefix_kind: PrefixKind,
prefer_no_std: bool,
prefer_prelude: bool,
) -> Option<ModPath> {
hir_def::find_path::find_path_prefixed(
db,
item.into().into(),
self.into(),
prefix_kind,
prefer_no_std,
prefer_prelude,
)
}
}
Expand Down
1 change: 1 addition & 0 deletions crates/ide-assists/src/assist_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,6 @@ pub struct AssistConfig {
pub allowed: Option<Vec<AssistKind>>,
pub insert_use: InsertUseConfig,
pub prefer_no_std: bool,
pub prefer_prelude: bool,
pub assist_emit_must_use: bool,
}
33 changes: 28 additions & 5 deletions crates/ide-assists/src/handlers/add_missing_match_arms.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,13 @@ pub(crate) fn add_missing_match_arms(acc: &mut Assists, ctx: &AssistContext<'_>)
.into_iter()
.filter_map(|variant| {
Some((
build_pat(ctx.db(), module, variant, ctx.config.prefer_no_std)?,
build_pat(
ctx.db(),
module,
variant,
ctx.config.prefer_no_std,
ctx.config.prefer_prelude,
)?,
variant.should_be_hidden(ctx.db(), module.krate()),
))
})
Expand Down Expand Up @@ -140,7 +146,13 @@ pub(crate) fn add_missing_match_arms(acc: &mut Assists, ctx: &AssistContext<'_>)
.iter()
.any(|variant| variant.should_be_hidden(ctx.db(), module.krate()));
let patterns = variants.into_iter().filter_map(|variant| {
build_pat(ctx.db(), module, variant, ctx.config.prefer_no_std)
build_pat(
ctx.db(),
module,
variant,
ctx.config.prefer_no_std,
ctx.config.prefer_prelude,
)
});

(ast::Pat::from(make::tuple_pat(patterns)), is_hidden)
Expand Down Expand Up @@ -173,7 +185,13 @@ pub(crate) fn add_missing_match_arms(acc: &mut Assists, ctx: &AssistContext<'_>)
.iter()
.any(|variant| variant.should_be_hidden(ctx.db(), module.krate()));
let patterns = variants.into_iter().filter_map(|variant| {
build_pat(ctx.db(), module, variant.clone(), ctx.config.prefer_no_std)
build_pat(
ctx.db(),
module,
variant.clone(),
ctx.config.prefer_no_std,
ctx.config.prefer_prelude,
)
});
(ast::Pat::from(make::slice_pat(patterns)), is_hidden)
})
Expand Down Expand Up @@ -440,11 +458,16 @@ fn build_pat(
module: hir::Module,
var: ExtendedVariant,
prefer_no_std: bool,
prefer_prelude: bool,
) -> Option<ast::Pat> {
match var {
ExtendedVariant::Variant(var) => {
let path =
mod_path_to_ast(&module.find_use_path(db, ModuleDef::from(var), prefer_no_std)?);
let path = mod_path_to_ast(&module.find_use_path(
db,
ModuleDef::from(var),
prefer_no_std,
prefer_prelude,
)?);

// FIXME: use HIR for this; it doesn't currently expose struct vs. tuple vs. unit variants though
Some(match var.source(db)?.value.kind() {
Expand Down
Loading

0 comments on commit ba61766

Please sign in to comment.