Merge #4344

4344: Use SourceChange for assists r=matklad a=matklad



bors r+
🤖

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

crates/ra_assists/src/assist_ctx.rs

@@ -1,8 +1,11 @@
 //! This module defines `AssistCtx` -- the API surface that is exposed to assists.
 use hir::Semantics;
-use ra_db::FileRange;
+use ra_db::{FileId, FileRange};
 use ra_fmt::{leading_indent, reindent};
-use ra_ide_db::RootDatabase;
+use ra_ide_db::{
+    source_change::{SingleFileChange, SourceChange},
+    RootDatabase,
+};
 use ra_syntax::{
     algo::{self, find_covering_element, find_node_at_offset, SyntaxRewriter},
     AstNode, SourceFile, SyntaxElement, SyntaxKind, SyntaxNode, SyntaxToken, TextRange, TextSize,
@@ -10,7 +13,7 @@ use ra_syntax::{
 };
 use ra_text_edit::TextEditBuilder;
 
-use crate::{AssistAction, AssistFile, AssistId, AssistLabel, GroupLabel, ResolvedAssist};
+use crate::{AssistFile, AssistId, AssistLabel, GroupLabel, ResolvedAssist};
 
 #[derive(Clone, Debug)]
 pub(crate) struct Assist(pub(crate) Vec<AssistInfo>);
@@ -19,7 +22,7 @@ pub(crate) struct Assist(pub(crate) Vec<AssistInfo>);
 pub(crate) struct AssistInfo {
     pub(crate) label: AssistLabel,
     pub(crate) group_label: Option<GroupLabel>,
-    pub(crate) action: Option<AssistAction>,
+    pub(crate) action: Option<SourceChange>,
 }
 
 impl AssistInfo {
@@ -27,7 +30,7 @@ impl AssistInfo {
         AssistInfo { label, group_label: None, action: None }
     }
 
-    fn resolved(self, action: AssistAction) -> AssistInfo {
+    fn resolved(self, action: SourceChange) -> AssistInfo {
         AssistInfo { action: Some(action), ..self }
     }
 
@@ -98,13 +101,13 @@ impl<'a> AssistCtx<'a> {
         f: impl FnOnce(&mut ActionBuilder),
     ) -> Option<Assist> {
         let label = AssistLabel::new(id, label.into(), None, target);
-
+        let change_label = label.label.clone();
         let mut info = AssistInfo::new(label);
         if self.should_compute_edit {
             let action = {
                 let mut edit = ActionBuilder::new(&self);
                 f(&mut edit);
-                edit.build()
+                edit.build(change_label, self.frange.file_id)
             };
             info = info.resolved(action)
         };
@@ -157,13 +160,13 @@ impl<'a> AssistGroup<'a> {
         f: impl FnOnce(&mut ActionBuilder),
     ) {
         let label = AssistLabel::new(id, label.into(), Some(self.group.clone()), target);
-
+        let change_label = label.label.clone();
         let mut info = AssistInfo::new(label).with_group(self.group.clone());
         if self.ctx.should_compute_edit {
             let action = {
                 let mut edit = ActionBuilder::new(&self.ctx);
                 f(&mut edit);
-                edit.build()
+                edit.build(change_label, self.ctx.frange.file_id)
             };
             info = info.resolved(action)
         };
@@ -255,11 +258,16 @@ impl<'a, 'b> ActionBuilder<'a, 'b> {
         self.file = assist_file
     }
 
-    fn build(self) -> AssistAction {
+    fn build(self, change_label: String, current_file: FileId) -> SourceChange {
         let edit = self.edit.finish();
         if edit.is_empty() && self.cursor_position.is_none() {
             panic!("Only call `add_assist` if the assist can be applied")
         }
-        AssistAction { edit, cursor_position: self.cursor_position, file: self.file }
+        let file = match self.file {
+            AssistFile::CurrentFile => current_file,
+            AssistFile::TargetFile(it) => it,
+        };
+        SingleFileChange { label: change_label, edit, cursor_position: self.cursor_position }
+            .into_source_change(file)
     }
 }

crates/ra_assists/src/lib.rs

@@ -19,9 +19,8 @@ pub mod ast_transform;
 
 use hir::Semantics;
 use ra_db::{FileId, FileRange};
-use ra_ide_db::RootDatabase;
+use ra_ide_db::{source_change::SourceChange, RootDatabase};
-use ra_syntax::{TextRange, TextSize};
+use ra_syntax::TextRange;
-use ra_text_edit::TextEdit;
 
 pub(crate) use crate::assist_ctx::{Assist, AssistCtx};
 
@@ -57,21 +56,14 @@ impl AssistLabel {
     }
 }
 
-#[derive(Debug, Clone)]
-pub struct AssistAction {
-    pub edit: TextEdit,
-    pub cursor_position: Option<TextSize>,
-    pub file: AssistFile,
-}
-
 #[derive(Debug, Clone)]
 pub struct ResolvedAssist {
     pub label: AssistLabel,
-    pub action: AssistAction,
+    pub action: SourceChange,
 }
 
 #[derive(Debug, Clone, Copy)]
-pub enum AssistFile {
+enum AssistFile {
     CurrentFile,
     TargetFile(FileId),
 }

crates/ra_assists/src/tests.rs

@@ -11,7 +11,7 @@ use test_utils::{
     RangeOrOffset,
 };
 
-use crate::{handlers::Handler, resolved_assists, AssistCtx, AssistFile};
+use crate::{handlers::Handler, resolved_assists, AssistCtx};
 
 pub(crate) fn with_single_file(text: &str) -> (RootDatabase, FileId) {
     let (mut db, file_id) = RootDatabase::with_single_file(text);
@@ -41,7 +41,7 @@ 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 assist = resolved_assists(&db, frange)
+    let mut assist = resolved_assists(&db, frange)
         .into_iter()
         .find(|assist| assist.label.id.0 == assist_id)
         .unwrap_or_else(|| {
@@ -57,8 +57,9 @@ fn check_doc_test(assist_id: &str, before: &str, after: &str) {
         });
 
     let actual = {
+        let change = assist.action.source_file_edits.pop().unwrap();
         let mut actual = before.clone();
-        assist.action.edit.apply(&mut actual);
+        change.edit.apply(&mut actual);
         actual
     };
     assert_eq_text!(after, &actual);
@@ -93,26 +94,23 @@ fn check(assist: Handler, before: &str, expected: ExpectedResult) {
 
     match (assist(assist_ctx), expected) {
         (Some(assist), ExpectedResult::After(after)) => {
-            let action = assist.0[0].action.clone().unwrap();
+            let mut action = assist.0[0].action.clone().unwrap();
+            let change = action.source_file_edits.pop().unwrap();
 
-            let mut actual = if let AssistFile::TargetFile(file_id) = action.file {
+            let mut actual = db.file_text(change.file_id).as_ref().to_owned();
-                db.file_text(file_id).as_ref().to_owned()
+            change.edit.apply(&mut actual);
-            } else {
-                text_without_caret
-            };
-            action.edit.apply(&mut actual);
 
             match action.cursor_position {
                 None => {
                     if let RangeOrOffset::Offset(before_cursor_pos) = range_or_offset {
-                        let off = action
+                        let off = change
                             .edit
                             .apply_to_offset(before_cursor_pos)
                             .expect("cursor position is affected by the edit");
                         actual = add_cursor(&actual, off)
                     }
                 }
-                Some(off) => actual = add_cursor(&actual, off),
+                Some(off) => actual = add_cursor(&actual, off.offset),
             };
 
             assert_eq_text!(after, &actual);

crates/ra_ide/src/assists.rs

@@ -1,42 +0,0 @@
-//! FIXME: write short doc here
-
-use ra_assists::{resolved_assists, AssistAction};
-use ra_db::{FilePosition, FileRange};
-use ra_ide_db::RootDatabase;
-
-use crate::{FileId, SourceChange, SourceFileEdit};
-
-pub use ra_assists::AssistId;
-
-#[derive(Debug)]
-pub struct Assist {
-    pub id: AssistId,
-    pub label: String,
-    pub group_label: Option<String>,
-    pub source_change: SourceChange,
-}
-
-pub(crate) fn assists(db: &RootDatabase, frange: FileRange) -> Vec<Assist> {
-    resolved_assists(db, frange)
-        .into_iter()
-        .map(|assist| {
-            let file_id = frange.file_id;
-            Assist {
-                id: assist.label.id,
-                label: assist.label.label.clone(),
-                group_label: assist.label.group.map(|it| it.0),
-                source_change: action_to_edit(assist.action, file_id, assist.label.label.clone()),
-            }
-        })
-        .collect()
-}
-
-fn action_to_edit(action: AssistAction, file_id: FileId, label: String) -> SourceChange {
-    let file_id = match action.file {
-        ra_assists::AssistFile::TargetFile(it) => it,
-        _ => file_id,
-    };
-    let file_edit = SourceFileEdit { file_id, edit: action.edit };
-    SourceChange::source_file_edit(label, file_edit)
-        .with_cursor_opt(action.cursor_position.map(|offset| FilePosition { offset, file_id }))
-}

crates/ra_ide/src/lib.rs

@@ -31,7 +31,6 @@ mod syntax_highlighting;
 mod parent_module;
 mod references;
 mod impls;
-mod assists;
 mod diagnostics;
 mod syntax_tree;
 mod folding_ranges;
@@ -64,7 +63,6 @@ use ra_syntax::{SourceFile, TextRange, TextSize};
 use crate::display::ToNav;
 
 pub use crate::{
-    assists::{Assist, AssistId},
     call_hierarchy::CallItem,
     completion::{
         CompletionConfig, CompletionItem, CompletionItemKind, CompletionScore, InsertTextFormat,
@@ -84,6 +82,7 @@ pub use crate::{
 };
 
 pub use hir::Documentation;
+pub use ra_assists::AssistId;
 pub use ra_db::{
     Canceled, CrateGraph, CrateId, Edition, FileId, FilePosition, FileRange, SourceRootId,
 };
@@ -134,10 +133,12 @@ pub struct AnalysisHost {
     db: RootDatabase,
 }
 
-impl Default for AnalysisHost {
+#[derive(Debug)]
-    fn default() -> AnalysisHost {
+pub struct Assist {
-        AnalysisHost::new(None)
+    pub id: AssistId,
-    }
+    pub label: String,
+    pub group_label: Option<String>,
+    pub source_change: SourceChange,
 }
 
 impl AnalysisHost {
@@ -187,6 +188,12 @@ impl AnalysisHost {
     }
 }
 
+impl Default for AnalysisHost {
+    fn default() -> AnalysisHost {
+        AnalysisHost::new(None)
+    }
+}
+
 /// Analysis is a snapshot of a world state at a moment in time. It is the main
 /// entry point for asking semantic information about the world. When the world
 /// state is advanced using `AnalysisHost::apply_change` method, all existing
@@ -464,7 +471,17 @@ impl Analysis {
     /// Computes assists (aka code actions aka intentions) for the given
     /// position.
     pub fn assists(&self, frange: FileRange) -> Cancelable<Vec<Assist>> {
-        self.with_db(|db| assists::assists(db, frange))
+        self.with_db(|db| {
+            ra_assists::resolved_assists(db, frange)
+                .into_iter()
+                .map(|assist| Assist {
+                    id: assist.label.id,
+                    label: assist.label.label,
+                    group_label: assist.label.group.map(|it| it.0),
+                    source_change: assist.action,
+                })
+                .collect()
+        })
     }
 
     /// Computes the set of diagnostics for the given file.

crates/ra_ide_db/src/source_change.rs

@@ -6,7 +6,7 @@
 use ra_db::{FileId, FilePosition, RelativePathBuf, SourceRootId};
 use ra_text_edit::{TextEdit, TextSize};
 
-#[derive(Debug)]
+#[derive(Debug, Clone)]
 pub struct SourceChange {
     /// For display in the undo log in the editor
     pub label: String,
@@ -90,13 +90,13 @@ impl SourceChange {
     }
 }
 
-#[derive(Debug)]
+#[derive(Debug, Clone)]
 pub struct SourceFileEdit {
     pub file_id: FileId,
     pub edit: TextEdit,
 }
 
-#[derive(Debug)]
+#[derive(Debug, Clone)]
 pub enum FileSystemEdit {
     CreateFile { source_root: SourceRootId, path: RelativePathBuf },
     MoveFile { src: FileId, dst_source_root: SourceRootId, dst_path: RelativePathBuf },