Skip to content

Commit ec74b70

Browse files
committed
fix: Properly handle aliased imports in remove_unused_imports
1 parent 00037c1 commit ec74b70

File tree

2 files changed

+97
-13
lines changed

2 files changed

+97
-13
lines changed

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

Lines changed: 80 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,10 @@ use ide_db::{
66
search::{FileReference, ReferenceCategory, SearchScope},
77
FxHashMap, RootDatabase,
88
};
9-
use syntax::{ast, AstNode};
9+
use syntax::{
10+
ast::{self, Rename},
11+
AstNode,
12+
};
1013
use text_edit::TextRange;
1114

1215
use crate::{AssistContext, AssistId, AssistKind, Assists};
@@ -100,19 +103,19 @@ pub(crate) fn remove_unused_imports(acc: &mut Assists, ctx: &AssistContext<'_>)
100103
hir::ScopeDef::ModuleDef(d) => Some(Definition::from(*d)),
101104
_ => None,
102105
})
103-
.any(|d| used_once_in_scope(ctx, d, scope))
106+
.any(|d| used_once_in_scope(ctx, d, u.rename(), scope))
104107
{
105108
return Some(u);
106109
}
107110
} else if let Definition::Trait(ref t) = def {
108111
// If the trait or any item is used.
109-
if !std::iter::once(def)
110-
.chain(t.items(ctx.db()).into_iter().map(Definition::from))
111-
.any(|d| used_once_in_scope(ctx, d, scope))
112+
if !std::iter::once((def, u.rename()))
113+
.chain(t.items(ctx.db()).into_iter().map(|item| (item.into(), None)))
114+
.any(|(d, rename)| used_once_in_scope(ctx, d, rename, scope))
112115
{
113116
return Some(u);
114117
}
115-
} else if !used_once_in_scope(ctx, def, scope) {
118+
} else if !used_once_in_scope(ctx, def, u.rename(), scope) {
116119
return Some(u);
117120
}
118121

@@ -138,11 +141,19 @@ pub(crate) fn remove_unused_imports(acc: &mut Assists, ctx: &AssistContext<'_>)
138141
}
139142
}
140143

