diff options
author | bors[bot] <bors[bot]@users.noreply.github.com> | 2019-02-09 08:52:09 +0000 |
---|---|---|
committer | bors[bot] <bors[bot]@users.noreply.github.com> | 2019-02-09 08:52:09 +0000 |
commit | 3e8351fb0607f8711749b00d80f68bf25de01a76 (patch) | |
tree | 97388dafe71ececcbaf97249021b9c8d49786ccf /crates/ra_assists/src/lib.rs | |
parent | 12e3b4c70b5ef23b2fdfc197296d483680e125f9 (diff) | |
parent | 4fdeb54bb5c7ba0704839a65996766d223c51fc1 (diff) |
Merge #768
768: Sort assists by the range of the affected element r=matklad a=robojumper
Closes #763.
https://github.com/rust-analyzer/rust-analyzer/blob/3be98f2ac93b278828e76eb813bdd8033f647b12/crates/ra_assists/src/lib.rs#L233-L236
This could be made more robust by a) adding a way to identify actions by things other than their label and b) allowing arbitrary actions to appear in the list as long as the tested actions are there in the correct order. Let me know if I should do any of that.
Co-authored-by: robojumper <[email protected]>
Diffstat (limited to 'crates/ra_assists/src/lib.rs')
-rw-r--r-- | crates/ra_assists/src/lib.rs | 100 |
1 files changed, 96 insertions, 4 deletions
diff --git a/crates/ra_assists/src/lib.rs b/crates/ra_assists/src/lib.rs index 881db6347..7928b4983 100644 --- a/crates/ra_assists/src/lib.rs +++ b/crates/ra_assists/src/lib.rs | |||
@@ -8,7 +8,7 @@ | |||
8 | mod assist_ctx; | 8 | mod assist_ctx; |
9 | 9 | ||
10 | use ra_text_edit::TextEdit; | 10 | use ra_text_edit::TextEdit; |
11 | use ra_syntax::{TextUnit, SyntaxNode, Direction}; | 11 | use ra_syntax::{TextRange, TextUnit, SyntaxNode, Direction}; |
12 | use ra_db::FileRange; | 12 | use ra_db::FileRange; |
13 | use hir::db::HirDatabase; | 13 | use hir::db::HirDatabase; |
14 | 14 | ||
@@ -23,6 +23,7 @@ pub struct AssistLabel { | |||
23 | pub struct AssistAction { | 23 | pub struct AssistAction { |
24 | pub edit: TextEdit, | 24 | pub edit: TextEdit, |
25 | pub cursor_position: Option<TextUnit>, | 25 | pub cursor_position: Option<TextUnit>, |
26 | pub target: Option<TextRange>, | ||
26 | } | 27 | } |
27 | 28 | ||
28 | /// Return all the assists applicable at the given position. | 29 | /// Return all the assists applicable at the given position. |
@@ -53,15 +54,24 @@ pub fn assists<H>(db: &H, range: FileRange) -> Vec<(AssistLabel, AssistAction)> | |||
53 | where | 54 | where |
54 | H: HirDatabase + 'static, | 55 | H: HirDatabase + 'static, |
55 | { | 56 | { |
57 | use std::cmp::Ordering; | ||
58 | |||
56 | AssistCtx::with_ctx(db, range, true, |ctx| { | 59 | AssistCtx::with_ctx(db, range, true, |ctx| { |
57 | all_assists() | 60 | let mut a = all_assists() |
58 | .iter() | 61 | .iter() |
59 | .filter_map(|f| f(ctx.clone())) | 62 | .filter_map(|f| f(ctx.clone())) |
60 | .map(|a| match a { | 63 | .map(|a| match a { |
61 | Assist::Resolved(label, action) => (label, action), | 64 | Assist::Resolved(label, action) => (label, action), |
62 | Assist::Unresolved(..) => unreachable!(), | 65 | Assist::Unresolved(..) => unreachable!(), |
63 | }) | 66 | }) |
64 | .collect() | 67 | .collect::<Vec<(AssistLabel, AssistAction)>>(); |
68 | a.sort_by(|a, b| match (a.1.target, b.1.target) { | ||
69 | (Some(a), Some(b)) => a.len().cmp(&b.len()), | ||
70 | (Some(_), None) => Ordering::Less, | ||
71 | (None, Some(_)) => Ordering::Greater, | ||
72 | (None, None) => Ordering::Equal, | ||
73 | }); | ||
74 | a | ||
65 | }) | 75 | }) |
66 | } | 76 | } |
67 | 77 | ||
@@ -97,7 +107,7 @@ mod helpers { | |||
97 | use hir::mock::MockDatabase; | 107 | use hir::mock::MockDatabase; |
98 | use ra_syntax::TextRange; | 108 | use ra_syntax::TextRange; |
99 | use ra_db::FileRange; | 109 | use ra_db::FileRange; |
100 | use test_utils::{extract_offset, assert_eq_text, add_cursor, extract_range}; | 110 | use test_utils::{extract_offset, extract_range, assert_eq_text, add_cursor}; |
101 | 111 | ||
102 | use crate::{AssistCtx, Assist}; | 112 | use crate::{AssistCtx, Assist}; |
103 | 113 | ||
@@ -151,6 +161,45 @@ mod helpers { | |||
151 | assert_eq_text!(after, &actual); | 161 | assert_eq_text!(after, &actual); |
152 | } | 162 | } |
153 | 163 | ||
164 | pub(crate) fn check_assist_target( | ||
165 | assist: fn(AssistCtx<MockDatabase>) -> Option<Assist>, | ||
166 | before: &str, | ||
167 | target: &str, | ||
168 | ) { | ||
169 | let (before_cursor_pos, before) = extract_offset(before); | ||
170 | let (db, _source_root, file_id) = MockDatabase::with_single_file(&before); | ||
171 | let frange = | ||
172 | FileRange { file_id, range: TextRange::offset_len(before_cursor_pos, 0.into()) }; | ||
173 | let assist = | ||
174 | AssistCtx::with_ctx(&db, frange, true, assist).expect("code action is not applicable"); | ||
175 | let action = match assist { | ||
176 | Assist::Unresolved(_) => unreachable!(), | ||
177 | Assist::Resolved(_, it) => it, | ||
178 | }; | ||
179 | |||
180 | let range = action.target.expect("expected target on action"); | ||
181 | assert_eq_text!(&before[range.start().to_usize()..range.end().to_usize()], target); | ||
182 | } | ||
183 | |||
184 | pub(crate) fn check_assist_range_target( | ||
185 | assist: fn(AssistCtx<MockDatabase>) -> Option<Assist>, | ||
186 | before: &str, | ||
187 | target: &str, | ||
188 | ) { | ||
189 | let (range, before) = extract_range(before); | ||
190 | let (db, _source_root, file_id) = MockDatabase::with_single_file(&before); | ||
191 | let frange = FileRange { file_id, range }; | ||
192 | let assist = | ||
193 | AssistCtx::with_ctx(&db, frange, true, assist).expect("code action is not applicable"); | ||
194 | let action = match assist { | ||
195 | Assist::Unresolved(_) => unreachable!(), | ||
196 | Assist::Resolved(_, it) => it, | ||
197 | }; | ||
198 | |||
199 | let range = action.target.expect("expected target on action"); | ||
200 | assert_eq_text!(&before[range.start().to_usize()..range.end().to_usize()], target); | ||
201 | } | ||
202 | |||
154 | pub(crate) fn check_assist_not_applicable( | 203 | pub(crate) fn check_assist_not_applicable( |
155 | assist: fn(AssistCtx<MockDatabase>) -> Option<Assist>, | 204 | assist: fn(AssistCtx<MockDatabase>) -> Option<Assist>, |
156 | before: &str, | 205 | before: &str, |
@@ -162,5 +211,48 @@ mod helpers { | |||
162 | let assist = AssistCtx::with_ctx(&db, frange, true, assist); | 211 | let assist = AssistCtx::with_ctx(&db, frange, true, assist); |
163 | assert!(assist.is_none()); | 212 | assert!(assist.is_none()); |
164 | } | 213 | } |
214 | } | ||
215 | |||
216 | #[cfg(test)] | ||
217 | mod tests { | ||
218 | use hir::mock::MockDatabase; | ||
219 | use ra_syntax::TextRange; | ||
220 | use ra_db::FileRange; | ||
221 | use test_utils::{extract_offset}; | ||
222 | |||
223 | #[test] | ||
224 | fn assist_order_field_struct() { | ||
225 | let before = "struct Foo { <|>bar: u32 }"; | ||
226 | let (before_cursor_pos, before) = extract_offset(before); | ||
227 | let (db, _source_root, file_id) = MockDatabase::with_single_file(&before); | ||
228 | let frange = | ||
229 | FileRange { file_id, range: TextRange::offset_len(before_cursor_pos, 0.into()) }; | ||
230 | let assists = super::assists(&db, frange); | ||
231 | let mut assists = assists.iter(); | ||
232 | |||
233 | assert_eq!(assists.next().expect("expected assist").0.label, "make pub(crate)"); | ||
234 | assert_eq!(assists.next().expect("expected assist").0.label, "add `#[derive]`"); | ||
235 | } | ||
236 | |||
237 | #[test] | ||
238 | fn assist_order_if_expr() { | ||
239 | let before = " | ||
240 | pub fn test_some_range(a: int) -> bool { | ||
241 | if let 2..6 = 5<|> { | ||
242 | true | ||
243 | } else { | ||
244 | false | ||
245 | } | ||
246 | }"; | ||
247 | let (before_cursor_pos, before) = extract_offset(before); | ||
248 | let (db, _source_root, file_id) = MockDatabase::with_single_file(&before); | ||
249 | let frange = | ||
250 | FileRange { file_id, range: TextRange::offset_len(before_cursor_pos, 0.into()) }; | ||
251 | let assists = super::assists(&db, frange); | ||
252 | let mut assists = assists.iter(); | ||
253 | |||
254 | assert_eq!(assists.next().expect("expected assist").0.label, "introduce variable"); | ||
255 | assert_eq!(assists.next().expect("expected assist").0.label, "replace with match"); | ||
256 | } | ||
165 | 257 | ||
166 | } | 258 | } |