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 From a173e31890c1eb03d9d4c123986baae4154cd4fa Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Thu, 6 Feb 2020 16:40:28 +0100 Subject: Make assists use ImportsLocator directly --- crates/ra_assists/src/lib.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 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 625ebc4a2..0ebb8e8b0 100644 --- a/crates/ra_assists/src/lib.rs +++ b/crates/ra_assists/src/lib.rs @@ -16,6 +16,7 @@ pub mod ast_transform; use either::Either; use hir::{db::HirDatabase, ModuleDef}; use ra_db::FileRange; +use ra_ide_db::{imports_locator::ImportsLocatorIde, RootDatabase}; use ra_syntax::{TextRange, TextUnit}; use ra_text_edit::TextEdit; @@ -88,20 +89,19 @@ pub trait ImportsLocator { fn find_imports(&mut self, name_to_import: &str) -> Vec; } +impl ImportsLocator for ImportsLocatorIde<'_> { + fn find_imports(&mut self, name_to_import: &str) -> Vec { + self.find_imports(name_to_import) + } +} + /// 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( - db: &H, - range: FileRange, - mut imports_locator: F, -) -> Vec -where - H: HirDatabase + 'static, - F: ImportsLocator, -{ +pub fn assists_with_imports_locator(db: &RootDatabase, range: FileRange) -> Vec { + let mut imports_locator = ImportsLocatorIde::new(db); AssistCtx::with_ctx(db, range, true, |ctx| { let mut assists = assists::all() .iter() -- cgit v1.2.3 From 2c922ef54958a82ce745e6db6834062f97f21bed Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Thu, 6 Feb 2020 16:53:42 +0100 Subject: Start switching assists to a root database --- crates/ra_assists/src/lib.rs | 61 ++++++++++++++++++++------------------------ 1 file changed, 28 insertions(+), 33 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 0ebb8e8b0..a2109b751 100644 --- a/crates/ra_assists/src/lib.rs +++ b/crates/ra_assists/src/lib.rs @@ -14,7 +14,7 @@ mod test_db; pub mod ast_transform; use either::Either; -use hir::{db::HirDatabase, ModuleDef}; +use hir::ModuleDef; use ra_db::FileRange; use ra_ide_db::{imports_locator::ImportsLocatorIde, RootDatabase}; use ra_syntax::{TextRange, TextUnit}; @@ -62,10 +62,7 @@ impl ResolvedAssist { /// /// Assists are returned in the "unresolved" state, that is only labels are /// returned, without actual edits. -pub fn applicable_assists(db: &H, range: FileRange) -> Vec -where - H: HirDatabase + 'static, -{ +pub fn applicable_assists(db: &RootDatabase, range: FileRange) -> Vec { AssistCtx::with_ctx(db, range, false, |ctx| { assists::all() .iter() @@ -126,10 +123,7 @@ pub fn assists_with_imports_locator(db: &RootDatabase, range: FileRange) -> Vec< /// /// Assists are returned in the "resolved" state, that is with edit fully /// computed. -pub fn assists(db: &H, range: FileRange) -> Vec -where - H: HirDatabase + 'static, -{ +pub fn assists(db: &RootDatabase, range: FileRange) -> Vec { AssistCtx::with_ctx(db, range, true, |ctx| { let mut a = assists::all() .iter() @@ -231,17 +225,18 @@ 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, ImportsLocator}; + use crate::{Assist, AssistCtx, ImportsLocator}; + use ra_ide_db::RootDatabase; use std::sync::Arc; // FIXME remove the `ModuleDefId` reexport from `ra_hir` when this gets removed. pub(crate) struct TestImportsLocator { - db: Arc, + db: Arc, test_file_id: FileId, } impl TestImportsLocator { - pub(crate) fn new(db: Arc, test_file_id: FileId) -> Self { + pub(crate) fn new(db: Arc, test_file_id: FileId) -> Self { TestImportsLocator { db, test_file_id } } } @@ -282,12 +277,12 @@ mod helpers { } pub(crate) fn check_assist( - assist: fn(AssistCtx) -> Option, + assist: fn(AssistCtx) -> Option, before: &str, after: &str, ) { let (before_cursor_pos, before) = extract_offset(before); - let (db, file_id) = TestDB::with_single_file(&before); + let (db, file_id) = RootDatabase::with_single_file(&before); let frange = FileRange { file_id, range: TextRange::offset_len(before_cursor_pos, 0.into()) }; let assist = @@ -310,13 +305,13 @@ mod helpers { } pub(crate) fn check_assist_with_imports_locator( - assist: fn(AssistCtx, &mut F) -> Option, - imports_locator_provider: fn(db: Arc, file_id: FileId) -> F, + assist: fn(AssistCtx, &mut F) -> Option, + 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, file_id) = RootDatabase::with_single_file(&before); let db = Arc::new(db); let mut imports_locator = imports_locator_provider(Arc::clone(&db), file_id); let frange = @@ -342,12 +337,12 @@ mod helpers { } pub(crate) fn check_assist_range( - assist: fn(AssistCtx) -> Option, + assist: fn(AssistCtx) -> Option, before: &str, after: &str, ) { let (range, before) = extract_range(before); - let (db, file_id) = TestDB::with_single_file(&before); + let (db, file_id) = RootDatabase::with_single_file(&before); let frange = FileRange { file_id, range }; let assist = AssistCtx::with_ctx(&db, frange, true, assist).expect("code action is not applicable"); @@ -364,12 +359,12 @@ mod helpers { } pub(crate) fn check_assist_target( - assist: fn(AssistCtx) -> Option, + assist: fn(AssistCtx) -> Option, before: &str, target: &str, ) { let (before_cursor_pos, before) = extract_offset(before); - let (db, file_id) = TestDB::with_single_file(&before); + let (db, file_id) = RootDatabase::with_single_file(&before); let frange = FileRange { file_id, range: TextRange::offset_len(before_cursor_pos, 0.into()) }; let assist = @@ -384,12 +379,12 @@ mod helpers { } pub(crate) fn check_assist_range_target( - assist: fn(AssistCtx) -> Option, + assist: fn(AssistCtx) -> Option, before: &str, target: &str, ) { let (range, before) = extract_range(before); - let (db, file_id) = TestDB::with_single_file(&before); + let (db, file_id) = RootDatabase::with_single_file(&before); let frange = FileRange { file_id, range }; let assist = AssistCtx::with_ctx(&db, frange, true, assist).expect("code action is not applicable"); @@ -403,11 +398,11 @@ mod helpers { } pub(crate) fn check_assist_not_applicable( - assist: fn(AssistCtx) -> Option, + assist: fn(AssistCtx) -> Option, before: &str, ) { let (before_cursor_pos, before) = extract_offset(before); - let (db, file_id) = TestDB::with_single_file(&before); + let (db, file_id) = RootDatabase::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, assist); @@ -415,12 +410,12 @@ mod helpers { } pub(crate) fn check_assist_with_imports_locator_not_applicable( - assist: fn(AssistCtx, &mut F) -> Option, - imports_locator_provider: fn(db: Arc, file_id: FileId) -> F, + assist: fn(AssistCtx, &mut F) -> Option, + 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, file_id) = RootDatabase::with_single_file(&before); let db = Arc::new(db); let mut imports_locator = imports_locator_provider(Arc::clone(&db), file_id); let frange = @@ -431,11 +426,11 @@ mod helpers { } pub(crate) fn check_assist_range_not_applicable( - assist: fn(AssistCtx) -> Option, + assist: fn(AssistCtx) -> Option, before: &str, ) { let (range, before) = extract_range(before); - let (db, file_id) = TestDB::with_single_file(&before); + let (db, file_id) = RootDatabase::with_single_file(&before); let frange = FileRange { file_id, range }; let assist = AssistCtx::with_ctx(&db, frange, true, assist); assert!(assist.is_none()); @@ -448,13 +443,13 @@ mod tests { use ra_syntax::TextRange; use test_utils::{extract_offset, extract_range}; - use crate::test_db::TestDB; + use ra_ide_db::RootDatabase; #[test] fn assist_order_field_struct() { let before = "struct Foo { <|>bar: u32 }"; let (before_cursor_pos, before) = extract_offset(before); - let (db, file_id) = TestDB::with_single_file(&before); + let (db, file_id) = RootDatabase::with_single_file(&before); let frange = FileRange { file_id, range: TextRange::offset_len(before_cursor_pos, 0.into()) }; let assists = super::assists(&db, frange); @@ -478,7 +473,7 @@ mod tests { } }"; let (range, before) = extract_range(before); - let (db, file_id) = TestDB::with_single_file(&before); + let (db, file_id) = RootDatabase::with_single_file(&before); let frange = FileRange { file_id, range }; let assists = super::assists(&db, frange); let mut assists = assists.iter(); -- cgit v1.2.3 From f8965ffafd5cf467b3f0482ca962ba2bfd090161 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Thu, 6 Feb 2020 16:54:22 +0100 Subject: Remove assists TestDB --- crates/ra_assists/src/lib.rs | 2 -- 1 file changed, 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 a2109b751..be6e06842 100644 --- a/crates/ra_assists/src/lib.rs +++ b/crates/ra_assists/src/lib.rs @@ -9,8 +9,6 @@ mod assist_ctx; mod marks; #[cfg(test)] mod doc_tests; -#[cfg(test)] -mod test_db; pub mod ast_transform; use either::Either; -- cgit v1.2.3 From cf812c12d1ac7944d1c18877ee93bea02d91e99f Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Thu, 6 Feb 2020 16:58:57 +0100 Subject: Assists are not generic --- crates/ra_assists/src/lib.rs | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 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 be6e06842..ad8438b6c 100644 --- a/crates/ra_assists/src/lib.rs +++ b/crates/ra_assists/src/lib.rs @@ -148,7 +148,6 @@ fn sort_assists(assists: &mut Vec) { mod assists { use crate::{Assist, AssistCtx, ImportsLocator}; - use hir::db::HirDatabase; mod add_derive; mod add_explicit_type; @@ -176,7 +175,7 @@ mod assists { mod move_bounds; mod early_return; - pub(crate) fn all() -> &'static [fn(AssistCtx) -> Option] { + pub(crate) fn all() -> &'static [fn(AssistCtx) -> Option] { &[ add_derive::add_derive, add_explicit_type::add_explicit_type, @@ -210,8 +209,8 @@ mod assists { ] } - pub(crate) fn all_with_imports_locator<'a, DB: HirDatabase, F: ImportsLocator>( - ) -> &'a [fn(AssistCtx, &mut F) -> Option] { + pub(crate) fn all_with_imports_locator<'a, F: ImportsLocator>( + ) -> &'a [fn(AssistCtx, &mut F) -> Option] { &[auto_import::auto_import] } } @@ -274,11 +273,7 @@ mod helpers { } } - pub(crate) fn check_assist( - assist: fn(AssistCtx) -> Option, - before: &str, - after: &str, - ) { + pub(crate) fn check_assist(assist: fn(AssistCtx) -> Option, before: &str, after: &str) { let (before_cursor_pos, before) = extract_offset(before); let (db, file_id) = RootDatabase::with_single_file(&before); let frange = @@ -303,7 +298,7 @@ mod helpers { } pub(crate) fn check_assist_with_imports_locator( - assist: fn(AssistCtx, &mut F) -> Option, + assist: fn(AssistCtx, &mut F) -> Option, imports_locator_provider: fn(db: Arc, file_id: FileId) -> F, before: &str, after: &str, @@ -335,7 +330,7 @@ mod helpers { } pub(crate) fn check_assist_range( - assist: fn(AssistCtx) -> Option, + assist: fn(AssistCtx) -> Option, before: &str, after: &str, ) { @@ -357,7 +352,7 @@ mod helpers { } pub(crate) fn check_assist_target( - assist: fn(AssistCtx) -> Option, + assist: fn(AssistCtx) -> Option, before: &str, target: &str, ) { @@ -377,7 +372,7 @@ mod helpers { } pub(crate) fn check_assist_range_target( - assist: fn(AssistCtx) -> Option, + assist: fn(AssistCtx) -> Option, before: &str, target: &str, ) { @@ -396,7 +391,7 @@ mod helpers { } pub(crate) fn check_assist_not_applicable( - assist: fn(AssistCtx) -> Option, + assist: fn(AssistCtx) -> Option, before: &str, ) { let (before_cursor_pos, before) = extract_offset(before); @@ -408,7 +403,7 @@ mod helpers { } pub(crate) fn check_assist_with_imports_locator_not_applicable( - assist: fn(AssistCtx, &mut F) -> Option, + assist: fn(AssistCtx, &mut F) -> Option, imports_locator_provider: fn(db: Arc, file_id: FileId) -> F, before: &str, ) { @@ -424,7 +419,7 @@ mod helpers { } pub(crate) fn check_assist_range_not_applicable( - assist: fn(AssistCtx) -> Option, + assist: fn(AssistCtx) -> Option, before: &str, ) { let (range, before) = extract_range(before); -- cgit v1.2.3 From ad204f7562747150c4f570d7ce648f2539530b76 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Thu, 6 Feb 2020 17:17:51 +0100 Subject: Mostly remove ImoportLocator infra --- crates/ra_assists/src/lib.rs | 158 ++----------------------------------------- 1 file changed, 6 insertions(+), 152 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 ad8438b6c..8285e93a4 100644 --- a/crates/ra_assists/src/lib.rs +++ b/crates/ra_assists/src/lib.rs @@ -12,9 +12,8 @@ mod doc_tests; pub mod ast_transform; use either::Either; -use hir::ModuleDef; use ra_db::FileRange; -use ra_ide_db::{imports_locator::ImportsLocatorIde, RootDatabase}; +use ra_ide_db::RootDatabase; use ra_syntax::{TextRange, TextUnit}; use ra_text_edit::TextEdit; @@ -73,50 +72,6 @@ pub fn applicable_assists(db: &RootDatabase, range: FileRange) -> Vec Vec; -} - -impl ImportsLocator for ImportsLocatorIde<'_> { - fn find_imports(&mut self, name_to_import: &str) -> Vec { - self.find_imports(name_to_import) - } -} - -/// 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(db: &RootDatabase, range: FileRange) -> Vec { - let mut imports_locator = ImportsLocatorIde::new(db); - 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 @@ -147,7 +102,7 @@ fn sort_assists(assists: &mut Vec) { } mod assists { - use crate::{Assist, AssistCtx, ImportsLocator}; + use crate::{Assist, AssistCtx}; mod add_derive; mod add_explicit_type; @@ -206,72 +161,19 @@ mod assists { raw_string::make_usual_string, raw_string::remove_hash, early_return::convert_to_guarded_return, + auto_import::auto_import, ] } - - pub(crate) fn all_with_imports_locator<'a, F: ImportsLocator>( - ) -> &'a [fn(AssistCtx, &mut F) -> Option] { - &[auto_import::auto_import] - } } #[cfg(test)] mod helpers { - use hir::db::DefDatabase; - use ra_db::{fixture::WithFixture, FileId, FileRange}; + use ra_db::{fixture::WithFixture, FileRange}; + use ra_ide_db::RootDatabase; use ra_syntax::TextRange; use test_utils::{add_cursor, assert_eq_text, extract_offset, extract_range}; - use crate::{Assist, AssistCtx, ImportsLocator}; - use ra_ide_db::RootDatabase; - 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, - } - - 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::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::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() - { - 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 - } - } + use crate::{Assist, AssistCtx}; pub(crate) fn check_assist(assist: fn(AssistCtx) -> Option, before: &str, after: &str) { let (before_cursor_pos, before) = extract_offset(before); @@ -297,38 +199,6 @@ mod helpers { assert_eq_text!(after, &actual); } - pub(crate) fn check_assist_with_imports_locator( - assist: fn(AssistCtx, &mut F) -> Option, - 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) = RootDatabase::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.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(), - }; - - 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, @@ -402,22 +272,6 @@ mod helpers { assert!(assist.is_none()); } - pub(crate) fn check_assist_with_imports_locator_not_applicable( - assist: fn(AssistCtx, &mut F) -> Option, - imports_locator_provider: fn(db: Arc, file_id: FileId) -> F, - before: &str, - ) { - let (before_cursor_pos, before) = extract_offset(before); - let (db, file_id) = RootDatabase::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.as_ref(), frange, true, |ctx| assist(ctx, &mut 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 d1e8b8d134da802eecef5cbcd5486bd542ad75b5 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Thu, 6 Feb 2020 17:42:17 +0100 Subject: Fix tests --- crates/ra_assists/src/lib.rs | 35 +++++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 12 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 8285e93a4..1343043dd 100644 --- a/crates/ra_assists/src/lib.rs +++ b/crates/ra_assists/src/lib.rs @@ -168,16 +168,27 @@ mod assists { #[cfg(test)] mod helpers { - use ra_db::{fixture::WithFixture, FileRange}; - use ra_ide_db::RootDatabase; + use std::sync::Arc; + + use ra_db::{fixture::WithFixture, FileId, FileRange, SourceDatabaseExt}; + use ra_ide_db::{symbol_index::SymbolsDatabase, RootDatabase}; use ra_syntax::TextRange; use test_utils::{add_cursor, assert_eq_text, extract_offset, extract_range}; use crate::{Assist, AssistCtx}; + pub(crate) fn with_single_file(text: &str) -> (RootDatabase, FileId) { + let (mut db, file_id) = RootDatabase::with_single_file(text); + // FIXME: ideally, this should be done by the above `RootDatabase::with_single_file`, + // but it looks like this might need specialization? :( + let local_roots = vec![db.file_source_root(file_id)]; + db.set_local_roots(Arc::new(local_roots)); + (db, file_id) + } + pub(crate) fn check_assist(assist: fn(AssistCtx) -> Option, before: &str, after: &str) { let (before_cursor_pos, before) = extract_offset(before); - let (db, file_id) = RootDatabase::with_single_file(&before); + let (db, file_id) = with_single_file(&before); let frange = FileRange { file_id, range: TextRange::offset_len(before_cursor_pos, 0.into()) }; let assist = @@ -205,7 +216,7 @@ mod helpers { after: &str, ) { let (range, before) = extract_range(before); - let (db, file_id) = RootDatabase::with_single_file(&before); + let (db, file_id) = with_single_file(&before); let frange = FileRange { file_id, range }; let assist = AssistCtx::with_ctx(&db, frange, true, assist).expect("code action is not applicable"); @@ -227,7 +238,7 @@ mod helpers { target: &str, ) { let (before_cursor_pos, before) = extract_offset(before); - let (db, file_id) = RootDatabase::with_single_file(&before); + let (db, file_id) = with_single_file(&before); let frange = FileRange { file_id, range: TextRange::offset_len(before_cursor_pos, 0.into()) }; let assist = @@ -247,7 +258,7 @@ mod helpers { target: &str, ) { let (range, before) = extract_range(before); - let (db, file_id) = RootDatabase::with_single_file(&before); + let (db, file_id) = with_single_file(&before); let frange = FileRange { file_id, range }; let assist = AssistCtx::with_ctx(&db, frange, true, assist).expect("code action is not applicable"); @@ -265,7 +276,7 @@ mod helpers { before: &str, ) { let (before_cursor_pos, before) = extract_offset(before); - let (db, file_id) = RootDatabase::with_single_file(&before); + let (db, file_id) = 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, assist); @@ -277,7 +288,7 @@ mod helpers { before: &str, ) { let (range, before) = extract_range(before); - let (db, file_id) = RootDatabase::with_single_file(&before); + let (db, file_id) = with_single_file(&before); let frange = FileRange { file_id, range }; let assist = AssistCtx::with_ctx(&db, frange, true, assist); assert!(assist.is_none()); @@ -286,17 +297,17 @@ mod helpers { #[cfg(test)] mod tests { - use ra_db::{fixture::WithFixture, FileRange}; + use ra_db::FileRange; use ra_syntax::TextRange; use test_utils::{extract_offset, extract_range}; - use ra_ide_db::RootDatabase; + use crate::helpers; #[test] fn assist_order_field_struct() { let before = "struct Foo { <|>bar: u32 }"; let (before_cursor_pos, before) = extract_offset(before); - let (db, file_id) = RootDatabase::with_single_file(&before); + let (db, file_id) = helpers::with_single_file(&before); let frange = FileRange { file_id, range: TextRange::offset_len(before_cursor_pos, 0.into()) }; let assists = super::assists(&db, frange); @@ -320,7 +331,7 @@ mod tests { } }"; let (range, before) = extract_range(before); - let (db, file_id) = RootDatabase::with_single_file(&before); + let (db, file_id) = helpers::with_single_file(&before); let frange = FileRange { file_id, range }; let assists = super::assists(&db, frange); let mut assists = assists.iter(); -- cgit v1.2.3 From 7e73b7a5f8d594a85627786a13e76d9d70163770 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Thu, 6 Feb 2020 18:46:11 +0100 Subject: Minor rename --- crates/ra_assists/src/lib.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 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 1343043dd..3f3df3f96 100644 --- a/crates/ra_assists/src/lib.rs +++ b/crates/ra_assists/src/lib.rs @@ -59,7 +59,7 @@ impl ResolvedAssist { /// /// Assists are returned in the "unresolved" state, that is only labels are /// returned, without actual edits. -pub fn applicable_assists(db: &RootDatabase, range: FileRange) -> Vec { +pub fn unresolved_assists(db: &RootDatabase, range: FileRange) -> Vec { AssistCtx::with_ctx(db, range, false, |ctx| { assists::all() .iter() @@ -76,7 +76,7 @@ pub fn applicable_assists(db: &RootDatabase, range: FileRange) -> Vec Vec { +pub fn resolved_assists(db: &RootDatabase, range: FileRange) -> Vec { AssistCtx::with_ctx(db, range, true, |ctx| { let mut a = assists::all() .iter() @@ -301,7 +301,7 @@ mod tests { use ra_syntax::TextRange; use test_utils::{extract_offset, extract_range}; - use crate::helpers; + use crate::{helpers, resolved_assists}; #[test] fn assist_order_field_struct() { @@ -310,7 +310,7 @@ mod tests { let (db, file_id) = helpers::with_single_file(&before); let frange = FileRange { file_id, range: TextRange::offset_len(before_cursor_pos, 0.into()) }; - let assists = super::assists(&db, frange); + let assists = resolved_assists(&db, frange); let mut assists = assists.iter(); assert_eq!( @@ -333,7 +333,7 @@ mod tests { let (range, before) = extract_range(before); let (db, file_id) = helpers::with_single_file(&before); let frange = FileRange { file_id, range }; - let assists = super::assists(&db, frange); + let assists = resolved_assists(&db, frange); let mut assists = assists.iter(); assert_eq!(assists.next().expect("expected assist").label.label, "Extract into variable"); -- cgit v1.2.3 From b831b17b3dbc475420924834b250f07bccd673d0 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Fri, 7 Feb 2020 14:53:50 +0100 Subject: Simplify --- crates/ra_assists/src/lib.rs | 55 ++++++++++++++++++++++---------------------- 1 file changed, 27 insertions(+), 28 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 3f3df3f96..fcdfe6c14 100644 --- a/crates/ra_assists/src/lib.rs +++ b/crates/ra_assists/src/lib.rs @@ -37,6 +37,7 @@ pub struct AssistAction { pub label: Option, pub edit: TextEdit, pub cursor_position: Option, + // FIXME: This belongs to `AssistLabel` pub target: Option, } @@ -60,16 +61,15 @@ impl ResolvedAssist { /// Assists are returned in the "unresolved" state, that is only labels are /// returned, without actual edits. pub fn unresolved_assists(db: &RootDatabase, range: FileRange) -> Vec { - AssistCtx::with_ctx(db, range, false, |ctx| { - assists::all() - .iter() - .filter_map(|f| f(ctx.clone())) - .map(|a| match a { - Assist::Unresolved { label } => label, - Assist::Resolved { .. } => unreachable!(), - }) - .collect() - }) + let ctx = AssistCtx::new(db, range, false); + assists::all() + .iter() + .filter_map(|f| f(ctx.clone())) + .map(|a| match a { + Assist::Unresolved { label } => label, + Assist::Resolved { .. } => unreachable!(), + }) + .collect() } /// Return all the assists applicable at the given position. @@ -77,18 +77,17 @@ pub fn unresolved_assists(db: &RootDatabase, range: FileRange) -> Vec Vec { - AssistCtx::with_ctx(db, range, true, |ctx| { - let mut a = assists::all() - .iter() - .filter_map(|f| f(ctx.clone())) - .map(|a| match a { - Assist::Resolved { assist } => assist, - Assist::Unresolved { .. } => unreachable!(), - }) - .collect(); - sort_assists(&mut a); - a - }) + let ctx = AssistCtx::new(db, range, true); + let mut a = assists::all() + .iter() + .filter_map(|f| f(ctx.clone())) + .map(|a| match a { + Assist::Resolved { assist } => assist, + Assist::Unresolved { .. } => unreachable!(), + }) + .collect(); + sort_assists(&mut a); + a } fn sort_assists(assists: &mut Vec) { @@ -192,7 +191,7 @@ mod helpers { let frange = FileRange { file_id, range: TextRange::offset_len(before_cursor_pos, 0.into()) }; let assist = - AssistCtx::with_ctx(&db, frange, true, assist).expect("code action is not applicable"); + assist(AssistCtx::new(&db, frange, true)).expect("code action is not applicable"); let action = match assist { Assist::Unresolved { .. } => unreachable!(), Assist::Resolved { assist } => assist.get_first_action(), @@ -219,7 +218,7 @@ mod helpers { let (db, file_id) = with_single_file(&before); let frange = FileRange { file_id, range }; let assist = - AssistCtx::with_ctx(&db, frange, true, assist).expect("code action is not applicable"); + assist(AssistCtx::new(&db, frange, true)).expect("code action is not applicable"); let action = match assist { Assist::Unresolved { .. } => unreachable!(), Assist::Resolved { assist } => assist.get_first_action(), @@ -242,7 +241,7 @@ mod helpers { let frange = FileRange { file_id, range: TextRange::offset_len(before_cursor_pos, 0.into()) }; let assist = - AssistCtx::with_ctx(&db, frange, true, assist).expect("code action is not applicable"); + assist(AssistCtx::new(&db, frange, true)).expect("code action is not applicable"); let action = match assist { Assist::Unresolved { .. } => unreachable!(), Assist::Resolved { assist } => assist.get_first_action(), @@ -261,7 +260,7 @@ mod helpers { let (db, file_id) = with_single_file(&before); let frange = FileRange { file_id, range }; let assist = - AssistCtx::with_ctx(&db, frange, true, assist).expect("code action is not applicable"); + assist(AssistCtx::new(&db, frange, true)).expect("code action is not applicable"); let action = match assist { Assist::Unresolved { .. } => unreachable!(), Assist::Resolved { assist } => assist.get_first_action(), @@ -279,7 +278,7 @@ mod helpers { let (db, file_id) = 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, assist); + let assist = assist(AssistCtx::new(&db, frange, true)); assert!(assist.is_none()); } @@ -290,7 +289,7 @@ mod helpers { let (range, before) = extract_range(before); let (db, file_id) = with_single_file(&before); let frange = FileRange { file_id, range }; - let assist = AssistCtx::with_ctx(&db, frange, true, assist); + let assist = assist(AssistCtx::new(&db, frange, true)); assert!(assist.is_none()); } } -- cgit v1.2.3 From 2d95047f7c273f9e97c33b93487c9091ec6abcb7 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Fri, 7 Feb 2020 14:55:47 +0100 Subject: Cleanup --- crates/ra_assists/src/lib.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 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 fcdfe6c14..b71df7e5d 100644 --- a/crates/ra_assists/src/lib.rs +++ b/crates/ra_assists/src/lib.rs @@ -11,6 +11,8 @@ mod marks; mod doc_tests; pub mod ast_transform; +use std::cmp::Ordering; + use either::Either; use ra_db::FileRange; use ra_ide_db::RootDatabase; @@ -85,13 +87,12 @@ pub fn resolved_assists(db: &RootDatabase, range: FileRange) -> Vec assist, Assist::Unresolved { .. } => unreachable!(), }) - .collect(); + .collect::>(); sort_assists(&mut a); a } -fn sort_assists(assists: &mut Vec) { - use std::cmp::Ordering; +fn sort_assists(assists: &mut [ResolvedAssist]) { 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, -- cgit v1.2.3 From 6ac9c4ad6ae2ce246a8c70d468ae2dabb484a03d Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Fri, 7 Feb 2020 15:04:50 +0100 Subject: Cleanup --- crates/ra_assists/src/lib.rs | 8 ++++++++ 1 file changed, 8 insertions(+) (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 b71df7e5d..d476088a2 100644 --- a/crates/ra_assists/src/lib.rs +++ b/crates/ra_assists/src/lib.rs @@ -34,6 +34,14 @@ pub struct AssistLabel { pub id: AssistId, } +impl AssistLabel { + pub(crate) fn new(label: String, id: AssistId) -> AssistLabel { + // FIXME: make fields private, so that this invariant can't be broken + assert!(label.chars().nth(0).unwrap().is_uppercase()); + AssistLabel { label: label.into(), id } + } +} + #[derive(Debug, Clone)] pub struct AssistAction { pub label: Option, -- cgit v1.2.3 From 561b4b11ff1d87ea1ff2477dcba6ae1f396573a3 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Fri, 7 Feb 2020 15:53:31 +0100 Subject: Name assist handlers --- crates/ra_assists/src/lib.rs | 46 ++++++++++++++------------------------------ 1 file changed, 14 insertions(+), 32 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 d476088a2..7b08e8fd7 100644 --- a/crates/ra_assists/src/lib.rs +++ b/crates/ra_assists/src/lib.rs @@ -19,8 +19,8 @@ use ra_ide_db::RootDatabase; use ra_syntax::{TextRange, TextUnit}; use ra_text_edit::TextEdit; -pub(crate) use crate::assist_ctx::{Assist, AssistCtx}; -pub use crate::assists::add_import::auto_import_text_edit; +pub(crate) use crate::assist_ctx::{Assist, AssistCtx, AssistHandler}; +pub use crate::handlers::add_import::auto_import_text_edit; /// Unique identifier of the assist, should not be shown to the user /// directly. @@ -72,7 +72,7 @@ impl ResolvedAssist { /// returned, without actual edits. pub fn unresolved_assists(db: &RootDatabase, range: FileRange) -> Vec { let ctx = AssistCtx::new(db, range, false); - assists::all() + handlers::all() .iter() .filter_map(|f| f(ctx.clone())) .map(|a| match a { @@ -88,7 +88,7 @@ pub fn unresolved_assists(db: &RootDatabase, range: FileRange) -> Vec Vec { let ctx = AssistCtx::new(db, range, true); - let mut a = assists::all() + let mut a = handlers::all() .iter() .filter_map(|f| f(ctx.clone())) .map(|a| match a { @@ -109,8 +109,8 @@ fn sort_assists(assists: &mut [ResolvedAssist]) { }); } -mod assists { - use crate::{Assist, AssistCtx}; +mod handlers { + use crate::AssistHandler; mod add_derive; mod add_explicit_type; @@ -138,7 +138,7 @@ mod assists { mod move_bounds; mod early_return; - pub(crate) fn all() -> &'static [fn(AssistCtx) -> Option] { + pub(crate) fn all() -> &'static [AssistHandler] { &[ add_derive::add_derive, add_explicit_type::add_explicit_type, @@ -183,7 +183,7 @@ mod helpers { use ra_syntax::TextRange; use test_utils::{add_cursor, assert_eq_text, extract_offset, extract_range}; - use crate::{Assist, AssistCtx}; + use crate::{Assist, AssistCtx, AssistHandler}; pub(crate) fn with_single_file(text: &str) -> (RootDatabase, FileId) { let (mut db, file_id) = RootDatabase::with_single_file(text); @@ -194,7 +194,7 @@ mod helpers { (db, file_id) } - pub(crate) fn check_assist(assist: fn(AssistCtx) -> Option, before: &str, after: &str) { + pub(crate) fn check_assist(assist: AssistHandler, before: &str, after: &str) { let (before_cursor_pos, before) = extract_offset(before); let (db, file_id) = with_single_file(&before); let frange = @@ -218,11 +218,7 @@ mod helpers { assert_eq_text!(after, &actual); } - pub(crate) fn check_assist_range( - assist: fn(AssistCtx) -> Option, - before: &str, - after: &str, - ) { + pub(crate) fn check_assist_range(assist: AssistHandler, before: &str, after: &str) { let (range, before) = extract_range(before); let (db, file_id) = with_single_file(&before); let frange = FileRange { file_id, range }; @@ -240,11 +236,7 @@ mod helpers { assert_eq_text!(after, &actual); } - pub(crate) fn check_assist_target( - assist: fn(AssistCtx) -> Option, - before: &str, - target: &str, - ) { + pub(crate) fn check_assist_target(assist: AssistHandler, before: &str, target: &str) { let (before_cursor_pos, before) = extract_offset(before); let (db, file_id) = with_single_file(&before); let frange = @@ -260,11 +252,7 @@ mod helpers { assert_eq_text!(&before[range.start().to_usize()..range.end().to_usize()], target); } - pub(crate) fn check_assist_range_target( - assist: fn(AssistCtx) -> Option, - before: &str, - target: &str, - ) { + pub(crate) fn check_assist_range_target(assist: AssistHandler, before: &str, target: &str) { let (range, before) = extract_range(before); let (db, file_id) = with_single_file(&before); let frange = FileRange { file_id, range }; @@ -279,10 +267,7 @@ mod helpers { assert_eq_text!(&before[range.start().to_usize()..range.end().to_usize()], target); } - pub(crate) fn check_assist_not_applicable( - assist: fn(AssistCtx) -> Option, - before: &str, - ) { + pub(crate) fn check_assist_not_applicable(assist: AssistHandler, before: &str) { let (before_cursor_pos, before) = extract_offset(before); let (db, file_id) = with_single_file(&before); let frange = @@ -291,10 +276,7 @@ mod helpers { assert!(assist.is_none()); } - pub(crate) fn check_assist_range_not_applicable( - assist: fn(AssistCtx) -> Option, - before: &str, - ) { + pub(crate) fn check_assist_range_not_applicable(assist: AssistHandler, before: &str) { let (range, before) = extract_range(before); let (db, file_id) = with_single_file(&before); let frange = FileRange { file_id, range }; -- cgit v1.2.3 From d00add1f1fec59494c3c1a99c27937ae3891458d Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Fri, 7 Feb 2020 15:57:38 +0100 Subject: Introduce assists utils --- crates/ra_assists/src/lib.rs | 1 + 1 file changed, 1 insertion(+) (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 7b08e8fd7..eca6dec4b 100644 --- a/crates/ra_assists/src/lib.rs +++ b/crates/ra_assists/src/lib.rs @@ -9,6 +9,7 @@ mod assist_ctx; mod marks; #[cfg(test)] mod doc_tests; +mod utils; pub mod ast_transform; use std::cmp::Ordering; -- cgit v1.2.3 From 740a26b7d26a68cc46becda3cca39091e8da67fc Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Fri, 7 Feb 2020 23:35:34 +0200 Subject: Rename add import assist --- crates/ra_assists/src/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 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 eca6dec4b..f79189ae8 100644 --- a/crates/ra_assists/src/lib.rs +++ b/crates/ra_assists/src/lib.rs @@ -21,7 +21,7 @@ use ra_syntax::{TextRange, TextUnit}; use ra_text_edit::TextEdit; pub(crate) use crate::assist_ctx::{Assist, AssistCtx, AssistHandler}; -pub use crate::handlers::add_import::auto_import_text_edit; +pub use crate::handlers::replace_qualified_name_with_use::insert_use_statement; /// Unique identifier of the assist, should not be shown to the user /// directly. @@ -133,7 +133,7 @@ mod handlers { mod replace_if_let_with_match; mod split_import; mod remove_dbg; - pub(crate) mod add_import; + pub(crate) mod replace_qualified_name_with_use; mod add_missing_impl_members; mod move_guard; mod move_bounds; @@ -158,7 +158,7 @@ mod handlers { replace_if_let_with_match::replace_if_let_with_match, split_import::split_import, remove_dbg::remove_dbg, - add_import::add_import, + replace_qualified_name_with_use::replace_qualified_name_with_use, add_missing_impl_members::add_missing_impl_members, add_missing_impl_members::add_missing_default_members, inline_local_variable::inline_local_variable, -- cgit v1.2.3 From 9769c5140c9c406a4cc880e698593a6c4bcc6826 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sun, 9 Feb 2020 15:32:53 +0100 Subject: Simplify Assists interface Instead of building a physical tree structure, just "tag" related assists with the same group --- crates/ra_assists/src/lib.rs | 64 +++++++++++--------------------------------- 1 file changed, 15 insertions(+), 49 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 f79189ae8..828a8e9e8 100644 --- a/crates/ra_assists/src/lib.rs +++ b/crates/ra_assists/src/lib.rs @@ -12,9 +12,6 @@ mod doc_tests; mod utils; pub mod ast_transform; -use std::cmp::Ordering; - -use either::Either; use ra_db::FileRange; use ra_ide_db::RootDatabase; use ra_syntax::{TextRange, TextUnit}; @@ -35,6 +32,9 @@ pub struct AssistLabel { pub id: AssistId, } +#[derive(Clone, Debug)] +pub struct GroupLabel(pub String); + impl AssistLabel { pub(crate) fn new(label: String, id: AssistId) -> AssistLabel { // FIXME: make fields private, so that this invariant can't be broken @@ -45,7 +45,6 @@ impl AssistLabel { #[derive(Debug, Clone)] pub struct AssistAction { - pub label: Option, pub edit: TextEdit, pub cursor_position: Option, // FIXME: This belongs to `AssistLabel` @@ -55,16 +54,8 @@ pub struct AssistAction { #[derive(Debug, Clone)] pub struct ResolvedAssist { pub label: AssistLabel, - pub action_data: Either>, -} - -impl ResolvedAssist { - pub fn get_first_action(&self) -> AssistAction { - match &self.action_data { - Either::Left(action) => action.clone(), - Either::Right(actions) => actions[0].clone(), - } - } + pub group_label: Option, + pub action: AssistAction, } /// Return all the assists applicable at the given position. @@ -76,10 +67,8 @@ pub fn unresolved_assists(db: &RootDatabase, range: FileRange) -> Vec label, - Assist::Resolved { .. } => unreachable!(), - }) + .flat_map(|it| it.0) + .map(|a| a.label) .collect() } @@ -92,24 +81,13 @@ pub fn resolved_assists(db: &RootDatabase, range: FileRange) -> Vec assist, - Assist::Unresolved { .. } => unreachable!(), - }) + .flat_map(|it| it.0) + .map(|it| it.into_resolved().unwrap()) .collect::>(); - sort_assists(&mut a); + a.sort_by_key(|it| it.action.target.map_or(TextUnit::from(!0u32), |it| it.len())); a } -fn sort_assists(assists: &mut [ResolvedAssist]) { - 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 handlers { use crate::AssistHandler; @@ -184,7 +162,7 @@ mod helpers { use ra_syntax::TextRange; use test_utils::{add_cursor, assert_eq_text, extract_offset, extract_range}; - use crate::{Assist, AssistCtx, AssistHandler}; + use crate::{AssistCtx, AssistHandler}; pub(crate) fn with_single_file(text: &str) -> (RootDatabase, FileId) { let (mut db, file_id) = RootDatabase::with_single_file(text); @@ -202,10 +180,7 @@ mod helpers { FileRange { file_id, range: TextRange::offset_len(before_cursor_pos, 0.into()) }; let assist = assist(AssistCtx::new(&db, frange, true)).expect("code action is not applicable"); - let action = match assist { - Assist::Unresolved { .. } => unreachable!(), - Assist::Resolved { assist } => assist.get_first_action(), - }; + let action = assist.0[0].action.clone().unwrap(); let actual = action.edit.apply(&before); let actual_cursor_pos = match action.cursor_position { @@ -225,10 +200,7 @@ mod helpers { let frange = FileRange { file_id, range }; let assist = assist(AssistCtx::new(&db, frange, true)).expect("code action is not applicable"); - let action = match assist { - Assist::Unresolved { .. } => unreachable!(), - Assist::Resolved { assist } => assist.get_first_action(), - }; + let action = assist.0[0].action.clone().unwrap(); let mut actual = action.edit.apply(&before); if let Some(pos) = action.cursor_position { @@ -244,10 +216,7 @@ mod helpers { FileRange { file_id, range: TextRange::offset_len(before_cursor_pos, 0.into()) }; let assist = assist(AssistCtx::new(&db, frange, true)).expect("code action is not applicable"); - let action = match assist { - Assist::Unresolved { .. } => unreachable!(), - Assist::Resolved { assist } => assist.get_first_action(), - }; + let action = assist.0[0].action.clone().unwrap(); let range = action.target.expect("expected target on action"); assert_eq_text!(&before[range.start().to_usize()..range.end().to_usize()], target); @@ -259,10 +228,7 @@ mod helpers { let frange = FileRange { file_id, range }; let assist = assist(AssistCtx::new(&db, frange, true)).expect("code action is not applicable"); - let action = match assist { - Assist::Unresolved { .. } => unreachable!(), - Assist::Resolved { assist } => assist.get_first_action(), - }; + let action = assist.0[0].action.clone().unwrap(); let range = action.target.expect("expected target on action"); assert_eq_text!(&before[range.start().to_usize()..range.end().to_usize()], target); -- cgit v1.2.3