Merge #8794

8794: Give MergeBehaviour variants better names r=Veykril a=Veykril

I never really liked the variant names I gave this enum from the beginning and then I found out about rustfmt's `imports_granularity` config:

> imports_granularity
>
> How imports should be grouped into use statements. Imports will be merged or split to the configured level of granularity.
> 
>     Default value: Preserve
>     Possible values: Preserve, Crate, Module, Item
>     Stable: No

I personally prefer using `crate` over `full` and `module` over last, they seem more descriptive. Keeping these similar between tooling also seems like a good plus point to me.

We might even wanna take over the entire enum at some point if we have a `format/cleanup imports` assists in the future which would probably want to also have the `preserve` and `item` options.

Co-authored-by: Lukas Wirth <[email protected]>

9 files changed, 77 insertions, 67 deletions

diff --git a/crates/ide_assists/src/handlers/merge_imports.rs b/crates/ide_assists/src/handlers/merge_imports.rs
index add7b8e37..3cd090737 100644
--- a/crates/ide_assists/src/handlers/merge_imports.rs
+++ b/crates/ide_assists/src/handlers/merge_imports.rs
@@ -27,14 +27,14 @@ pub(crate) fn merge_imports(acc: &mut Assists, ctx: &AssistContext) -> Option<()
     if let Some(use_item) = tree.syntax().parent().and_then(ast::Use::cast) {
         let (merged, to_remove) =
             next_prev().filter_map(|dir| neighbor(&use_item, dir)).find_map(|use_item2| {
-                try_merge_imports(&use_item, &use_item2, MergeBehavior::Full).zip(Some(use_item2))
+                try_merge_imports(&use_item, &use_item2, MergeBehavior::Crate).zip(Some(use_item2))
             })?;
 
         imports = Some((use_item, merged, to_remove));
     } else {
         let (merged, to_remove) =
             next_prev().filter_map(|dir| neighbor(&tree, dir)).find_map(|use_tree| {
-                try_merge_trees(&tree, &use_tree, MergeBehavior::Full).zip(Some(use_tree))
+                try_merge_trees(&tree, &use_tree, MergeBehavior::Crate).zip(Some(use_tree))
             })?;
 
         uses = Some((tree.clone(), merged, to_remove))
diff --git a/crates/ide_assists/src/tests.rs b/crates/ide_assists/src/tests.rs
index 9c2847998..0d3969c36 100644
--- a/crates/ide_assists/src/tests.rs
+++ b/crates/ide_assists/src/tests.rs
@@ -21,7 +21,7 @@ pub(crate) const TEST_CONFIG: AssistConfig = AssistConfig {
     snippet_cap: SnippetCap::new(true),
     allowed: None,
     insert_use: InsertUseConfig {
-        merge: Some(MergeBehavior::Full),
+        merge: Some(MergeBehavior::Crate),
         prefix_kind: hir::PrefixKind::Plain,
         group: true,
     },
diff --git a/crates/ide_completion/src/test_utils.rs b/crates/ide_completion/src/test_utils.rs
index c9857ec5f..939fb2d47 100644
--- a/crates/ide_completion/src/test_utils.rs
+++ b/crates/ide_completion/src/test_utils.rs
@@ -20,7 +20,7 @@ pub(crate) const TEST_CONFIG: CompletionConfig = CompletionConfig {
     add_call_argument_snippets: true,
     snippet_cap: SnippetCap::new(true),
     insert_use: InsertUseConfig {
-        merge: Some(MergeBehavior::Full),
+        merge: Some(MergeBehavior::Crate),
         prefix_kind: PrefixKind::Plain,
         group: true,
     },
diff --git a/crates/ide_db/src/helpers/insert_use/tests.rs b/crates/ide_db/src/helpers/insert_use/tests.rs
index 048c213e2..248227d29 100644
--- a/crates/ide_db/src/helpers/insert_use/tests.rs
+++ b/crates/ide_db/src/helpers/insert_use/tests.rs
@@ -44,7 +44,7 @@ fn insert_not_group_empty() {
 
 #[test]
 fn insert_existing() {
-    check_full("std::fs", "use std::fs;", "use std::fs;")
+    check_crate("std::fs", "use std::fs;", "use std::fs;")
 }
 
 #[test]
@@ -249,7 +249,7 @@ use self::fmt;",
 
 #[test]
 fn insert_no_imports() {
-    check_full(
+    check_crate(
         "foo::bar",
         "fn main() {}",
         r"use foo::bar;
@@ -263,7 +263,7 @@ fn insert_empty_file() {
     cov_mark::check!(insert_group_empty_file);
     // empty files will get two trailing newlines
     // this is due to the test case insert_no_imports above
-    check_full(
+    check_crate(
         "foo::bar",
         "",
         r"use foo::bar;
@@ -290,7 +290,7 @@ fn insert_empty_module() {
 #[test]
 fn insert_after_inner_attr() {
     cov_mark::check!(insert_group_empty_inner_attr);
-    check_full(
+    check_crate(
         "foo::bar",
         r"#![allow(unused_imports)]",
         r"#![allow(unused_imports)]
@@ -301,7 +301,7 @@ use foo::bar;",
 
 #[test]
 fn insert_after_inner_attr2() {
-    check_full(
+    check_crate(
         "foo::bar",
         r"#![allow(unused_imports)]
 
@@ -371,12 +371,12 @@ fn main() {}"#,
 
 #[test]
 fn merge_groups() {
-    check_last("std::io", r"use std::fmt;", r"use std::{fmt, io};")
+    check_module("std::io", r"use std::fmt;", r"use std::{fmt, io};")
 }
 
 #[test]
 fn merge_groups_last() {
-    check_last(
+    check_module(
         "std::io",
         r"use std::fmt::{Result, Display};",
         r"use std::fmt::{Result, Display};
@@ -386,12 +386,12 @@ use std::io;",
 
 #[test]
 fn merge_last_into_self() {
-    check_last("foo::bar::baz", r"use foo::bar;", r"use foo::bar::{self, baz};");
+    check_module("foo::bar::baz", r"use foo::bar;", r"use foo::bar::{self, baz};");
 }
 
 #[test]
 fn merge_groups_full() {
-    check_full(
+    check_crate(
         "std::io",
         r"use std::fmt::{Result, Display};",
         r"use std::{fmt::{Result, Display}, io};",
@@ -400,17 +400,21 @@ fn merge_groups_full() {
 
 #[test]
 fn merge_groups_long_full() {
-    check_full("std::foo::bar::Baz", r"use std::foo::bar::Qux;", r"use std::foo::bar::{Baz, Qux};")
+    check_crate("std::foo::bar::Baz", r"use std::foo::bar::Qux;", r"use std::foo::bar::{Baz, Qux};")
 }
 
 #[test]
 fn merge_groups_long_last() {
-    check_last("std::foo::bar::Baz", r"use std::foo::bar::Qux;", r"use std::foo::bar::{Baz, Qux};")
+    check_module(
+        "std::foo::bar::Baz",
+        r"use std::foo::bar::Qux;",
+        r"use std::foo::bar::{Baz, Qux};",
+    )
 }
 
 #[test]
 fn merge_groups_long_full_list() {
-    check_full(
+    check_crate(
         "std::foo::bar::Baz",
         r"use std::foo::bar::{Qux, Quux};",
         r"use std::foo::bar::{Baz, Quux, Qux};",
@@ -419,7 +423,7 @@ fn merge_groups_long_full_list() {
 
 #[test]
 fn merge_groups_long_last_list() {
-    check_last(
+    check_module(
         "std::foo::bar::Baz",
         r"use std::foo::bar::{Qux, Quux};",
         r"use std::foo::bar::{Baz, Quux, Qux};",
@@ -428,7 +432,7 @@ fn merge_groups_long_last_list() {
 
 #[test]
 fn merge_groups_long_full_nested() {
-    check_full(
+    check_crate(
         "std::foo::bar::Baz",
         r"use std::foo::bar::{Qux, quux::{Fez, Fizz}};",
         r"use std::foo::bar::{Baz, Qux, quux::{Fez, Fizz}};",
@@ -437,7 +441,7 @@ fn merge_groups_long_full_nested() {
 
 #[test]
 fn merge_groups_long_last_nested() {
-    check_last(
+    check_module(
         "std::foo::bar::Baz",
         r"use std::foo::bar::{Qux, quux::{Fez, Fizz}};",
         r"use std::foo::bar::Baz;
@@ -447,7 +451,7 @@ use std::foo::bar::{Qux, quux::{Fez, Fizz}};",
 
 #[test]
 fn merge_groups_full_nested_deep() {
-    check_full(
+    check_crate(
         "std::foo::bar::quux::Baz",
         r"use std::foo::bar::{Qux, quux::{Fez, Fizz}};",
         r"use std::foo::bar::{Qux, quux::{Baz, Fez, Fizz}};",
@@ -456,7 +460,7 @@ fn merge_groups_full_nested_deep() {
 
 #[test]
 fn merge_groups_full_nested_long() {
-    check_full(
+    check_crate(
         "std::foo::bar::Baz",
         r"use std::{foo::bar::Qux};",
         r"use std::{foo::bar::{Baz, Qux}};",
@@ -465,7 +469,7 @@ fn merge_groups_full_nested_long() {
 
 #[test]
 fn merge_groups_last_nested_long() {
-    check_full(
+    check_crate(
         "std::foo::bar::Baz",
         r"use std::{foo::bar::Qux};",
         r"use std::{foo::bar::{Baz, Qux}};",
@@ -474,7 +478,7 @@ fn merge_groups_last_nested_long() {
 
 #[test]
 fn merge_groups_skip_pub() {
-    check_full(
+    check_crate(
         "std::io",
         r"pub use std::fmt::{Result, Display};",
         r"pub use std::fmt::{Result, Display};
@@ -484,7 +488,7 @@ use std::io;",
 
 #[test]
 fn merge_groups_skip_pub_crate() {
-    check_full(
+    check_crate(
         "std::io",
         r"pub(crate) use std::fmt::{Result, Display};",
         r"pub(crate) use std::fmt::{Result, Display};
@@ -494,7 +498,7 @@ use std::io;",
 
 #[test]
 fn merge_groups_skip_attributed() {
-    check_full(
+    check_crate(
         "std::io",
         r#"
 #[cfg(feature = "gated")] use std::fmt::{Result, Display};
@@ -509,7 +513,7 @@ use std::io;
 #[test]
 #[ignore] // FIXME: Support this
 fn split_out_merge() {
-    check_last(
+    check_module(
         "std::fmt::Result",
         r"use std::{fmt, io};",
         r"use std::fmt::{self, Result};
@@ -519,29 +523,33 @@ use std::io;",
 
 #[test]
 fn merge_into_module_import() {
-    check_full("std::fmt::Result", r"use std::{fmt, io};", r"use std::{fmt::{self, Result}, io};")
+    check_crate("std::fmt::Result", r"use std::{fmt, io};", r"use std::{fmt::{self, Result}, io};")
 }
 
 #[test]
 fn merge_groups_self() {
-    check_full("std::fmt::Debug", r"use std::fmt;", r"use std::fmt::{self, Debug};")
+    check_crate("std::fmt::Debug", r"use std::fmt;", r"use std::fmt::{self, Debug};")
 }
 
 #[test]
 fn merge_mod_into_glob() {
-    check_full("token::TokenKind", r"use token::TokenKind::*;", r"use token::TokenKind::{*, self};")
+    check_crate(
+        "token::TokenKind",
+        r"use token::TokenKind::*;",
+        r"use token::TokenKind::{*, self};",
+    )
     // FIXME: have it emit `use token::TokenKind::{self, *}`?
 }
 
 #[test]
 fn merge_self_glob() {
-    check_full("self", r"use self::*;", r"use self::{*, self};")
+    check_crate("self", r"use self::*;", r"use self::{*, self};")
     // FIXME: have it emit `use {self, *}`?
 }
 
 #[test]
 fn merge_glob_nested() {
-    check_full(
+    check_crate(
         "foo::bar::quux::Fez",
         r"use foo::bar::{Baz, quux::*};",
         r"use foo::bar::{Baz, quux::{self::*, Fez}};",
@@ -550,7 +558,7 @@ fn merge_glob_nested() {
 
 #[test]
 fn merge_nested_considers_first_segments() {
-    check_full(
+    check_crate(
         "hir_ty::display::write_bounds_like_dyn_trait",
         r"use hir_ty::{autoderef, display::{HirDisplayError, HirFormatter}, method_resolution};",
         r"use hir_ty::{autoderef, display::{HirDisplayError, HirFormatter, write_bounds_like_dyn_trait}, method_resolution};",
@@ -559,7 +567,7 @@ fn merge_nested_considers_first_segments() {
 
 #[test]
 fn skip_merge_last_too_long() {
-    check_last(
+    check_module(
         "foo::bar",
         r"use foo::bar::baz::Qux;",
         r"use foo::bar;
@@ -569,7 +577,7 @@ use foo::bar::baz::Qux;",
 
 #[test]
 fn skip_merge_last_too_long2() {
-    check_last(
+    check_module(
         "foo::bar::baz::Qux",
         r"use foo::bar;",
         r"use foo::bar;
@@ -592,7 +600,7 @@ fn merge_last_fail() {
     check_merge_only_fail(
         r"use foo::bar::{baz::{Qux, Fez}};",
         r"use foo::bar::{baaz::{Quux, Feez}};",
-        MergeBehavior::Last,
+        MergeBehavior::Module,
     );
 }
 
@@ -601,7 +609,7 @@ fn merge_last_fail1() {
     check_merge_only_fail(
         r"use foo::bar::{baz::{Qux, Fez}};",
         r"use foo::bar::baaz::{Quux, Feez};",
-        MergeBehavior::Last,
+        MergeBehavior::Module,
     );
 }
 
@@ -610,7 +618,7 @@ fn merge_last_fail2() {
     check_merge_only_fail(
         r"use foo::bar::baz::{Qux, Fez};",
         r"use foo::bar::{baaz::{Quux, Feez}};",
-        MergeBehavior::Last,
+        MergeBehavior::Module,
     );
 }
 
@@ -619,7 +627,7 @@ fn merge_last_fail3() {
     check_merge_only_fail(
         r"use foo::bar::baz::{Qux, Fez};",
         r"use foo::bar::baaz::{Quux, Feez};",
-        MergeBehavior::Last,
+        MergeBehavior::Module,
     );
 }
 
@@ -648,12 +656,12 @@ fn check(
     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, true)
+fn check_crate(path: &str, ra_fixture_before: &str, ra_fixture_after: &str) {
+    check(path, ra_fixture_before, ra_fixture_after, Some(MergeBehavior::Crate), 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, true)
+fn check_module(path: &str, ra_fixture_before: &str, ra_fixture_after: &str) {
+    check(path, ra_fixture_before, ra_fixture_after, Some(MergeBehavior::Module), false, true)
 }
 
 fn check_none(path: &str, ra_fixture_before: &str, ra_fixture_after: &str) {
diff --git a/crates/ide_db/src/helpers/merge_imports.rs b/crates/ide_db/src/helpers/merge_imports.rs
index 148297279..af2a51a4d 100644
--- a/crates/ide_db/src/helpers/merge_imports.rs
+++ b/crates/ide_db/src/helpers/merge_imports.rs
@@ -9,19 +9,19 @@ use syntax::ast::{
 /// What type of merges are allowed.
 #[derive(Copy, Clone, Debug, PartialEq, Eq)]
 pub enum MergeBehavior {
-    /// Merge everything together creating deeply nested imports.
-    Full,
-    /// Only merge the last import level, doesn't allow import nesting.
-    Last,
+    /// Merge imports from the same crate into a single use statement.
+    Crate,
+    /// Merge imports from the same module into a single use statement.
+    Module,
 }
 
 impl MergeBehavior {
     #[inline]
     fn is_tree_allowed(&self, tree: &ast::UseTree) -> bool {
         match self {
-            MergeBehavior::Full => true,
+            MergeBehavior::Crate => true,
             // only simple single segment paths are allowed
-            MergeBehavior::Last => {
+            MergeBehavior::Module => {
                 tree.use_tree_list().is_none() && tree.path().map(path_len) <= Some(1)
             }
         }
@@ -153,7 +153,7 @@ fn recursive_merge(
                 }
             }
             Err(_)
-                if merge == MergeBehavior::Last
+                if merge == MergeBehavior::Module
                     && use_trees.len() > 0
                     && rhs_t.use_tree_list().is_some() =>
             {
diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs
index 8879a9161..d83670bda 100644
--- a/crates/rust-analyzer/src/config.rs
+++ b/crates/rust-analyzer/src/config.rs
@@ -36,7 +36,7 @@ config_data! {
     struct ConfigData {
         /// The strategy to use when inserting new imports or merging imports.
         assist_importMergeBehavior |
-        assist_importMergeBehaviour: MergeBehaviorDef  = "\"full\"",
+        assist_importMergeBehaviour: MergeBehaviorDef  = "\"crate\"",
         /// 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.
@@ -604,8 +604,8 @@ impl Config {
         InsertUseConfig {
             merge: match self.data.assist_importMergeBehavior {
                 MergeBehaviorDef::None => None,
-                MergeBehaviorDef::Full => Some(MergeBehavior::Full),
-                MergeBehaviorDef::Last => Some(MergeBehavior::Last),
+                MergeBehaviorDef::Crate => Some(MergeBehavior::Crate),
+                MergeBehaviorDef::Module => Some(MergeBehavior::Module),
             },
             prefix_kind: match self.data.assist_importPrefix {
                 ImportPrefixDef::Plain => PrefixKind::Plain,
@@ -709,8 +709,10 @@ enum ManifestOrProjectJson {
 #[serde(rename_all = "snake_case")]
 enum MergeBehaviorDef {
     None,
-    Full,
-    Last,
+    #[serde(alias = "full")]
+    Crate,
+    #[serde(alias = "last")]
+    Module,
 }
 
 #[derive(Deserialize, Debug, Clone)]
@@ -867,11 +869,11 @@ fn field_props(field: &str, ty: &str, doc: &[&str], default: &str) -> serde_json
         },
         "MergeBehaviorDef" => set! {
             "type": "string",
-            "enum": ["none", "full", "last"],
+            "enum": ["none", "crate", "module"],
             "enumDescriptions": [
-                "No merging",
-                "Merge all layers of the import trees",
-                "Only merge the last layer of the import trees"
+                "Do not merge imports at all.",
+                "Merge imports from the same crate into a single `use` statement.",
+                "Merge imports from the same module into a single `use` statement."
             ],
         },
         "ImportPrefixDef" => set! {
diff --git a/crates/rust-analyzer/src/integrated_benchmarks.rs b/crates/rust-analyzer/src/integrated_benchmarks.rs
index 3dcbe397a..56de9681c 100644
--- a/crates/rust-analyzer/src/integrated_benchmarks.rs
+++ b/crates/rust-analyzer/src/integrated_benchmarks.rs
@@ -133,7 +133,7 @@ fn integrated_completion_benchmark() {
             add_call_argument_snippets: true,
             snippet_cap: SnippetCap::new(true),
             insert_use: InsertUseConfig {
-                merge: Some(MergeBehavior::Full),
+                merge: Some(MergeBehavior::Crate),
                 prefix_kind: hir::PrefixKind::ByCrate,
                 group: true,
             },
@@ -166,7 +166,7 @@ fn integrated_completion_benchmark() {
             add_call_argument_snippets: true,
             snippet_cap: SnippetCap::new(true),
             insert_use: InsertUseConfig {
-                merge: Some(MergeBehavior::Full),
+                merge: Some(MergeBehavior::Crate),
                 prefix_kind: hir::PrefixKind::ByCrate,
                 group: true,
             },
diff --git a/docs/user/generated_config.adoc b/docs/user/generated_config.adoc
index e28423e99..f70558200 100644
--- a/docs/user/generated_config.adoc
+++ b/docs/user/generated_config.adoc
@@ -1,4 +1,4 @@
-[[rust-analyzer.assist.importMergeBehavior]]rust-analyzer.assist.importMergeBehavior (default: `"full"`)::
+[[rust-analyzer.assist.importMergeBehavior]]rust-analyzer.assist.importMergeBehavior (default: `"crate"`)::
 +
 --
 The strategy to use when inserting new imports or merging imports.
diff --git a/editors/code/package.json b/editors/code/package.json
index 0cc265aa4..f35d30898 100644
--- a/editors/code/package.json
+++ b/editors/code/package.json
@@ -382,17 +382,17 @@
                 "$generated-start": false,
                 "rust-analyzer.assist.importMergeBehavior": {
                     "markdownDescription": "The strategy to use when inserting new imports or merging imports.",
-                    "default": "full",
+                    "default": "crate",
                     "type": "string",
                     "enum": [
                         "none",
-                        "full",
-                        "last"
+                        "crate",
+                        "module"
                     ],
                     "enumDescriptions": [
-                        "No merging",
-                        "Merge all layers of the import trees",
-                        "Only merge the last layer of the import trees"
+                        "Do not merge imports at all.",
+                        "Merge imports from the same crate into a single `use` statement.",
+                        "Merge imports from the same module into a single `use` statement."
                     ]
                 },
                 "rust-analyzer.assist.importPrefix": {