Skip to content

Commit 4f95064

Browse files
Merge #2887
2887: Initial auto import action implementation r=matklad a=SomeoneToIgnore Closes #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: #2180 (comment) * 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]>
2 parents a108f22 + 9be1ab7 commit 4f95064

File tree

10 files changed

+509
-18
lines changed

10 files changed

+509
-18
lines changed

crates/ra_assists/src/assist_ctx.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,6 @@ impl<'a, DB: HirDatabase> AssistCtx<'a, DB> {
101101
Some(assist)
102102
}
103103

104-
#[allow(dead_code)] // will be used for auto import assist with multiple actions
105104
pub(crate) fn add_assist_group(
106105
self,
107106
id: AssistId,
@@ -168,7 +167,6 @@ pub(crate) struct ActionBuilder {
168167
}
169168

170169
impl ActionBuilder {
171-
#[allow(dead_code)]
172170
/// Adds a custom label to the action, if it needs to be different from the assist label
173171
pub(crate) fn label(&mut self, label: impl Into<String>) {
174172
self.label = Some(label.into())
Lines changed: 222 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,222 @@
1+
use hir::db::HirDatabase;
2+
use ra_syntax::{
3+
ast::{self, AstNode},
4+
SmolStr, SyntaxElement,
5+
SyntaxKind::{NAME_REF, USE_ITEM},
6+
SyntaxNode,
7+
};
8+
9+
use 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+
// ```
31+
pub(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+
72+
fn 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+
87+
fn 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)]
100+
mod 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+
}

crates/ra_assists/src/doc_tests.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,10 @@ use test_utils::{assert_eq_text, extract_range_or_offset};
1111
use crate::test_db::TestDB;
1212

1313
fn 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+
}
1418
let (selection, before) = extract_range_or_offset(before);
1519
let (db, file_id) = TestDB::with_single_file(&before);
1620
let frange = FileRange { file_id, range: selection.into() };

crates/ra_assists/src/doc_tests/generated.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,25 @@ fn main() {
214214
)
215215
}
216216

217+
#[test]
218+
fn doctest_auto_import() {
219+
check(
220+
"auto_import",
221+
r#####"
222+
fn main() {
223+
let map = HashMap<|>::new();
224+
}
225+
"#####,
226+
r#####"
227+
use std::collections::HashMap;
228+
229+
fn main() {
230+
let map = HashMap<|>::new();
231+
}
232+
"#####,
233+
)
234+
}
235+
217236
#[test]
218237
fn doctest_change_visibility() {
219238
check(

0 commit comments

Comments
 (0)