about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2022-06-02 12:37:17 +0000
committerbors <bors@rust-lang.org>2022-06-02 12:37:17 +0000
commit2f0814ea351d323ee236f6757c3f7603fc7d714a (patch)
tree85c225f83fce5758eb4cc178501d5311e4620df7
parent6f7c5589abfc93fbdfc071cc2716d1ea7b527e2e (diff)
parent8a1ef52f5c39e1d549bf4c731f5d7756fe54cd0e (diff)
downloadrust-2f0814ea351d323ee236f6757c3f7603fc7d714a.tar.gz
rust-2f0814ea351d323ee236f6757c3f7603fc7d714a.zip
Auto merge of #12347 - feniljain:fix_extract_module, r=Veykril
fix(extract_module) resolving import panics and improve import resolution

- Should solve #11766
- While adding a test case for this issue, I observed another issue:
For this test case:
```rust
            mod x {
                pub struct Foo;
                pub struct Bar;
            }

            use x::{Bar, Foo};

            $0type A = (Foo, Bar);$0
```
extract module should yield this:

```rust
            mod x {
                pub struct Foo;
                pub struct Bar;
            }

            use x::{};

            mod modname {
                use super::x::Bar;

                use super::x::Foo;

                pub(crate) type A = (Foo, Bar);
            }
```

instead it gave:

```rust
            mod x {
                pub struct Foo;
                pub struct Bar;
            }

            use x::{};

            mod modname {
                use x::Bar;

                use x::Foo;

                pub(crate) type A = (Foo, Bar);
            }
```

So fixed this problem with second commit
-rw-r--r--crates/ide-assists/src/handlers/extract_module.rs98
1 files changed, 92 insertions, 6 deletions
diff --git a/crates/ide-assists/src/handlers/extract_module.rs b/crates/ide-assists/src/handlers/extract_module.rs
index b5ed5699c90..11349b45d3c 100644
--- a/crates/ide-assists/src/handlers/extract_module.rs
+++ b/crates/ide-assists/src/handlers/extract_module.rs
@@ -413,7 +413,10 @@ impl Module {
                                         ctx,
                                     )
                                 {
-                                    import_paths_to_be_removed.push(import_path);
+                                    check_intersection_and_push(
+                                        &mut import_paths_to_be_removed,
+                                        import_path,
+                                    );
                                 }
                             }
                         }
@@ -439,7 +442,10 @@ impl Module {
                                         ctx,
                                     )
                                 {
-                                    import_paths_to_be_removed.push(import_path);
+                                    check_intersection_and_push(
+                                        &mut import_paths_to_be_removed,
+                                        import_path,
+                                    );
                                 }
                             }
                         }
@@ -543,12 +549,25 @@ impl Module {
         } else if exists_inside_sel && !exists_outside_sel {
             //Changes to be made inside new module, and remove import from outside
 
-            if let Some((use_tree_str, text_range_opt)) =
+            if let Some((mut use_tree_str, text_range_opt)) =
                 self.process_use_stmt_for_import_resolve(use_stmt_opt, node_syntax)
             {
                 if let Some(text_range) = text_range_opt {
                     import_path_to_be_removed = Some(text_range);
                 }
+
+                if source_exists_outside_sel_in_same_mod {
+                    if let Some(first_path_in_use_tree) = use_tree_str.last() {
+                        let first_path_in_use_tree_str = first_path_in_use_tree.to_string();
+                        if !first_path_in_use_tree_str.contains("super")
+                            && !first_path_in_use_tree_str.contains("crate")
+                        {
+                            let super_path = make::ext::ident_path("super");
+                            use_tree_str.push(super_path);
+                        }
+                    }
+                }
+
                 use_tree_str_opt = Some(use_tree_str);
             } else if source_exists_outside_sel_in_same_mod {
                 self.make_use_stmt_of_node_with_super(node_syntax);
@@ -558,9 +577,16 @@ impl Module {
         if let Some(use_tree_str) = use_tree_str_opt {
             let mut use_tree_str = use_tree_str;
             use_tree_str.reverse();
-            if use_tree_str[0].to_string().contains("super") {
-                let super_path = make::ext::ident_path("super");
-                use_tree_str.insert(0, super_path)
+
+            if !(!exists_outside_sel && exists_inside_sel && source_exists_outside_sel_in_same_mod)
+            {
+                if let Some(first_path_in_use_tree) = use_tree_str.first() {
+                    let first_path_in_use_tree_str = first_path_in_use_tree.to_string();
+                    if first_path_in_use_tree_str.contains("super") {
+                        let super_path = make::ext::ident_path("super");
+                        use_tree_str.insert(0, super_path)
+                    }
+                }
             }
 
             let use_ =
@@ -621,6 +647,32 @@ impl Module {
     }
 }
 
+fn check_intersection_and_push(
+    import_paths_to_be_removed: &mut Vec<TextRange>,
+    import_path: TextRange,
+) {
+    if import_paths_to_be_removed.len() > 0 {
+        // Text ranges recieved 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);
+    }
+}
+
 fn does_source_exists_outside_sel_in_same_mod(
     def: Definition,
     ctx: &AssistContext,
@@ -1495,4 +1547,38 @@ mod modname {
         ",
         )
     }
+
+    #[test]
+    fn test_issue_11766() {
+        //https://github.com/rust-lang/rust-analyzer/issues/11766
+        check_assist(
+            extract_module,
+            r"
+            mod x {
+                pub struct Foo;
+                pub struct Bar;
+            }
+
+            use x::{Bar, Foo};
+
+            $0type A = (Foo, Bar);$0
+        ",
+            r"
+            mod x {
+                pub struct Foo;
+                pub struct Bar;
+            }
+
+            use x::{};
+
+            mod modname {
+                use super::x::Bar;
+
+                use super::x::Foo;
+
+                pub(crate) type A = (Foo, Bar);
+            }
+        ",
+        )
+    }
 }