Skip to content

Commit bb07e6b

Browse files
bors[bot]matklad
andauthored
Merge #5821
5821: **Remove Unused Parameter** refactoring r=matklad a=matklad bors r+ 🤖 Co-authored-by: Aleksey Kladov <[email protected]>
2 parents f5b7540 + 4b5b55f commit bb07e6b

File tree

7 files changed

+163
-7
lines changed

7 files changed

+163
-7
lines changed

crates/assists/src/handlers/merge_imports.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use syntax::{
88

99
use crate::{
1010
assist_context::{AssistContext, Assists},
11+
utils::next_prev,
1112
AssistId, AssistKind,
1213
};
1314

@@ -66,10 +67,6 @@ pub(crate) fn merge_imports(acc: &mut Assists, ctx: &AssistContext) -> Option<()
6667
)
6768
}
6869

69-
fn next_prev() -> impl Iterator<Item = Direction> {
70-
[Direction::Next, Direction::Prev].iter().copied()
71-
}
72-
7370
fn try_merge_trees(old: &ast::UseTree, new: &ast::UseTree) -> Option<ast::UseTree> {
7471
let lhs_path = old.path()?;
7572
let rhs_path = new.path()?;

crates/assists/src/handlers/remove_dbg.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,9 +82,10 @@ fn is_valid_macrocall(macro_call: &ast::MacroCall, macro_name: &str) -> Option<b
8282

8383
#[cfg(test)]
8484
mod tests {
85-
use super::*;
8685
use crate::tests::{check_assist, check_assist_not_applicable, check_assist_target};
8786

87+
use super::*;
88+
8889
#[test]
8990
fn test_remove_dbg() {
9091
check_assist(remove_dbg, "<|>dbg!(1 + 1)", "1 + 1");
Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
use ide_db::{defs::Definition, search::Reference};
2+
use syntax::{
3+
algo::find_node_at_range,
4+
ast::{self, ArgListOwner},
5+
AstNode, SyntaxNode, TextRange, T,
6+
};
7+
use test_utils::mark;
8+
9+
use crate::{
10+
assist_context::AssistBuilder, utils::next_prev, AssistContext, AssistId, AssistKind, Assists,
11+
};
12+
13+
// Assist: remove_unused_param
14+
//
15+
// Removes unused function parameter.
16+
//
17+
// ```
18+
// fn frobnicate(x: i32<|>) {}
19+
//
20+
// fn main() {
21+
// frobnicate(92);
22+
// }
23+
// ```
24+
// ->
25+
// ```
26+
// fn frobnicate() {}
27+
//
28+
// fn main() {
29+
// frobnicate();
30+
// }
31+
// ```
32+
pub(crate) fn remove_unused_param(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
33+
let param: ast::Param = ctx.find_node_at_offset()?;
34+
let ident_pat = match param.pat()? {
35+
ast::Pat::IdentPat(it) => it,
36+
_ => return None,
37+
};
38+
let func = param.syntax().ancestors().find_map(ast::Fn::cast)?;
39+
let param_position = func.param_list()?.params().position(|it| it == param)?;
40+
41+
let fn_def = {
42+
let func = ctx.sema.to_def(&func)?;
43+
Definition::ModuleDef(func.into())
44+
};
45+
46+
let param_def = {
47+
let local = ctx.sema.to_def(&ident_pat)?;
48+
Definition::Local(local)
49+
};
50+
if param_def.usages(&ctx.sema).at_least_one() {
51+
mark::hit!(keep_used);
52+
return None;
53+
}
54+
acc.add(
55+
AssistId("remove_unused_param", AssistKind::Refactor),
56+
"Remove unused parameter",
57+
param.syntax().text_range(),
58+
|builder| {
59+
builder.delete(range_with_coma(param.syntax()));
60+
for usage in fn_def.usages(&ctx.sema).all() {
61+
process_usage(ctx, builder, usage, param_position);
62+
}
63+
},
64+
)
65+
}
66+
67+
fn process_usage(
68+
ctx: &AssistContext,
69+
builder: &mut AssistBuilder,
70+
usage: Reference,
71+
arg_to_remove: usize,
72+
) -> Option<()> {
73+
let source_file = ctx.sema.parse(usage.file_range.file_id);
74+
let call_expr: ast::CallExpr =
75+
find_node_at_range(source_file.syntax(), usage.file_range.range)?;
76+
if call_expr.expr()?.syntax().text_range() != usage.file_range.range {
77+
return None;
78+
}
79+
let arg = call_expr.arg_list()?.args().nth(arg_to_remove)?;
80+
81+
builder.edit_file(usage.file_range.file_id);
82+
builder.delete(range_with_coma(arg.syntax()));
83+
84+
Some(())
85+
}
86+
87+
fn range_with_coma(node: &SyntaxNode) -> TextRange {
88+
let up_to = next_prev().find_map(|dir| {
89+
node.siblings_with_tokens(dir)
90+
.filter_map(|it| it.into_token())
91+
.find(|it| it.kind() == T![,])
92+
});
93+
let up_to = up_to.map_or(node.text_range(), |it| it.text_range());
94+
node.text_range().cover(up_to)
95+
}
96+
97+
#[cfg(test)]
98+
mod tests {
99+
use crate::tests::{check_assist, check_assist_not_applicable};
100+
101+
use super::*;
102+
103+
#[test]
104+
fn remove_unused() {
105+
check_assist(
106+
remove_unused_param,
107+
r#"
108+
fn a() { foo(9, 2) }
109+
fn foo(x: i32, <|>y: i32) { x; }
110+
fn b() { foo(9, 2,) }
111+
"#,
112+
r#"
113+
fn a() { foo(9) }
114+
fn foo(x: i32) { x; }
115+
fn b() { foo(9, ) }
116+
"#,
117+
);
118+
}
119+
120+
#[test]
121+
fn keep_used() {
122+
mark::check!(keep_used);
123+
check_assist_not_applicable(
124+
remove_unused_param,
125+
r#"
126+
fn foo(x: i32, <|>y: i32) { y; }
127+
fn main() { foo(9, 2) }
128+
"#,
129+
);
130+
}
131+
}

crates/assists/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,7 @@ mod handlers {
152152
mod raw_string;
153153
mod remove_dbg;
154154
mod remove_mut;
155+
mod remove_unused_param;
155156
mod reorder_fields;
156157
mod replace_if_let_with_match;
157158
mod replace_let_with_if_let;
@@ -198,6 +199,7 @@ mod handlers {
198199
raw_string::remove_hash,
199200
remove_dbg::remove_dbg,
200201
remove_mut::remove_mut,
202+
remove_unused_param::remove_unused_param,
201203
reorder_fields::reorder_fields,
202204
replace_if_let_with_match::replace_if_let_with_match,
203205
replace_let_with_if_let::replace_let_with_if_let,

crates/assists/src/tests/generated.rs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -750,6 +750,27 @@ impl Walrus {
750750
)
751751
}
752752

753+
#[test]
754+
fn doctest_remove_unused_param() {
755+
check_doc_test(
756+
"remove_unused_param",
757+
r#####"
758+
fn frobnicate(x: i32<|>) {}
759+
760+
fn main() {
761+
frobnicate(92);
762+
}
763+
"#####,
764+
r#####"
765+
fn frobnicate() {}
766+
767+
fn main() {
768+
frobnicate();
769+
}
770+
"#####,
771+
)
772+
}
773+
753774
#[test]
754775
fn doctest_reorder_fields() {
755776
check_doc_test(

crates/assists/src/utils.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use itertools::Itertools;
99
use rustc_hash::FxHashSet;
1010
use syntax::{
1111
ast::{self, make, NameOwner},
12-
AstNode,
12+
AstNode, Direction,
1313
SyntaxKind::*,
1414
SyntaxNode, TextSize, T,
1515
};
@@ -311,3 +311,7 @@ pub use prelude::*;
311311
Some(def)
312312
}
313313
}
314+
315+
pub(crate) fn next_prev() -> impl Iterator<Item = Direction> {
316+
[Direction::Next, Direction::Prev].iter().copied()
317+
}

crates/ide_db/src/search.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ impl<'a> FindUsages<'a> {
203203
}
204204

205205
pub fn at_least_one(self) -> bool {
206-
self.all().is_empty()
206+
!self.all().is_empty()
207207
}
208208

209209
pub fn all(self) -> Vec<Reference> {

0 commit comments

Comments
 (0)