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/assist_ctx.rs | 83 +++++++++++++++++-------------------- crates/ra_assists/src/doc_tests.rs | 2 +- crates/ra_assists/src/lib.rs | 64 +++++++--------------------- 3 files changed, 54 insertions(+), 95 deletions(-) (limited to 'crates/ra_assists/src') diff --git a/crates/ra_assists/src/assist_ctx.rs b/crates/ra_assists/src/assist_ctx.rs index 5924a3fd5..5aab5fb8b 100644 --- a/crates/ra_assists/src/assist_ctx.rs +++ b/crates/ra_assists/src/assist_ctx.rs @@ -1,5 +1,4 @@ //! This module defines `AssistCtx` -- the API surface that is exposed to assists. -use either::Either; use hir::{InFile, SourceAnalyzer, SourceBinder}; use ra_db::{FileRange, SourceDatabase}; use ra_fmt::{leading_indent, reindent}; @@ -11,12 +10,36 @@ use ra_syntax::{ }; use ra_text_edit::TextEditBuilder; -use crate::{AssistAction, AssistId, AssistLabel, ResolvedAssist}; +use crate::{AssistAction, AssistId, AssistLabel, GroupLabel, ResolvedAssist}; #[derive(Clone, Debug)] -pub(crate) enum Assist { - Unresolved { label: AssistLabel }, - Resolved { assist: ResolvedAssist }, +pub(crate) struct Assist(pub(crate) Vec); + +#[derive(Clone, Debug)] +pub(crate) struct AssistInfo { + pub(crate) label: AssistLabel, + pub(crate) group_label: Option, + pub(crate) action: Option, +} + +impl AssistInfo { + fn new(label: AssistLabel) -> AssistInfo { + AssistInfo { label, group_label: None, action: None } + } + + fn resolved(self, action: AssistAction) -> AssistInfo { + AssistInfo { action: Some(action), ..self } + } + + fn with_group(self, group_label: GroupLabel) -> AssistInfo { + AssistInfo { group_label: Some(group_label), ..self } + } + + pub(crate) fn into_resolved(self) -> Option { + let label = self.label; + let group_label = self.group_label; + self.action.map(|action| ResolvedAssist { label, group_label, action }) + } } pub(crate) type AssistHandler = fn(AssistCtx) -> Option; @@ -84,18 +107,17 @@ impl<'a> AssistCtx<'a> { ) -> Option { let label = AssistLabel::new(label.into(), id); - let assist = if self.should_compute_edit { + let mut info = AssistInfo::new(label); + if self.should_compute_edit { let action = { let mut edit = ActionBuilder::default(); f(&mut edit); edit.build() }; - Assist::Resolved { assist: ResolvedAssist { label, action_data: Either::Left(action) } } - } else { - Assist::Unresolved { label } + info = info.resolved(action) }; - Some(assist) + Some(Assist(vec![info])) } pub(crate) fn add_assist_group(self, group_name: impl Into) -> AssistGroup<'a> { @@ -136,7 +158,7 @@ impl<'a> AssistCtx<'a> { pub(crate) struct AssistGroup<'a> { ctx: AssistCtx<'a>, group_name: String, - assists: Vec, + assists: Vec, } impl<'a> AssistGroup<'a> { @@ -148,49 +170,22 @@ impl<'a> AssistGroup<'a> { ) { let label = AssistLabel::new(label.into(), id); - let assist = if self.ctx.should_compute_edit { + let mut info = AssistInfo::new(label).with_group(GroupLabel(self.group_name.clone())); + if self.ctx.should_compute_edit { let action = { let mut edit = ActionBuilder::default(); f(&mut edit); edit.build() }; - Assist::Resolved { assist: ResolvedAssist { label, action_data: Either::Left(action) } } - } else { - Assist::Unresolved { label } + info = info.resolved(action) }; - self.assists.push(assist) + self.assists.push(info) } pub(crate) fn finish(self) -> Option { assert!(!self.assists.is_empty()); - let mut label = match &self.assists[0] { - Assist::Unresolved { label } => label.clone(), - Assist::Resolved { assist } => assist.label.clone(), - }; - label.label = self.group_name; - let assist = if self.ctx.should_compute_edit { - Assist::Resolved { - assist: ResolvedAssist { - label, - action_data: Either::Right( - self.assists - .into_iter() - .map(|assist| match assist { - Assist::Resolved { - assist: - ResolvedAssist { label: _, action_data: Either::Left(it) }, - } => it, - _ => unreachable!(), - }) - .collect(), - ), - }, - } - } else { - Assist::Unresolved { label } - }; - Some(assist) + Some(Assist(self.assists)) } } @@ -199,7 +194,6 @@ pub(crate) struct ActionBuilder { edit: TextEditBuilder, cursor_position: Option, target: Option, - label: Option, } impl ActionBuilder { @@ -261,7 +255,6 @@ impl ActionBuilder { edit: self.edit.finish(), cursor_position: self.cursor_position, target: self.target, - label: self.label, } } } diff --git a/crates/ra_assists/src/doc_tests.rs b/crates/ra_assists/src/doc_tests.rs index ae0e5605c..c0f9bc1fb 100644 --- a/crates/ra_assists/src/doc_tests.rs +++ b/crates/ra_assists/src/doc_tests.rs @@ -30,6 +30,6 @@ fn check(assist_id: &str, before: &str, after: &str) { ) }); - let actual = assist.get_first_action().edit.apply(&before); + let actual = assist.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 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