Apply the api design suggestions

3 files changed, 45 insertions, 28 deletions

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<AssistAction> },
+    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<String>,
-        f: impl FnOnce() -> (ActionBuilder, Vec<ActionBuilder>),
+        f: impl FnOnce() -> Vec<ActionBuilder>,
     ) -> Option<Assist> {
         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::<Vec<_>>()
                     .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<TextRange>,
 }
 
+#[derive(Debug, Clone)]
+pub struct ResolvedAssist {
+    pub label: AssistLabel,
+    pub action_data: Either<AssistAction, Vec<AssistAction>>,
+}
+
+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<H>(db: &H, range: FileRange) -> Vec<(AssistLabel, AssistAction, Vec<AssistAction>)>
+pub fn assists<H>(db: &H, range: FileRange) -> Vec<ResolvedAssist>
 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::<Vec<_>>();
-        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");
     }
 }