diff options
author | Aleksey Kladov <[email protected]> | 2020-08-19 17:44:33 +0100 |
---|---|---|
committer | Aleksey Kladov <[email protected]> | 2020-08-19 18:40:55 +0100 |
commit | 4b5b55f6f3d1879cd974f290e2f0d92f487acc4b (patch) | |
tree | a51117f4fed220e42ed145b4d82d752545f28d82 /crates | |
parent | f5b7540f388e815b3c4c2fb28b8233c724e0a838 (diff) |
**Remove Unused Parameter** refactoring
Diffstat (limited to 'crates')
-rw-r--r-- | crates/assists/src/handlers/merge_imports.rs | 5 | ||||
-rw-r--r-- | crates/assists/src/handlers/remove_dbg.rs | 3 | ||||
-rw-r--r-- | crates/assists/src/handlers/remove_unused_param.rs | 131 | ||||
-rw-r--r-- | crates/assists/src/lib.rs | 2 | ||||
-rw-r--r-- | crates/assists/src/tests/generated.rs | 21 | ||||
-rw-r--r-- | crates/assists/src/utils.rs | 6 | ||||
-rw-r--r-- | crates/ide_db/src/search.rs | 2 |
7 files changed, 163 insertions, 7 deletions
diff --git a/crates/assists/src/handlers/merge_imports.rs b/crates/assists/src/handlers/merge_imports.rs index 47d465404..35b884206 100644 --- a/crates/assists/src/handlers/merge_imports.rs +++ b/crates/assists/src/handlers/merge_imports.rs | |||
@@ -8,6 +8,7 @@ use syntax::{ | |||
8 | 8 | ||
9 | use crate::{ | 9 | use crate::{ |
10 | assist_context::{AssistContext, Assists}, | 10 | assist_context::{AssistContext, Assists}, |
11 | utils::next_prev, | ||
11 | AssistId, AssistKind, | 12 | AssistId, AssistKind, |
12 | }; | 13 | }; |
13 | 14 | ||
@@ -66,10 +67,6 @@ pub(crate) fn merge_imports(acc: &mut Assists, ctx: &AssistContext) -> Option<() | |||
66 | ) | 67 | ) |
67 | } | 68 | } |
68 | 69 | ||
69 | fn next_prev() -> impl Iterator<Item = Direction> { | ||
70 | [Direction::Next, Direction::Prev].iter().copied() | ||
71 | } | ||
72 | |||
73 | fn try_merge_trees(old: &ast::UseTree, new: &ast::UseTree) -> Option<ast::UseTree> { | 70 | fn try_merge_trees(old: &ast::UseTree, new: &ast::UseTree) -> Option<ast::UseTree> { |
74 | let lhs_path = old.path()?; | 71 | let lhs_path = old.path()?; |
75 | let rhs_path = new.path()?; | 72 | let rhs_path = new.path()?; |
diff --git a/crates/assists/src/handlers/remove_dbg.rs b/crates/assists/src/handlers/remove_dbg.rs index f3dcca534..4e252edf0 100644 --- a/crates/assists/src/handlers/remove_dbg.rs +++ b/crates/assists/src/handlers/remove_dbg.rs | |||
@@ -82,9 +82,10 @@ fn is_valid_macrocall(macro_call: &ast::MacroCall, macro_name: &str) -> Option<b | |||
82 | 82 | ||
83 | #[cfg(test)] | 83 | #[cfg(test)] |
84 | mod tests { | 84 | mod tests { |
85 | use super::*; | ||
86 | use crate::tests::{check_assist, check_assist_not_applicable, check_assist_target}; | 85 | use crate::tests::{check_assist, check_assist_not_applicable, check_assist_target}; |
87 | 86 | ||
87 | use super::*; | ||
88 | |||
88 | #[test] | 89 | #[test] |
89 | fn test_remove_dbg() { | 90 | fn test_remove_dbg() { |
90 | check_assist(remove_dbg, "<|>dbg!(1 + 1)", "1 + 1"); | 91 | check_assist(remove_dbg, "<|>dbg!(1 + 1)", "1 + 1"); |
diff --git a/crates/assists/src/handlers/remove_unused_param.rs b/crates/assists/src/handlers/remove_unused_param.rs new file mode 100644 index 000000000..5fccca54b --- /dev/null +++ b/crates/assists/src/handlers/remove_unused_param.rs | |||
@@ -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 | } | ||
diff --git a/crates/assists/src/lib.rs b/crates/assists/src/lib.rs index 14834480a..2e0d191a6 100644 --- a/crates/assists/src/lib.rs +++ b/crates/assists/src/lib.rs | |||
@@ -152,6 +152,7 @@ mod handlers { | |||
152 | mod raw_string; | 152 | mod raw_string; |
153 | mod remove_dbg; | 153 | mod remove_dbg; |
154 | mod remove_mut; | 154 | mod remove_mut; |
155 | mod remove_unused_param; | ||
155 | mod reorder_fields; | 156 | mod reorder_fields; |
156 | mod replace_if_let_with_match; | 157 | mod replace_if_let_with_match; |
157 | mod replace_let_with_if_let; | 158 | mod replace_let_with_if_let; |
@@ -198,6 +199,7 @@ mod handlers { | |||
198 | raw_string::remove_hash, | 199 | raw_string::remove_hash, |
199 | remove_dbg::remove_dbg, | 200 | remove_dbg::remove_dbg, |
200 | remove_mut::remove_mut, | 201 | remove_mut::remove_mut, |
202 | remove_unused_param::remove_unused_param, | ||
201 | reorder_fields::reorder_fields, | 203 | reorder_fields::reorder_fields, |
202 | replace_if_let_with_match::replace_if_let_with_match, | 204 | replace_if_let_with_match::replace_if_let_with_match, |
203 | replace_let_with_if_let::replace_let_with_if_let, | 205 | replace_let_with_if_let::replace_let_with_if_let, |
diff --git a/crates/assists/src/tests/generated.rs b/crates/assists/src/tests/generated.rs index 173567003..04c8fd1f9 100644 --- a/crates/assists/src/tests/generated.rs +++ b/crates/assists/src/tests/generated.rs | |||
@@ -751,6 +751,27 @@ impl Walrus { | |||
751 | } | 751 | } |
752 | 752 | ||
753 | #[test] | 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 | |||
774 | #[test] | ||
754 | fn doctest_reorder_fields() { | 775 | fn doctest_reorder_fields() { |
755 | check_doc_test( | 776 | check_doc_test( |
756 | "reorder_fields", | 777 | "reorder_fields", |
diff --git a/crates/assists/src/utils.rs b/crates/assists/src/utils.rs index 84ccacafe..d071d6502 100644 --- a/crates/assists/src/utils.rs +++ b/crates/assists/src/utils.rs | |||
@@ -9,7 +9,7 @@ use itertools::Itertools; | |||
9 | use rustc_hash::FxHashSet; | 9 | use rustc_hash::FxHashSet; |
10 | use syntax::{ | 10 | use syntax::{ |
11 | ast::{self, make, NameOwner}, | 11 | ast::{self, make, NameOwner}, |
12 | AstNode, | 12 | AstNode, Direction, |
13 | SyntaxKind::*, | 13 | SyntaxKind::*, |
14 | SyntaxNode, TextSize, T, | 14 | SyntaxNode, TextSize, T, |
15 | }; | 15 | }; |
@@ -311,3 +311,7 @@ pub use prelude::*; | |||
311 | Some(def) | 311 | Some(def) |
312 | } | 312 | } |
313 | } | 313 | } |
314 | |||
315 | pub(crate) fn next_prev() -> impl Iterator<Item = Direction> { | ||
316 | [Direction::Next, Direction::Prev].iter().copied() | ||
317 | } | ||
diff --git a/crates/ide_db/src/search.rs b/crates/ide_db/src/search.rs index ce7631c69..fa0830b23 100644 --- a/crates/ide_db/src/search.rs +++ b/crates/ide_db/src/search.rs | |||
@@ -203,7 +203,7 @@ impl<'a> FindUsages<'a> { | |||
203 | } | 203 | } |
204 | 204 | ||
205 | pub fn at_least_one(self) -> bool { | 205 | pub fn at_least_one(self) -> bool { |
206 | self.all().is_empty() | 206 | !self.all().is_empty() |
207 | } | 207 | } |
208 | 208 | ||
209 | pub fn all(self) -> Vec<Reference> { | 209 | pub fn all(self) -> Vec<Reference> { |