aboutsummaryrefslogtreecommitdiff
path: root/crates/ra_assists
diff options
context:
space:
mode:
authorbors[bot] <26634292+bors[bot]@users.noreply.github.com>2020-01-27 13:04:53 +0000
committerGitHub <[email protected]>2020-01-27 13:04:53 +0000
commit4f9506416c95bbf4fbd46c6dafb13bb30bb2feee (patch)
tree38e8d121c8a9790d2e3f2ab83c8dcb0a778681cd /crates/ra_assists
parenta108f22d835c8d67c93c66758d89371fff179b9a (diff)
parent9be1ab7ff948d89334a8acbc309c8235d4ab374f (diff)
Merge #2887
2887: Initial auto import action implementation r=matklad a=SomeoneToIgnore Closes https://github.com/rust-analyzer/rust-analyzer/issues/2180 Adds an auto import action implementation. This implementation is not ideal and has a few limitations: * The import search functionality should be moved into a separate crate accessible from ra_assists. This requires a lot of changes and a preliminary design. Currently the functionality is provided as a trait impl, more on that here: https://github.com/rust-analyzer/rust-analyzer/issues/2180#issuecomment-575690942 * Due to the design desicion from the previous item, no doctests are run for the new aciton (look for a new FIXME in the PR) * For the same reason, I have to create the mock trait implementaion to test the assist * Ideally, I think we should have this feature as a diagnostics (that detects an absense of an import) that has a corresponding quickfix action that gets evaluated on demand. Curretly we perform the import search every time we resolve the import which looks suboptimal. This requires `classify_name_ref` to be moved from ra_ide, so not done currently. A few improvements to the imports mechanism to be considered later: * Constants like `ra_syntax::SyntaxKind::NAME` are not imported, because they are not present in the database * Method usages are not imported, they are found in the database, but `find_use_path` does not return any import paths for them * Some import paths returned by the `find_use_path` method end up in `core::` or `alloc::` instead of `std:`, for example: `core::fmt::Debug` instead of `std::fmt::Debug`. This is not an error techically, but still looks weird. * No detection of cases where a trait should be imported in order to be able to call a method * Improve `auto_import_text_edit` functionality: refactor it and move away from the place it is now, add better logic for merging the new import with already existing imports Co-authored-by: Kirill Bulatov <[email protected]>
Diffstat (limited to 'crates/ra_assists')
-rw-r--r--crates/ra_assists/src/assist_ctx.rs2
-rw-r--r--crates/ra_assists/src/assists/auto_import.rs222
-rw-r--r--crates/ra_assists/src/doc_tests.rs4
-rw-r--r--crates/ra_assists/src/doc_tests/generated.rs19
-rw-r--r--crates/ra_assists/src/lib.rs177
5 files changed, 409 insertions, 15 deletions
diff --git a/crates/ra_assists/src/assist_ctx.rs b/crates/ra_assists/src/assist_ctx.rs
index 43f0d664b..2ab65ab99 100644
--- a/crates/ra_assists/src/assist_ctx.rs
+++ b/crates/ra_assists/src/assist_ctx.rs
@@ -101,7 +101,6 @@ impl<'a, DB: HirDatabase> AssistCtx<'a, DB> {
101 Some(assist) 101 Some(assist)
102 } 102 }
103 103
104 #[allow(dead_code)] // will be used for auto import assist with multiple actions
105 pub(crate) fn add_assist_group( 104 pub(crate) fn add_assist_group(
106 self, 105 self,
107 id: AssistId, 106 id: AssistId,
@@ -168,7 +167,6 @@ pub(crate) struct ActionBuilder {
168} 167}
169 168
170impl ActionBuilder { 169impl ActionBuilder {
171 #[allow(dead_code)]
172 /// Adds a custom label to the action, if it needs to be different from the assist label 170 /// Adds a custom label to the action, if it needs to be different from the assist label
173 pub(crate) fn label(&mut self, label: impl Into<String>) { 171 pub(crate) fn label(&mut self, label: impl Into<String>) {
174 self.label = Some(label.into()) 172 self.label = Some(label.into())
diff --git a/crates/ra_assists/src/assists/auto_import.rs b/crates/ra_assists/src/assists/auto_import.rs
new file mode 100644
index 000000000..9163cc662
--- /dev/null
+++ b/crates/ra_assists/src/assists/auto_import.rs
@@ -0,0 +1,222 @@
1use hir::db::HirDatabase;
2use ra_syntax::{
3 ast::{self, AstNode},
4 SmolStr, SyntaxElement,
5 SyntaxKind::{NAME_REF, USE_ITEM},
6 SyntaxNode,
7};
8
9use crate::{
10 assist_ctx::{ActionBuilder, Assist, AssistCtx},
11 auto_import_text_edit, AssistId, ImportsLocator,
12};
13
14// Assist: auto_import
15//
16// If the name is unresolved, provides all possible imports for it.
17//
18// ```
19// fn main() {
20// let map = HashMap<|>::new();
21// }
22// ```
23// ->
24// ```
25// use std::collections::HashMap;
26//
27// fn main() {
28// let map = HashMap<|>::new();
29// }
30// ```
31pub(crate) fn auto_import<F: ImportsLocator>(
32 ctx: AssistCtx<impl HirDatabase>,
33 imports_locator: &mut F,
34) -> Option<Assist> {
35 let path: ast::Path = ctx.find_node_at_offset()?;
36 let module = path.syntax().ancestors().find_map(ast::Module::cast);
37 let position = match module.and_then(|it| it.item_list()) {
38 Some(item_list) => item_list.syntax().clone(),
39 None => {
40 let current_file = path.syntax().ancestors().find_map(ast::SourceFile::cast)?;
41 current_file.syntax().clone()
42 }
43 };
44 let source_analyzer = ctx.source_analyzer(&position, None);
45 let module_with_name_to_import = source_analyzer.module()?;
46 let path_to_import = ctx.covering_element().ancestors().find_map(ast::Path::cast)?;
47 if source_analyzer.resolve_path(ctx.db, &path_to_import).is_some() {
48 return None;
49 }
50
51 let name_to_import = &find_applicable_name_ref(ctx.covering_element())?.syntax().to_string();
52 let proposed_imports = imports_locator
53 .find_imports(&name_to_import.to_string())
54 .into_iter()
55 .filter_map(|module_def| module_with_name_to_import.find_use_path(ctx.db, module_def))
56 .filter(|use_path| !use_path.segments.is_empty())
57 .take(20)
58 .map(|import| import.to_string())
59 .collect::<std::collections::BTreeSet<_>>();
60 if proposed_imports.is_empty() {
61 return None;
62 }
63
64 ctx.add_assist_group(AssistId("auto_import"), "auto import", || {
65 proposed_imports
66 .into_iter()
67 .map(|import| import_to_action(import, &position, &path_to_import.syntax()))
68 .collect()
69 })
70}
71
72fn find_applicable_name_ref(element: SyntaxElement) -> Option<ast::NameRef> {
73 if element.ancestors().find(|ancestor| ancestor.kind() == USE_ITEM).is_some() {
74 None
75 } else if element.kind() == NAME_REF {
76 Some(element.as_node().cloned().and_then(ast::NameRef::cast)?)
77 } else {
78 let parent = element.parent()?;
79 if parent.kind() == NAME_REF {
80 Some(ast::NameRef::cast(parent)?)
81 } else {
82 None
83 }
84 }
85}
86
87fn import_to_action(import: String, position: &SyntaxNode, anchor: &SyntaxNode) -> ActionBuilder {
88 let mut action_builder = ActionBuilder::default();
89 action_builder.label(format!("Import `{}`", &import));
90 auto_import_text_edit(
91 position,
92 anchor,
93 &[SmolStr::new(import)],
94 action_builder.text_edit_builder(),
95 );
96 action_builder
97}
98
99#[cfg(test)]
100mod tests {
101 use super::*;
102 use crate::helpers::{
103 check_assist_with_imports_locator, check_assist_with_imports_locator_not_applicable,
104 TestImportsLocator,
105 };
106
107 #[test]
108 fn applicable_when_found_an_import() {
109 check_assist_with_imports_locator(
110 auto_import,
111 TestImportsLocator::new,
112 r"
113 PubStruct<|>
114
115 pub mod PubMod {
116 pub struct PubStruct;
117 }
118 ",
119 r"
120 use PubMod::PubStruct;
121
122 PubStruct<|>
123
124 pub mod PubMod {
125 pub struct PubStruct;
126 }
127 ",
128 );
129 }
130
131 #[test]
132 fn applicable_when_found_multiple_imports() {
133 check_assist_with_imports_locator(
134 auto_import,
135 TestImportsLocator::new,
136 r"
137 PubStruct<|>
138
139 pub mod PubMod1 {
140 pub struct PubStruct;
141 }
142 pub mod PubMod2 {
143 pub struct PubStruct;
144 }
145 pub mod PubMod3 {
146 pub struct PubStruct;
147 }
148 ",
149 r"
150 use PubMod1::PubStruct;
151
152 PubStruct<|>
153
154 pub mod PubMod1 {
155 pub struct PubStruct;
156 }
157 pub mod PubMod2 {
158 pub struct PubStruct;
159 }
160 pub mod PubMod3 {
161 pub struct PubStruct;
162 }
163 ",
164 );
165 }
166
167 #[test]
168 fn not_applicable_for_already_imported_types() {
169 check_assist_with_imports_locator_not_applicable(
170 auto_import,
171 TestImportsLocator::new,
172 r"
173 use PubMod::PubStruct;
174
175 PubStruct<|>
176
177 pub mod PubMod {
178 pub struct PubStruct;
179 }
180 ",
181 );
182 }
183
184 #[test]
185 fn not_applicable_for_types_with_private_paths() {
186 check_assist_with_imports_locator_not_applicable(
187 auto_import,
188 TestImportsLocator::new,
189 r"
190 PrivateStruct<|>
191
192 pub mod PubMod {
193 struct PrivateStruct;
194 }
195 ",
196 );
197 }
198
199 #[test]
200 fn not_applicable_when_no_imports_found() {
201 check_assist_with_imports_locator_not_applicable(
202 auto_import,
203 TestImportsLocator::new,
204 "
205 PubStruct<|>",
206 );
207 }
208
209 #[test]
210 fn not_applicable_in_import_statements() {
211 check_assist_with_imports_locator_not_applicable(
212 auto_import,
213 TestImportsLocator::new,
214 r"
215 use PubStruct<|>;
216
217 pub mod PubMod {
218 pub struct PubStruct;
219 }",
220 );
221 }
222}
diff --git a/crates/ra_assists/src/doc_tests.rs b/crates/ra_assists/src/doc_tests.rs
index 5dc1ee233..65d51428b 100644
--- a/crates/ra_assists/src/doc_tests.rs
+++ b/crates/ra_assists/src/doc_tests.rs
@@ -11,6 +11,10 @@ use test_utils::{assert_eq_text, extract_range_or_offset};
11use crate::test_db::TestDB; 11use crate::test_db::TestDB;
12 12
13fn check(assist_id: &str, before: &str, after: &str) { 13fn check(assist_id: &str, before: &str, after: &str) {
14 // FIXME we cannot get the imports search functionality here yet, but still need to generate a test and a doc for an assist
15 if assist_id == "auto_import" {
16 return;
17 }
14 let (selection, before) = extract_range_or_offset(before); 18 let (selection, before) = extract_range_or_offset(before);
15 let (db, file_id) = TestDB::with_single_file(&before); 19 let (db, file_id) = TestDB::with_single_file(&before);
16 let frange = FileRange { file_id, range: selection.into() }; 20 let frange = FileRange { file_id, range: selection.into() };
diff --git a/crates/ra_assists/src/doc_tests/generated.rs b/crates/ra_assists/src/doc_tests/generated.rs
index 7d84dc8fb..ec4587ce7 100644
--- a/crates/ra_assists/src/doc_tests/generated.rs
+++ b/crates/ra_assists/src/doc_tests/generated.rs
@@ -215,6 +215,25 @@ fn main() {
215} 215}
216 216
217#[test] 217#[test]
218fn doctest_auto_import() {
219 check(
220 "auto_import",
221 r#####"
222fn main() {
223 let map = HashMap<|>::new();
224}
225"#####,
226 r#####"
227use std::collections::HashMap;
228
229fn main() {
230 let map = HashMap<|>::new();
231}
232"#####,
233 )
234}
235
236#[test]
218fn doctest_change_visibility() { 237fn doctest_change_visibility() {
219 check( 238 check(
220 "change_visibility", 239 "change_visibility",
diff --git a/crates/ra_assists/src/lib.rs b/crates/ra_assists/src/lib.rs
index 3337805a5..625ebc4a2 100644
--- a/crates/ra_assists/src/lib.rs
+++ b/crates/ra_assists/src/lib.rs
@@ -14,7 +14,7 @@ mod test_db;
14pub mod ast_transform; 14pub mod ast_transform;
15 15
16use either::Either; 16use either::Either;
17use hir::db::HirDatabase; 17use hir::{db::HirDatabase, ModuleDef};
18use ra_db::FileRange; 18use ra_db::FileRange;
19use ra_syntax::{TextRange, TextUnit}; 19use ra_syntax::{TextRange, TextUnit};
20use ra_text_edit::TextEdit; 20use ra_text_edit::TextEdit;
@@ -77,6 +77,51 @@ where
77 }) 77 })
78} 78}
79 79
80/// A functionality for locating imports for the given name.
81///
82/// Currently has to be a trait with the real implementation provided by the ra_ide_api crate,
83/// due to the search functionality located there.
84/// Later, this trait should be removed completely and the search functionality moved to a separate crate,
85/// accessible from the ra_assists crate.
86pub trait ImportsLocator {
87 /// Finds all imports for the given name and the module that contains this name.
88 fn find_imports(&mut self, name_to_import: &str) -> Vec<ModuleDef>;
89}
90
91/// Return all the assists applicable at the given position
92/// and additional assists that need the imports locator functionality to work.
93///
94/// Assists are returned in the "resolved" state, that is with edit fully
95/// computed.
96pub fn assists_with_imports_locator<H, F>(
97 db: &H,
98 range: FileRange,
99 mut imports_locator: F,
100) -> Vec<ResolvedAssist>
101where
102 H: HirDatabase + 'static,
103 F: ImportsLocator,
104{
105 AssistCtx::with_ctx(db, range, true, |ctx| {
106 let mut assists = assists::all()
107 .iter()
108 .map(|f| f(ctx.clone()))
109 .chain(
110 assists::all_with_imports_locator()
111 .iter()
112 .map(|f| f(ctx.clone(), &mut imports_locator)),
113 )
114 .filter_map(std::convert::identity)
115 .map(|a| match a {
116 Assist::Resolved { assist } => assist,
117 Assist::Unresolved { .. } => unreachable!(),
118 })
119 .collect();
120 sort_assists(&mut assists);
121 assists
122 })
123}
124
80/// Return all the assists applicable at the given position. 125/// Return all the assists applicable at the given position.
81/// 126///
82/// Assists are returned in the "resolved" state, that is with edit fully 127/// Assists are returned in the "resolved" state, that is with edit fully
@@ -85,8 +130,6 @@ pub fn assists<H>(db: &H, range: FileRange) -> Vec<ResolvedAssist>
85where 130where
86 H: HirDatabase + 'static, 131 H: HirDatabase + 'static,
87{ 132{
88 use std::cmp::Ordering;
89
90 AssistCtx::with_ctx(db, range, true, |ctx| { 133 AssistCtx::with_ctx(db, range, true, |ctx| {
91 let mut a = assists::all() 134 let mut a = assists::all()
92 .iter() 135 .iter()
@@ -95,19 +138,24 @@ where
95 Assist::Resolved { assist } => assist, 138 Assist::Resolved { assist } => assist,
96 Assist::Unresolved { .. } => unreachable!(), 139 Assist::Unresolved { .. } => unreachable!(),
97 }) 140 })
98 .collect::<Vec<_>>(); 141 .collect();
99 a.sort_by(|a, b| match (a.get_first_action().target, b.get_first_action().target) { 142 sort_assists(&mut a);
100 (Some(a), Some(b)) => a.len().cmp(&b.len()),
101 (Some(_), None) => Ordering::Less,
102 (None, Some(_)) => Ordering::Greater,
103 (None, None) => Ordering::Equal,
104 });
105 a 143 a
106 }) 144 })
107} 145}
108 146
147fn sort_assists(assists: &mut Vec<ResolvedAssist>) {
148 use std::cmp::Ordering;
149 assists.sort_by(|a, b| match (a.get_first_action().target, b.get_first_action().target) {
150 (Some(a), Some(b)) => a.len().cmp(&b.len()),
151 (Some(_), None) => Ordering::Less,
152 (None, Some(_)) => Ordering::Greater,
153 (None, None) => Ordering::Equal,
154 });
155}
156
109mod assists { 157mod assists {
110 use crate::{Assist, AssistCtx}; 158 use crate::{Assist, AssistCtx, ImportsLocator};
111 use hir::db::HirDatabase; 159 use hir::db::HirDatabase;
112 160
113 mod add_derive; 161 mod add_derive;
@@ -116,6 +164,7 @@ mod assists {
116 mod add_custom_impl; 164 mod add_custom_impl;
117 mod add_new; 165 mod add_new;
118 mod apply_demorgan; 166 mod apply_demorgan;
167 mod auto_import;
119 mod invert_if; 168 mod invert_if;
120 mod flip_comma; 169 mod flip_comma;
121 mod flip_binexpr; 170 mod flip_binexpr;
@@ -168,15 +217,69 @@ mod assists {
168 early_return::convert_to_guarded_return, 217 early_return::convert_to_guarded_return,
169 ] 218 ]
170 } 219 }
220
221 pub(crate) fn all_with_imports_locator<'a, DB: HirDatabase, F: ImportsLocator>(
222 ) -> &'a [fn(AssistCtx<DB>, &mut F) -> Option<Assist>] {
223 &[auto_import::auto_import]
224 }
171} 225}
172 226
173#[cfg(test)] 227#[cfg(test)]
174mod helpers { 228mod helpers {
175 use ra_db::{fixture::WithFixture, FileRange}; 229 use hir::db::DefDatabase;
230 use ra_db::{fixture::WithFixture, FileId, FileRange};
176 use ra_syntax::TextRange; 231 use ra_syntax::TextRange;
177 use test_utils::{add_cursor, assert_eq_text, extract_offset, extract_range}; 232 use test_utils::{add_cursor, assert_eq_text, extract_offset, extract_range};
178 233
179 use crate::{test_db::TestDB, Assist, AssistCtx}; 234 use crate::{test_db::TestDB, Assist, AssistCtx, ImportsLocator};
235 use std::sync::Arc;
236
237 // FIXME remove the `ModuleDefId` reexport from `ra_hir` when this gets removed.
238 pub(crate) struct TestImportsLocator {
239 db: Arc<TestDB>,
240 test_file_id: FileId,
241 }
242
243 impl TestImportsLocator {
244 pub(crate) fn new(db: Arc<TestDB>, test_file_id: FileId) -> Self {
245 TestImportsLocator { db, test_file_id }
246 }
247 }
248
249 impl ImportsLocator for TestImportsLocator {
250 fn find_imports(&mut self, name_to_import: &str) -> Vec<hir::ModuleDef> {
251 let crate_def_map = self.db.crate_def_map(self.db.test_crate());
252 let mut findings = Vec::new();
253
254 let mut module_ids_to_process =
255 crate_def_map.modules_for_file(self.test_file_id).collect::<Vec<_>>();
256
257 while !module_ids_to_process.is_empty() {
258 let mut more_ids_to_process = Vec::new();
259 for local_module_id in module_ids_to_process.drain(..) {
260 for (name, namespace_data) in
261 crate_def_map[local_module_id].scope.entries_without_primitives()
262 {
263 let found_a_match = &name.to_string() == name_to_import;
264 vec![namespace_data.types, namespace_data.values]
265 .into_iter()
266 .filter_map(std::convert::identity)
267 .for_each(|(module_def_id, _)| {
268 if found_a_match {
269 findings.push(module_def_id.into());
270 }
271 if let hir::ModuleDefId::ModuleId(module_id) = module_def_id {
272 more_ids_to_process.push(module_id.local_id);
273 }
274 });
275 }
276 }
277 module_ids_to_process = more_ids_to_process;
278 }
279
280 findings
281 }
282 }
180 283
181 pub(crate) fn check_assist( 284 pub(crate) fn check_assist(
182 assist: fn(AssistCtx<TestDB>) -> Option<Assist>, 285 assist: fn(AssistCtx<TestDB>) -> Option<Assist>,
@@ -206,6 +309,38 @@ mod helpers {
206 assert_eq_text!(after, &actual); 309 assert_eq_text!(after, &actual);
207 } 310 }
208 311
312 pub(crate) fn check_assist_with_imports_locator<F: ImportsLocator>(
313 assist: fn(AssistCtx<TestDB>, &mut F) -> Option<Assist>,
314 imports_locator_provider: fn(db: Arc<TestDB>, file_id: FileId) -> F,
315 before: &str,
316 after: &str,
317 ) {
318 let (before_cursor_pos, before) = extract_offset(before);
319 let (db, file_id) = TestDB::with_single_file(&before);
320 let db = Arc::new(db);
321 let mut imports_locator = imports_locator_provider(Arc::clone(&db), file_id);
322 let frange =
323 FileRange { file_id, range: TextRange::offset_len(before_cursor_pos, 0.into()) };
324 let assist =
325 AssistCtx::with_ctx(db.as_ref(), frange, true, |ctx| assist(ctx, &mut imports_locator))
326 .expect("code action is not applicable");
327 let action = match assist {
328 Assist::Unresolved { .. } => unreachable!(),
329 Assist::Resolved { assist } => assist.get_first_action(),
330 };
331
332 let actual = action.edit.apply(&before);
333 let actual_cursor_pos = match action.cursor_position {
334 None => action
335 .edit
336 .apply_to_offset(before_cursor_pos)
337 .expect("cursor position is affected by the edit"),
338 Some(off) => off,
339 };
340 let actual = add_cursor(&actual, actual_cursor_pos);
341 assert_eq_text!(after, &actual);
342 }
343
209 pub(crate) fn check_assist_range( 344 pub(crate) fn check_assist_range(
210 assist: fn(AssistCtx<TestDB>) -> Option<Assist>, 345 assist: fn(AssistCtx<TestDB>) -> Option<Assist>,
211 before: &str, 346 before: &str,
@@ -279,6 +414,22 @@ mod helpers {
279 assert!(assist.is_none()); 414 assert!(assist.is_none());
280 } 415 }
281 416
417 pub(crate) fn check_assist_with_imports_locator_not_applicable<F: ImportsLocator>(
418 assist: fn(AssistCtx<TestDB>, &mut F) -> Option<Assist>,
419 imports_locator_provider: fn(db: Arc<TestDB>, file_id: FileId) -> F,
420 before: &str,
421 ) {
422 let (before_cursor_pos, before) = extract_offset(before);
423 let (db, file_id) = TestDB::with_single_file(&before);
424 let db = Arc::new(db);
425 let mut imports_locator = imports_locator_provider(Arc::clone(&db), file_id);
426 let frange =
427 FileRange { file_id, range: TextRange::offset_len(before_cursor_pos, 0.into()) };
428 let assist =
429 AssistCtx::with_ctx(db.as_ref(), frange, true, |ctx| assist(ctx, &mut imports_locator));
430 assert!(assist.is_none());
431 }
432
282 pub(crate) fn check_assist_range_not_applicable( 433 pub(crate) fn check_assist_range_not_applicable(
283 assist: fn(AssistCtx<TestDB>) -> Option<Assist>, 434 assist: fn(AssistCtx<TestDB>) -> Option<Assist>,
284 before: &str, 435 before: &str,