Merge #7889

7889: Make group imports configurable r=lnicola a=asv1



Co-authored-by: asv <[email protected]>

12 files changed, 72 insertions, 23 deletions

diff --git a/crates/ide_assists/src/handlers/auto_import.rs b/crates/ide_assists/src/handlers/auto_import.rs
index dc38f90e9..1422224ac 100644
--- a/crates/ide_assists/src/handlers/auto_import.rs
+++ b/crates/ide_assists/src/handlers/auto_import.rs
@@ -99,8 +99,7 @@ pub(crate) fn auto_import(acc: &mut Assists, ctx: &AssistContext) -> Option<()>
             format!("Import `{}`", &import),
             range,
             |builder| {
-                let rewriter =
-                    insert_use(&scope, mod_path_to_ast(&import), ctx.config.insert_use.merge);
+                let rewriter = insert_use(&scope, mod_path_to_ast(&import), ctx.config.insert_use);
                 builder.rewrite(rewriter);
             },
         );
diff --git a/crates/ide_assists/src/handlers/extract_struct_from_enum_variant.rs b/crates/ide_assists/src/handlers/extract_struct_from_enum_variant.rs
index 5c7678b53..4f0422e96 100644
--- a/crates/ide_assists/src/handlers/extract_struct_from_enum_variant.rs
+++ b/crates/ide_assists/src/handlers/extract_struct_from_enum_variant.rs
@@ -154,7 +154,7 @@ fn insert_import(
         mod_path.pop_segment();
         mod_path.push_segment(variant_hir_name.clone());
         let scope = ImportScope::find_insert_use_container(scope_node, &ctx.sema)?;
-        *rewriter += insert_use(&scope, mod_path_to_ast(&mod_path), ctx.config.insert_use.merge);
+        *rewriter += insert_use(&scope, mod_path_to_ast(&mod_path), ctx.config.insert_use);
     }
     Some(())
 }
diff --git a/crates/ide_assists/src/handlers/replace_qualified_name_with_use.rs b/crates/ide_assists/src/handlers/replace_qualified_name_with_use.rs
index f3bc6cf39..55481af34 100644
--- a/crates/ide_assists/src/handlers/replace_qualified_name_with_use.rs
+++ b/crates/ide_assists/src/handlers/replace_qualified_name_with_use.rs
@@ -44,7 +44,7 @@ pub(crate) fn replace_qualified_name_with_use(
             let mut rewriter = SyntaxRewriter::default();
             shorten_paths(&mut rewriter, syntax.clone(), &path);
             if let Some(ref import_scope) = ImportScope::from(syntax.clone()) {
-                rewriter += insert_use(import_scope, path, ctx.config.insert_use.merge);
+                rewriter += insert_use(import_scope, path, ctx.config.insert_use);
                 builder.rewrite(rewriter);
             }
         },
diff --git a/crates/ide_assists/src/tests.rs b/crates/ide_assists/src/tests.rs
index b7f616760..a7a923beb 100644
--- a/crates/ide_assists/src/tests.rs
+++ b/crates/ide_assists/src/tests.rs
@@ -23,6 +23,7 @@ pub(crate) const TEST_CONFIG: AssistConfig = AssistConfig {
     insert_use: InsertUseConfig {
         merge: Some(MergeBehavior::Full),
         prefix_kind: hir::PrefixKind::Plain,
+        group: true,
     },
 };
 
