Add config for preferring / ignoring prelude modules in find_path

Author: Veykril

crates/hir-def/src/find_path.rs

@@ -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(
@@ -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)]
@@ -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 {
@@ -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);
     }
@@ -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());
@@ -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)
@@ -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;
@@ -236,6 +243,7 @@ fn find_path_for_module(
         from,
         prefixed,
         prefer_no_std,
+        prefer_prelude,
         scope_name,
     )
 }
@@ -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 {
@@ -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();
@@ -388,6 +400,7 @@ fn calculate_best_path(
                     max_len - 1,
                     prefixed,
                     prefer_no_std,
+                    prefer_prelude,
                 ) else {
                     continue;
                 };
@@ -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);
@@ -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 {
@@ -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),
     }
 }
 
@@ -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};"));
@@ -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:?}");
     }
 
@@ -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]
@@ -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",
+        );
+    }
 }

crates/hir-ty/src/display.rs

@@ -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 {

crates/hir/src/lib.rs

@@ -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
@@ -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,
         )
     }
 }

crates/ide-assists/src/assist_config.rs

@@ -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,
 }

crates/ide-assists/src/handlers/add_missing_match_arms.rs

@@ -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()),
                 ))
             })
@@ -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)
@@ -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)
             })
@@ -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() {