Auto merge of #13871 - lowr:fix/extract-module-merge-multiple-ranges, r=lnicola

fix: merge multiple intersecting ranges

Fixes #13791

In `check_intersection_and_push()`, there may exist two ranges we should merge with the new one. We've been assuming there should be only one range that intersects, which lead to [this assertion](da15d92a32/crates/text-edit/src/lib.rs (L192)) to fail under specific circumstances.

Cargo.lock

@@ -655,6 +655,7 @@ dependencies = [
  "ide-db",
  "itertools",
  "profile",
+ "smallvec",
  "sourcegen",
  "stdx",
  "syntax",

crates/ide-assists/Cargo.toml

@@ -14,6 +14,7 @@ cov-mark = "2.0.0-pre.1"
 
 itertools = "0.10.5"
 either = "1.7.0"
+smallvec = "1.10.0"
 
 stdx = { path = "../stdx", version = "0.0.0" }
 syntax = { path = "../syntax", version = "0.0.0" }

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

@@ -10,6 +10,8 @@ use ide_db::{
     defs::{Definition, NameClass, NameRefClass},
     search::{FileReference, SearchScope},
 };
+use itertools::Itertools;
+use smallvec::SmallVec;
 use stdx::format_to;
 use syntax::{
     algo::find_node_at_range,
@@ -657,28 +659,23 @@ impl Module {
 
 fn check_intersection_and_push(
     import_paths_to_be_removed: &mut Vec<TextRange>,
-    import_path: TextRange,
+    mut import_path: TextRange,
 ) {
-    if import_paths_to_be_removed.len() > 0 {
-        // Text ranges received here for imports are extended to the
-        // next/previous comma which can cause intersections among them
-        // and later deletion of these can cause panics similar
-        // to reported in #11766. So to mitigate it, we
-        // check for intersection between all current members
-        // and if it exists we combine both text ranges into
-        // one
-        let r = import_paths_to_be_removed
-            .into_iter()
-            .position(|it| it.intersect(import_path).is_some());
-        match r {
-            Some(it) => {
-                import_paths_to_be_removed[it] = import_paths_to_be_removed[it].cover(import_path)
-            }
-            None => import_paths_to_be_removed.push(import_path),
-        }
-    } else {
-        import_paths_to_be_removed.push(import_path);
+    // Text ranges received here for imports are extended to the
+    // next/previous comma which can cause intersections among them
+    // and later deletion of these can cause panics similar
+    // to reported in #11766. So to mitigate it, we
+    // check for intersection between all current members
+    // and combine all such ranges into one.
+    let s: SmallVec<[_; 2]> = import_paths_to_be_removed
+        .into_iter()
+        .positions(|it| it.intersect(import_path).is_some())
+        .collect();
+    for pos in s.into_iter().rev() {
+        let intersecting_path = import_paths_to_be_removed.swap_remove(pos);
+        import_path = import_path.cover(intersecting_path);
     }
+    import_paths_to_be_removed.push(import_path);
 }
 
 fn does_source_exists_outside_sel_in_same_mod(
@@ -1766,4 +1763,49 @@ mod modname {
         ",
         )
     }
+
+    #[test]
+    fn test_merge_multiple_intersections() {
+        check_assist(
+            extract_module,
+            r#"
+mod dep {
+    pub struct A;
+    pub struct B;
+    pub struct C;
+}
+
+use dep::{A, B, C};
+
+$0struct S {
+    inner: A,
+    state: C,
+    condvar: B,
+}$0
+"#,
+            r#"
+mod dep {
+    pub struct A;
+    pub struct B;
+    pub struct C;
+}
+
+use dep::{};
+
+mod modname {
+    use super::dep::B;
+
+    use super::dep::C;
+
+    use super::dep::A;
+
+    pub(crate) struct S {
+        pub(crate) inner: A,
+        pub(crate) state: C,
+        pub(crate) condvar: B,
+    }
+}
+"#,
+        );
+    }
 }