diff --git a/crates/ide_completion/src/item.rs b/crates/ide_completion/src/item.rs
index 884711f11..9b2435c4b 100644
--- a/crates/ide_completion/src/item.rs
+++ b/crates/ide_completion/src/item.rs
@@ -5,7 +5,7 @@ use std::fmt;
 use hir::{Documentation, ModPath, Mutability};
 use ide_db::{
     helpers::{
-        insert_use::{self, ImportScope, MergeBehavior},
+        insert_use::{self, ImportScope, InsertUseConfig},
         mod_path_to_ast, SnippetCap,
     },
     SymbolKind,
@@ -280,14 +280,11 @@ pub struct ImportEdit {
 impl ImportEdit {
     /// Attempts to insert the import to the given scope, producing a text edit.
     /// May return no edit in edge cases, such as scope already containing the import.
-    pub fn to_text_edit(&self, merge_behavior: Option<MergeBehavior>) -> Option<TextEdit> {
+    pub fn to_text_edit(&self, cfg: InsertUseConfig) -> Option<TextEdit> {
         let _p = profile::span("ImportEdit::to_text_edit");
 
-        let rewriter = insert_use::insert_use(
-            &self.import_scope,
-            mod_path_to_ast(&self.import_path),
-            merge_behavior,
-        );
+        let rewriter =
+            insert_use::insert_use(&self.import_scope, mod_path_to_ast(&self.import_path), cfg);
         let old_ast = rewriter.rewrite_root()?;
         let mut import_insert = TextEdit::builder();
         algo::diff(&old_ast, &rewriter.rewrite(&old_ast)).into_text_edit(&mut import_insert);
diff --git a/crates/ide_completion/src/lib.rs b/crates/ide_completion/src/lib.rs
index 76f31de9e..b0b809791 100644
--- a/crates/ide_completion/src/lib.rs
+++ b/crates/ide_completion/src/lib.rs
@@ -156,7 +156,7 @@ pub fn resolve_completion_edits(
         .find(|mod_path| mod_path.to_string() == full_import_path)?;
 
     ImportEdit { import_path, import_scope, import_for_trait_assoc_item }
-        .to_text_edit(config.insert_use.merge)
+        .to_text_edit(config.insert_use)
         .map(|edit| vec![edit])
 }
 
diff --git a/crates/ide_completion/src/test_utils.rs b/crates/ide_completion/src/test_utils.rs
index baff83305..9da844031 100644
--- a/crates/ide_completion/src/test_utils.rs
+++ b/crates/ide_completion/src/test_utils.rs
@@ -25,6 +25,7 @@ pub(crate) const TEST_CONFIG: CompletionConfig = CompletionConfig {
     insert_use: InsertUseConfig {
         merge: Some(MergeBehavior::Full),
         prefix_kind: PrefixKind::Plain,
+        group: true,
     },
 };
 
@@ -119,7 +120,7 @@ pub(crate) fn check_edit_with_config(
 
     let mut combined_edit = completion.text_edit().to_owned();
     if let Some(import_text_edit) =
-        completion.import_to_add().and_then(|edit| edit.to_text_edit(config.insert_use.merge))
+        completion.import_to_add().and_then(|edit| edit.to_text_edit(config.insert_use))
     {
         combined_edit.union(import_text_edit).expect(
             "Failed to apply completion resolve changes: change ranges overlap, but should not",
diff --git a/crates/ide_db/src/helpers/insert_use.rs b/crates/ide_db/src/helpers/insert_use.rs
index fd4035198..f52aee344 100644
--- a/crates/ide_db/src/helpers/insert_use.rs
+++ b/crates/ide_db/src/helpers/insert_use.rs
@@ -19,6 +19,7 @@ use test_utils::mark;
 pub struct InsertUseConfig {
     pub merge: Option<MergeBehavior>,
     pub prefix_kind: hir::PrefixKind,
+    pub group: bool,
 }
 
 #[derive(Debug, Clone)]
@@ -99,13 +100,13 @@ fn is_inner_comment(token: SyntaxToken) -> bool {
 pub fn insert_use<'a>(
     scope: &ImportScope,
     path: ast::Path,
-    merge: Option<MergeBehavior>,
+    cfg: InsertUseConfig,
 ) -> SyntaxRewriter<'a> {
     let _p = profile::span("insert_use");
     let mut rewriter = SyntaxRewriter::default();
     let use_item = make::use_(None, make::use_tree(path.clone(), None, None, false));
     // merge into existing imports if possible
-    if let Some(mb) = merge {
+    if let Some(mb) = cfg.merge {
         for existing_use in scope.as_syntax_node().children().filter_map(ast::Use::cast) {
             if let Some(merged) = try_merge_imports(&existing_use, &use_item, mb) {
                 rewriter.replace(existing_use.syntax(), merged.syntax());
@@ -116,7 +117,7 @@ pub fn insert_use<'a>(
 
     // either we weren't allowed to merge or there is no import that fits the merge conditions
     // so look for the place we have to insert to
-    let (insert_position, add_blank) = find_insert_position(scope, path);
+    let (insert_position, add_blank) = find_insert_position(scope, path, cfg.group);
 
     let indent = if let ident_level @ 1..=usize::MAX = scope.indent_level().0 as usize {
         Some(make::tokens::whitespace(&" ".repeat(4 * ident_level)).into())
@@ -538,6 +539,7 @@ impl AddBlankLine {
 fn find_insert_position(
     scope: &ImportScope,
     insert_path: ast::Path,
+    group_imports: bool,
 ) -> (InsertPosition<SyntaxElement>, AddBlankLine) {
     let group = ImportGroup::new(&insert_path);
     let path_node_iter = scope
@@ -550,6 +552,14 @@ fn find_insert_position(
             let has_tl = tree.use_tree_list().is_some();
             Some((path, has_tl, node))
         });
+
+    if !group_imports {
+        if let Some((_, _, node)) = path_node_iter.last() {
+            return (InsertPosition::After(node.into()), AddBlankLine::Before);
+        }
+        return (InsertPosition::First, AddBlankLine::AfterTwice);
+    }
+
     // Iterator that discards anything thats not in the required grouping
     // This implementation allows the user to rearrange their import groups as this only takes the first group that fits
     let group_iter = path_node_iter
@@ -565,6 +575,7 @@ fn find_insert_position(
             use_tree_path_cmp(&insert_path, false, path, has_tl) != Ordering::Greater
         },
     );
+
     match post_insert {
         // insert our import before that element
         Some((.., node)) => (InsertPosition::Before(node.into()), AddBlankLine::After),
diff --git a/crates/ide_db/src/helpers/insert_use/tests.rs b/crates/ide_db/src/helpers/insert_use/tests.rs
index 4bbe66f1f..67d0d6fb6 100644
--- a/crates/ide_db/src/helpers/insert_use/tests.rs
+++ b/crates/ide_db/src/helpers/insert_use/tests.rs
@@ -1,8 +1,32 @@
 use super::*;
 
+use hir::PrefixKind;
 use test_utils::assert_eq_text;
 
 #[test]
+fn insert_not_group() {
+    check(
+        "use external_crate2::bar::A",
+        r"
+use std::bar::B;
+use external_crate::bar::A;
+use crate::bar::A;
+use self::bar::A;
+use super::bar::A;",
+        r"
+use std::bar::B;
+use external_crate::bar::A;
+use crate::bar::A;
+use self::bar::A;
+use super::bar::A;
+use external_crate2::bar::A;",
+        None,
+        false,
+        false,
+    );
+}
+
+#[test]
 fn insert_existing() {
     check_full("std::fs", "use std::fs;", "use std::fs;")
 }
@@ -240,6 +264,7 @@ fn insert_empty_module() {
 }",
         None,
         true,
+        true,
     )
 }
 
@@ -584,6 +609,7 @@ fn check(
     ra_fixture_after: &str,
     mb: Option<MergeBehavior>,
     module: bool,
+    group: bool,
 ) {
     let mut syntax = ast::SourceFile::parse(ra_fixture_before).tree().syntax().clone();
     if module {
@@ -597,21 +623,25 @@ fn check(
         .find_map(ast::Path::cast)
         .unwrap();
 
-    let rewriter = insert_use(&file, path, mb);
+    let rewriter = insert_use(
+        &file,
+        path,
+        InsertUseConfig { merge: mb, prefix_kind: PrefixKind::Plain, group },
+    );
     let result = rewriter.rewrite(file.as_syntax_node()).to_string();
     assert_eq_text!(ra_fixture_after, &result);
 }
 
 fn check_full(path: &str, ra_fixture_before: &str, ra_fixture_after: &str) {
-    check(path, ra_fixture_before, ra_fixture_after, Some(MergeBehavior::Full), false)
+    check(path, ra_fixture_before, ra_fixture_after, Some(MergeBehavior::Full), false, true)
 }
 
 fn check_last(path: &str, ra_fixture_before: &str, ra_fixture_after: &str) {
-    check(path, ra_fixture_before, ra_fixture_after, Some(MergeBehavior::Last), false)
+    check(path, ra_fixture_before, ra_fixture_after, Some(MergeBehavior::Last), false, true)
 }
 
 fn check_none(path: &str, ra_fixture_before: &str, ra_fixture_after: &str) {
-    check(path, ra_fixture_before, ra_fixture_after, None, false)
+    check(path, ra_fixture_before, ra_fixture_after, None, false, true)
 }
 
 fn check_merge_only_fail(ra_fixture0: &str, ra_fixture1: &str, mb: MergeBehavior) {
diff --git a/crates/rust-analyzer/src/cli/analysis_bench.rs b/crates/rust-analyzer/src/cli/analysis_bench.rs
index 3bd7e678d..49994824f 100644
--- a/crates/rust-analyzer/src/cli/analysis_bench.rs
+++ b/crates/rust-analyzer/src/cli/analysis_bench.rs
@@ -108,7 +108,11 @@ impl BenchCmd {
                         add_call_parenthesis: true,
                         add_call_argument_snippets: true,
                         snippet_cap: SnippetCap::new(true),
-                        insert_use: InsertUseConfig { merge: None, prefix_kind: PrefixKind::Plain },
+                        insert_use: InsertUseConfig {
+                            merge: None,
+                            prefix_kind: PrefixKind::Plain,
+                            group: true,
+                        },
                     };
                     let res = do_work(&mut host, file_id, |analysis| {
                         analysis.completions(&options, file_position)
diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs
index 367136702..cac48e911 100644
--- a/crates/rust-analyzer/src/config.rs
+++ b/crates/rust-analyzer/src/config.rs
@@ -35,7 +35,8 @@ config_data! {
         assist_importMergeBehaviour: MergeBehaviorDef  = "\"full\"",
         /// The path structure for newly inserted paths to use.
         assist_importPrefix: ImportPrefixDef           = "\"plain\"",
-
+        /// Group inserted imports by the [following order](https://rust-analyzer.github.io/manual.html#auto-import). Groups are separated by newlines.
+        assist_importGroup: bool                       = "true",
         /// Show function name and docs in parameter hints.
         callInfo_full: bool = "true",
 
@@ -574,6 +575,7 @@ impl Config {
                 ImportPrefixDef::ByCrate => PrefixKind::ByCrate,
                 ImportPrefixDef::BySelf => PrefixKind::BySelf,
             },
+            group: self.data.assist_importGroup,
         }
     }
     pub fn completion(&self) -> CompletionConfig {
diff --git a/crates/rust-analyzer/src/to_proto.rs b/crates/rust-analyzer/src/to_proto.rs
index c1ca88df6..4235eb6dd 100644
--- a/crates/rust-analyzer/src/to_proto.rs
+++ b/crates/rust-analyzer/src/to_proto.rs
@@ -1087,7 +1087,11 @@ mod tests {
                     add_call_parenthesis: true,
                     add_call_argument_snippets: true,
                     snippet_cap: SnippetCap::new(true),
-                    insert_use: InsertUseConfig { merge: None, prefix_kind: PrefixKind::Plain },
+                    insert_use: InsertUseConfig {
+                        merge: None,
+                        prefix_kind: PrefixKind::Plain,
+                        group: true,
+                    },
                 },
                 ide_db::base_db::FilePosition { file_id, offset },
             )