Auto merge of #14723 - obsgolem:master, r=Veykril

Added remove unused imports assist

This resolves the most important part of #5131. I needed to make a couple of cosmetic changes to the search infrastructure to do this.

A few open questions:
* Should imports that don't resolve to anything be considered unused? I figured probably not, but it would be a trivial change to make if we want it.
* Is there a cleaner way to make the edits to the use list?
* Is there a cleaner way to get the list of uses that intersect the current selection?
* Is the performance acceptable? When testing this on itself, it takes a good couple seconds to perform the assist.
* Is there a way to hide the rustc diagnostics that overlap with this functionality?
This commit is contained in:
bors 2023-08-01 09:50:16 +00:00
commit 62dcf39ef0
16 changed files with 822 additions and 28 deletions

View File

@ -114,7 +114,7 @@ fn collect_data(ident_pat: IdentPat, ctx: &AssistContext<'_>) -> Option<TupleDat
let usages = ctx.sema.to_def(&ident_pat).map(|def| {
Definition::Local(def)
.usages(&ctx.sema)
.in_scope(SearchScope::single_file(ctx.file_id()))
.in_scope(&SearchScope::single_file(ctx.file_id()))
.all()
});

View File

@ -120,7 +120,7 @@ fn find_use_tree(n: SyntaxNode) -> Option<(ast::UseTree, ast::Path)> {
fn def_is_referenced_in(def: Definition, ctx: &AssistContext<'_>) -> bool {
let search_scope = SearchScope::single_file(ctx.file_id());
def.usages(&ctx.sema).in_scope(search_scope).at_least_one()
def.usages(&ctx.sema).in_scope(&search_scope).at_least_one()
}
#[derive(Debug, Clone)]

View File

@ -384,7 +384,7 @@ fn find_local_usages(ctx: &AssistContext<'_>, var: Local) -> Self {
Self(
Definition::Local(var)
.usages(&ctx.sema)
.in_scope(SearchScope::single_file(ctx.file_id()))
.in_scope(&SearchScope::single_file(ctx.file_id()))
.all(),
)
}

View File

@ -478,7 +478,7 @@ fn process_names_and_namerefs_for_import_resolve(
let selection_range = ctx.selection_trimmed();
let curr_file_id = ctx.file_id();
let search_scope = SearchScope::single_file(curr_file_id);
let usage_res = def.usages(&ctx.sema).in_scope(search_scope).all();
let usage_res = def.usages(&ctx.sema).in_scope(&search_scope).all();
let file = ctx.sema.parse(curr_file_id);
let mut exists_inside_sel = false;

View File

@ -80,7 +80,7 @@ pub(crate) fn inline_into_callers(acc: &mut Assists, ctx: &AssistContext<'_>) ->
let is_recursive_fn = usages
.clone()
.in_scope(SearchScope::file_range(FileRange {
.in_scope(&SearchScope::file_range(FileRange {
file_id: def_file,
range: func_body.syntax().text_range(),
}))

View File

@ -82,17 +82,19 @@ pub(crate) fn move_const_to_impl(acc: &mut Assists, ctx: &AssistContext<'_>) ->
return None;
}
let usages =
Definition::Const(def).usages(&ctx.sema).in_scope(SearchScope::file_range(FileRange {
file_id: ctx.file_id(),
range: parent_fn.syntax().text_range(),
}));
acc.add(
AssistId("move_const_to_impl", crate::AssistKind::RefactorRewrite),
"Move const to impl block",
const_.syntax().text_range(),
|builder| {
let usages = Definition::Const(def)
.usages(&ctx.sema)
.in_scope(&SearchScope::file_range(FileRange {
file_id: ctx.file_id(),
range: parent_fn.syntax().text_range(),
}))
.all();
let range_to_delete = match const_.syntax().next_sibling_or_token() {
Some(s) if matches!(s.kind(), SyntaxKind::WHITESPACE) => {
// Remove following whitespaces too.
@ -103,7 +105,7 @@ pub(crate) fn move_const_to_impl(acc: &mut Assists, ctx: &AssistContext<'_>) ->
builder.delete(range_to_delete);
let const_ref = format!("Self::{}", name.display(ctx.db()));
for range in usages.all().file_ranges().map(|it| it.range) {
for range in usages.file_ranges().map(|it| it.range) {
builder.replace(range, const_ref.clone());
}

View File

@ -0,0 +1,739 @@
use std::collections::{hash_map::Entry, HashMap};
use hir::{InFile, Module, ModuleSource};
use ide_db::{
base_db::FileRange,
defs::Definition,
search::{FileReference, ReferenceCategory, SearchScope},
RootDatabase,
};
use syntax::{ast, AstNode};
use text_edit::TextRange;
use crate::{AssistContext, AssistId, AssistKind, Assists};
// Assist: remove_unused_imports
//
// Removes any use statements in the current selection that are unused.
//
// ```
// struct X();
// mod foo {
// use super::X$0;
// }
// ```
// ->
// ```
// struct X();
// mod foo {
// }
// ```
pub(crate) fn remove_unused_imports(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> {
// First, grab the uses that intersect with the current selection.
let selected_el = match ctx.covering_element() {
syntax::NodeOrToken::Node(n) => n,
syntax::NodeOrToken::Token(t) => t.parent()?,
};
// This applies to all uses that are selected, or are ancestors of our selection.
let uses_up = selected_el.ancestors().skip(1).filter_map(ast::Use::cast);
let uses_down = selected_el
.descendants()
.filter(|x| x.text_range().intersect(ctx.selection_trimmed()).is_some())
.filter_map(ast::Use::cast);
let uses = uses_up.chain(uses_down).collect::<Vec<_>>();
// Maps use nodes to the scope that we should search through to find
let mut search_scopes = HashMap::<Module, Vec<SearchScope>>::new();
// iterator over all unused use trees
let mut unused = uses
.into_iter()
.flat_map(|u| u.syntax().descendants().filter_map(ast::UseTree::cast))
.filter(|u| u.use_tree_list().is_none())
.filter_map(|u| {
// Find any uses trees that are unused
let use_module = ctx.sema.scope(&u.syntax()).map(|s| s.module())?;
let scope = match search_scopes.entry(use_module) {
Entry::Occupied(o) => o.into_mut(),
Entry::Vacant(v) => v.insert(module_search_scope(ctx.db(), use_module)),
};
// Gets the path associated with this use tree. If there isn't one, then ignore this use tree.
let path = if let Some(path) = u.path() {
path
} else if u.star_token().is_some() {
// This case maps to the situation where the * token is braced.
// In this case, the parent use tree's path is the one we should use to resolve the glob.
match u.syntax().ancestors().skip(1).find_map(ast::UseTree::cast) {
Some(parent_u) if parent_u.path().is_some() => parent_u.path().unwrap(),
_ => return None,
}
} else {
return None;
};
// Get the actual definition associated with this use item.
let res = match ctx.sema.resolve_path(&path) {
Some(x) => x,
None => {
return None;
}
};
let def = match res {
hir::PathResolution::Def(d) => Definition::from(d),
_ => return None,
};
if u.star_token().is_some() {
// Check if any of the children of this module are used
let def_mod = match def {
Definition::Module(module) => module,
_ => return None,
};
if !def_mod
.scope(ctx.db(), Some(use_module))
.iter()
.filter_map(|(_, x)| match x {
hir::ScopeDef::ModuleDef(d) => Some(Definition::from(*d)),
_ => None,
})
.any(|d| used_once_in_scope(ctx, d, 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))
{
return Some(u);
}
} else {
if !used_once_in_scope(ctx, def, &scope) {
return Some(u);
}
}
None
})
.peekable();
// Peek so we terminate early if an unused use is found. Only do the rest of the work if the user selects the assist.
if unused.peek().is_some() {
acc.add(
AssistId("remove_unused_imports", AssistKind::QuickFix),
"Remove all the unused imports",
selected_el.text_range(),
|builder| {
let unused: Vec<ast::UseTree> = unused.map(|x| builder.make_mut(x)).collect();
for node in unused {
node.remove_recursive();
}
},
)
} else {
None
}
}
fn used_once_in_scope(ctx: &AssistContext<'_>, def: Definition, scopes: &Vec<SearchScope>) -> bool {
let mut found = false;
for scope in scopes {
let mut search_non_import = |_, r: FileReference| {
// The import itself is a use; we must skip that.
if r.category != Some(ReferenceCategory::Import) {
found = true;
true
} else {
false
}
};
def.usages(&ctx.sema).in_scope(scope).search(&mut search_non_import);
if found {
break;
}
}
found
}
/// Build a search scope spanning the given module but none of its submodules.
fn module_search_scope(db: &RootDatabase, module: hir::Module) -> Vec<SearchScope> {
let (file_id, range) = {
let InFile { file_id, value } = module.definition_source(db);
if let Some((file_id, call_source)) = file_id.original_call_node(db) {
(file_id, Some(call_source.text_range()))
} else {
(
file_id.original_file(db),
match value {
ModuleSource::SourceFile(_) => None,
ModuleSource::Module(it) => Some(it.syntax().text_range()),
ModuleSource::BlockExpr(it) => Some(it.syntax().text_range()),
},
)
}
};
fn split_at_subrange(first: TextRange, second: TextRange) -> (TextRange, Option<TextRange>) {
let intersect = first.intersect(second);
if let Some(intersect) = intersect {
let start_range = TextRange::new(first.start(), intersect.start());
if intersect.end() < first.end() {
(start_range, Some(TextRange::new(intersect.end(), first.end())))
} else {
(start_range, None)
}
} else {
(first, None)
}
}
let mut scopes = Vec::new();
if let Some(range) = range {
let mut ranges = vec![range];
for child in module.children(db) {
let rng = match child.definition_source(db).value {
ModuleSource::SourceFile(_) => continue,
ModuleSource::Module(it) => it.syntax().text_range(),
ModuleSource::BlockExpr(_) => continue,
};
let mut new_ranges = Vec::new();
for old_range in ranges.iter_mut() {
let split = split_at_subrange(old_range.clone(), rng);
*old_range = split.0;
new_ranges.extend(split.1);
}
ranges.append(&mut new_ranges);
}
for range in ranges {
scopes.push(SearchScope::file_range(FileRange { file_id, range }));
}
} else {
scopes.push(SearchScope::single_file(file_id));
}
scopes
}
#[cfg(test)]
mod tests {
use crate::tests::{check_assist, check_assist_not_applicable};
use super::*;
#[test]
fn remove_unused() {
check_assist(
remove_unused_imports,
r#"
struct X();
struct Y();
mod z {
$0use super::X;
use super::Y;$0
}
"#,
r#"
struct X();
struct Y();
mod z {
}
"#,
);
}
#[test]
fn remove_unused_is_precise() {
check_assist(
remove_unused_imports,
r#"
struct X();
mod z {
$0use super::X;$0
fn w() {
struct X();
let x = X();
}
}
"#,
r#"
struct X();
mod z {
fn w() {
struct X();
let x = X();
}
}
"#,
);
}
#[test]
fn trait_name_use_is_use() {
check_assist_not_applicable(
remove_unused_imports,
r#"
struct X();
trait Y {
fn f();
}
impl Y for X {
fn f() {}
}
mod z {
$0use super::X;
use super::Y;$0
fn w() {
X::f();
}
}
"#,
);
}
#[test]
fn trait_item_use_is_use() {
check_assist_not_applicable(
remove_unused_imports,
r#"
struct X();
trait Y {
fn f(self);
}
impl Y for X {
fn f(self) {}
}
mod z {
$0use super::X;
use super::Y;$0
fn w() {
let x = X();
x.f();
}
}
"#,
);
}
#[test]
fn ranamed_trait_item_use_is_use() {
check_assist_not_applicable(
remove_unused_imports,
r#"
struct X();
trait Y {
fn f(self);
}
impl Y for X {
fn f(self) {}
}
mod z {
$0use super::X;
use super::Y as Z;$0
fn w() {
let x = X();
x.f();
}
}
"#,
);
}
#[test]
fn ranamed_underscore_trait_item_use_is_use() {
check_assist_not_applicable(
remove_unused_imports,
r#"
struct X();
trait Y {
fn f(self);
}
impl Y for X {
fn f(self) {}
}
mod z {
$0use super::X;
use super::Y as _;$0
fn w() {
let x = X();
x.f();
}
}
"#,
);
}
#[test]
fn dont_remove_used() {
check_assist_not_applicable(
remove_unused_imports,
r#"
struct X();
struct Y();
mod z {
$0use super::X;
use super::Y;$0
fn w() {
let x = X();
let y = Y();
}
}
"#,
);
}
#[test]
fn remove_unused_in_braces() {
check_assist(
remove_unused_imports,
r#"
struct X();
struct Y();
mod z {
$0use super::{X, Y};$0
fn w() {
let x = X();
}
}
"#,
r#"
struct X();
struct Y();
mod z {
use super::{X};
fn w() {
let x = X();
}
}
"#,
);
}
#[test]
fn remove_unused_under_cursor() {
check_assist(
remove_unused_imports,
r#"
struct X();
mod z {
use super::X$0;
}
"#,
r#"
struct X();
mod z {
}
"#,
);
}
#[test]
fn remove_multi_use_block() {
check_assist(
remove_unused_imports,
r#"
struct X();
$0mod y {
use super::X;
}
mod z {
use super::X;
}$0
"#,
r#"
struct X();
mod y {
}
mod z {
}
"#,
);
}
#[test]
fn remove_nested() {
check_assist(
remove_unused_imports,
r#"
struct X();
mod y {
struct Y();
mod z {
use crate::{X, y::Y}$0;
fn f() {
let x = X();
}
}
}
"#,
r#"
struct X();
mod y {
struct Y();
mod z {
use crate::{X};
fn f() {
let x = X();
}
}
}
"#,
);
}
#[test]
fn remove_nested_first_item() {
check_assist(
remove_unused_imports,
r#"
struct X();
mod y {
struct Y();
mod z {
use crate::{X, y::Y}$0;
fn f() {
let y = Y();
}
}
}
"#,
r#"
struct X();
mod y {
struct Y();
mod z {
use crate::{y::Y};
fn f() {
let y = Y();
}
}
}
"#,
);
}
#[test]
fn remove_nested_all_unused() {
check_assist(
remove_unused_imports,
r#"
struct X();
mod y {
struct Y();
mod z {
use crate::{X, y::Y}$0;
}
}
"#,
r#"
struct X();
mod y {
struct Y();
mod z {
}
}
"#,
);
}
#[test]
fn remove_unused_glob() {
check_assist(
remove_unused_imports,
r#"
struct X();
struct Y();
mod z {
use super::*$0;
}
"#,
r#"
struct X();
struct Y();
mod z {
}
"#,
);
}
#[test]
fn remove_unused_braced_glob() {
check_assist(
remove_unused_imports,
r#"
struct X();
struct Y();
mod z {
use super::{*}$0;
}
"#,
r#"
struct X();
struct Y();
mod z {
}
"#,
);
}
#[test]
fn dont_remove_used_glob() {
check_assist_not_applicable(
remove_unused_imports,
r#"
struct X();
struct Y();
mod z {
use super::*$0;
fn f() {
let x = X();
}
}
"#,
);
}
#[test]
fn only_remove_from_selection() {
check_assist(
remove_unused_imports,
r#"
struct X();
struct Y();
mod z {
$0use super::X;$0
use super::Y;
}
mod w {
use super::Y;
}
"#,
r#"
struct X();
struct Y();
mod z {
use super::Y;
}
mod w {
use super::Y;
}
"#,
);
}
#[test]
fn test_several_files() {
check_assist(
remove_unused_imports,
r#"
//- /foo.rs
pub struct X();
pub struct Y();
//- /main.rs
$0use foo::X;
use foo::Y;
$0
mod foo;
mod z {
use crate::foo::X;
}
"#,
r#"
mod foo;
mod z {
use crate::foo::X;
}
"#,
);
}
#[test]
fn use_in_submodule_doesnt_count() {
check_assist(
remove_unused_imports,
r#"
struct X();
mod z {
use super::X$0;
mod w {
use crate::X;
fn f() {
let x = X();
}
}
}
"#,
r#"
struct X();
mod z {
mod w {
use crate::X;
fn f() {
let x = X();
}
}
}
"#,
);
}
#[test]
fn use_in_submodule_file_doesnt_count() {
check_assist(
remove_unused_imports,
r#"
//- /z/foo.rs
use crate::X;
fn f() {
let x = X();
}
//- /main.rs
pub struct X();
mod z {
use crate::X$0;
mod foo;
}
"#,
r#"
pub struct X();
mod z {
mod foo;
}
"#,
);
}
}

View File

@ -157,7 +157,7 @@ fn find_usages(
file_id: FileId,
) -> UsageSearchResult {
let file_range = FileRange { file_id, range: fn_.syntax().text_range() };
type_param_def.usages(sema).in_scope(SearchScope::file_range(file_range)).all()
type_param_def.usages(sema).in_scope(&SearchScope::file_range(file_range)).all()
}
fn check_valid_usages(usages: &UsageSearchResult, param_list_range: TextRange) -> bool {

View File

@ -184,6 +184,7 @@ mod handlers {
mod raw_string;
mod remove_dbg;
mod remove_mut;
mod remove_unused_imports;
mod remove_unused_param;
mod remove_parentheses;
mod reorder_fields;
@ -294,6 +295,7 @@ pub(crate) fn all() -> &'static [Handler] {
raw_string::make_usual_string,
raw_string::remove_hash,
remove_mut::remove_mut,
remove_unused_imports::remove_unused_imports,
remove_unused_param::remove_unused_param,
remove_parentheses::remove_parentheses,
reorder_fields::reorder_fields,

View File

@ -2233,6 +2233,24 @@ fn main() {
)
}
#[test]
fn doctest_remove_unused_imports() {
check_doc_test(
"remove_unused_imports",
r#####"
struct X();
mod foo {
use super::X$0;
}
"#####,
r#####"
struct X();
mod foo {
}
"#####,
)
}
#[test]
fn doctest_remove_unused_param() {
check_doc_test(

View File

@ -127,7 +127,7 @@ fn krate(db: &RootDatabase, of: hir::Crate) -> SearchScope {
}
/// Build a search scope spanning the given module and all its submodules.
fn module_and_children(db: &RootDatabase, module: hir::Module) -> SearchScope {
pub fn module_and_children(db: &RootDatabase, module: hir::Module) -> SearchScope {
let mut entries = IntMap::default();
let (file_id, range) = {
@ -329,7 +329,7 @@ pub fn usages<'a>(self, sema: &'a Semantics<'_, RootDatabase>) -> FindUsages<'a>
pub struct FindUsages<'a> {
def: Definition,
sema: &'a Semantics<'a, RootDatabase>,
scope: Option<SearchScope>,
scope: Option<&'a SearchScope>,
/// The container of our definition should it be an assoc item
assoc_item_container: Option<hir::AssocItemContainer>,
/// whether to search for the `Self` type of the definition
@ -338,7 +338,7 @@ pub struct FindUsages<'a> {
search_self_mod: bool,
}
impl FindUsages<'_> {
impl<'a> FindUsages<'a> {
/// Enable searching for `Self` when the definition is a type or `self` for modules.
pub fn include_self_refs(mut self) -> Self {
self.include_self_kw_refs = def_to_ty(self.sema, &self.def);
@ -347,12 +347,12 @@ pub fn include_self_refs(mut self) -> Self {
}
/// Limit the search to a given [`SearchScope`].
pub fn in_scope(self, scope: SearchScope) -> Self {
pub fn in_scope(self, scope: &'a SearchScope) -> Self {
self.set_scope(Some(scope))
}
/// Limit the search to a given [`SearchScope`].
pub fn set_scope(mut self, scope: Option<SearchScope>) -> Self {
pub fn set_scope(mut self, scope: Option<&'a SearchScope>) -> Self {
assert!(self.scope.is_none());
self.scope = scope;
self
@ -376,7 +376,7 @@ pub fn all(self) -> UsageSearchResult {
res
}
fn search(&self, sink: &mut dyn FnMut(FileId, FileReference) -> bool) {
pub fn search(&self, sink: &mut dyn FnMut(FileId, FileReference) -> bool) {
let _p = profile::span("FindUsages:search");
let sema = self.sema;

View File

@ -121,7 +121,7 @@ fn find_usages<'a>(
// cache miss. This is a limitation of NLL and is fixed with Polonius. For now we do two
// lookups in the case of a cache hit.
if usage_cache.find(&definition).is_none() {
let usages = definition.usages(&self.sema).in_scope(self.search_scope()).all();
let usages = definition.usages(&self.sema).in_scope(&self.search_scope()).all();
usage_cache.usages.push((definition, usages));
return &usage_cache.usages.last().unwrap().1;
}

View File

@ -100,10 +100,7 @@ fn highlight_closure_captures(
.flat_map(|local| {
let usages = Definition::Local(local)
.usages(sema)
.set_scope(Some(SearchScope::file_range(FileRange {
file_id,
range: search_range,
})))
.in_scope(&SearchScope::file_range(FileRange { file_id, range: search_range }))
.include_self_refs()
.all()
.references
@ -139,7 +136,7 @@ fn highlight_references(
.iter()
.filter_map(|&d| {
d.usages(sema)
.set_scope(Some(SearchScope::single_file(file_id)))
.in_scope(&SearchScope::single_file(file_id))
.include_self_refs()
.all()
.references
@ -183,7 +180,7 @@ fn highlight_references(
.filter_map(|item| {
Definition::from(item)
.usages(sema)
.set_scope(Some(SearchScope::file_range(FileRange {
.set_scope(Some(&SearchScope::file_range(FileRange {
file_id,
range: trait_item_use_scope.text_range(),
})))

View File

@ -74,7 +74,7 @@ pub(crate) fn find_all_refs(
}
});
let mut usages =
def.usages(sema).set_scope(search_scope.clone()).include_self_refs().all();
def.usages(sema).set_scope(search_scope.as_ref()).include_self_refs().all();
if literal_search {
retain_adt_literal_usages(&mut usages, def, sema);

View File

@ -232,7 +232,7 @@ fn find_related_tests(
for def in defs {
let defs = def
.usages(sema)
.set_scope(search_scope.clone())
.set_scope(search_scope.as_ref())
.all()
.references
.into_values()

View File

@ -380,6 +380,26 @@ fn remove(&self) {
}
impl ast::UseTree {
/// Deletes the usetree node represented by the input. Recursively removes parents, including use nodes that become empty.
pub fn remove_recursive(self) {
let parent = self.syntax().parent();
self.remove();
if let Some(u) = parent.clone().and_then(ast::Use::cast) {
if u.use_tree().is_none() {
u.remove();
}
} else if let Some(u) = parent.and_then(ast::UseTreeList::cast) {
if u.use_trees().next().is_none() {
let parent = u.syntax().parent().and_then(ast::UseTree::cast);
if let Some(u) = parent {
u.remove_recursive();
}
}
}
}
pub fn get_or_create_use_tree_list(&self) -> ast::UseTreeList {
match self.use_tree_list() {
Some(it) => it,
@ -487,6 +507,22 @@ fn remove(&self) {
}
}
}
let prev_ws = self
.syntax()
.prev_sibling_or_token()
.and_then(|it| it.into_token())
.and_then(ast::Whitespace::cast);
if let Some(prev_ws) = prev_ws {
let ws_text = prev_ws.syntax().text();
let prev_newline = ws_text.rfind('\n').map(|x| x + 1).unwrap_or(0);
let rest = &ws_text[0..prev_newline];
if rest.is_empty() {
ted::remove(prev_ws.syntax());
} else {
ted::replace(prev_ws.syntax(), make::tokens::whitespace(rest));
}
}
ted::remove(self.syntax());
}
}