141-
fn used_once_in_scope(ctx: &AssistContext<'_>, def: Definition, scopes: &Vec<SearchScope>) -> bool {
144+
fn used_once_in_scope(
145+
ctx: &AssistContext<'_>,
146+
def: Definition,
147+
rename: Option<Rename>,
148+
scopes: &Vec<SearchScope>,
149+
) -> bool {
142150
let mut found = false;
143151

152+
dbg!(&def);
153+
144154
for scope in scopes {
145155
let mut search_non_import = |_, r: FileReference| {
156+
dbg!(&r);
146157
// The import itself is a use; we must skip that.
147158
if !r.category.contains(ReferenceCategory::IMPORT) {
148159
found = true;
@@ -151,7 +162,10 @@ fn used_once_in_scope(ctx: &AssistContext<'_>, def: Definition, scopes: &Vec<Sea
151162
false
152163
}
153164
};
154-
def.usages(&ctx.sema).in_scope(scope).search(&mut search_non_import);
165+
def.usages(&ctx.sema)
166+
.in_scope(scope)
167+
.with_rename(rename.as_ref())
168+
.search(&mut search_non_import);
155169
if found {
156170
break;
157171
}
@@ -330,7 +344,7 @@ fn w() {
330344
}
331345

332346
#[test]
333-
fn ranamed_trait_item_use_is_use() {
347+
fn renamed_trait_item_use_is_use() {
334348
check_assist_not_applicable(
335349
remove_unused_imports,
336350
r#"
@@ -356,7 +370,7 @@ fn w() {
356370
}
357371

358372
#[test]
359-
fn ranamed_underscore_trait_item_use_is_use() {
373+
fn renamed_underscore_trait_item_use_is_use() {
360374
check_assist_not_applicable(
361375
remove_unused_imports,
362376
r#"
@@ -942,6 +956,62 @@ pub struct X();
942956
mod z {
943957
mod foo;
944958
}
959+
"#,
960+
);
961+
}
962+
963+
#[test]
964+
fn use_as_alias() {
965+
check_assist_not_applicable(
966+
remove_unused_imports,
967+
r#"
968+
mod foo {
969+
pub struct Foo {}
970+
}
971+
972+
use foo::Foo as Bar$0;
973+
974+
fn test(_: Bar) {}
975+
"#,
976+
);
977+
978+
check_assist(
979+
remove_unused_imports,
980+
r#"
981+
mod foo {
982+
pub struct Foo {}
983+
pub struct Bar {}
984+
pub struct Qux {}
985+
pub trait Quux {
986+
fn quxx(&self) {}
987+
}
988+
impl<T> Quxx for T {}
989+
}
990+
991+
use foo::{Foo as Bar, Bar as Baz, Qux as _, Quxx as _}$0;
992+
993+
fn test(_: Bar) {
994+
let a = ();
995+
a.quxx();
996+
}
997+
"#,
998+
r#"
999+
mod foo {
1000+
pub struct Foo {}
1001+
pub struct Bar {}
1002+
pub struct Qux {}
1003+
pub trait Quux {
1004+
fn quxx(&self) {}
1005+
}
1006+
impl<T> Quxx for T {}
1007+
}
1008+
1009+
use foo::{Foo as Bar, Quxx as _};
1010+
1011+
fn test(_: Bar) {
1012+
let a = ();
1013+
a.quxx();
1014+
}
9451015
"#,
9461016
);
9471017
}

crates/ide-db/src/search.rs

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ use parser::SyntaxKind;
1919
use rustc_hash::{FxHashMap, FxHashSet};
2020
use span::EditionedFileId;
2121
use syntax::{
22-
ast::{self, HasName},
22+
ast::{self, HasName, Rename},
2323
match_ast, AstNode, AstToken, SmolStr, SyntaxElement, SyntaxNode, TextRange, TextSize,
2424
ToSmolStr,
2525
};
@@ -405,6 +405,7 @@ impl Definition {
405405
pub fn usages<'a>(self, sema: &'a Semantics<'_, RootDatabase>) -> FindUsages<'a> {
406406
FindUsages {
407407
def: self,
408+
rename: None,
408409
assoc_item_container: self.as_assoc_item(sema.db).map(|a| a.container(sema.db)),
409410
sema,
410411
scope: None,
@@ -417,6 +418,7 @@ impl Definition {
417418
#[derive(Clone)]
418419
pub struct FindUsages<'a> {
419420
def: Definition,
421+
rename: Option<&'a Rename>,
420422
sema: &'a Semantics<'a, RootDatabase>,
421423
scope: Option<&'a SearchScope>,
422424
/// The container of our definition should it be an assoc item
@@ -447,6 +449,11 @@ impl<'a> FindUsages<'a> {
447449
self
448450
}
449451

452+
pub fn with_rename(mut self, rename: Option<&'a Rename>) -> Self {
453+
self.rename = rename;
454+
self
455+
}
456+
450457
pub fn at_least_one(&self) -> bool {
451458
let mut found = false;
452459
self.search(&mut |_, _| {
@@ -884,9 +891,16 @@ impl<'a> FindUsages<'a> {
884891
}
885892
};
886893

887-
let name = match self.def {
894+
let name = match (self.rename, self.def) {
895+
(Some(rename), _) => {
896+
if rename.underscore_token().is_some() {
897+
None
898+
} else {
899+
rename.name().map(|n| n.to_smolstr())
900+
}
901+
}
888902
// special case crate modules as these do not have a proper name
889-
Definition::Module(module) if module.is_crate_root() => {
903+
(_, Definition::Module(module)) if module.is_crate_root() => {
890904
// FIXME: This assumes the crate name is always equal to its display name when it
891905
// really isn't
892906
// we should instead look at the dependency edge name and recursively search our way

0 commit comments

Comments
 (0)