Merge #2982

2982: Merge imports when auto importing r=flodiebold a=SomeoneToIgnore



Co-authored-by: Kirill Bulatov <mail4score@gmail.com>

Author: bors[bot]

crates/ra_assists/src/assists/add_import.rs

@@ -1,4 +1,4 @@
-use hir::{self, db::HirDatabase};
+use hir::{self, db::HirDatabase, ModPath};
 use ra_syntax::{
     ast::{self, NameOwner},
     AstNode, Direction, SmolStr,
@@ -20,10 +20,10 @@ pub fn auto_import_text_edit(
     position: &SyntaxNode,
     // The statement to use as anchor (last resort)
     anchor: &SyntaxNode,
-    // The path to import as a sequence of strings
-    target: &[SmolStr],
+    path_to_import: &ModPath,
     edit: &mut TextEditBuilder,
 ) {
+    let target = path_to_import.to_string().split("::").map(SmolStr::new).collect::<Vec<_>>();
     let container = position.ancestors().find_map(|n| {
         if let Some(module) = ast::Module::cast(n.clone()) {
             return module.item_list().map(|it| it.syntax().clone());
@@ -32,8 +32,8 @@ pub fn auto_import_text_edit(
     });
 
     if let Some(container) = container {
-        let action = best_action_for_target(container, anchor.clone(), target);
-        make_assist(&action, target, edit);
+        let action = best_action_for_target(container, anchor.clone(), &target);
+        make_assist(&action, &target, edit);
     }
 }
 

crates/ra_assists/src/assists/auto_import.rs

@@ -1,7 +1,6 @@
-use hir::db::HirDatabase;
+use hir::{db::HirDatabase, ModPath};
 use ra_syntax::{
     ast::{self, AstNode},
-    SmolStr,
     SyntaxKind::USE_ITEM,
     SyntaxNode,
 };
@@ -58,7 +57,6 @@ pub(crate) fn auto_import<F: ImportsLocator>(
         .filter_map(|module_def| module_with_name_to_import.find_use_path(ctx.db, module_def))
         .filter(|use_path| !use_path.segments.is_empty())
         .take(20)
-        .map(|import| import.to_string())
         .collect::<std::collections::BTreeSet<_>>();
     if proposed_imports.is_empty() {
         return None;
@@ -76,15 +74,10 @@ pub(crate) fn auto_import<F: ImportsLocator>(
     )
 }
 
-fn import_to_action(import: String, position: &SyntaxNode, anchor: &SyntaxNode) -> ActionBuilder {
+fn import_to_action(import: ModPath, position: &SyntaxNode, anchor: &SyntaxNode) -> ActionBuilder {
     let mut action_builder = ActionBuilder::default();
     action_builder.label(format!("Import `{}`", &import));
-    auto_import_text_edit(
-        position,
-        anchor,
-        &[SmolStr::new(import)],
-        action_builder.text_edit_builder(),
-    );
+    auto_import_text_edit(position, anchor, &import, action_builder.text_edit_builder());
     action_builder
 }
 
@@ -120,6 +113,34 @@ mod tests {
         );
     }
 
+    #[test]
+    fn auto_imports_are_merged() {
+        check_assist_with_imports_locator(
+            auto_import,
+            TestImportsLocator::new,
+            r"
+            use PubMod::PubStruct1;
+
+            PubStruct2<|>
+
+            pub mod PubMod {
+                pub struct PubStruct1;
+                pub struct PubStruct2;
+            }
+            ",
+            r"
+            use PubMod::{PubStruct2, PubStruct1};
+
+            PubStruct2<|>
+
+            pub mod PubMod {
+                pub struct PubStruct1;
+                pub struct PubStruct2;
+            }
+            ",
+        );
+    }
+
     #[test]
     fn applicable_when_found_multiple_imports() {
         check_assist_with_imports_locator(

crates/ra_hir/src/lib.rs

@@ -59,6 +59,7 @@ pub use hir_def::{
     ModuleDefId, // FIXME this is exposed and should be used for implementing the `TestImportsLocator` in `ra_assists` only, should be removed later along with the trait and the implementation.
 };
 pub use hir_expand::{
-    name::Name, HirFileId, InFile, MacroCallId, MacroCallLoc, MacroDefId, MacroFile, Origin,
+    name::{name, Name},
+    HirFileId, InFile, MacroCallId, MacroCallLoc, MacroDefId, MacroFile, Origin,
 };
 pub use hir_ty::{display::HirDisplay, CallableDef};

crates/ra_hir_def/src/path.rs

@@ -16,13 +16,13 @@ use ra_syntax::ast;
 
 use crate::{type_ref::TypeRef, InFile};
 
-#[derive(Debug, Clone, PartialEq, Eq, Hash)]
+#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub struct ModPath {
     pub kind: PathKind,
     pub segments: Vec<Name>,
 }
 
-#[derive(Debug, Clone, PartialEq, Eq, Hash)]
+#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub enum PathKind {
     Plain,
     /// `self::` is `Super(0)`

crates/ra_hir_expand/src/name.rs

@@ -187,6 +187,13 @@ pub mod known {
         PartialOrd,
         Eq,
         PartialEq,
+        // FIXME delete those after `ImportResolver` is removed.
+        hash,
+        fmt,
+        io,
+        Display,
+        Iterator,
+        Hasher,
     );
 
     // self/Self cannot be used as an identifier

crates/ra_ide/src/completion/complete_scope.rs

@@ -6,6 +6,7 @@ use ra_text_edit::TextEditBuilder;
 use rustc_hash::FxHashMap;
 
 use crate::completion::{CompletionContext, CompletionItem, CompletionKind, Completions};
+use hir::{ModPath, PathKind};
 
 pub(super) fn complete_scope(acc: &mut Completions, ctx: &CompletionContext) {
     if !ctx.is_trivial_path {
@@ -54,58 +55,76 @@ pub(super) fn complete_scope(acc: &mut Completions, ctx: &CompletionContext) {
     }
 }
 
-fn build_import_label(name: &str, path: &[SmolStr]) -> String {
+fn build_import_label(name: &str, path: &ModPath) -> String {
     let mut buf = String::with_capacity(64);
     buf.push_str(name);
     buf.push_str(" (");
-    fmt_import_path(path, &mut buf);
+    buf.push_str(&path.to_string());
     buf.push_str(")");
     buf
 }
 
-fn fmt_import_path(path: &[SmolStr], buf: &mut String) {
-    let mut segments = path.iter();
-    if let Some(s) = segments.next() {
-        buf.push_str(&s);
-    }
-    for s in segments {
-        buf.push_str("::");
-        buf.push_str(&s);
-    }
-}
-
 #[derive(Debug, Clone, Default)]
 pub(crate) struct ImportResolver {
     // todo: use fst crate or something like that
-    dummy_names: Vec<(SmolStr, Vec<SmolStr>)>,
+    dummy_names: Vec<(SmolStr, ModPath)>,
 }
 
 impl ImportResolver {
     pub(crate) fn new() -> Self {
+        use hir::name;
+
         let dummy_names = vec![
-            (SmolStr::new("fmt"), vec![SmolStr::new("std"), SmolStr::new("fmt")]),
-            (SmolStr::new("io"), vec![SmolStr::new("std"), SmolStr::new("io")]),
-            (SmolStr::new("iter"), vec![SmolStr::new("std"), SmolStr::new("iter")]),
-            (SmolStr::new("hash"), vec![SmolStr::new("std"), SmolStr::new("hash")]),
+            (
+                SmolStr::new("fmt"),
+                ModPath { kind: PathKind::Plain, segments: vec![name![std], name![fmt]] },
+            ),
+            (
+                SmolStr::new("io"),
+                ModPath { kind: PathKind::Plain, segments: vec![name![std], name![io]] },
+            ),
+            (
+                SmolStr::new("iter"),
+                ModPath { kind: PathKind::Plain, segments: vec![name![std], name![iter]] },
+            ),
+            (
+                SmolStr::new("hash"),
+                ModPath { kind: PathKind::Plain, segments: vec![name![std], name![hash]] },
+            ),
             (
                 SmolStr::new("Debug"),
-                vec![SmolStr::new("std"), SmolStr::new("fmt"), SmolStr::new("Debug")],
+                ModPath {
+                    kind: PathKind::Plain,
+                    segments: vec![name![std], name![fmt], name![Debug]],
+                },
             ),
             (
                 SmolStr::new("Display"),
-                vec![SmolStr::new("std"), SmolStr::new("fmt"), SmolStr::new("Display")],
+                ModPath {
+                    kind: PathKind::Plain,
+                    segments: vec![name![std], name![fmt], name![Display]],
+                },
             ),
             (
                 SmolStr::new("Hash"),
-                vec![SmolStr::new("std"), SmolStr::new("hash"), SmolStr::new("Hash")],
+                ModPath {
+                    kind: PathKind::Plain,
+                    segments: vec![name![std], name![hash], name![Hash]],
+                },
             ),
             (
                 SmolStr::new("Hasher"),
-                vec![SmolStr::new("std"), SmolStr::new("hash"), SmolStr::new("Hasher")],
+                ModPath {
+                    kind: PathKind::Plain,
+                    segments: vec![name![std], name![hash], name![Hasher]],
+                },
             ),
             (
                 SmolStr::new("Iterator"),
-                vec![SmolStr::new("std"), SmolStr::new("iter"), SmolStr::new("Iterator")],
+                ModPath {
+                    kind: PathKind::Plain,
+                    segments: vec![name![std], name![iter], name![Iterator]],
+                },
             ),
         ];
 
@@ -115,7 +134,7 @@ impl ImportResolver {
     // Returns a map of importable items filtered by name.
     // The map associates item name with its full path.
     // todo: should return Resolutions
-    pub(crate) fn all_names(&self, name: &str) -> FxHashMap<SmolStr, Vec<SmolStr>> {
+    pub(crate) fn all_names(&self, name: &str) -> FxHashMap<SmolStr, ModPath> {
         if name.len() > 1 {
             self.dummy_names.iter().filter(|(n, _)| n.contains(name)).cloned().collect()
         } else {