Allow merge_imports assists to merge imports of equal visibility

2 files changed, 61 insertions, 6 deletions

diff --git a/crates/assists/src/handlers/merge_imports.rs b/crates/assists/src/handlers/merge_imports.rs
index da084d5fb..0bd679260 100644
--- a/crates/assists/src/handlers/merge_imports.rs
+++ b/crates/assists/src/handlers/merge_imports.rs
@@ -150,6 +150,56 @@ pub use std::fmt::Display;
     }
 
     #[test]
+    fn skip_pub_crate_pub() {
+        check_assist_not_applicable(
+            merge_imports,
+            r"
+pub(crate) use std::fmt<|>::Debug;
+pub use std::fmt::Display;
+",
+        );
+    }
+
+    #[test]
+    fn skip_pub_pub_crate() {
+        check_assist_not_applicable(
+            merge_imports,
+            r"
+pub use std::fmt<|>::Debug;
+pub(crate) use std::fmt::Display;
+",
+        );
+    }
+
+    #[test]
+    fn merge_pub() {
+        check_assist(
+            merge_imports,
+            r"
+pub use std::fmt<|>::Debug;
+pub use std::fmt::Display;
+",
+            r"
+pub use std::fmt::{Debug, Display};
+",
+        )
+    }
+
+    #[test]
+    fn merge_pub_crate() {
+        check_assist(
+            merge_imports,
+            r"
+pub(crate) use std::fmt<|>::Debug;
+pub(crate) use std::fmt::Display;
+",
+            r"
+pub(crate) use std::fmt::{Debug, Display};
+",
+        )
+    }
+
+    #[test]
     fn test_merge_nested() {
         check_assist(
             merge_imports,
diff --git a/crates/assists/src/utils/insert_use.rs b/crates/assists/src/utils/insert_use.rs
index a920e12c5..98553b2e0 100644
--- a/crates/assists/src/utils/insert_use.rs
+++ b/crates/assists/src/utils/insert_use.rs
@@ -138,18 +138,23 @@ pub(crate) fn insert_use(
     algo::insert_children(scope.as_syntax_node(), insert_position, to_insert)
 }
 
+fn eq_visibility(vis0: Option<ast::Visibility>, vis1: Option<ast::Visibility>) -> bool {
+    match (vis0, vis1) {
+        (None, None) => true,
+        // FIXME: Don't use the string representation to check for equality
+        // spaces inside of the node would break this comparison
+        (Some(vis0), Some(vis1)) => vis0.to_string() == vis1.to_string(),
+        _ => false,
+    }
+}
+
 pub(crate) fn try_merge_imports(
     old: &ast::Use,
     new: &ast::Use,
     merge_behaviour: MergeBehaviour,
 ) -> Option<ast::Use> {
     // don't merge imports with different visibilities
-    if old
-        .visibility()
-        .and_then(|vis| vis.pub_token())
-        .or_else(|| new.visibility().and_then(|vis| vis.pub_token()))
-        .is_some()
-    {
+    if !eq_visibility(old.visibility(), new.visibility()) {
         return None;
     }
     let old_tree = old.use_tree()?;