Skip to content

Commit

Permalink
Auto merge of #15587 - dfireBird:fix-15128, r=Veykril
Browse files Browse the repository at this point in the history
Fix autoimport does nothing when importing trait that is as _ imports

Potentially fixes #15128

There are two cases of imports:
1. With simple path
2. With use tree list (or say complex path).

On deeper inspection, the [`recursive_merge`](https://github.com/rust-lang/rust-analyzer/blob/994df3d6a31d39f11600f30a6df0b744b13937c1/crates/ide-db/src/imports/merge_imports.rs#L87) function (called by [`try_merge_trees_mut`)](https://github.com/rust-lang/rust-analyzer/blob/994df3d6a31d39f11600f30a6df0b744b13937c1/crates/ide-db/src/imports/merge_imports.rs#L69) is meaningful only in the case of complex path (i.e when the UseTree contains a UseTreeList).

The [`recursive_merge`](https://github.com/rust-lang/rust-analyzer/blob/994df3d6a31d39f11600f30a6df0b744b13937c1/crates/ide-db/src/imports/merge_imports.rs#L87) function has [match with `Ok` arm](https://github.com/rust-lang/rust-analyzer/blob/994df3d6a31d39f11600f30a6df0b744b13937c1/crates/ide-db/src/imports/merge_imports.rs#L106), that is only executed when both LHS and RHS has `PathSegment` with same `NameRef`. The removal of underscore is implemented in this arm in the case of complex path.

For simple paths, the underscore is removed by checking if both LHS and RHS are simple paths and if their `Path` is same (the check is done [here](https://github.com/rust-lang/rust-analyzer/blob/994df3d6a31d39f11600f30a6df0b744b13937c1/crates/ide-db/src/imports/merge_imports.rs#L74)) and remove the underscore if one is found (I made an assumption here that RHS will always be what rust-analyzer suggests to import, because at this point I'm not sure how to remove underscore with help of `ted::replace`).
  • Loading branch information
bors committed Sep 22, 2023
2 parents df75809 + df1239b commit 5855bd8
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 0 deletions.
40 changes: 40 additions & 0 deletions crates/ide-db/src/imports/insert_use/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -993,6 +993,46 @@ use foo::bar::qux;
);
}

#[test]
fn insert_with_renamed_import_simple_use() {
check_with_config(
"use self::foo::Foo",
r#"
use self::foo::Foo as _;
"#,
r#"
use self::foo::Foo;
"#,
&InsertUseConfig {
granularity: ImportGranularity::Crate,
prefix_kind: hir::PrefixKind::BySelf,
enforce_granularity: true,
group: true,
skip_glob_imports: true,
},
);
}

#[test]
fn insert_with_renamed_import_complex_use() {
check_with_config(
"use self::foo::Foo;",
r#"
use self::foo::{self, Foo as _, Bar};
"#,
r#"
use self::foo::{self, Foo, Bar};
"#,
&InsertUseConfig {
granularity: ImportGranularity::Crate,
prefix_kind: hir::PrefixKind::BySelf,
enforce_granularity: true,
group: true,
skip_glob_imports: true,
},
);
}

fn check_with_config(
path: &str,
ra_fixture_before: &str,
Expand Down
11 changes: 11 additions & 0 deletions crates/ide-db/src/imports/merge_imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,10 @@ fn try_merge_trees_mut(lhs: &ast::UseTree, rhs: &ast::UseTree, merge: MergeBehav
{
lhs.split_prefix(&lhs_prefix);
rhs.split_prefix(&rhs_prefix);
} else {
ted::replace(lhs.syntax(), rhs.syntax());
// we can safely return here, in this case `recursive_merge` doesn't do anything
return Some(());
}
recursive_merge(lhs, rhs, merge)
}
Expand Down Expand Up @@ -123,6 +127,13 @@ fn recursive_merge(lhs: &ast::UseTree, rhs: &ast::UseTree, merge: MergeBehavior)
// so they need to be handled explicitly
.or_else(|| tree.star_token().map(|_| false))
};

if lhs_t.rename().and_then(|x| x.underscore_token()).is_some() {
ted::replace(lhs_t.syntax(), rhs_t.syntax());
*lhs_t = rhs_t;
continue;
}

match (tree_contains_self(lhs_t), tree_contains_self(&rhs_t)) {
(Some(true), None) => continue,
(None, Some(true)) => {
Expand Down

0 comments on commit 5855bd8

Please sign in to comment.