Skip to content

Commit

Permalink
Auto merge of rust-lang#12347 - feniljain:fix_extract_module, r=Veykril
Browse files Browse the repository at this point in the history
fix(extract_module) resolving import panics and improve import resolution

- Should solve rust-lang#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
  • Loading branch information
bors committed Jun 2, 2022
2 parents 6f7c558 + 8a1ef52 commit 2f0814e
Showing 1 changed file with 92 additions and 6 deletions.
98 changes: 92 additions & 6 deletions crates/ide-assists/src/handlers/extract_module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
);
}
}
}
Expand All @@ -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,
);
}
}
}
Expand Down Expand Up @@ -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);
Expand All @@ -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_ =
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
}
",
)
}
}

0 comments on commit 2f0814e

Please sign in to comment.