From e5cdcb8b124f5b7d59950429787e760e46388f72 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Mon, 3 May 2021 17:08:09 +0300 Subject: Add a way to resolve certain assists --- crates/ide_assists/src/assist_context.rs | 10 ++++++---- crates/ide_assists/src/lib.rs | 24 +++++++++++++++++++++--- crates/ide_assists/src/tests.rs | 21 ++++++++++++--------- 3 files changed, 39 insertions(+), 16 deletions(-) (limited to 'crates/ide_assists/src') diff --git a/crates/ide_assists/src/assist_context.rs b/crates/ide_assists/src/assist_context.rs index 8714e4978..19e9f179e 100644 --- a/crates/ide_assists/src/assist_context.rs +++ b/crates/ide_assists/src/assist_context.rs @@ -19,7 +19,9 @@ use syntax::{ }; use text_edit::{TextEdit, TextEditBuilder}; -use crate::{assist_config::AssistConfig, Assist, AssistId, AssistKind, GroupLabel}; +use crate::{ + assist_config::AssistConfig, Assist, AssistId, AssistKind, AssistResolveStrategy, GroupLabel, +}; /// `AssistContext` allows to apply an assist or check if it could be applied. /// @@ -105,14 +107,14 @@ impl<'a> AssistContext<'a> { } pub(crate) struct Assists { - resolve: bool, file: FileId, + resolve: AssistResolveStrategy, buf: Vec, allowed: Option>, } impl Assists { - pub(crate) fn new(ctx: &AssistContext, resolve: bool) -> Assists { + pub(crate) fn new(ctx: &AssistContext, resolve: AssistResolveStrategy) -> Assists { Assists { resolve, file: ctx.frange.file_id, @@ -158,7 +160,7 @@ impl Assists { } fn add_impl(&mut self, mut assist: Assist, f: impl FnOnce(&mut AssistBuilder)) -> Option<()> { - let source_change = if self.resolve { + let source_change = if self.resolve.should_resolve(&assist.id) { let mut builder = AssistBuilder::new(self.file); f(&mut builder); Some(builder.finish()) diff --git a/crates/ide_assists/src/lib.rs b/crates/ide_assists/src/lib.rs index 88ae5c9a9..397d2a3d0 100644 --- a/crates/ide_assists/src/lib.rs +++ b/crates/ide_assists/src/lib.rs @@ -26,7 +26,7 @@ pub(crate) use crate::assist_context::{AssistContext, Assists}; pub use assist_config::AssistConfig; -#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub enum AssistKind { // FIXME: does the None variant make sense? Probably not. None, @@ -60,9 +60,27 @@ impl AssistKind { /// Unique identifier of the assist, should not be shown to the user /// directly. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub struct AssistId(pub &'static str, pub AssistKind); +// TODO kb docs +#[derive(Debug, Clone, Copy)] +pub enum AssistResolveStrategy { + None, + All, + Single(AssistId), +} + +impl AssistResolveStrategy { + pub fn should_resolve(&self, id: &AssistId) -> bool { + match self { + AssistResolveStrategy::None => false, + AssistResolveStrategy::All => true, + AssistResolveStrategy::Single(id_to_resolve) => id_to_resolve == id, + } + } +} + #[derive(Clone, Debug)] pub struct GroupLabel(pub String); @@ -91,7 +109,7 @@ impl Assist { pub fn get( db: &RootDatabase, config: &AssistConfig, - resolve: bool, + resolve: AssistResolveStrategy, range: FileRange, ) -> Vec { let sema = Semantics::new(db); diff --git a/crates/ide_assists/src/tests.rs b/crates/ide_assists/src/tests.rs index 6f4f97361..227687766 100644 --- a/crates/ide_assists/src/tests.rs +++ b/crates/ide_assists/src/tests.rs @@ -12,7 +12,10 @@ use stdx::{format_to, trim_indent}; use syntax::TextRange; use test_utils::{assert_eq_text, extract_offset}; -use crate::{handlers::Handler, Assist, AssistConfig, AssistContext, AssistKind, Assists}; +use crate::{ + handlers::Handler, Assist, AssistConfig, AssistContext, AssistKind, AssistResolveStrategy, + Assists, +}; pub(crate) const TEST_CONFIG: AssistConfig = AssistConfig { snippet_cap: SnippetCap::new(true), @@ -65,14 +68,14 @@ fn check_doc_test(assist_id: &str, before: &str, after: &str) { let before = db.file_text(file_id).to_string(); let frange = FileRange { file_id, range: selection.into() }; - let assist = Assist::get(&db, &TEST_CONFIG, true, frange) + let assist = Assist::get(&db, &TEST_CONFIG, AssistResolveStrategy::All, frange) .into_iter() .find(|assist| assist.id.0 == assist_id) .unwrap_or_else(|| { panic!( "\n\nAssist is not applicable: {}\nAvailable assists: {}", assist_id, - Assist::get(&db, &TEST_CONFIG, false, frange) + Assist::get(&db, &TEST_CONFIG, AssistResolveStrategy::None, frange) .into_iter() .map(|assist| assist.id.0) .collect::>() @@ -108,7 +111,7 @@ fn check(handler: Handler, before: &str, expected: ExpectedResult, assist_label: let sema = Semantics::new(&db); let config = TEST_CONFIG; let ctx = AssistContext::new(sema, &config, frange); - let mut acc = Assists::new(&ctx, true); + let mut acc = Assists::new(&ctx, AssistResolveStrategy::All); handler(&mut acc, &ctx); let mut res = acc.finish(); @@ -186,7 +189,7 @@ fn assist_order_field_struct() { let (before_cursor_pos, before) = extract_offset(before); let (db, file_id) = with_single_file(&before); let frange = FileRange { file_id, range: TextRange::empty(before_cursor_pos) }; - let assists = Assist::get(&db, &TEST_CONFIG, false, frange); + let assists = Assist::get(&db, &TEST_CONFIG, AssistResolveStrategy::None, frange); let mut assists = assists.iter(); assert_eq!(assists.next().expect("expected assist").label, "Change visibility to pub(crate)"); @@ -211,7 +214,7 @@ pub fn test_some_range(a: int) -> bool { "#, ); - let assists = Assist::get(&db, &TEST_CONFIG, false, frange); + let assists = Assist::get(&db, &TEST_CONFIG, AssistResolveStrategy::None, frange); let expected = labels(&assists); expect![[r#" @@ -240,7 +243,7 @@ pub fn test_some_range(a: int) -> bool { let mut cfg = TEST_CONFIG; cfg.allowed = Some(vec![AssistKind::Refactor]); - let assists = Assist::get(&db, &cfg, false, frange); + let assists = Assist::get(&db, &cfg, AssistResolveStrategy::None, frange); let expected = labels(&assists); expect![[r#" @@ -255,7 +258,7 @@ pub fn test_some_range(a: int) -> bool { { let mut cfg = TEST_CONFIG; cfg.allowed = Some(vec![AssistKind::RefactorExtract]); - let assists = Assist::get(&db, &cfg, false, frange); + let assists = Assist::get(&db, &cfg, AssistResolveStrategy::None, frange); let expected = labels(&assists); expect![[r#" @@ -268,7 +271,7 @@ pub fn test_some_range(a: int) -> bool { { let mut cfg = TEST_CONFIG; cfg.allowed = Some(vec![AssistKind::QuickFix]); - let assists = Assist::get(&db, &cfg, false, frange); + let assists = Assist::get(&db, &cfg, AssistResolveStrategy::None, frange); let expected = labels(&assists); expect![[r#""#]].assert_eq(&expected); -- cgit v1.2.3 From 1679a376f30c5ad8971c0f855074a3f489fee5fa Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Mon, 3 May 2021 18:03:28 +0300 Subject: Resolve single assist only --- crates/ide_assists/src/lib.rs | 39 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 36 insertions(+), 3 deletions(-) (limited to 'crates/ide_assists/src') diff --git a/crates/ide_assists/src/lib.rs b/crates/ide_assists/src/lib.rs index 397d2a3d0..01addffe9 100644 --- a/crates/ide_assists/src/lib.rs +++ b/crates/ide_assists/src/lib.rs @@ -17,6 +17,8 @@ mod tests; pub mod utils; pub mod ast_transform; +use std::str::FromStr; + use hir::Semantics; use ide_db::base_db::FileRange; use ide_db::{label::Label, source_change::SourceChange, RootDatabase}; @@ -56,6 +58,35 @@ impl AssistKind { _ => return false, } } + + pub fn name(&self) -> &str { + match self { + AssistKind::None => "None", + AssistKind::QuickFix => "QuickFix", + AssistKind::Generate => "Generate", + AssistKind::Refactor => "Refactor", + AssistKind::RefactorExtract => "RefactorExtract", + AssistKind::RefactorInline => "RefactorInline", + AssistKind::RefactorRewrite => "RefactorRewrite", + } + } +} + +impl FromStr for AssistKind { + type Err = String; + + fn from_str(s: &str) -> Result { + match s { + "None" => Ok(AssistKind::None), + "QuickFix" => Ok(AssistKind::QuickFix), + "Generate" => Ok(AssistKind::Generate), + "Refactor" => Ok(AssistKind::Refactor), + "RefactorExtract" => Ok(AssistKind::RefactorExtract), + "RefactorInline" => Ok(AssistKind::RefactorInline), + "RefactorRewrite" => Ok(AssistKind::RefactorRewrite), + unknown => Err(format!("Unknown AssistKind: '{}'", unknown)), + } + } } /// Unique identifier of the assist, should not be shown to the user @@ -64,11 +95,11 @@ impl AssistKind { pub struct AssistId(pub &'static str, pub AssistKind); // TODO kb docs -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone)] pub enum AssistResolveStrategy { None, All, - Single(AssistId), + Single(String, AssistKind), } impl AssistResolveStrategy { @@ -76,7 +107,9 @@ impl AssistResolveStrategy { match self { AssistResolveStrategy::None => false, AssistResolveStrategy::All => true, - AssistResolveStrategy::Single(id_to_resolve) => id_to_resolve == id, + AssistResolveStrategy::Single(id_to_resolve, kind) => { + id_to_resolve == id.0 && kind == &id.1 + } } } } -- cgit v1.2.3 From 28293d370ffc4270bb6244579166f0df18962951 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Mon, 3 May 2021 18:16:35 +0300 Subject: Add docs and use better naming --- crates/ide_assists/src/lib.rs | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) (limited to 'crates/ide_assists/src') diff --git a/crates/ide_assists/src/lib.rs b/crates/ide_assists/src/lib.rs index 01addffe9..5a0047f03 100644 --- a/crates/ide_assists/src/lib.rs +++ b/crates/ide_assists/src/lib.rs @@ -94,12 +94,27 @@ impl FromStr for AssistKind { #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub struct AssistId(pub &'static str, pub AssistKind); -// TODO kb docs -#[derive(Debug, Clone)] +/// A way to control how many asssist to resolve during the assist resolution. +/// When an assist is resolved, its edits are calculated that might be costly to always do by default. +#[derive(Debug)] pub enum AssistResolveStrategy { + /// No assists should be resolved. None, + /// All assists should be resolved. All, - Single(String, AssistKind), + /// Only a certain assists should be resolved. + Single(SingleResolve), +} + +/// Hold the [`AssistId`] data of a certain assist to resolve. +/// The original id object cannot be used due to a `'static` lifetime +/// and the requirement to construct this struct dynamically during the resolve handling. +#[derive(Debug)] +pub struct SingleResolve { + /// The id of the assist. + pub assist_id: String, + // The kind of the assist. + pub assist_kind: AssistKind, } impl AssistResolveStrategy { @@ -107,8 +122,8 @@ impl AssistResolveStrategy { match self { AssistResolveStrategy::None => false, AssistResolveStrategy::All => true, - AssistResolveStrategy::Single(id_to_resolve, kind) => { - id_to_resolve == id.0 && kind == &id.1 + AssistResolveStrategy::Single(single_resolve) => { + single_resolve.assist_id == id.0 && single_resolve.assist_kind == id.1 } } } -- cgit v1.2.3 From 8089a227f4b40872cf8491cfb9e065a8b05705a2 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Mon, 3 May 2021 18:40:04 +0300 Subject: Tests added --- crates/ide_assists/src/tests.rs | 243 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 242 insertions(+), 1 deletion(-) (limited to 'crates/ide_assists/src') diff --git a/crates/ide_assists/src/tests.rs b/crates/ide_assists/src/tests.rs index 227687766..9c2847998 100644 --- a/crates/ide_assists/src/tests.rs +++ b/crates/ide_assists/src/tests.rs @@ -14,7 +14,7 @@ use test_utils::{assert_eq_text, extract_offset}; use crate::{ handlers::Handler, Assist, AssistConfig, AssistContext, AssistKind, AssistResolveStrategy, - Assists, + Assists, SingleResolve, }; pub(crate) const TEST_CONFIG: AssistConfig = AssistConfig { @@ -277,3 +277,244 @@ pub fn test_some_range(a: int) -> bool { expect![[r#""#]].assert_eq(&expected); } } + +#[test] +fn various_resolve_strategies() { + let (db, frange) = RootDatabase::with_range( + r#" +pub fn test_some_range(a: int) -> bool { + if let 2..6 = $05$0 { + true + } else { + false + } +} +"#, + ); + + let mut cfg = TEST_CONFIG; + cfg.allowed = Some(vec![AssistKind::RefactorExtract]); + + { + let assists = Assist::get(&db, &cfg, AssistResolveStrategy::None, frange); + assert_eq!(2, assists.len()); + let mut assists = assists.into_iter(); + + let extract_into_variable_assist = assists.next().unwrap(); + expect![[r#" + Assist { + id: AssistId( + "extract_variable", + RefactorExtract, + ), + label: "Extract into variable", + group: None, + target: 59..60, + source_change: None, + } + "#]] + .assert_debug_eq(&extract_into_variable_assist); + + let extract_into_function_assist = assists.next().unwrap(); + expect![[r#" + Assist { + id: AssistId( + "extract_function", + RefactorExtract, + ), + label: "Extract into function", + group: None, + target: 59..60, + source_change: None, + } + "#]] + .assert_debug_eq(&extract_into_function_assist); + } + + { + let assists = Assist::get( + &db, + &cfg, + AssistResolveStrategy::Single(SingleResolve { + assist_id: "SOMETHING_MISMATCHING".to_string(), + assist_kind: AssistKind::RefactorExtract, + }), + frange, + ); + assert_eq!(2, assists.len()); + let mut assists = assists.into_iter(); + + let extract_into_variable_assist = assists.next().unwrap(); + expect![[r#" + Assist { + id: AssistId( + "extract_variable", + RefactorExtract, + ), + label: "Extract into variable", + group: None, + target: 59..60, + source_change: None, + } + "#]] + .assert_debug_eq(&extract_into_variable_assist); + + let extract_into_function_assist = assists.next().unwrap(); + expect![[r#" + Assist { + id: AssistId( + "extract_function", + RefactorExtract, + ), + label: "Extract into function", + group: None, + target: 59..60, + source_change: None, + } + "#]] + .assert_debug_eq(&extract_into_function_assist); + } + + { + let assists = Assist::get( + &db, + &cfg, + AssistResolveStrategy::Single(SingleResolve { + assist_id: "extract_variable".to_string(), + assist_kind: AssistKind::RefactorExtract, + }), + frange, + ); + assert_eq!(2, assists.len()); + let mut assists = assists.into_iter(); + + let extract_into_variable_assist = assists.next().unwrap(); + expect![[r#" + Assist { + id: AssistId( + "extract_variable", + RefactorExtract, + ), + label: "Extract into variable", + group: None, + target: 59..60, + source_change: Some( + SourceChange { + source_file_edits: { + FileId( + 0, + ): TextEdit { + indels: [ + Indel { + insert: "let $0var_name = 5;\n ", + delete: 45..45, + }, + Indel { + insert: "var_name", + delete: 59..60, + }, + ], + }, + }, + file_system_edits: [], + is_snippet: true, + }, + ), + } + "#]] + .assert_debug_eq(&extract_into_variable_assist); + + let extract_into_function_assist = assists.next().unwrap(); + expect![[r#" + Assist { + id: AssistId( + "extract_function", + RefactorExtract, + ), + label: "Extract into function", + group: None, + target: 59..60, + source_change: None, + } + "#]] + .assert_debug_eq(&extract_into_function_assist); + } + + { + let assists = Assist::get(&db, &cfg, AssistResolveStrategy::All, frange); + assert_eq!(2, assists.len()); + let mut assists = assists.into_iter(); + + let extract_into_variable_assist = assists.next().unwrap(); + expect![[r#" + Assist { + id: AssistId( + "extract_variable", + RefactorExtract, + ), + label: "Extract into variable", + group: None, + target: 59..60, + source_change: Some( + SourceChange { + source_file_edits: { + FileId( + 0, + ): TextEdit { + indels: [ + Indel { + insert: "let $0var_name = 5;\n ", + delete: 45..45, + }, + Indel { + insert: "var_name", + delete: 59..60, + }, + ], + }, + }, + file_system_edits: [], + is_snippet: true, + }, + ), + } + "#]] + .assert_debug_eq(&extract_into_variable_assist); + + let extract_into_function_assist = assists.next().unwrap(); + expect![[r#" + Assist { + id: AssistId( + "extract_function", + RefactorExtract, + ), + label: "Extract into function", + group: None, + target: 59..60, + source_change: Some( + SourceChange { + source_file_edits: { + FileId( + 0, + ): TextEdit { + indels: [ + Indel { + insert: "fun_name()", + delete: 59..60, + }, + Indel { + insert: "\n\nfn $0fun_name() -> i32 {\n 5\n}", + delete: 110..110, + }, + ], + }, + }, + file_system_edits: [], + is_snippet: true, + }, + ), + } + "#]] + .assert_debug_eq(&extract_into_function_assist); + } +} -- cgit v1.2.3 From 53a73de3d10e20a13153c94e050a8ad9230169eb Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Mon, 3 May 2021 18:44:58 +0300 Subject: Small fixes --- crates/ide_assists/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'crates/ide_assists/src') diff --git a/crates/ide_assists/src/lib.rs b/crates/ide_assists/src/lib.rs index 5a0047f03..723531078 100644 --- a/crates/ide_assists/src/lib.rs +++ b/crates/ide_assists/src/lib.rs @@ -28,7 +28,7 @@ pub(crate) use crate::assist_context::{AssistContext, Assists}; pub use assist_config::AssistConfig; -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum AssistKind { // FIXME: does the None variant make sense? Probably not. None, @@ -91,7 +91,7 @@ impl FromStr for AssistKind { /// Unique identifier of the assist, should not be shown to the user /// directly. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct AssistId(pub &'static str, pub AssistKind); /// A way to control how many asssist to resolve during the assist resolution. -- cgit v1.2.3 From 90fc32937785b3f17899f14d8cb2f7b3738a9850 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Mon, 3 May 2021 19:35:44 +0300 Subject: Index retrieval fix --- crates/ide_assists/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'crates/ide_assists/src') diff --git a/crates/ide_assists/src/lib.rs b/crates/ide_assists/src/lib.rs index 723531078..2e0c58504 100644 --- a/crates/ide_assists/src/lib.rs +++ b/crates/ide_assists/src/lib.rs @@ -102,7 +102,7 @@ pub enum AssistResolveStrategy { None, /// All assists should be resolved. All, - /// Only a certain assists should be resolved. + /// Only a certain assist should be resolved. Single(SingleResolve), } -- cgit v1.2.3