Merge #4341

4341: Move target to AssistLabel r=matklad a=matklad

Target is used for assists sorting, so we need it before we compute
the action.



bors r+
🤖

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

Author: bors[bot]

crates/ra_assists/src/assist_ctx.rs

@@ -94,9 +94,10 @@ impl<'a> AssistCtx<'a> {
         self,
         id: AssistId,
         label: impl Into<String>,
+        target: TextRange,
         f: impl FnOnce(&mut ActionBuilder),
     ) -> Option<Assist> {
-        let label = AssistLabel::new(id, label.into(), None);
+        let label = AssistLabel::new(id, label.into(), None, target);
 
         let mut info = AssistInfo::new(label);
         if self.should_compute_edit {
@@ -152,9 +153,10 @@ impl<'a> AssistGroup<'a> {
         &mut self,
         id: AssistId,
         label: impl Into<String>,
+        target: TextRange,
         f: impl FnOnce(&mut ActionBuilder),
     ) {
-        let label = AssistLabel::new(id, label.into(), Some(self.group.clone()));
+        let label = AssistLabel::new(id, label.into(), Some(self.group.clone()), target);
 
         let mut info = AssistInfo::new(label).with_group(self.group.clone());
         if self.ctx.should_compute_edit {
@@ -181,7 +183,6 @@ impl<'a> AssistGroup<'a> {
 pub(crate) struct ActionBuilder<'a, 'b> {
     edit: TextEditBuilder,
     cursor_position: Option<TextSize>,
-    target: Option<TextRange>,
     file: AssistFile,
     ctx: &'a AssistCtx<'b>,
 }
@@ -191,7 +192,6 @@ impl<'a, 'b> ActionBuilder<'a, 'b> {
         Self {
             edit: TextEditBuilder::default(),
             cursor_position: None,
-            target: None,
             file: AssistFile::default(),
             ctx,
         }
@@ -237,14 +237,6 @@ impl<'a, 'b> ActionBuilder<'a, 'b> {
         self.cursor_position = Some(offset)
     }
 
-    /// Specify that the assist should be active withing the `target` range.
-    ///
-    /// Target ranges are used to sort assists: the smaller the target range,
-    /// the more specific assist is, and so it should be sorted first.
-    pub(crate) fn target(&mut self, target: TextRange) {
-        self.target = Some(target)
-    }
-
     /// Get access to the raw `TextEditBuilder`.
     pub(crate) fn text_edit_builder(&mut self) -> &mut TextEditBuilder {
         &mut self.edit
@@ -267,7 +259,6 @@ impl<'a, 'b> ActionBuilder<'a, 'b> {
         AssistAction {
             edit: self.edit.finish(),
             cursor_position: self.cursor_position,
-            target: self.target,
             file: self.file,
         }
     }

crates/ra_assists/src/handlers/add_custom_impl.rs

@@ -48,9 +48,8 @@ pub(crate) fn add_custom_impl(ctx: AssistCtx) -> Option<Assist> {
     let label =
         format!("Add custom impl '{}' for '{}'", trait_token.text().as_str(), annotated_name);
 
-    ctx.add_assist(AssistId("add_custom_impl"), label, |edit| {
-        edit.target(attr.syntax().text_range());
-
+    let target = attr.syntax().text_range();
+    ctx.add_assist(AssistId("add_custom_impl"), label, target, |edit| {
         let new_attr_input = input
             .syntax()
             .descendants_with_tokens()

crates/ra_assists/src/handlers/add_derive.rs

@@ -27,7 +27,8 @@ use crate::{Assist, AssistCtx, AssistId};
 pub(crate) fn add_derive(ctx: AssistCtx) -> Option<Assist> {
     let nominal = ctx.find_node_at_offset::<ast::NominalDef>()?;
     let node_start = derive_insertion_offset(&nominal)?;
-    ctx.add_assist(AssistId("add_derive"), "Add `#[derive]`", |edit| {
+    let target = nominal.syntax().text_range();
+    ctx.add_assist(AssistId("add_derive"), "Add `#[derive]`", target, |edit| {
         let derive_attr = nominal
             .attrs()
             .filter_map(|x| x.as_simple_call())
@@ -41,7 +42,6 @@ pub(crate) fn add_derive(ctx: AssistCtx) -> Option<Assist> {
             }
             Some(tt) => tt.syntax().text_range().end() - TextSize::of(')'),
         };
-        edit.target(nominal.syntax().text_range());
         edit.set_cursor(offset)
     })
 }

crates/ra_assists/src/handlers/add_explicit_type.rs

@@ -62,8 +62,8 @@ pub(crate) fn add_explicit_type(ctx: AssistCtx) -> Option<Assist> {
     ctx.add_assist(
         AssistId("add_explicit_type"),
         format!("Insert explicit type '{}'", new_type_string),
+        pat_range,
         |edit| {
-            edit.target(pat_range);
             if let Some(ascribed_ty) = ascribed_ty {
                 edit.replace(ascribed_ty.syntax().text_range(), new_type_string);
             } else {

crates/ra_assists/src/handlers/add_from_impl_for_enum.rs

@@ -47,9 +47,11 @@ pub(crate) fn add_from_impl_for_enum(ctx: AssistCtx) -> Option<Assist> {
         return None;
     }
 
+    let target = variant.syntax().text_range();
     ctx.add_assist(
         AssistId("add_from_impl_for_enum"),
         "Add From impl for this enum variant",
+        target,
         |edit| {
             let start_offset = variant.parent_enum().syntax().text_range().end();
             let mut buf = String::new();

crates/ra_assists/src/handlers/add_function.rs

@@ -57,9 +57,9 @@ pub(crate) fn add_function(ctx: AssistCtx) -> Option<Assist> {
 
     let function_builder = FunctionBuilder::from_call(&ctx, &call, &path, target_module)?;
 
-    ctx.add_assist(AssistId("add_function"), "Add function", |edit| {
-        edit.target(call.syntax().text_range());
-
+    let target = call.syntax().text_range();
+    // TODO: assert here?
+    ctx.add_assist(AssistId("add_function"), "Add function", target, |edit| {
         if let Some(function_template) = function_builder.render() {
             edit.set_file(function_template.file);
             edit.set_cursor(function_template.cursor_offset);

crates/ra_assists/src/handlers/add_impl.rs

@@ -28,33 +28,40 @@ use crate::{Assist, AssistCtx, AssistId};
 pub(crate) fn add_impl(ctx: AssistCtx) -> Option<Assist> {
     let nominal = ctx.find_node_at_offset::<ast::NominalDef>()?;
     let name = nominal.name()?;
-    ctx.add_assist(AssistId("add_impl"), format!("Implement {}", name.text().as_str()), |edit| {
-        edit.target(nominal.syntax().text_range());
-        let type_params = nominal.type_param_list();
-        let start_offset = nominal.syntax().text_range().end();
-        let mut buf = String::new();
-        buf.push_str("\n\nimpl");
-        if let Some(type_params) = &type_params {
-            format_to!(buf, "{}", type_params.syntax());
-        }
-        buf.push_str(" ");
-        buf.push_str(name.text().as_str());
-        if let Some(type_params) = type_params {
-            let lifetime_params = type_params
-                .lifetime_params()
-                .filter_map(|it| it.lifetime_token())
-                .map(|it| it.text().clone());
-            let type_params =
-                type_params.type_params().filter_map(|it| it.name()).map(|it| it.text().clone());
+    let target = nominal.syntax().text_range();
+    ctx.add_assist(
+        AssistId("add_impl"),
+        format!("Implement {}", name.text().as_str()),
+        target,
+        |edit| {
+            let type_params = nominal.type_param_list();
+            let start_offset = nominal.syntax().text_range().end();
+            let mut buf = String::new();
+            buf.push_str("\n\nimpl");
+            if let Some(type_params) = &type_params {
+                format_to!(buf, "{}", type_params.syntax());
+            }
+            buf.push_str(" ");
+            buf.push_str(name.text().as_str());
+            if let Some(type_params) = type_params {
+                let lifetime_params = type_params
+                    .lifetime_params()
+                    .filter_map(|it| it.lifetime_token())
+                    .map(|it| it.text().clone());
+                let type_params = type_params
+                    .type_params()
+                    .filter_map(|it| it.name())
+                    .map(|it| it.text().clone());
 
-            let generic_params = lifetime_params.chain(type_params).sep_by(", ");
-            format_to!(buf, "<{}>", generic_params)
-        }
-        buf.push_str(" {\n");
-        edit.set_cursor(start_offset + TextSize::of(&buf));
-        buf.push_str("\n}");
-        edit.insert(start_offset, buf);
-    })
+                let generic_params = lifetime_params.chain(type_params).sep_by(", ");
+                format_to!(buf, "<{}>", generic_params)
+            }
+            buf.push_str(" {\n");
+            edit.set_cursor(start_offset + TextSize::of(&buf));
+            buf.push_str("\n}");
+            edit.insert(start_offset, buf);
+        },
+    )
 }
 
 #[cfg(test)]

crates/ra_assists/src/handlers/add_missing_impl_members.rs

@@ -107,10 +107,10 @@ fn add_missing_impl_members_inner(
     label: &'static str,
 ) -> Option<Assist> {
     let _p = ra_prof::profile("add_missing_impl_members_inner");
-    let impl_node = ctx.find_node_at_offset::<ast::ImplDef>()?;
-    let impl_item_list = impl_node.item_list()?;
+    let impl_def = ctx.find_node_at_offset::<ast::ImplDef>()?;
+    let impl_item_list = impl_def.item_list()?;
 
-    let trait_ = resolve_target_trait(&ctx.sema, &impl_node)?;
+    let trait_ = resolve_target_trait(&ctx.sema, &impl_def)?;
 
     let def_name = |item: &ast::AssocItem| -> Option<SmolStr> {
         match item {
@@ -121,7 +121,7 @@ fn add_missing_impl_members_inner(
         .map(|it| it.text().clone())
     };
 
-    let missing_items = get_missing_assoc_items(&ctx.sema, &impl_node)
+    let missing_items = get_missing_assoc_items(&ctx.sema, &impl_def)
         .iter()
         .map(|i| match i {
             hir::AssocItem::Function(i) => ast::AssocItem::FnDef(i.source(ctx.db).value),
@@ -143,13 +143,13 @@ fn add_missing_impl_members_inner(
     }
 
     let sema = ctx.sema;
-
-    ctx.add_assist(AssistId(assist_id), label, |edit| {
+    let target = impl_def.syntax().text_range();
+    ctx.add_assist(AssistId(assist_id), label, target, |edit| {
         let n_existing_items = impl_item_list.assoc_items().count();
         let source_scope = sema.scope_for_def(trait_);
         let target_scope = sema.scope(impl_item_list.syntax());
         let ast_transform = QualifyPaths::new(&target_scope, &source_scope)
-            .or(SubstituteTypeParams::for_trait_impl(&source_scope, trait_, impl_node));
+            .or(SubstituteTypeParams::for_trait_impl(&source_scope, trait_, impl_def));
         let items = missing_items
             .into_iter()
             .map(|it| ast_transform::apply(&*ast_transform, it))

crates/ra_assists/src/handlers/add_new.rs

@@ -41,9 +41,8 @@ pub(crate) fn add_new(ctx: AssistCtx) -> Option<Assist> {
     // Return early if we've found an existing new fn
     let impl_def = find_struct_impl(&ctx, &strukt)?;
 
-    ctx.add_assist(AssistId("add_new"), "Add default constructor", |edit| {
-        edit.target(strukt.syntax().text_range());
-
+    let target = strukt.syntax().text_range();
+    ctx.add_assist(AssistId("add_new"), "Add default constructor", target, |edit| {
         let mut buf = String::with_capacity(512);
 
         if impl_def.is_some() {

crates/ra_assists/src/handlers/apply_demorgan.rs

@@ -39,8 +39,7 @@ pub(crate) fn apply_demorgan(ctx: AssistCtx) -> Option<Assist> {
     let rhs_range = rhs.syntax().text_range();
     let not_rhs = invert_boolean_expression(rhs);
 
-    ctx.add_assist(AssistId("apply_demorgan"), "Apply De Morgan's law", |edit| {
-        edit.target(op_range);
+    ctx.add_assist(AssistId("apply_demorgan"), "Apply De Morgan's law", op_range, |edit| {
         edit.replace(op_range, opposite_op);
         edit.replace(lhs_range, format!("!({}", not_lhs.syntax().text()));
         edit.replace(rhs_range, format!("{})", not_rhs.syntax().text()));