Merge #4345

4345: Refactor assists a bit r=matklad a=matklad

bors r+
🤖

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

crates/ra_assists/src/assist_ctx.rs

@@ -13,7 +13,7 @@ use ra_syntax::{
 };
 use ra_text_edit::TextEditBuilder;
 
-use crate::{AssistFile, AssistId, AssistLabel, GroupLabel, ResolvedAssist};
+use crate::{AssistId, AssistLabel, GroupLabel, ResolvedAssist};
 
 #[derive(Clone, Debug)]
 pub(crate) struct Assist(pub(crate) Vec<AssistInfo>);
@@ -22,16 +22,16 @@ 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<SourceChange>,
+    pub(crate) source_change: Option<SourceChange>,
 }
 
 impl AssistInfo {
     fn new(label: AssistLabel) -> AssistInfo {
-        AssistInfo { label, group_label: None, action: None }
+        AssistInfo { label, group_label: None, source_change: None }
     }
 
-    fn resolved(self, action: SourceChange) -> AssistInfo {
-        AssistInfo { action: Some(action), ..self }
+    fn resolved(self, source_change: SourceChange) -> AssistInfo {
+        AssistInfo { source_change: Some(source_change), ..self }
     }
 
     fn with_group(self, group_label: GroupLabel) -> AssistInfo {
@@ -40,7 +40,7 @@ impl AssistInfo {
 
     pub(crate) fn into_resolved(self) -> Option<ResolvedAssist> {
         let label = self.label;
-        self.action.map(|action| ResolvedAssist { label, action })
+        self.source_change.map(|source_change| ResolvedAssist { label, source_change })
     }
 }
 
@@ -104,12 +104,12 @@ impl<'a> AssistCtx<'a> {
         let change_label = label.label.clone();
         let mut info = AssistInfo::new(label);
         if self.should_compute_edit {
-            let action = {
+            let source_change = {
                 let mut edit = ActionBuilder::new(&self);
                 f(&mut edit);
-                edit.build(change_label, self.frange.file_id)
+                edit.build(change_label)
             };
-            info = info.resolved(action)
+            info = info.resolved(source_change)
         };
 
         Some(Assist(vec![info]))
@@ -163,12 +163,12 @@ impl<'a> AssistGroup<'a> {
         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 source_change = {
                 let mut edit = ActionBuilder::new(&self.ctx);
                 f(&mut edit);
-                edit.build(change_label, self.ctx.frange.file_id)
+                edit.build(change_label)
             };
-            info = info.resolved(action)
+            info = info.resolved(source_change)
         };
 
         self.assists.push(info)
@@ -186,7 +186,7 @@ impl<'a> AssistGroup<'a> {
 pub(crate) struct ActionBuilder<'a, 'b> {
     edit: TextEditBuilder,
     cursor_position: Option<TextSize>,
-    file: AssistFile,
+    file: FileId,
     ctx: &'a AssistCtx<'b>,
 }
 
@@ -195,7 +195,7 @@ impl<'a, 'b> ActionBuilder<'a, 'b> {
         Self {
             edit: TextEditBuilder::default(),
             cursor_position: None,
-            file: AssistFile::default(),
+            file: ctx.frange.file_id,
             ctx,
         }
     }
@@ -254,20 +254,16 @@ impl<'a, 'b> ActionBuilder<'a, 'b> {
         algo::diff(&node, &new).into_text_edit(&mut self.edit)
     }
 
-    pub(crate) fn set_file(&mut self, assist_file: AssistFile) {
-        self.file = assist_file
+    pub(crate) fn set_file(&mut self, assist_file: FileId) {
+        self.file = assist_file;
     }
 
-    fn build(self, change_label: String, current_file: FileId) -> SourceChange {
+    fn build(self, change_label: String) -> 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")
         }
-        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)
+            .into_source_change(self.file)
     }
 }

crates/ra_assists/src/handlers/add_function.rs

@@ -3,9 +3,10 @@ use ra_syntax::{
     SyntaxKind, SyntaxNode, TextSize,
 };
 
-use crate::{Assist, AssistCtx, AssistFile, AssistId};
+use crate::{Assist, AssistCtx, AssistId};
 use ast::{edit::IndentLevel, ArgListOwner, ModuleItemOwner};
 use hir::HirDisplay;
+use ra_db::FileId;
 use rustc_hash::{FxHashMap, FxHashSet};
 
 // Assist: add_function
@@ -70,7 +71,7 @@ struct FunctionTemplate {
     insert_offset: TextSize,
     cursor_offset: TextSize,
     fn_def: ast::SourceFile,
-    file: AssistFile,
+    file: FileId,
 }
 
 struct FunctionBuilder {
@@ -78,7 +79,7 @@ struct FunctionBuilder {
     fn_name: ast::Name,
     type_params: Option<ast::TypeParamList>,
     params: ast::ParamList,
-    file: AssistFile,
+    file: FileId,
     needs_pub: bool,
 }
 
@@ -92,7 +93,7 @@ impl FunctionBuilder {
         target_module: Option<hir::InFile<hir::ModuleSource>>,
     ) -> Option<Self> {
         let needs_pub = target_module.is_some();
-        let mut file = AssistFile::default();
+        let mut file = ctx.frange.file_id;
         let target = if let Some(target_module) = target_module {
             let (in_file, target) = next_space_for_fn_in_module(ctx.sema.db, target_module)?;
             file = in_file;
@@ -253,9 +254,8 @@ fn next_space_for_fn_after_call_site(expr: &ast::CallExpr) -> Option<GeneratedFu
 fn next_space_for_fn_in_module(
     db: &dyn hir::db::AstDatabase,
     module: hir::InFile<hir::ModuleSource>,
-) -> Option<(AssistFile, GeneratedFunctionTarget)> {
+) -> Option<(FileId, GeneratedFunctionTarget)> {
     let file = module.file_id.original_file(db);
-    let assist_file = AssistFile::TargetFile(file);
     let assist_item = match module.value {
         hir::ModuleSource::SourceFile(it) => {
             if let Some(last_item) = it.items().last() {
@@ -272,7 +272,7 @@ fn next_space_for_fn_in_module(
             }
         }
     };
-    Some((assist_file, assist_item))
+    Some((file, assist_item))
 }
 
 #[cfg(test)]

crates/ra_assists/src/lib.rs

@@ -18,7 +18,7 @@ pub mod utils;
 pub mod ast_transform;
 
 use hir::Semantics;
-use ra_db::{FileId, FileRange};
+use ra_db::FileRange;
 use ra_ide_db::{source_change::SourceChange, RootDatabase};
 use ra_syntax::TextRange;
 
@@ -59,19 +59,7 @@ impl AssistLabel {
 #[derive(Debug, Clone)]
 pub struct ResolvedAssist {
     pub label: AssistLabel,
-    pub action: SourceChange,
-}
-
-#[derive(Debug, Clone, Copy)]
-enum AssistFile {
-    CurrentFile,
-    TargetFile(FileId),
-}
-
-impl Default for AssistFile {
-    fn default() -> Self {
-        Self::CurrentFile
-    }
+    pub source_change: SourceChange,
 }
 
 /// Return all the assists applicable at the given position.

crates/ra_assists/src/tests.rs

@@ -57,7 +57,7 @@ fn check_doc_test(assist_id: &str, before: &str, after: &str) {
         });
 
     let actual = {
-        let change = assist.action.source_file_edits.pop().unwrap();
+        let change = assist.source_change.source_file_edits.pop().unwrap();
         let mut actual = before.clone();
         change.edit.apply(&mut actual);
         actual
@@ -94,7 +94,7 @@ fn check(assist: Handler, before: &str, expected: ExpectedResult) {
 
     match (assist(assist_ctx), expected) {
         (Some(assist), ExpectedResult::After(after)) => {
-            let mut action = assist.0[0].action.clone().unwrap();
+            let mut action = assist.0[0].source_change.clone().unwrap();
             let change = action.source_file_edits.pop().unwrap();
 
             let mut actual = db.file_text(change.file_id).as_ref().to_owned();

crates/ra_ide/src/completion/completion_item.rs

@@ -2,11 +2,12 @@
 
 use std::fmt;
 
-use super::completion_config::SnippetCap;
 use hir::Documentation;
 use ra_syntax::TextRange;
 use ra_text_edit::TextEdit;
 
+use crate::completion::completion_config::SnippetCap;
+
 /// `CompletionItem` describes a single completion variant in the editor pop-up.
 /// It is basically a POD with various properties. To construct a
 /// `CompletionItem`, use `new` method and the `Builder` struct.

crates/ra_ide/src/lib.rs

@@ -478,7 +478,7 @@ impl Analysis {
                     id: assist.label.id,
                     label: assist.label.label,
                     group_label: assist.label.group.map(|it| it.0),
-                    source_change: assist.action,
+                    source_change: assist.source_change,
                 })
                 .collect()
         })