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/lib.rs | 126 ++++++++++++++++++++++++++++++++++++++----- 1 file changed, 113 insertions(+), 13 deletions(-) (limited to 'crates/ra_assists/src/lib.rs') diff --git a/crates/ra_assists/src/lib.rs b/crates/ra_assists/src/lib.rs index 3337805a5..4029962f7 100644 --- a/crates/ra_assists/src/lib.rs +++ b/crates/ra_assists/src/lib.rs @@ -14,9 +14,9 @@ mod test_db; pub mod ast_transform; use either::Either; -use hir::db::HirDatabase; +use hir::{db::HirDatabase, InFile, ModPath, Module}; use ra_db::FileRange; -use ra_syntax::{TextRange, TextUnit}; +use ra_syntax::{ast::NameRef, TextRange, TextUnit}; use ra_text_edit::TextEdit; pub(crate) use crate::assist_ctx::{Assist, AssistCtx}; @@ -77,6 +77,55 @@ where }) } +/// A functionality for locating imports for the given name. +/// +/// Currently has to be a trait with the real implementation provided by the ra_ide_api crate, +/// due to the search functionality located there. +/// Later, this trait should be removed completely and the search functionality moved to a separate crate, +/// accessible from the ra_assists crate. +pub trait ImportsLocator<'a> { + /// Finds all imports for the given name and the module that contains this name. + fn find_imports( + &mut self, + name_to_import: InFile<&NameRef>, + module_with_name_to_import: Module, + ) -> Option>; +} + +/// Return all the assists applicable at the given position +/// and additional assists that need the imports locator functionality to work. +/// +/// Assists are returned in the "resolved" state, that is with edit fully +/// computed. +pub fn assists_with_imports_locator<'a, H, F: 'a>( + db: &H, + range: FileRange, + mut imports_locator: F, +) -> Vec +where + H: HirDatabase + 'static, + F: ImportsLocator<'a>, +{ + AssistCtx::with_ctx(db, range, true, |ctx| { + let mut assists = assists::all() + .iter() + .map(|f| f(ctx.clone())) + .chain( + assists::all_with_imports_locator() + .iter() + .map(|f| f(ctx.clone(), &mut imports_locator)), + ) + .filter_map(std::convert::identity) + .map(|a| match a { + Assist::Resolved { assist } => assist, + Assist::Unresolved { .. } => unreachable!(), + }) + .collect(); + sort_assists(&mut assists); + assists + }) +} + /// Return all the assists applicable at the given position. /// /// Assists are returned in the "resolved" state, that is with edit fully @@ -85,8 +134,6 @@ pub fn assists(db: &H, range: FileRange) -> Vec where H: HirDatabase + 'static, { - use std::cmp::Ordering; - AssistCtx::with_ctx(db, range, true, |ctx| { let mut a = assists::all() .iter() @@ -95,19 +142,24 @@ where Assist::Resolved { assist } => assist, Assist::Unresolved { .. } => unreachable!(), }) - .collect::>(); - a.sort_by(|a, b| match (a.get_first_action().target, b.get_first_action().target) { - (Some(a), Some(b)) => a.len().cmp(&b.len()), - (Some(_), None) => Ordering::Less, - (None, Some(_)) => Ordering::Greater, - (None, None) => Ordering::Equal, - }); + .collect(); + sort_assists(&mut a); a }) } +fn sort_assists(assists: &mut Vec) { + use std::cmp::Ordering; + assists.sort_by(|a, b| match (a.get_first_action().target, b.get_first_action().target) { + (Some(a), Some(b)) => a.len().cmp(&b.len()), + (Some(_), None) => Ordering::Less, + (None, Some(_)) => Ordering::Greater, + (None, None) => Ordering::Equal, + }); +} + mod assists { - use crate::{Assist, AssistCtx}; + use crate::{Assist, AssistCtx, ImportsLocator}; use hir::db::HirDatabase; mod add_derive; @@ -116,6 +168,7 @@ mod assists { mod add_custom_impl; mod add_new; mod apply_demorgan; + mod auto_import; mod invert_if; mod flip_comma; mod flip_binexpr; @@ -168,6 +221,11 @@ mod assists { early_return::convert_to_guarded_return, ] } + + pub(crate) fn all_with_imports_locator<'a, DB: HirDatabase, F: ImportsLocator<'a>>( + ) -> &'a [fn(AssistCtx, &mut F) -> Option] { + &[auto_import::auto_import] + } } #[cfg(test)] @@ -176,7 +234,7 @@ mod helpers { use ra_syntax::TextRange; use test_utils::{add_cursor, assert_eq_text, extract_offset, extract_range}; - use crate::{test_db::TestDB, Assist, AssistCtx}; + use crate::{test_db::TestDB, Assist, AssistCtx, ImportsLocator}; pub(crate) fn check_assist( assist: fn(AssistCtx) -> Option, @@ -206,6 +264,35 @@ mod helpers { assert_eq_text!(after, &actual); } + pub(crate) fn check_assist_with_imports_locator<'a, F: ImportsLocator<'a>>( + assist: fn(AssistCtx, &mut F) -> Option, + imports_locator: &mut F, + before: &str, + after: &str, + ) { + let (before_cursor_pos, before) = extract_offset(before); + let (db, file_id) = TestDB::with_single_file(&before); + let frange = + FileRange { file_id, range: TextRange::offset_len(before_cursor_pos, 0.into()) }; + let assist = AssistCtx::with_ctx(&db, frange, true, |ctx| assist(ctx, imports_locator)) + .expect("code action is not applicable"); + let action = match assist { + Assist::Unresolved { .. } => unreachable!(), + Assist::Resolved { assist } => assist.get_first_action(), + }; + + let actual = action.edit.apply(&before); + let actual_cursor_pos = match action.cursor_position { + None => action + .edit + .apply_to_offset(before_cursor_pos) + .expect("cursor position is affected by the edit"), + Some(off) => off, + }; + let actual = add_cursor(&actual, actual_cursor_pos); + assert_eq_text!(after, &actual); + } + pub(crate) fn check_assist_range( assist: fn(AssistCtx) -> Option, before: &str, @@ -279,6 +366,19 @@ mod helpers { assert!(assist.is_none()); } + pub(crate) fn check_assist_with_imports_locator_not_applicable<'a, F: ImportsLocator<'a>>( + assist: fn(AssistCtx, &mut F) -> Option, + imports_locator: &mut F, + before: &str, + ) { + let (before_cursor_pos, before) = extract_offset(before); + let (db, file_id) = TestDB::with_single_file(&before); + let frange = + FileRange { file_id, range: TextRange::offset_len(before_cursor_pos, 0.into()) }; + let assist = AssistCtx::with_ctx(&db, frange, true, |ctx| assist(ctx, imports_locator)); + assert!(assist.is_none()); + } + pub(crate) fn check_assist_range_not_applicable( assist: fn(AssistCtx) -> Option, before: &str, -- 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/lib.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'crates/ra_assists/src/lib.rs') diff --git a/crates/ra_assists/src/lib.rs b/crates/ra_assists/src/lib.rs index 4029962f7..381a51df6 100644 --- a/crates/ra_assists/src/lib.rs +++ b/crates/ra_assists/src/lib.rs @@ -83,7 +83,7 @@ where /// due to the search functionality located there. /// Later, this trait should be removed completely and the search functionality moved to a separate crate, /// accessible from the ra_assists crate. -pub trait ImportsLocator<'a> { +pub trait ImportsLocator { /// Finds all imports for the given name and the module that contains this name. fn find_imports( &mut self, @@ -97,14 +97,14 @@ pub trait ImportsLocator<'a> { /// /// Assists are returned in the "resolved" state, that is with edit fully /// computed. -pub fn assists_with_imports_locator<'a, H, F: 'a>( +pub fn assists_with_imports_locator( db: &H, range: FileRange, mut imports_locator: F, ) -> Vec where H: HirDatabase + 'static, - F: ImportsLocator<'a>, + F: ImportsLocator, { AssistCtx::with_ctx(db, range, true, |ctx| { let mut assists = assists::all() @@ -222,7 +222,7 @@ mod assists { ] } - pub(crate) fn all_with_imports_locator<'a, DB: HirDatabase, F: ImportsLocator<'a>>( + pub(crate) fn all_with_imports_locator<'a, DB: HirDatabase, F: ImportsLocator>( ) -> &'a [fn(AssistCtx, &mut F) -> Option] { &[auto_import::auto_import] } @@ -264,7 +264,7 @@ mod helpers { assert_eq_text!(after, &actual); } - pub(crate) fn check_assist_with_imports_locator<'a, F: ImportsLocator<'a>>( + pub(crate) fn check_assist_with_imports_locator( assist: fn(AssistCtx, &mut F) -> Option, imports_locator: &mut F, before: &str, @@ -366,7 +366,7 @@ mod helpers { assert!(assist.is_none()); } - pub(crate) fn check_assist_with_imports_locator_not_applicable<'a, F: ImportsLocator<'a>>( + pub(crate) fn check_assist_with_imports_locator_not_applicable( assist: fn(AssistCtx, &mut F) -> Option, imports_locator: &mut F, before: &str, -- 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/lib.rs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) (limited to 'crates/ra_assists/src/lib.rs') diff --git a/crates/ra_assists/src/lib.rs b/crates/ra_assists/src/lib.rs index 381a51df6..9e4ebec47 100644 --- a/crates/ra_assists/src/lib.rs +++ b/crates/ra_assists/src/lib.rs @@ -14,9 +14,9 @@ mod test_db; pub mod ast_transform; use either::Either; -use hir::{db::HirDatabase, InFile, ModPath, Module}; +use hir::{db::HirDatabase, ModuleDef}; use ra_db::FileRange; -use ra_syntax::{ast::NameRef, TextRange, TextUnit}; +use ra_syntax::{TextRange, TextUnit}; use ra_text_edit::TextEdit; pub(crate) use crate::assist_ctx::{Assist, AssistCtx}; @@ -85,11 +85,7 @@ where /// accessible from the ra_assists crate. pub trait ImportsLocator { /// Finds all imports for the given name and the module that contains this name. - fn find_imports( - &mut self, - name_to_import: InFile<&NameRef>, - module_with_name_to_import: Module, - ) -> Option>; + fn find_imports(&mut self, name_to_import: &str) -> Vec; } /// Return all the assists applicable at the given position -- 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/lib.rs | 66 ++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 60 insertions(+), 6 deletions(-) (limited to 'crates/ra_assists/src/lib.rs') diff --git a/crates/ra_assists/src/lib.rs b/crates/ra_assists/src/lib.rs index 9e4ebec47..724bce191 100644 --- a/crates/ra_assists/src/lib.rs +++ b/crates/ra_assists/src/lib.rs @@ -226,11 +226,59 @@ mod assists { #[cfg(test)] mod helpers { - use ra_db::{fixture::WithFixture, FileRange}; + use hir::db::DefDatabase; + use ra_db::{fixture::WithFixture, FileId, FileRange}; use ra_syntax::TextRange; use test_utils::{add_cursor, assert_eq_text, extract_offset, extract_range}; use crate::{test_db::TestDB, Assist, AssistCtx, ImportsLocator}; + use std::sync::Arc; + + pub(crate) struct TestImportsLocator { + db: Arc, + test_file_id: FileId, + } + + impl TestImportsLocator { + pub(crate) fn new(db: Arc, test_file_id: FileId) -> Self { + TestImportsLocator { db, test_file_id } + } + } + + impl ImportsLocator for TestImportsLocator { + fn find_imports(&mut self, name_to_import: &str) -> Vec { + let crate_def_map = self.db.crate_def_map(self.db.test_crate()); + let mut findings = vec![]; + + let mut module_ids_to_process = + crate_def_map.modules_for_file(self.test_file_id).collect::>(); + + while !module_ids_to_process.is_empty() { + let mut more_ids_to_process = vec![]; + for local_module_id in module_ids_to_process.drain(..) { + for (name, namespace_data) in + crate_def_map[local_module_id].scope.entries_without_primitives() + { + let found_a_match = &name.to_string() == name_to_import; + vec![namespace_data.types, namespace_data.values] + .into_iter() + .filter_map(std::convert::identity) + .for_each(|(module_def_id, _)| { + if found_a_match { + findings.push(module_def_id.into()); + } + if let hir::ModuleDefId::ModuleId(module_id) = module_def_id { + more_ids_to_process.push(module_id.local_id); + } + }); + } + } + module_ids_to_process = more_ids_to_process; + } + + findings + } + } pub(crate) fn check_assist( assist: fn(AssistCtx) -> Option, @@ -262,16 +310,19 @@ mod helpers { pub(crate) fn check_assist_with_imports_locator( assist: fn(AssistCtx, &mut F) -> Option, - imports_locator: &mut F, + imports_locator_provider: fn(db: Arc, file_id: FileId) -> F, before: &str, after: &str, ) { let (before_cursor_pos, before) = extract_offset(before); let (db, file_id) = TestDB::with_single_file(&before); + let db = Arc::new(db); + let mut imports_locator = imports_locator_provider(Arc::clone(&db), file_id); let frange = FileRange { file_id, range: TextRange::offset_len(before_cursor_pos, 0.into()) }; - let assist = AssistCtx::with_ctx(&db, frange, true, |ctx| assist(ctx, imports_locator)) - .expect("code action is not applicable"); + let assist = + AssistCtx::with_ctx(db.as_ref(), frange, true, |ctx| assist(ctx, &mut imports_locator)) + .expect("code action is not applicable"); let action = match assist { Assist::Unresolved { .. } => unreachable!(), Assist::Resolved { assist } => assist.get_first_action(), @@ -364,14 +415,17 @@ mod helpers { pub(crate) fn check_assist_with_imports_locator_not_applicable( assist: fn(AssistCtx, &mut F) -> Option, - imports_locator: &mut F, + imports_locator_provider: fn(db: Arc, file_id: FileId) -> F, before: &str, ) { let (before_cursor_pos, before) = extract_offset(before); let (db, file_id) = TestDB::with_single_file(&before); + let db = Arc::new(db); + let mut imports_locator = imports_locator_provider(Arc::clone(&db), file_id); let frange = FileRange { file_id, range: TextRange::offset_len(before_cursor_pos, 0.into()) }; - let assist = AssistCtx::with_ctx(&db, frange, true, |ctx| assist(ctx, imports_locator)); + let assist = + AssistCtx::with_ctx(db.as_ref(), frange, true, |ctx| assist(ctx, &mut imports_locator)); assert!(assist.is_none()); } -- 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/lib.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'crates/ra_assists/src/lib.rs') diff --git a/crates/ra_assists/src/lib.rs b/crates/ra_assists/src/lib.rs index 724bce191..625ebc4a2 100644 --- a/crates/ra_assists/src/lib.rs +++ b/crates/ra_assists/src/lib.rs @@ -234,6 +234,7 @@ mod helpers { use crate::{test_db::TestDB, Assist, AssistCtx, ImportsLocator}; use std::sync::Arc; + // FIXME remove the `ModuleDefId` reexport from `ra_hir` when this gets removed. pub(crate) struct TestImportsLocator { db: Arc, test_file_id: FileId, @@ -248,13 +249,13 @@ mod helpers { impl ImportsLocator for TestImportsLocator { fn find_imports(&mut self, name_to_import: &str) -> Vec { let crate_def_map = self.db.crate_def_map(self.db.test_crate()); - let mut findings = vec![]; + let mut findings = Vec::new(); let mut module_ids_to_process = crate_def_map.modules_for_file(self.test_file_id).collect::>(); while !module_ids_to_process.is_empty() { - let mut more_ids_to_process = vec![]; + let mut more_ids_to_process = Vec::new(); for local_module_id in module_ids_to_process.drain(..) { for (name, namespace_data) in crate_def_map[local_module_id].scope.entries_without_primitives() -- cgit v1.2.3