Skip to content

Commit

Permalink
fix: Temporary fix for remove_unused_imports not handling import al…
Browse files Browse the repository at this point in the history
…iases correctly
  • Loading branch information
ShoyuVanilla committed Sep 24, 2024
1 parent 00037c1 commit d4de84f
Show file tree
Hide file tree
Showing 2 changed files with 97 additions and 13 deletions.
87 changes: 77 additions & 10 deletions crates/ide-assists/src/handlers/remove_unused_imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@ use ide_db::{
search::{FileReference, ReferenceCategory, SearchScope},
FxHashMap, RootDatabase,
};
use syntax::{ast, AstNode};
use syntax::{
ast::{self, Rename},
AstNode,
};
use text_edit::TextRange;

use crate::{AssistContext, AssistId, AssistKind, Assists};
Expand Down Expand Up @@ -100,19 +103,19 @@ pub(crate) fn remove_unused_imports(acc: &mut Assists, ctx: &AssistContext<'_>)
hir::ScopeDef::ModuleDef(d) => Some(Definition::from(*d)),
_ => None,
})
.any(|d| used_once_in_scope(ctx, d, scope))
.any(|d| used_once_in_scope(ctx, d, u.rename(), scope))
{
return Some(u);
}
} else if let Definition::Trait(ref t) = def {
// If the trait or any item is used.
if !std::iter::once(def)
.chain(t.items(ctx.db()).into_iter().map(Definition::from))
.any(|d| used_once_in_scope(ctx, d, scope))
if !std::iter::once((def, u.rename()))
.chain(t.items(ctx.db()).into_iter().map(|item| (item.into(), None)))
.any(|(d, rename)| used_once_in_scope(ctx, d, rename, scope))
{
return Some(u);
}
} else if !used_once_in_scope(ctx, def, scope) {
} else if !used_once_in_scope(ctx, def, u.rename(), scope) {
return Some(u);
}

Expand All @@ -138,7 +141,12 @@ pub(crate) fn remove_unused_imports(acc: &mut Assists, ctx: &AssistContext<'_>)
}
}

fn used_once_in_scope(ctx: &AssistContext<'_>, def: Definition, scopes: &Vec<SearchScope>) -> bool {
fn used_once_in_scope(
ctx: &AssistContext<'_>,
def: Definition,
rename: Option<Rename>,
scopes: &Vec<SearchScope>,
) -> bool {
let mut found = false;

for scope in scopes {
Expand All @@ -151,7 +159,10 @@ fn used_once_in_scope(ctx: &AssistContext<'_>, def: Definition, scopes: &Vec<Sea
false
}
};
def.usages(&ctx.sema).in_scope(scope).search(&mut search_non_import);
def.usages(&ctx.sema)
.in_scope(scope)
.with_rename(rename.as_ref())
.search(&mut search_non_import);
if found {
break;
}
Expand Down Expand Up @@ -330,7 +341,7 @@ fn w() {
}

#[test]
fn ranamed_trait_item_use_is_use() {
fn renamed_trait_item_use_is_use() {
check_assist_not_applicable(
remove_unused_imports,
r#"
Expand All @@ -356,7 +367,7 @@ fn w() {
}

#[test]
fn ranamed_underscore_trait_item_use_is_use() {
fn renamed_underscore_trait_item_use_is_use() {
check_assist_not_applicable(
remove_unused_imports,
r#"
Expand Down Expand Up @@ -942,6 +953,62 @@ pub struct X();
mod z {
mod foo;
}
"#,
);
}

#[test]
fn use_as_alias() {
check_assist_not_applicable(
remove_unused_imports,
r#"
mod foo {
pub struct Foo {}
}
use foo::Foo as Bar$0;
fn test(_: Bar) {}
"#,
);

check_assist(
remove_unused_imports,
r#"
mod foo {
pub struct Foo {}
pub struct Bar {}
pub struct Qux {}
pub trait Quux {
fn quxx(&self) {}
}
impl<T> Quxx for T {}
}
use foo::{Foo as Bar, Bar as Baz, Qux as _, Quxx as _}$0;
fn test(_: Bar) {
let a = ();
a.quxx();
}
"#,
r#"
mod foo {
pub struct Foo {}
pub struct Bar {}
pub struct Qux {}
pub trait Quux {
fn quxx(&self) {}
}
impl<T> Quxx for T {}
}
use foo::{Foo as Bar, Quxx as _};
fn test(_: Bar) {
let a = ();
a.quxx();
}
"#,
);
}
Expand Down
23 changes: 20 additions & 3 deletions crates/ide-db/src/search.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use parser::SyntaxKind;
use rustc_hash::{FxHashMap, FxHashSet};
use span::EditionedFileId;
use syntax::{
ast::{self, HasName},
ast::{self, HasName, Rename},
match_ast, AstNode, AstToken, SmolStr, SyntaxElement, SyntaxNode, TextRange, TextSize,
ToSmolStr,
};
Expand Down Expand Up @@ -405,6 +405,7 @@ impl Definition {
pub fn usages<'a>(self, sema: &'a Semantics<'_, RootDatabase>) -> FindUsages<'a> {
FindUsages {
def: self,
rename: None,
assoc_item_container: self.as_assoc_item(sema.db).map(|a| a.container(sema.db)),
sema,
scope: None,
Expand All @@ -417,6 +418,7 @@ impl Definition {
#[derive(Clone)]
pub struct FindUsages<'a> {
def: Definition,
rename: Option<&'a Rename>,
sema: &'a Semantics<'a, RootDatabase>,
scope: Option<&'a SearchScope>,
/// The container of our definition should it be an assoc item
Expand Down Expand Up @@ -447,6 +449,14 @@ impl<'a> FindUsages<'a> {
self
}

// FIXME: This is just a temporary fix for not handling import aliases like
// `use Foo as Bar`. We need to support them in a proper way.
// See issue #14079
pub fn with_rename(mut self, rename: Option<&'a Rename>) -> Self {
self.rename = rename;
self
}

pub fn at_least_one(&self) -> bool {
let mut found = false;
self.search(&mut |_, _| {
Expand Down Expand Up @@ -884,9 +894,16 @@ impl<'a> FindUsages<'a> {
}
};

let name = match self.def {
let name = match (self.rename, self.def) {
(Some(rename), _) => {
if rename.underscore_token().is_some() {
None
} else {
rename.name().map(|n| n.to_smolstr())
}
}
// special case crate modules as these do not have a proper name
Definition::Module(module) if module.is_crate_root() => {
(_, Definition::Module(module)) if module.is_crate_root() => {
// FIXME: This assumes the crate name is always equal to its display name when it
// really isn't
// we should instead look at the dependency edge name and recursively search our way
Expand Down

0 comments on commit d4de84f

Please sign in to comment.