Mirrors/rust-analyzer
2
0
Fork 0
mirror of https://github.com/rust-lang/rust-analyzer synced 2024-12-26 21:13:37 +00:00
Code Issues Projects Releases Packages Wiki Activity

fix: Fix find_path not respecting non-std preference config correctly

Browse source
This commit is contained in:
Lukas Wirth 2024-08-10 08:02:03 +02:00
parent 56f63dfd8a
commit 24c0e0bd48
11 changed files with 124 additions and 104 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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

@ -50,13 +50,13 @@ pub fn find_path(
prefix: prefix_kind,
cfg,
ignore_local_imports,
is_std_item: db.crate_graph()[item_module.krate()].origin.is_lang(),
from,
from_def_map: &from.def_map(db),
fuel: Cell::new(FIND_PATH_FUEL),
},
item,
MAX_PATH_LEN,
db.crate_graph()[item_module.krate()].origin.is_lang(),
)
}
@ -98,20 +98,16 @@ struct FindPathCtx<'db> {
prefix: PrefixKind,
cfg: ImportPathConfig,
ignore_local_imports: bool,
is_std_item: bool,
from: ModuleId,
from_def_map: &'db DefMap,
fuel: Cell<usize>,
}
/// Attempts to find a path to refer to the given `item` visible from the `from` ModuleId
fn find_path_inner(
ctx: &FindPathCtx<'_>,
item: ItemInNs,
max_len: usize,
is_std_item: bool,
) -> Option<ModPath> {
fn find_path_inner(ctx: &FindPathCtx<'_>, item: ItemInNs, max_len: usize) -> Option<ModPath> {
// - if the item is a module, jump straight to module search
if !is_std_item {
if !ctx.is_std_item {
if let ItemInNs::Types(ModuleDefId::ModuleId(module_id)) = item {
return find_path_for_module(ctx, &mut FxHashSet::default(), module_id, true, max_len)
.map(|choice| choice.path);
@ -138,12 +134,9 @@ fn find_path_inner(
if let Some(ModuleDefId::EnumVariantId(variant)) = item.as_module_def_id() {
// - if the item is an enum variant, refer to it via the enum
if let Some(mut path) = find_path_inner(
ctx,
ItemInNs::Types(variant.lookup(ctx.db).parent.into()),
max_len,
is_std_item,
) {
if let Some(mut path) =
find_path_inner(ctx, ItemInNs::Types(variant.lookup(ctx.db).parent.into()), max_len)
{
path.push_segment(ctx.db.enum_variant_data(variant).name.clone());
return Some(path);
}
@ -152,16 +145,6 @@ fn find_path_inner(
// variant somewhere
}
if is_std_item {
// The item we are searching for comes from the sysroot libraries, so skip prefer looking in
// the sysroot libraries directly.
// We do need to fallback as the item in question could be re-exported by another crate
// while not being a transitive dependency of the current crate.
if let Some(choice) = find_in_sysroot(ctx, &mut FxHashSet::default(), item, max_len) {
return Some(choice.path);
}
}
let mut best_choice = None;
calculate_best_path(ctx, &mut FxHashSet::default(), item, max_len, &mut best_choice);
best_choice.map(|choice| choice.path)
@ -366,6 +349,12 @@ fn calculate_best_path(
// Item was defined in the same crate that wants to import it. It cannot be found in any
// dependency in this case.
calculate_best_path_local(ctx, visited_modules, item, max_len, best_choice)
} else if ctx.is_std_item {
// The item we are searching for comes from the sysroot libraries, so skip prefer looking in
// the sysroot libraries directly.
// We do need to fallback as the item in question could be re-exported by another crate
// while not being a transitive dependency of the current crate.
find_in_sysroot(ctx, visited_modules, item, max_len, best_choice)
} else {
// Item was defined in some upstream crate. This means that it must be exported from one,
// too (unless we can't name it at all). It could *also* be (re)exported by the same crate
@ -382,10 +371,10 @@ fn find_in_sysroot(
visited_modules: &mut FxHashSet<(ItemInNs, ModuleId)>,
item: ItemInNs,
max_len: usize,
) -> Option<Choice> {
best_choice: &mut Option<Choice>,
) {
let crate_graph = ctx.db.crate_graph();
let dependencies = &crate_graph[ctx.from.krate].dependencies;
let mut best_choice = None;
let mut search = |lang, best_choice: &mut _| {
if let Some(dep) = dependencies.iter().filter(|it| it.is_sysroot()).find(|dep| {
match crate_graph[dep.crate_id].origin {
@ -397,29 +386,31 @@ fn find_in_sysroot(
}
};
if ctx.cfg.prefer_no_std {
search(LangCrateOrigin::Core, &mut best_choice);
search(LangCrateOrigin::Core, best_choice);
if matches!(best_choice, Some(Choice { stability: Stable, .. })) {
return best_choice;
return;
}
search(LangCrateOrigin::Std, &mut best_choice);
search(LangCrateOrigin::Std, best_choice);
if matches!(best_choice, Some(Choice { stability: Stable, .. })) {
return best_choice;
return;
}
} else {
search(LangCrateOrigin::Std, &mut best_choice);
search(LangCrateOrigin::Std, best_choice);
if matches!(best_choice, Some(Choice { stability: Stable, .. })) {
return best_choice;
return;
}
search(LangCrateOrigin::Core, &mut best_choice);
search(LangCrateOrigin::Core, best_choice);
if matches!(best_choice, Some(Choice { stability: Stable, .. })) {
return best_choice;
return;
}
}
let mut best_choice = None;
dependencies.iter().filter(|it| it.is_sysroot()).for_each(|dep| {
find_in_dep(ctx, visited_modules, item, max_len, &mut best_choice, dep.crate_id);
});
best_choice
dependencies
.iter()
.filter(|it| it.is_sysroot())
.chain(dependencies.iter().filter(|it| !it.is_sysroot()))
.for_each(|dep| {
find_in_dep(ctx, visited_modules, item, max_len, best_choice, dep.crate_id);
});
}
fn find_in_dep(
@ -491,6 +482,7 @@ fn calculate_best_path_local(
);
}
#[derive(Debug)]
struct Choice {
path: ModPath,
/// The length in characters of the path
@ -676,6 +668,7 @@ mod tests {
path: &str,
prefer_prelude: bool,
prefer_absolute: bool,
prefer_no_std: bool,
expect: Expect,
) {
let (db, pos) = TestDB::with_position(ra_fixture);
@ -717,7 +710,7 @@ mod tests {
module,
prefix,
ignore_local_imports,
ImportPathConfig { prefer_no_std: false, prefer_prelude, prefer_absolute },
ImportPathConfig { prefer_no_std, prefer_prelude, prefer_absolute },
);
format_to!(
res,
@ -732,15 +725,19 @@ mod tests {
}
fn check_found_path(ra_fixture: &str, path: &str, expect: Expect) {
check_found_path_(ra_fixture, path, false, false, expect);
check_found_path_(ra_fixture, path, false, false, false, expect);
}
fn check_found_path_prelude(ra_fixture: &str, path: &str, expect: Expect) {
check_found_path_(ra_fixture, path, true, false, expect);
check_found_path_(ra_fixture, path, true, false, false, expect);
}
fn check_found_path_absolute(ra_fixture: &str, path: &str, expect: Expect) {
check_found_path_(ra_fixture, path, false, true, expect);
check_found_path_(ra_fixture, path, false, true, false, expect);
}
fn check_found_path_prefer_no_std(ra_fixture: &str, path: &str, expect: Expect) {
check_found_path_(ra_fixture, path, false, false, true, expect);
}
#[test]
@ -1361,9 +1358,66 @@ pub mod sync {
"#]],
);
}
#[test]
fn prefer_core_paths_over_std_for_mod_reexport() {
check_found_path_prefer_no_std(
r#"
//- /main.rs crate:main deps:core,std
$0
//- /stdlib.rs crate:std deps:core
pub use core::pin;
//- /corelib.rs crate:core
pub mod pin {
pub struct Pin;
}
"#,
"std::pin::Pin",
expect![[r#"
Plain (imports ✔): core::pin::Pin
Plain (imports ✖): core::pin::Pin
ByCrate(imports ✔): core::pin::Pin
ByCrate(imports ✖): core::pin::Pin
BySelf (imports ✔): core::pin::Pin
BySelf (imports ✖): core::pin::Pin
"#]],
);
}
#[test]
fn prefer_core_paths_over_std() {
check_found_path_prefer_no_std(
r#"
//- /main.rs crate:main deps:core,std
$0
//- /std.rs crate:std deps:core
pub mod fmt {
pub use core::fmt::Error;
}
//- /zzz.rs crate:core
pub mod fmt {
pub struct Error;
}
"#,
"core::fmt::Error",
expect![[r#"
Plain (imports ✔): core::fmt::Error
Plain (imports ✖): core::fmt::Error
ByCrate(imports ✔): core::fmt::Error
ByCrate(imports ✖): core::fmt::Error
BySelf (imports ✔): core::fmt::Error
BySelf (imports ✖): core::fmt::Error
"#]],
);
check_found_path(
r#"
//- /main.rs crate:main deps:core,std
@ -1878,10 +1932,9 @@ pub mod ops {
#[test]
fn respect_unstable_modules() {
check_found_path(
check_found_path_prefer_no_std(
r#"
//- /main.rs crate:main deps:std,core
#![no_std]
extern crate std;
$0
//- /longer.rs crate:std deps:core

2
crates/hir-def/src/lib.rs
View file

@ -105,7 +105,7 @@ use crate::{
type FxIndexMap<K, V> =
indexmap::IndexMap<K, V, std::hash::BuildHasherDefault<rustc_hash::FxHasher>>;
/// A wrapper around two booleans, [`ImportPathConfig::prefer_no_std`] and [`ImportPathConfig::prefer_prelude`].
/// A wrapper around three booleans
#[derive(Debug, Clone, PartialEq, Eq, Hash, Copy)]
pub struct ImportPathConfig {
/// If true, prefer to unconditionally use imports of the `core` and `alloc` crate

1
crates/hir-ty/src/display.rs
View file

@ -1069,6 +1069,7 @@ impl HirDisplay for Ty {
module_id,
PrefixKind::Plain,
false,
// FIXME: no_std Cfg?
ImportPathConfig {
prefer_no_std: false,
prefer_prelude: true,

8
crates/ide-completion/src/completions.rs
View file

@ -24,7 +24,7 @@ pub(crate) mod vis;
use std::iter;
use hir::{sym, HasAttrs, ImportPathConfig, Name, ScopeDef, Variant};
use hir::{sym, HasAttrs, Name, ScopeDef, Variant};
use ide_db::{imports::import_assets::LocatedImport, RootDatabase, SymbolKind};
use syntax::{ast, SmolStr, ToSmolStr};
@ -645,11 +645,7 @@ fn enum_variants_with_paths(
if let Some(path) = ctx.module.find_path(
ctx.db,
hir::ModuleDef::from(variant),
ImportPathConfig {
prefer_no_std: ctx.config.prefer_no_std,
prefer_prelude: ctx.config.prefer_prelude,
prefer_absolute: ctx.config.prefer_absolute,
},
ctx.config.import_path_config(),
) {
// Variants with trivial paths are already added by the existing completion logic,
// so we should avoid adding these twice

14
crates/ide-completion/src/completions/expr.rs
View file

@ -1,6 +1,6 @@
//! Completion of names from the current scope in expression position.
use hir::{sym, ImportPathConfig, Name, ScopeDef};
use hir::{sym, Name, ScopeDef};
use syntax::ast;
use crate::{
@ -174,11 +174,7 @@ pub(crate) fn complete_expr_path(
.find_path(
ctx.db,
hir::ModuleDef::from(strukt),
ImportPathConfig {
prefer_no_std: ctx.config.prefer_no_std,
prefer_prelude: ctx.config.prefer_prelude,
prefer_absolute: ctx.config.prefer_absolute,
},
ctx.config.import_path_config(),
)
.filter(|it| it.len() > 1);
@ -200,11 +196,7 @@ pub(crate) fn complete_expr_path(
.find_path(
ctx.db,
hir::ModuleDef::from(un),
ImportPathConfig {
prefer_no_std: ctx.config.prefer_no_std,
prefer_prelude: ctx.config.prefer_prelude,
prefer_absolute: ctx.config.prefer_absolute,
},
ctx.config.import_path_config(),
)
.filter(|it| it.len() > 1);

21
crates/ide-completion/src/completions/flyimport.rs
View file

@ -1,5 +1,5 @@
//! See [`import_on_the_fly`].
use hir::{ImportPathConfig, ItemInNs, ModuleDef};
use hir::{ItemInNs, ModuleDef};
use ide_db::imports::{
import_assets::{ImportAssets, LocatedImport},
insert_use::ImportScope,
@ -256,11 +256,7 @@ fn import_on_the_fly(
};
let user_input_lowercased = potential_import_name.to_lowercase();
let import_cfg = ImportPathConfig {
prefer_no_std: ctx.config.prefer_no_std,
prefer_prelude: ctx.config.prefer_prelude,
prefer_absolute: ctx.config.prefer_absolute,
};
let import_cfg = ctx.config.import_path_config();
import_assets
.search_for_imports(&ctx.sema, import_cfg, ctx.config.insert_use.prefix_kind)
@ -306,12 +302,7 @@ fn import_on_the_fly_pat_(
ItemInNs::Values(def) => matches!(def, hir::ModuleDef::Const(_)),
};
let user_input_lowercased = potential_import_name.to_lowercase();
let cfg = ImportPathConfig {
prefer_no_std: ctx.config.prefer_no_std,
prefer_prelude: ctx.config.prefer_prelude,
prefer_absolute: ctx.config.prefer_absolute,
};
let cfg = ctx.config.import_path_config();
import_assets
.search_for_imports(&ctx.sema, cfg, ctx.config.insert_use.prefix_kind)
@ -353,11 +344,7 @@ fn import_on_the_fly_method(
let user_input_lowercased = potential_import_name.to_lowercase();
let cfg = ImportPathConfig {
prefer_no_std: ctx.config.prefer_no_std,
prefer_prelude: ctx.config.prefer_prelude,
prefer_absolute: ctx.config.prefer_absolute,
};
let cfg = ctx.config.import_path_config();
import_assets
.search_for_imports(&ctx.sema, cfg, ctx.config.insert_use.prefix_kind)

8
crates/ide-completion/src/completions/postfix.rs
View file

@ -2,7 +2,7 @@
mod format_like;
use hir::{ImportPathConfig, ItemInNs};
use hir::ItemInNs;
use ide_db::{
documentation::{Documentation, HasDocs},
imports::insert_use::ImportScope,
@ -60,11 +60,7 @@ pub(crate) fn complete_postfix(
None => return,
};
let cfg = ImportPathConfig {
prefer_no_std: ctx.config.prefer_no_std,
prefer_prelude: ctx.config.prefer_prelude,
prefer_absolute: ctx.config.prefer_absolute,
};
let cfg = ctx.config.import_path_config();
if let Some(drop_trait) = ctx.famous_defs().core_ops_Drop() {
if receiver_ty.impls_trait(ctx.db, drop_trait, &[]) {

9
crates/ide-completion/src/config.rs
View file

@ -4,6 +4,7 @@
//! module, and we use to statically check that we only produce snippet
//! completions if we are allowed to.
use hir::ImportPathConfig;
use ide_db::{imports::insert_use::InsertUseConfig, SnippetCap};
use crate::snippet::Snippet;
@ -45,4 +46,12 @@ impl CompletionConfig {
.iter()
.flat_map(|snip| snip.prefix_triggers.iter().map(move |trigger| (&**trigger, snip)))
}
pub fn import_path_config(&self) -> ImportPathConfig {
ImportPathConfig {
prefer_no_std: self.prefer_no_std,
prefer_prelude: self.prefer_prelude,
prefer_absolute: self.prefer_absolute,
}
}
}

7
crates/ide-completion/src/lib.rs
View file

@ -10,7 +10,6 @@ mod snippet;
#[cfg(test)]
mod tests;
use hir::ImportPathConfig;
use ide_db::{
helpers::mod_path_to_ast,
imports::{
@ -249,11 +248,7 @@ pub fn resolve_completion_edits(
let new_ast = scope.clone_for_update();
let mut import_insert = TextEdit::builder();
let cfg = ImportPathConfig {
prefer_no_std: config.prefer_no_std,
prefer_prelude: config.prefer_prelude,
prefer_absolute: config.prefer_absolute,
};
let cfg = config.import_path_config();
imports.into_iter().for_each(|(full_import_path, imported_name)| {
let items_with_name = items_locator::items_with_name(

8
crates/ide-completion/src/render.rs
View file

@ -10,7 +10,7 @@ pub(crate) mod type_alias;
pub(crate) mod union_literal;
pub(crate) mod variant;
use hir::{sym, AsAssocItem, HasAttrs, HirDisplay, ImportPathConfig, ModuleDef, ScopeDef, Type};
use hir::{sym, AsAssocItem, HasAttrs, HirDisplay, ModuleDef, ScopeDef, Type};
use ide_db::{
documentation::{Documentation, HasDocs},
helpers::item_name,
@ -294,11 +294,7 @@ pub(crate) fn render_expr(
.unwrap_or_else(|| String::from("..."))
};
let cfg = ImportPathConfig {
prefer_no_std: ctx.config.prefer_no_std,
prefer_prelude: ctx.config.prefer_prelude,
prefer_absolute: ctx.config.prefer_absolute,
};
let cfg = ctx.config.import_path_config();
let label = expr.gen_source_code(&ctx.scope, &mut label_formatter, cfg).ok()?;

7
crates/ide-completion/src/snippet.rs
View file

@ -100,7 +100,6 @@
// }
// ----
use hir::ImportPathConfig;
use ide_db::imports::import_assets::LocatedImport;
use itertools::Itertools;
use syntax::{ast, AstNode, GreenNode, SyntaxNode};
@ -169,11 +168,7 @@ impl Snippet {
}
fn import_edits(ctx: &CompletionContext<'_>, requires: &[GreenNode]) -> Option<Vec<LocatedImport>> {
let import_cfg = ImportPathConfig {
prefer_no_std: ctx.config.prefer_no_std,
prefer_prelude: ctx.config.prefer_prelude,
prefer_absolute: ctx.config.prefer_absolute,
};
let import_cfg = ctx.config.import_path_config();
let resolve = |import: &GreenNode| {
let path = ast::Path::cast(SyntaxNode::new_root(import.clone()))?;