From 316795e074dff8f627f8c70c85d236420ecfb3a6 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 24 Dec 2019 02:19:09 +0200 Subject: Initial auto import action implementation --- crates/ra_assists/src/assists/auto_import.rs | 181 +++++++++++++++++++++++++++ 1 file changed, 181 insertions(+) create mode 100644 crates/ra_assists/src/assists/auto_import.rs (limited to 'crates/ra_assists/src/assists') 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..fe226521e --- /dev/null +++ b/crates/ra_assists/src/assists/auto_import.rs @@ -0,0 +1,181 @@ +use hir::db::HirDatabase; +use ra_syntax::{ + ast::{self, AstNode}, + SmolStr, SyntaxElement, + SyntaxKind::{NAME_REF, USE_ITEM}, + SyntaxNode, +}; + +use crate::{ + assist_ctx::{ActionBuilder, Assist, AssistCtx}, + auto_import_text_edit, AssistId, ImportsLocator, +}; + +// Assist: auto_import +// +// If the name is unresolved, provides all possible imports for it. +// +// ``` +// fn main() { +// let map = HashMap<|>::new(); +// } +// ``` +// -> +// ``` +// use std::collections::HashMap; +// +// fn main() { +// let map = HashMap<|>::new(); +// } +// ``` +pub(crate) fn auto_import<'a, F: ImportsLocator<'a>>( + ctx: AssistCtx, + imports_locator: &mut F, +) -> Option { + let path: ast::Path = ctx.find_node_at_offset()?; + let module = path.syntax().ancestors().find_map(ast::Module::cast); + let position = match module.and_then(|it| it.item_list()) { + Some(item_list) => item_list.syntax().clone(), + None => { + let current_file = path.syntax().ancestors().find_map(ast::SourceFile::cast)?; + current_file.syntax().clone() + } + }; + + let module_with_name_to_import = ctx.source_analyzer(&position, None).module()?; + let name_to_import = hir::InFile { + file_id: ctx.frange.file_id.into(), + value: &find_applicable_name_ref(ctx.covering_element())?, + }; + + let proposed_imports = + imports_locator.find_imports(name_to_import, module_with_name_to_import)?; + if proposed_imports.is_empty() { + return None; + } + + ctx.add_assist_group(AssistId("auto_import"), "auto import", || { + proposed_imports + .into_iter() + .map(|import| import_to_action(import.to_string(), &position, &path)) + .collect() + }) +} + +fn find_applicable_name_ref(element: SyntaxElement) -> Option { + if element.ancestors().find(|ancestor| ancestor.kind() == USE_ITEM).is_some() { + None + } else if element.kind() == NAME_REF { + Some(element.as_node().cloned().and_then(ast::NameRef::cast)?) + } else { + let parent = element.parent()?; + if parent.kind() == NAME_REF { + Some(ast::NameRef::cast(parent)?) + } else { + None + } + } +} + +fn import_to_action(import: String, position: &SyntaxNode, path: &ast::Path) -> ActionBuilder { + let mut action_builder = ActionBuilder::default(); + action_builder.label(format!("Import `{}`", &import)); + auto_import_text_edit( + position, + &path.syntax().clone(), + &[SmolStr::new(import)], + action_builder.text_edit_builder(), + ); + action_builder +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::helpers::{ + check_assist_with_imports_locator, check_assist_with_imports_locator_not_applicable, + }; + use hir::Name; + + #[derive(Clone)] + struct TestImportsLocator<'a> { + import_path: &'a [Name], + } + + impl<'a> TestImportsLocator<'a> { + fn new(import_path: &'a [Name]) -> Self { + TestImportsLocator { import_path } + } + } + + impl<'a> ImportsLocator<'_> for TestImportsLocator<'_> { + fn find_imports( + &mut self, + _: hir::InFile<&ast::NameRef>, + _: hir::Module, + ) -> Option> { + if self.import_path.is_empty() { + None + } else { + Some(vec![hir::ModPath { + kind: hir::PathKind::Plain, + segments: self.import_path.to_owned(), + }]) + } + } + } + + #[test] + fn applicable_when_found_an_import() { + let import_path = &[hir::name::known::std, hir::name::known::ops, hir::name::known::Debug]; + let mut imports_locator = TestImportsLocator::new(import_path); + check_assist_with_imports_locator( + auto_import, + &mut imports_locator, + " + fn main() { + } + + Debug<|>", + &format!( + " + use {}; + + fn main() {{ + }} + + Debug<|>", + import_path + .into_iter() + .map(|name| name.to_string()) + .collect::>() + .join("::") + ), + ); + } + + #[test] + fn not_applicable_when_no_imports_found() { + let mut imports_locator = TestImportsLocator::new(&[]); + check_assist_with_imports_locator_not_applicable( + auto_import, + &mut imports_locator, + " + fn main() { + } + + Debug<|>", + ); + } + + #[test] + fn not_applicable_in_import_statements() { + let import_path = &[hir::name::known::std, hir::name::known::ops, hir::name::known::Debug]; + let mut imports_locator = TestImportsLocator::new(import_path); + check_assist_with_imports_locator_not_applicable( + auto_import, + &mut imports_locator, + "use Debug<|>;", + ); + } +} -- cgit v1.2.3 From f57239729cb9d6f60eb09d8cfd8244066b5e182c Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Thu, 23 Jan 2020 21:15:16 +0200 Subject: Remove unnecessary lifetime parameter --- crates/ra_assists/src/assists/auto_import.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'crates/ra_assists/src/assists') diff --git a/crates/ra_assists/src/assists/auto_import.rs b/crates/ra_assists/src/assists/auto_import.rs index fe226521e..ae216944b 100644 --- a/crates/ra_assists/src/assists/auto_import.rs +++ b/crates/ra_assists/src/assists/auto_import.rs @@ -28,7 +28,7 @@ use crate::{ // let map = HashMap<|>::new(); // } // ``` -pub(crate) fn auto_import<'a, F: ImportsLocator<'a>>( +pub(crate) fn auto_import( ctx: AssistCtx, imports_locator: &mut F, ) -> Option { @@ -108,7 +108,7 @@ mod tests { } } - impl<'a> ImportsLocator<'_> for TestImportsLocator<'_> { + impl<'a> ImportsLocator for TestImportsLocator<'a> { fn find_imports( &mut self, _: hir::InFile<&ast::NameRef>, -- cgit v1.2.3 From d0a782ef1c53ef5c5d3b49b02c498f7a688c3a4d Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Fri, 24 Jan 2020 09:33:18 +0200 Subject: Have a better trait interface --- crates/ra_assists/src/assists/auto_import.rs | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) (limited to 'crates/ra_assists/src/assists') diff --git a/crates/ra_assists/src/assists/auto_import.rs b/crates/ra_assists/src/assists/auto_import.rs index ae216944b..d196f6c5c 100644 --- a/crates/ra_assists/src/assists/auto_import.rs +++ b/crates/ra_assists/src/assists/auto_import.rs @@ -1,4 +1,4 @@ -use hir::db::HirDatabase; +use hir::{db::HirDatabase, AsName}; use ra_syntax::{ ast::{self, AstNode}, SmolStr, SyntaxElement, @@ -41,15 +41,21 @@ pub(crate) fn auto_import( current_file.syntax().clone() } }; + let source_analyzer = ctx.source_analyzer(&position, None); + let module_with_name_to_import = source_analyzer.module()?; + let path_to_import = ctx.covering_element().ancestors().find_map(ast::Path::cast)?; + if source_analyzer.resolve_path(ctx.db, &path_to_import).is_some() { + return None; + } - let module_with_name_to_import = ctx.source_analyzer(&position, None).module()?; - let name_to_import = hir::InFile { - file_id: ctx.frange.file_id.into(), - value: &find_applicable_name_ref(ctx.covering_element())?, - }; - - let proposed_imports = - imports_locator.find_imports(name_to_import, module_with_name_to_import)?; + let name_to_import = &find_applicable_name_ref(ctx.covering_element())?.as_name(); + let proposed_imports = imports_locator + .find_imports(&name_to_import.to_string()) + .into_iter() + .filter_map(|module_def| module_with_name_to_import.find_use_path(ctx.db, module_def)) + .filter(|use_path| !use_path.segments.is_empty()) + .take(20) + .collect::>(); if proposed_imports.is_empty() { return None; } @@ -57,7 +63,7 @@ pub(crate) fn auto_import( ctx.add_assist_group(AssistId("auto_import"), "auto import", || { proposed_imports .into_iter() - .map(|import| import_to_action(import.to_string(), &position, &path)) + .map(|import| import_to_action(import.to_string(), &position, &path_to_import)) .collect() }) } -- cgit v1.2.3 From 1a78991df69630b581b4210083c9e94157bab0e1 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Mon, 27 Jan 2020 00:16:18 +0200 Subject: Adjust the tests --- crates/ra_assists/src/assists/auto_import.rs | 144 +++++++++++++++++---------- 1 file changed, 89 insertions(+), 55 deletions(-) (limited to 'crates/ra_assists/src/assists') diff --git a/crates/ra_assists/src/assists/auto_import.rs b/crates/ra_assists/src/assists/auto_import.rs index d196f6c5c..295fdf2e2 100644 --- a/crates/ra_assists/src/assists/auto_import.rs +++ b/crates/ra_assists/src/assists/auto_import.rs @@ -100,88 +100,122 @@ mod tests { use super::*; use crate::helpers::{ check_assist_with_imports_locator, check_assist_with_imports_locator_not_applicable, + TestImportsLocator, }; - use hir::Name; - #[derive(Clone)] - struct TestImportsLocator<'a> { - import_path: &'a [Name], - } + #[test] + fn applicable_when_found_an_import() { + check_assist_with_imports_locator( + auto_import, + TestImportsLocator::new, + r" + PubStruct<|> - impl<'a> TestImportsLocator<'a> { - fn new(import_path: &'a [Name]) -> Self { - TestImportsLocator { import_path } - } - } + pub mod PubMod { + pub struct PubStruct; + } + ", + r" + use PubMod::PubStruct; - impl<'a> ImportsLocator for TestImportsLocator<'a> { - fn find_imports( - &mut self, - _: hir::InFile<&ast::NameRef>, - _: hir::Module, - ) -> Option> { - if self.import_path.is_empty() { - None - } else { - Some(vec![hir::ModPath { - kind: hir::PathKind::Plain, - segments: self.import_path.to_owned(), - }]) + PubStruct<|> + + pub mod PubMod { + pub struct PubStruct; } - } + ", + ); } #[test] - fn applicable_when_found_an_import() { - let import_path = &[hir::name::known::std, hir::name::known::ops, hir::name::known::Debug]; - let mut imports_locator = TestImportsLocator::new(import_path); + fn applicable_when_found_multiple_imports() { check_assist_with_imports_locator( auto_import, - &mut imports_locator, - " - fn main() { + TestImportsLocator::new, + r" + PubStruct<|> + + pub mod PubMod1 { + pub struct PubStruct; + } + pub mod PubMod2 { + pub struct PubStruct; + } + pub mod PubMod3 { + pub struct PubStruct; + } + ", + r" + use PubMod1::PubStruct; + + PubStruct<|> + + pub mod PubMod1 { + pub struct PubStruct; + } + pub mod PubMod2 { + pub struct PubStruct; } + pub mod PubMod3 { + pub struct PubStruct; + } + ", + ); + } + + #[test] + fn not_applicable_for_already_imported_types() { + check_assist_with_imports_locator_not_applicable( + auto_import, + TestImportsLocator::new, + r" + use PubMod::PubStruct; + + PubStruct<|> - Debug<|>", - &format!( - " - use {}; - - fn main() {{ - }} - - Debug<|>", - import_path - .into_iter() - .map(|name| name.to_string()) - .collect::>() - .join("::") - ), + pub mod PubMod { + pub struct PubStruct; + } + ", ); } #[test] - fn not_applicable_when_no_imports_found() { - let mut imports_locator = TestImportsLocator::new(&[]); + fn not_applicable_for_types_with_private_paths() { check_assist_with_imports_locator_not_applicable( auto_import, - &mut imports_locator, - " - fn main() { + TestImportsLocator::new, + r" + PrivateStruct<|> + + pub mod PubMod { + struct PrivateStruct; } + ", + ); + } - Debug<|>", + #[test] + fn not_applicable_when_no_imports_found() { + check_assist_with_imports_locator_not_applicable( + auto_import, + TestImportsLocator::new, + " + PubStruct<|>", ); } #[test] fn not_applicable_in_import_statements() { - let import_path = &[hir::name::known::std, hir::name::known::ops, hir::name::known::Debug]; - let mut imports_locator = TestImportsLocator::new(import_path); check_assist_with_imports_locator_not_applicable( auto_import, - &mut imports_locator, - "use Debug<|>;", + TestImportsLocator::new, + r" + use PubStruct<|>; + + pub mod PubMod { + pub struct PubStruct; + }", ); } } -- cgit v1.2.3 From 9a6b5c6183b2d7aa3d577c3fb12d519721f4a4d0 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Mon, 27 Jan 2020 01:53:59 +0200 Subject: Enforce alphabetical import sorting --- crates/ra_assists/src/assists/auto_import.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'crates/ra_assists/src/assists') diff --git a/crates/ra_assists/src/assists/auto_import.rs b/crates/ra_assists/src/assists/auto_import.rs index 295fdf2e2..4c3793ac7 100644 --- a/crates/ra_assists/src/assists/auto_import.rs +++ b/crates/ra_assists/src/assists/auto_import.rs @@ -55,7 +55,8 @@ pub(crate) fn auto_import( .filter_map(|module_def| module_with_name_to_import.find_use_path(ctx.db, module_def)) .filter(|use_path| !use_path.segments.is_empty()) .take(20) - .collect::>(); + .map(|import| import.to_string()) + .collect::>(); if proposed_imports.is_empty() { return None; } @@ -63,7 +64,7 @@ pub(crate) fn auto_import( ctx.add_assist_group(AssistId("auto_import"), "auto import", || { proposed_imports .into_iter() - .map(|import| import_to_action(import.to_string(), &position, &path_to_import)) + .map(|import| import_to_action(import, &position, &path_to_import)) .collect() }) } -- cgit v1.2.3 From 9be1ab7ff948d89334a8acbc309c8235d4ab374f Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Mon, 27 Jan 2020 14:42:45 +0200 Subject: Code review fixes --- crates/ra_assists/src/assists/auto_import.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'crates/ra_assists/src/assists') diff --git a/crates/ra_assists/src/assists/auto_import.rs b/crates/ra_assists/src/assists/auto_import.rs index 4c3793ac7..9163cc662 100644 --- a/crates/ra_assists/src/assists/auto_import.rs +++ b/crates/ra_assists/src/assists/auto_import.rs @@ -1,4 +1,4 @@ -use hir::{db::HirDatabase, AsName}; +use hir::db::HirDatabase; use ra_syntax::{ ast::{self, AstNode}, SmolStr, SyntaxElement, @@ -48,7 +48,7 @@ pub(crate) fn auto_import( return None; } - let name_to_import = &find_applicable_name_ref(ctx.covering_element())?.as_name(); + let name_to_import = &find_applicable_name_ref(ctx.covering_element())?.syntax().to_string(); let proposed_imports = imports_locator .find_imports(&name_to_import.to_string()) .into_iter() @@ -64,7 +64,7 @@ pub(crate) fn auto_import( ctx.add_assist_group(AssistId("auto_import"), "auto import", || { proposed_imports .into_iter() - .map(|import| import_to_action(import, &position, &path_to_import)) + .map(|import| import_to_action(import, &position, &path_to_import.syntax())) .collect() }) } @@ -84,12 +84,12 @@ fn find_applicable_name_ref(element: SyntaxElement) -> Option { } } -fn import_to_action(import: String, position: &SyntaxNode, path: &ast::Path) -> ActionBuilder { +fn import_to_action(import: String, position: &SyntaxNode, anchor: &SyntaxNode) -> ActionBuilder { let mut action_builder = ActionBuilder::default(); action_builder.label(format!("Import `{}`", &import)); auto_import_text_edit( position, - &path.syntax().clone(), + anchor, &[SmolStr::new(import)], action_builder.text_edit_builder(), ); -- cgit v1.2.3