diff options
author | bors[bot] <26634292+bors[bot]@users.noreply.github.com> | 2020-09-11 14:41:15 +0100 |
---|---|---|
committer | GitHub <[email protected]> | 2020-09-11 14:41:15 +0100 |
commit | 87c8dfcadd42f3ca85a8f1cb22679266415a3596 (patch) | |
tree | ef464426799fe6adb2d38270f7f3a8062aa81c34 /crates/assists/src/handlers | |
parent | 4f1167d8ddac9d392f034341e6bc032b51014918 (diff) | |
parent | b477f99bd9d6ae81001451cc310efe0ac950eb61 (diff) |
Merge #5970
5970: Use better heuristics for replacement text when removing dbg! r=jonas-schievink a=SomeoneToIgnore
Closes https://github.com/rust-analyzer/rust-analyzer/issues/5911
Co-authored-by: Kirill Bulatov <[email protected]>
Diffstat (limited to 'crates/assists/src/handlers')
-rw-r--r-- | crates/assists/src/handlers/remove_dbg.rs | 155 |
1 files changed, 118 insertions, 37 deletions
diff --git a/crates/assists/src/handlers/remove_dbg.rs b/crates/assists/src/handlers/remove_dbg.rs index 4e252edf0..0b581dc22 100644 --- a/crates/assists/src/handlers/remove_dbg.rs +++ b/crates/assists/src/handlers/remove_dbg.rs | |||
@@ -1,6 +1,6 @@ | |||
1 | use syntax::{ | 1 | use syntax::{ |
2 | ast::{self, AstNode}, | 2 | ast::{self, AstNode}, |
3 | TextRange, TextSize, T, | 3 | SyntaxElement, TextRange, TextSize, T, |
4 | }; | 4 | }; |
5 | 5 | ||
6 | use crate::{AssistContext, AssistId, AssistKind, Assists}; | 6 | use crate::{AssistContext, AssistId, AssistKind, Assists}; |
@@ -22,62 +22,118 @@ use crate::{AssistContext, AssistId, AssistKind, Assists}; | |||
22 | // ``` | 22 | // ``` |
23 | pub(crate) fn remove_dbg(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { | 23 | pub(crate) fn remove_dbg(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { |
24 | let macro_call = ctx.find_node_at_offset::<ast::MacroCall>()?; | 24 | let macro_call = ctx.find_node_at_offset::<ast::MacroCall>()?; |
25 | let new_contents = adjusted_macro_contents(¯o_call)?; | ||
25 | 26 | ||
26 | if !is_valid_macrocall(¯o_call, "dbg")? { | 27 | let macro_text_range = macro_call.syntax().text_range(); |
27 | return None; | ||
28 | } | ||
29 | |||
30 | let is_leaf = macro_call.syntax().next_sibling().is_none(); | ||
31 | |||
32 | let macro_end = if macro_call.semicolon_token().is_some() { | 28 | let macro_end = if macro_call.semicolon_token().is_some() { |
33 | macro_call.syntax().text_range().end() - TextSize::of(';') | 29 | macro_text_range.end() - TextSize::of(';') |
34 | } else { | 30 | } else { |
35 | macro_call.syntax().text_range().end() | 31 | macro_text_range.end() |
36 | }; | 32 | }; |
37 | 33 | ||
38 | // macro_range determines what will be deleted and replaced with macro_content | 34 | acc.add( |
39 | let macro_range = TextRange::new(macro_call.syntax().text_range().start(), macro_end); | 35 | AssistId("remove_dbg", AssistKind::Refactor), |
40 | let paste_instead_of_dbg = { | 36 | "Remove dbg!()", |
41 | let text = macro_call.token_tree()?.syntax().text(); | 37 | macro_text_range, |
42 | 38 | |builder| { | |
43 | // leafiness determines if we should include the parenthesis or not | 39 | builder.replace(TextRange::new(macro_text_range.start(), macro_end), new_contents); |
44 | let slice_index: TextRange = if is_leaf { | 40 | }, |
45 | // leaf means - we can extract the contents of the dbg! in text | 41 | ) |
46 | TextRange::new(TextSize::of('('), text.len() - TextSize::of(')')) | 42 | } |
47 | } else { | ||
48 | // not leaf - means we should keep the parens | ||
49 | TextRange::up_to(text.len()) | ||
50 | }; | ||
51 | text.slice(slice_index).to_string() | ||
52 | }; | ||
53 | 43 | ||
54 | let target = macro_call.syntax().text_range(); | 44 | fn adjusted_macro_contents(macro_call: &ast::MacroCall) -> Option<String> { |
55 | acc.add(AssistId("remove_dbg", AssistKind::Refactor), "Remove dbg!()", target, |builder| { | 45 | let contents = get_valid_macrocall_contents(¯o_call, "dbg")?; |
56 | builder.replace(macro_range, paste_instead_of_dbg); | 46 | let is_leaf = macro_call.syntax().next_sibling().is_none(); |
57 | }) | 47 | let macro_text_with_brackets = macro_call.token_tree()?.syntax().text(); |
48 | let slice_index = if is_leaf || !needs_parentheses_around_macro_contents(contents) { | ||
49 | TextRange::new(TextSize::of('('), macro_text_with_brackets.len() - TextSize::of(')')) | ||
50 | } else { | ||
51 | // leave parenthesis around macro contents to preserve the semantics | ||
52 | TextRange::up_to(macro_text_with_brackets.len()) | ||
53 | }; | ||
54 | Some(macro_text_with_brackets.slice(slice_index).to_string()) | ||
58 | } | 55 | } |
59 | 56 | ||
60 | /// Verifies that the given macro_call actually matches the given name | 57 | /// Verifies that the given macro_call actually matches the given name |
61 | /// and contains proper ending tokens | 58 | /// and contains proper ending tokens, then returns the contents between the ending tokens |
62 | fn is_valid_macrocall(macro_call: &ast::MacroCall, macro_name: &str) -> Option<bool> { | 59 | fn get_valid_macrocall_contents( |
60 | macro_call: &ast::MacroCall, | ||
61 | macro_name: &str, | ||
62 | ) -> Option<Vec<SyntaxElement>> { | ||
63 | let path = macro_call.path()?; | 63 | let path = macro_call.path()?; |
64 | let name_ref = path.segment()?.name_ref()?; | 64 | let name_ref = path.segment()?.name_ref()?; |
65 | 65 | ||
66 | // Make sure it is actually a dbg-macro call, dbg followed by ! | 66 | // Make sure it is actually a dbg-macro call, dbg followed by ! |
67 | let excl = path.syntax().next_sibling_or_token()?; | 67 | let excl = path.syntax().next_sibling_or_token()?; |
68 | |||
69 | if name_ref.text() != macro_name || excl.kind() != T![!] { | 68 | if name_ref.text() != macro_name || excl.kind() != T![!] { |
70 | return None; | 69 | return None; |
71 | } | 70 | } |
72 | 71 | ||
73 | let node = macro_call.token_tree()?.syntax().clone(); | 72 | let mut children_with_tokens = macro_call.token_tree()?.syntax().children_with_tokens(); |
74 | let first_child = node.first_child_or_token()?; | 73 | let first_child = children_with_tokens.next()?; |
75 | let last_child = node.last_child_or_token()?; | 74 | let mut contents_between_brackets = children_with_tokens.collect::<Vec<_>>(); |
75 | let last_child = contents_between_brackets.pop()?; | ||
76 | |||
77 | if contents_between_brackets.is_empty() { | ||
78 | None | ||
79 | } else { | ||
80 | match (first_child.kind(), last_child.kind()) { | ||
81 | (T!['('], T![')']) | (T!['['], T![']']) | (T!['{'], T!['}']) => { | ||
82 | Some(contents_between_brackets) | ||
83 | } | ||
84 | _ => None, | ||
85 | } | ||
86 | } | ||
87 | } | ||
88 | |||
89 | fn needs_parentheses_around_macro_contents(macro_contents: Vec<SyntaxElement>) -> bool { | ||
90 | if macro_contents.len() < 2 { | ||
91 | return false; | ||
92 | } | ||
93 | |||
94 | let mut macro_contents_kind_not_in_brackets = Vec::with_capacity(macro_contents.len()); | ||
76 | 95 | ||
77 | match (first_child.kind(), last_child.kind()) { | 96 | let mut first_bracket_in_macro = None; |
78 | (T!['('], T![')']) | (T!['['], T![']']) | (T!['{'], T!['}']) => Some(true), | 97 | let mut unpaired_brackets_in_contents = Vec::new(); |
79 | _ => Some(false), | 98 | for element in macro_contents { |
99 | match element.kind() { | ||
100 | T!['('] | T!['['] | T!['{'] => { | ||
101 | if let None = first_bracket_in_macro { | ||
102 | first_bracket_in_macro = Some(element.clone()) | ||
103 | } | ||
104 | unpaired_brackets_in_contents.push(element); | ||
105 | } | ||
106 | T![')'] => { | ||
107 | if !matches!(unpaired_brackets_in_contents.pop(), Some(correct_bracket) if correct_bracket.kind() == T!['(']) | ||
108 | { | ||
109 | return true; | ||
110 | } | ||
111 | } | ||
112 | T![']'] => { | ||
113 | if !matches!(unpaired_brackets_in_contents.pop(), Some(correct_bracket) if correct_bracket.kind() == T!['[']) | ||
114 | { | ||
115 | return true; | ||
116 | } | ||
117 | } | ||
118 | T!['}'] => { | ||
119 | if !matches!(unpaired_brackets_in_contents.pop(), Some(correct_bracket) if correct_bracket.kind() == T!['{']) | ||
120 | { | ||
121 | return true; | ||
122 | } | ||
123 | } | ||
124 | other_kind => { | ||
125 | if unpaired_brackets_in_contents.is_empty() { | ||
126 | macro_contents_kind_not_in_brackets.push(other_kind); | ||
127 | } | ||
128 | } | ||
129 | } | ||
80 | } | 130 | } |
131 | |||
132 | !unpaired_brackets_in_contents.is_empty() | ||
133 | || matches!(first_bracket_in_macro, Some(bracket) if bracket.kind() != T!['(']) | ||
134 | || macro_contents_kind_not_in_brackets | ||
135 | .into_iter() | ||
136 | .any(|macro_contents_kind| macro_contents_kind.is_punct()) | ||
81 | } | 137 | } |
82 | 138 | ||
83 | #[cfg(test)] | 139 | #[cfg(test)] |
@@ -157,12 +213,37 @@ fn foo(n: usize) { | |||
157 | } | 213 | } |
158 | 214 | ||
159 | #[test] | 215 | #[test] |
216 | fn remove_dbg_from_non_leaf_simple_expression() { | ||
217 | check_assist( | ||
218 | remove_dbg, | ||
219 | " | ||
220 | fn main() { | ||
221 | let mut a = 1; | ||
222 | while dbg!<|>(a) < 10000 { | ||
223 | a += 1; | ||
224 | } | ||
225 | } | ||
226 | ", | ||
227 | " | ||
228 | fn main() { | ||
229 | let mut a = 1; | ||
230 | while a < 10000 { | ||
231 | a += 1; | ||
232 | } | ||
233 | } | ||
234 | ", | ||
235 | ); | ||
236 | } | ||
237 | |||
238 | #[test] | ||
160 | fn test_remove_dbg_keep_expression() { | 239 | fn test_remove_dbg_keep_expression() { |
161 | check_assist( | 240 | check_assist( |
162 | remove_dbg, | 241 | remove_dbg, |
163 | r#"let res = <|>dbg!(a + b).foo();"#, | 242 | r#"let res = <|>dbg!(a + b).foo();"#, |
164 | r#"let res = (a + b).foo();"#, | 243 | r#"let res = (a + b).foo();"#, |
165 | ); | 244 | ); |
245 | |||
246 | check_assist(remove_dbg, r#"let res = <|>dbg!(2 + 2) * 5"#, r#"let res = (2 + 2) * 5"#); | ||
166 | } | 247 | } |
167 | 248 | ||
168 | #[test] | 249 | #[test] |