Merge #4364

4364: Touch up assists public API r=matklad a=matklad

bors r+
🤖

Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>

crates/ra_assists/src/assist_context.rs

@@ -15,7 +15,7 @@ use ra_syntax::{
 };
 use ra_text_edit::TextEditBuilder;
 
-use crate::{AssistId, AssistLabel, GroupLabel, ResolvedAssist};
+use crate::{Assist, AssistId, GroupLabel, ResolvedAssist};
 
 /// `AssistContext` allows to apply an assist or check if it could be applied.
 ///
@@ -76,8 +76,7 @@ impl<'a> AssistContext<'a> {
         find_node_at_offset(self.source_file.syntax(), self.offset())
     }
     pub(crate) fn find_node_at_offset_with_descend<N: AstNode>(&self) -> Option<N> {
-        self.sema
+        self.sema.find_node_at_offset_with_descend(self.source_file.syntax(), self.offset())
-            .find_node_at_offset_with_descend(self.source_file.syntax(), self.frange.range.start())
     }
     pub(crate) fn covering_element(&self) -> SyntaxElement {
         find_covering_element(self.source_file.syntax(), self.frange.range)
@@ -91,7 +90,7 @@ impl<'a> AssistContext<'a> {
 pub(crate) struct Assists {
     resolve: bool,
     file: FileId,
-    buf: Vec<(AssistLabel, Option<SourceChange>)>,
+    buf: Vec<(Assist, Option<SourceChange>)>,
 }
 
 impl Assists {
@@ -102,7 +101,7 @@ impl Assists {
         Assists { resolve: false, file: ctx.frange.file_id, buf: Vec::new() }
     }
 
-    pub(crate) fn finish_unresolved(self) -> Vec<AssistLabel> {
+    pub(crate) fn finish_unresolved(self) -> Vec<Assist> {
         assert!(!self.resolve);
         self.finish()
             .into_iter()
@@ -117,7 +116,7 @@ impl Assists {
         assert!(self.resolve);
         self.finish()
             .into_iter()
-            .map(|(label, edit)| ResolvedAssist { label, source_change: edit.unwrap() })
+            .map(|(label, edit)| ResolvedAssist { assist: label, source_change: edit.unwrap() })
             .collect()
     }
 
@@ -128,7 +127,7 @@ impl Assists {
         target: TextRange,
         f: impl FnOnce(&mut AssistBuilder),
     ) -> Option<()> {
-        let label = AssistLabel::new(id, label.into(), None, target);
+        let label = Assist::new(id, label.into(), None, target);
         self.add_impl(label, f)
     }
     pub(crate) fn add_group(
@@ -139,10 +138,10 @@ impl Assists {
         target: TextRange,
         f: impl FnOnce(&mut AssistBuilder),
     ) -> Option<()> {
-        let label = AssistLabel::new(id, label.into(), Some(group.clone()), target);
+        let label = Assist::new(id, label.into(), Some(group.clone()), target);
         self.add_impl(label, f)
     }
-    fn add_impl(&mut self, label: AssistLabel, f: impl FnOnce(&mut AssistBuilder)) -> Option<()> {
+    fn add_impl(&mut self, label: Assist, f: impl FnOnce(&mut AssistBuilder)) -> Option<()> {
         let change_label = label.label.clone();
         let source_change = if self.resolve {
             let mut builder = AssistBuilder::new(self.file);
@@ -156,7 +155,7 @@ impl Assists {
         Some(())
     }
 
-    fn finish(mut self) -> Vec<(AssistLabel, Option<SourceChange>)> {
+    fn finish(mut self) -> Vec<(Assist, Option<SourceChange>)> {
         self.buf.sort_by_key(|(label, _edit)| label.target.len());
         self.buf
     }

crates/ra_assists/src/handlers/early_return.rs

@@ -93,7 +93,7 @@ pub(crate) fn convert_to_guarded_return(acc: &mut Assists, ctx: &AssistContext)
     }
 
     then_block.syntax().last_child_or_token().filter(|t| t.kind() == R_CURLY)?;
-    let cursor_position = ctx.frange.range.start();
+    let cursor_position = ctx.offset();
 
     let target = if_expr.syntax().text_range();
     acc.add(AssistId("convert_to_guarded_return"), "Convert to guarded return", target, |edit| {

crates/ra_assists/src/handlers/merge_imports.rs

@@ -26,7 +26,7 @@ use crate::{
 pub(crate) fn merge_imports(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
     let tree: ast::UseTree = ctx.find_node_at_offset()?;
     let mut rewriter = SyntaxRewriter::default();
-    let mut offset = ctx.frange.range.start();
+    let mut offset = ctx.offset();
 
     if let Some(use_item) = tree.syntax().parent().and_then(ast::UseItem::cast) {
         let (merged, to_delete) = next_prev()

crates/ra_assists/src/handlers/merge_match_arms.rs

@@ -45,7 +45,7 @@ pub(crate) fn merge_match_arms(acc: &mut Assists, ctx: &AssistContext) -> Option
         InExpr(TextSize),
         InPat(TextSize),
     }
-    let cursor_pos = ctx.frange.range.start();
+    let cursor_pos = ctx.offset();
     let cursor_pos = if current_expr.syntax().text_range().contains(cursor_pos) {
         CursorPos::InExpr(current_text_range.end() - cursor_pos)
     } else {

crates/ra_assists/src/handlers/split_import.rs

@@ -26,7 +26,7 @@ pub(crate) fn split_import(acc: &mut Assists, ctx: &AssistContext) -> Option<()>
     if new_tree == use_tree {
         return None;
     }
-    let cursor = ctx.frange.range.start();
+    let cursor = ctx.offset();
 
     let target = colon_colon.text_range();
     acc.add(AssistId("split_import"), "Split import", target, |edit| {

crates/ra_assists/src/lib.rs

@@ -29,8 +29,11 @@ pub(crate) use crate::assist_context::{AssistContext, Assists};
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub struct AssistId(pub &'static str);
 
+#[derive(Clone, Debug)]
+pub struct GroupLabel(pub String);
+
 #[derive(Debug, Clone)]
-pub struct AssistLabel {
+pub struct Assist {
     pub id: AssistId,
     /// Short description of the assist, as shown in the UI.
     pub label: String,
@@ -40,33 +43,18 @@ pub struct AssistLabel {
     pub target: TextRange,
 }
 
-#[derive(Clone, Debug)]
-pub struct GroupLabel(pub String);
-
-impl AssistLabel {
-    pub(crate) fn new(
-        id: AssistId,
-        label: String,
-        group: Option<GroupLabel>,
-        target: TextRange,
-    ) -> AssistLabel {
-        // FIXME: make fields private, so that this invariant can't be broken
-        assert!(label.starts_with(|c: char| c.is_uppercase()));
-        AssistLabel { id, label, group, target }
-    }
-}
-
 #[derive(Debug, Clone)]
 pub struct ResolvedAssist {
-    pub label: AssistLabel,
+    pub assist: Assist,
     pub source_change: SourceChange,
 }
 
-/// Return all the assists applicable at the given position.
+impl Assist {
-///
+    /// Return all the assists applicable at the given position.
-/// Assists are returned in the "unresolved" state, that is only labels are
+    ///
-/// returned, without actual edits.
+    /// Assists are returned in the "unresolved" state, that is only labels are
-pub fn unresolved_assists(db: &RootDatabase, range: FileRange) -> Vec<AssistLabel> {
+    /// returned, without actual edits.
+    pub fn unresolved(db: &RootDatabase, range: FileRange) -> Vec<Assist> {
         let sema = Semantics::new(db);
         let ctx = AssistContext::new(sema, range);
         let mut acc = Assists::new_unresolved(&ctx);
@@ -74,13 +62,13 @@ pub fn unresolved_assists(db: &RootDatabase, range: FileRange) -> Vec<AssistLabe
             handler(&mut acc, &ctx);
         });
         acc.finish_unresolved()
-}
+    }
 
-/// Return all the assists applicable at the given position.
+    /// Return all the assists applicable at the given position.
-///
+    ///
-/// Assists are returned in the "resolved" state, that is with edit fully
+    /// Assists are returned in the "resolved" state, that is with edit fully
-/// computed.
+    /// computed.
-pub fn resolved_assists(db: &RootDatabase, range: FileRange) -> Vec<ResolvedAssist> {
+    pub fn resolved(db: &RootDatabase, range: FileRange) -> Vec<ResolvedAssist> {
         let sema = Semantics::new(db);
         let ctx = AssistContext::new(sema, range);
         let mut acc = Assists::new_resolved(&ctx);
@@ -88,6 +76,18 @@ pub fn resolved_assists(db: &RootDatabase, range: FileRange) -> Vec<ResolvedAssi
             handler(&mut acc, &ctx);
         });
         acc.finish_resolved()
+    }
+
+    pub(crate) fn new(
+        id: AssistId,
+        label: String,
+        group: Option<GroupLabel>,
+        target: TextRange,
+    ) -> Assist {
+        // FIXME: make fields private, so that this invariant can't be broken
+        assert!(label.starts_with(|c: char| c.is_uppercase()));
+        Assist { id, label, group, target }
+    }
 }
 
 mod handlers {

crates/ra_assists/src/tests.rs

@@ -11,7 +11,7 @@ use test_utils::{
     RangeOrOffset,
 };
 
-use crate::{handlers::Handler, resolved_assists, AssistContext, Assists};
+use crate::{handlers::Handler, Assist, AssistContext, Assists};
 
 pub(crate) fn with_single_file(text: &str) -> (RootDatabase, FileId) {
     let (mut db, file_id) = RootDatabase::with_single_file(text);
@@ -41,16 +41,16 @@ fn check_doc_test(assist_id: &str, before: &str, after: &str) {
     let (db, file_id) = crate::tests::with_single_file(&before);
     let frange = FileRange { file_id, range: selection.into() };
 
-    let mut assist = resolved_assists(&db, frange)
+    let mut assist = Assist::resolved(&db, frange)
         .into_iter()
-        .find(|assist| assist.label.id.0 == assist_id)
+        .find(|assist| assist.assist.id.0 == assist_id)
         .unwrap_or_else(|| {
             panic!(
                 "\n\nAssist is not applicable: {}\nAvailable assists: {}",
                 assist_id,
-                resolved_assists(&db, frange)
+                Assist::resolved(&db, frange)
                     .into_iter()
-                    .map(|assist| assist.label.id.0)
+                    .map(|assist| assist.assist.id.0)
                     .collect::<Vec<_>>()
                     .join(", ")
             )
@@ -119,7 +119,7 @@ fn check(handler: Handler, before: &str, expected: ExpectedResult) {
             assert_eq_text!(after, &actual);
         }
         (Some(assist), ExpectedResult::Target(target)) => {
-            let range = assist.label.target;
+            let range = assist.assist.target;
             assert_eq_text!(&text_without_caret[range], target);
         }
         (Some(_), ExpectedResult::NotApplicable) => panic!("assist should not be applicable!"),
@@ -136,14 +136,14 @@ 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 = resolved_assists(&db, frange);
+    let assists = Assist::resolved(&db, frange);
     let mut assists = assists.iter();
 
     assert_eq!(
-        assists.next().expect("expected assist").label.label,
+        assists.next().expect("expected assist").assist.label,
         "Change visibility to pub(crate)"
     );
-    assert_eq!(assists.next().expect("expected assist").label.label, "Add `#[derive]`");
+    assert_eq!(assists.next().expect("expected assist").assist.label, "Add `#[derive]`");
 }
 
 #[test]
@@ -159,9 +159,9 @@ fn assist_order_if_expr() {
     let (range, before) = extract_range(before);
     let (db, file_id) = with_single_file(&before);
     let frange = FileRange { file_id, range };
-    let assists = resolved_assists(&db, frange);
+    let assists = Assist::resolved(&db, frange);
     let mut assists = assists.iter();
 
-    assert_eq!(assists.next().expect("expected assist").label.label, "Extract into variable");
+    assert_eq!(assists.next().expect("expected assist").assist.label, "Extract into variable");
-    assert_eq!(assists.next().expect("expected assist").label.label, "Replace with match");
+    assert_eq!(assists.next().expect("expected assist").assist.label, "Replace with match");
 }

crates/ra_ide/src/lib.rs

@@ -472,12 +472,12 @@ impl Analysis {
     /// position.
     pub fn assists(&self, frange: FileRange) -> Cancelable<Vec<Assist>> {
         self.with_db(|db| {
-            ra_assists::resolved_assists(db, frange)
+            ra_assists::Assist::resolved(db, frange)
                 .into_iter()
                 .map(|assist| Assist {
-                    id: assist.label.id,
+                    id: assist.assist.id,
-                    label: assist.label.label,
+                    label: assist.assist.label,
-                    group_label: assist.label.group.map(|it| it.0),
+                    group_label: assist.assist.group.map(|it| it.0),
                     source_change: assist.source_change,
                 })
                 .collect()