From 78a21253b494e573885ac8336bff6e93b401046f Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Sun, 12 Jan 2020 00:40:36 +0200 Subject: Apply the api design suggestions --- crates/ra_assists/src/assist_ctx.rs | 25 +++++++++++++---------- crates/ra_assists/src/doc_tests.rs | 8 ++++---- crates/ra_assists/src/lib.rs | 40 +++++++++++++++++++++++++------------ 3 files changed, 45 insertions(+), 28 deletions(-) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/assist_ctx.rs b/crates/ra_assists/src/assist_ctx.rs index 879216a36..e13f969c7 100644 --- a/crates/ra_assists/src/assist_ctx.rs +++ b/crates/ra_assists/src/assist_ctx.rs @@ -1,5 +1,6 @@ //! This module defines `AssistCtx` -- the API surface that is exposed to assists. use hir::{db::HirDatabase, InFile, SourceAnalyzer}; +use itertools::Either; use ra_db::FileRange; use ra_fmt::{leading_indent, reindent}; use ra_syntax::{ @@ -9,12 +10,12 @@ use ra_syntax::{ }; use ra_text_edit::TextEditBuilder; -use crate::{AssistAction, AssistId, AssistLabel}; +use crate::{AssistAction, AssistId, AssistLabel, ResolvedAssist}; #[derive(Clone, Debug)] pub(crate) enum Assist { Unresolved { label: AssistLabel }, - Resolved { label: AssistLabel, action: AssistAction, alternative_actions: Vec }, + Resolved { assist: ResolvedAssist }, } /// `AssistCtx` allows to apply an assist or check if it could be applied. @@ -92,7 +93,7 @@ impl<'a, DB: HirDatabase> AssistCtx<'a, DB> { f(&mut edit); edit.build() }; - Assist::Resolved { label, action, alternative_actions: Vec::default() } + Assist::Resolved { assist: ResolvedAssist { label, action_data: Either::Left(action) } } } else { Assist::Unresolved { label } }; @@ -105,18 +106,20 @@ impl<'a, DB: HirDatabase> AssistCtx<'a, DB> { self, id: AssistId, label: impl Into, - f: impl FnOnce() -> (ActionBuilder, Vec), + f: impl FnOnce() -> Vec, ) -> Option { let label = AssistLabel { label: label.into(), id }; let assist = if self.should_compute_edit { - let (action, alternative_actions) = f(); + let actions = f(); + assert!(!actions.is_empty(), "Assist cannot have no"); + Assist::Resolved { - label, - action: action.build(), - alternative_actions: alternative_actions - .into_iter() - .map(ActionBuilder::build) - .collect(), + assist: ResolvedAssist { + label, + action_data: Either::Right( + actions.into_iter().map(ActionBuilder::build).collect(), + ), + }, } } else { Assist::Unresolved { label } diff --git a/crates/ra_assists/src/doc_tests.rs b/crates/ra_assists/src/doc_tests.rs index 21bee228d..5dc1ee233 100644 --- a/crates/ra_assists/src/doc_tests.rs +++ b/crates/ra_assists/src/doc_tests.rs @@ -15,21 +15,21 @@ fn check(assist_id: &str, before: &str, after: &str) { let (db, file_id) = TestDB::with_single_file(&before); let frange = FileRange { file_id, range: selection.into() }; - let (_assist_id, action, _) = crate::assists(&db, frange) + let assist = crate::assists(&db, frange) .into_iter() - .find(|(id, _, _)| id.id.0 == assist_id) + .find(|assist| assist.label.id.0 == assist_id) .unwrap_or_else(|| { panic!( "\n\nAssist is not applicable: {}\nAvailable assists: {}", assist_id, crate::assists(&db, frange) .into_iter() - .map(|(id, _, _)| id.id.0) + .map(|assist| assist.label.id.0) .collect::>() .join(", ") ) }); - let actual = action.edit.apply(&before); + let actual = assist.get_first_action().edit.apply(&before); assert_eq_text!(after, &actual); } diff --git a/crates/ra_assists/src/lib.rs b/crates/ra_assists/src/lib.rs index 95a530821..a2983ae87 100644 --- a/crates/ra_assists/src/lib.rs +++ b/crates/ra_assists/src/lib.rs @@ -14,6 +14,7 @@ mod test_db; pub mod ast_transform; use hir::db::HirDatabase; +use itertools::Either; use ra_db::FileRange; use ra_syntax::{TextRange, TextUnit}; use ra_text_edit::TextEdit; @@ -41,6 +42,21 @@ pub struct AssistAction { pub target: Option, } +#[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(), + } + } +} + /// Return all the assists applicable at the given position. /// /// Assists are returned in the "unresolved" state, that is only labels are @@ -65,7 +81,7 @@ where /// /// Assists are returned in the "resolved" state, that is with edit fully /// computed. -pub fn assists(db: &H, range: FileRange) -> Vec<(AssistLabel, AssistAction, Vec)> +pub fn assists(db: &H, range: FileRange) -> Vec where H: HirDatabase + 'static, { @@ -76,13 +92,11 @@ where .iter() .filter_map(|f| f(ctx.clone())) .map(|a| match a { - Assist::Resolved { label, action, alternative_actions } => { - (label, action, alternative_actions) - } + Assist::Resolved { assist } => assist, Assist::Unresolved { .. } => unreachable!(), }) .collect::>(); - a.sort_by(|a, b| match (a.1.target, b.1.target) { + 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, @@ -177,7 +191,7 @@ mod helpers { AssistCtx::with_ctx(&db, frange, true, assist).expect("code action is not applicable"); let action = match assist { Assist::Unresolved { .. } => unreachable!(), - Assist::Resolved { action, .. } => action, + Assist::Resolved { assist } => assist.get_first_action(), }; let actual = action.edit.apply(&before); @@ -204,7 +218,7 @@ mod helpers { AssistCtx::with_ctx(&db, frange, true, assist).expect("code action is not applicable"); let action = match assist { Assist::Unresolved { .. } => unreachable!(), - Assist::Resolved { action, .. } => action, + Assist::Resolved { assist } => assist.get_first_action(), }; let mut actual = action.edit.apply(&before); @@ -227,7 +241,7 @@ mod helpers { AssistCtx::with_ctx(&db, frange, true, assist).expect("code action is not applicable"); let action = match assist { Assist::Unresolved { .. } => unreachable!(), - Assist::Resolved { action, .. } => action, + Assist::Resolved { assist } => assist.get_first_action(), }; let range = action.target.expect("expected target on action"); @@ -246,7 +260,7 @@ mod helpers { AssistCtx::with_ctx(&db, frange, true, assist).expect("code action is not applicable"); let action = match assist { Assist::Unresolved { .. } => unreachable!(), - Assist::Resolved { action, .. } => action, + Assist::Resolved { assist } => assist.get_first_action(), }; let range = action.target.expect("expected target on action"); @@ -296,10 +310,10 @@ mod tests { let mut assists = assists.iter(); assert_eq!( - assists.next().expect("expected assist").0.label, + assists.next().expect("expected assist").label.label, "Change visibility to pub(crate)" ); - assert_eq!(assists.next().expect("expected assist").0.label, "Add `#[derive]`"); + assert_eq!(assists.next().expect("expected assist").label.label, "Add `#[derive]`"); } #[test] @@ -318,7 +332,7 @@ mod tests { let assists = super::assists(&db, frange); let mut assists = assists.iter(); - assert_eq!(assists.next().expect("expected assist").0.label, "Extract into variable"); - assert_eq!(assists.next().expect("expected assist").0.label, "Replace with match"); + assert_eq!(assists.next().expect("expected assist").label.label, "Extract into variable"); + assert_eq!(assists.next().expect("expected assist").label.label, "Replace with match"); } } -- cgit v1.2.3