Auto merge of rust-lang#16577 - DropDemBits:structured-snippet-migrate-7, r=Veykril

internal: Migrate assists to the structured snippet API, part 7/7

Continuing from rust-lang#16467

Migrates the following assists:

- `generate_trait_from_impl`

This adds `add_placeholder_snippet_group`, which adds a group of placeholder snippets which are linked together and allows for renaming generated items without going through a separate rename step.

This also removes the last usages of `SourceChangeBuilder::{insert,replace}_snippet`, as all assists have finally been migrated to the structured snippet versions of those methods.

Author: bors

crates/ide-assists/src/handlers/fix_visibility.rs

@@ -79,7 +79,7 @@ fn add_vis_to_referenced_module_def(acc: &mut Assists, ctx: &AssistContext<'_>)
         edit.edit_file(target_file);
 
         let vis_owner = edit.make_mut(vis_owner);
-        vis_owner.set_visibility(missing_visibility.clone_for_update());
+        vis_owner.set_visibility(Some(missing_visibility.clone_for_update()));
 
         if let Some((cap, vis)) = ctx.config.snippet_cap.zip(vis_owner.visibility()) {
             edit.add_tabstop_before(cap, vis);
@@ -131,7 +131,7 @@ fn add_vis_to_referenced_record_field(acc: &mut Assists, ctx: &AssistContext<'_>
         edit.edit_file(target_file);
 
         let vis_owner = edit.make_mut(vis_owner);
-        vis_owner.set_visibility(missing_visibility.clone_for_update());
+        vis_owner.set_visibility(Some(missing_visibility.clone_for_update()));
 
         if let Some((cap, vis)) = ctx.config.snippet_cap.zip(vis_owner.visibility()) {
             edit.add_tabstop_before(cap, vis);

crates/ide-assists/src/handlers/generate_trait_from_impl.rs

@@ -1,8 +1,13 @@
 use crate::assist_context::{AssistContext, Assists};
 use ide_db::assists::AssistId;
 use syntax::{
-    ast::{self, edit::IndentLevel, make, HasGenericParams, HasVisibility},
-    ted, AstNode, SyntaxKind,
+    ast::{
+        self,
+        edit_in_place::{HasVisibilityEdit, Indent},
+        make, HasGenericParams, HasName,
+    },
+    ted::{self, Position},
+    AstNode, SyntaxKind, T,
 };
 
 // NOTES :
@@ -44,7 +49,7 @@ use syntax::{
 //     };
 // }
 //
-// trait ${0:TraitName}<const N: usize> {
+// trait ${0:NewTrait}<const N: usize> {
 //     // Used as an associated constant.
 //     const CONST_ASSOC: usize = N * 4;
 //
@@ -53,7 +58,7 @@ use syntax::{
 //     const_maker! {i32, 7}
 // }
 //
-// impl<const N: usize> ${0:TraitName}<N> for Foo<N> {
+// impl<const N: usize> ${0:NewTrait}<N> for Foo<N> {
 //     // Used as an associated constant.
 //     const CONST_ASSOC: usize = N * 4;
 //
@@ -94,8 +99,10 @@ pub(crate) fn generate_trait_from_impl(acc: &mut Assists, ctx: &AssistContext<'_
         "Generate trait from impl",
         impl_ast.syntax().text_range(),
         |builder| {
+            let impl_ast = builder.make_mut(impl_ast);
             let trait_items = assoc_items.clone_for_update();
-            let impl_items = assoc_items.clone_for_update();
+            let impl_items = builder.make_mut(assoc_items);
+            let impl_name = builder.make_mut(impl_name);
 
             trait_items.assoc_items().for_each(|item| {
                 strip_body(&item);
@@ -112,46 +119,42 @@ pub(crate) fn generate_trait_from_impl(acc: &mut Assists, ctx: &AssistContext<'_
                 impl_ast.generic_param_list(),
                 impl_ast.where_clause(),
                 trait_items,
-            );
+            )
+            .clone_for_update();
+
+            let trait_name = trait_ast.name().expect("new trait should have a name");
+            let trait_name_ref = make::name_ref(&trait_name.to_string()).clone_for_update();
 
             // Change `impl Foo` to `impl NewTrait for Foo`
-            let arg_list = if let Some(genpars) = impl_ast.generic_param_list() {
-                genpars.to_generic_args().to_string()
-            } else {
-                "".to_owned()
-            };
-
-            if let Some(snippet_cap) = ctx.config.snippet_cap {
-                builder.replace_snippet(
-                    snippet_cap,
-                    impl_name.syntax().text_range(),
-                    format!("${{0:TraitName}}{} for {}", arg_list, impl_name),
-                );
+            let mut elements = vec![
+                trait_name_ref.syntax().clone().into(),
+                make::tokens::single_space().into(),
+                make::token(T![for]).into(),
+            ];
+
+            if let Some(params) = impl_ast.generic_param_list() {
+                let gen_args = &params.to_generic_args().clone_for_update();
+                elements.insert(1, gen_args.syntax().clone().into());
+            }
 
-                // Insert trait before TraitImpl
-                builder.insert_snippet(
-                    snippet_cap,
-                    impl_ast.syntax().text_range().start(),
-                    format!(
-                        "{}\n\n{}",
-                        trait_ast.to_string().replace("NewTrait", "${0:TraitName}"),
-                        IndentLevel::from_node(impl_ast.syntax())
-                    ),
-                );
-            } else {
-                builder.replace(
-                    impl_name.syntax().text_range(),
-                    format!("NewTrait{} for {}", arg_list, impl_name),
-                );
+            ted::insert_all(Position::before(impl_name.syntax()), elements);
+
+            // Insert trait before TraitImpl
+            ted::insert_all_raw(
+                Position::before(impl_ast.syntax()),
+                vec![
+                    trait_ast.syntax().clone().into(),
+                    make::tokens::whitespace(&format!("\n\n{}", impl_ast.indent_level())).into(),
+                ],
+            );
 
-                // Insert trait before TraitImpl
-                builder.insert(
-                    impl_ast.syntax().text_range().start(),
-                    format!("{}\n\n{}", trait_ast, IndentLevel::from_node(impl_ast.syntax())),
+            // Link the trait name & trait ref names together as a placeholder snippet group
+            if let Some(cap) = ctx.config.snippet_cap {
+                builder.add_placeholder_snippet_group(
+                    cap,
+                    vec![trait_name.syntax().clone(), trait_name_ref.syntax().clone()],
                 );
             }
-
-            builder.replace(assoc_items.syntax().text_range(), impl_items.to_string());
         },
     );
 
@@ -160,23 +163,8 @@ pub(crate) fn generate_trait_from_impl(acc: &mut Assists, ctx: &AssistContext<'_
 
 /// `E0449` Trait items always share the visibility of their trait
 fn remove_items_visibility(item: &ast::AssocItem) {
-    match item {
-        ast::AssocItem::Const(c) => {
-            if let Some(vis) = c.visibility() {
-                ted::remove(vis.syntax());
-            }
-        }
-        ast::AssocItem::Fn(f) => {
-            if let Some(vis) = f.visibility() {
-                ted::remove(vis.syntax());
-            }
-        }
-        ast::AssocItem::TypeAlias(t) => {
-            if let Some(vis) = t.visibility() {
-                ted::remove(vis.syntax());
-            }
-        }
-        _ => (),
+    if let Some(has_vis) = ast::AnyHasVisibility::cast(item.syntax().clone()) {
+        has_vis.set_visibility(None);
     }
 }
 
@@ -404,12 +392,12 @@ impl<const N: usize> F$0oo<N> {
             r#"
 struct Foo<const N: usize>([i32; N]);
 
-trait ${0:TraitName}<const N: usize> {
+trait ${0:NewTrait}<const N: usize> {
     // Used as an associated constant.
     const CONST: usize = N * 4;
 }
 
-impl<const N: usize> ${0:TraitName}<N> for Foo<N> {
+impl<const N: usize> ${0:NewTrait}<N> for Foo<N> {
     // Used as an associated constant.
     const CONST: usize = N * 4;
 }

crates/ide-assists/src/tests/generated.rs

@@ -1665,7 +1665,7 @@ macro_rules! const_maker {
     };
 }
 
-trait ${0:TraitName}<const N: usize> {
+trait ${0:NewTrait}<const N: usize> {
     // Used as an associated constant.
     const CONST_ASSOC: usize = N * 4;
 
@@ -1674,7 +1674,7 @@ trait ${0:TraitName}<const N: usize> {
     const_maker! {i32, 7}
 }
 
-impl<const N: usize> ${0:TraitName}<N> for Foo<N> {
+impl<const N: usize> ${0:NewTrait}<N> for Foo<N> {
     // Used as an associated constant.
     const CONST_ASSOC: usize = N * 4;
 

crates/ide-db/src/source_change.rs

@@ -138,19 +138,21 @@ impl SnippetEdit {
             .into_iter()
             .zip(1..)
             .with_position()
-            .map(|pos| {
+            .flat_map(|pos| {
                 let (snippet, index) = match pos {
                     (itertools::Position::First, it) | (itertools::Position::Middle, it) => it,
                     // last/only snippet gets index 0
                     (itertools::Position::Last, (snippet, _))
                     | (itertools::Position::Only, (snippet, _)) => (snippet, 0),
                 };
 
-                let range = match snippet {
-                    Snippet::Tabstop(pos) => TextRange::empty(pos),
-                    Snippet::Placeholder(range) => range,
-                };
-                (index, range)
+                match snippet {
+                    Snippet::Tabstop(pos) => vec![(index, TextRange::empty(pos))],
+                    Snippet::Placeholder(range) => vec![(index, range)],
+                    Snippet::PlaceholderGroup(ranges) => {
+                        ranges.into_iter().map(|range| (index, range)).collect()
+                    }
+                }
             })
             .collect_vec();
 
@@ -248,7 +250,7 @@ impl SourceChangeBuilder {
     fn commit(&mut self) {
         let snippet_edit = self.snippet_builder.take().map(|builder| {
             SnippetEdit::new(
-                builder.places.into_iter().map(PlaceSnippet::finalize_position).collect_vec(),
+                builder.places.into_iter().flat_map(PlaceSnippet::finalize_position).collect(),
             )
         });
 
@@ -287,30 +289,10 @@ impl SourceChangeBuilder {
     pub fn insert(&mut self, offset: TextSize, text: impl Into<String>) {
         self.edit.insert(offset, text.into())
     }
-    /// Append specified `snippet` at the given `offset`
-    pub fn insert_snippet(
-        &mut self,
-        _cap: SnippetCap,
-        offset: TextSize,
-        snippet: impl Into<String>,
-    ) {
-        self.source_change.is_snippet = true;
-        self.insert(offset, snippet);
-    }
     /// Replaces specified `range` of text with a given string.
     pub fn replace(&mut self, range: TextRange, replace_with: impl Into<String>) {
         self.edit.replace(range, replace_with.into())
     }
-    /// Replaces specified `range` of text with a given `snippet`.
-    pub fn replace_snippet(
-        &mut self,
-        _cap: SnippetCap,
-        range: TextRange,
-        snippet: impl Into<String>,
-    ) {
-        self.source_change.is_snippet = true;
-        self.replace(range, snippet);
-    }
     pub fn replace_ast<N: AstNode>(&mut self, old: N, new: N) {
         algo::diff(old.syntax(), new.syntax()).into_text_edit(&mut self.edit)
     }
@@ -356,6 +338,17 @@ impl SourceChangeBuilder {
         self.add_snippet(PlaceSnippet::Over(node.syntax().clone().into()))
     }
 
+    /// Adds a snippet to move the cursor selected over `nodes`
+    ///
+    /// This allows for renaming newly generated items without having to go
+    /// through a separate rename step.
+    pub fn add_placeholder_snippet_group(&mut self, _cap: SnippetCap, nodes: Vec<SyntaxNode>) {
+        assert!(nodes.iter().all(|node| node.parent().is_some()));
+        self.add_snippet(PlaceSnippet::OverGroup(
+            nodes.into_iter().map(|node| node.into()).collect(),
+        ))
+    }
+
     fn add_snippet(&mut self, snippet: PlaceSnippet) {
         let snippet_builder = self.snippet_builder.get_or_insert(SnippetBuilder { places: vec![] });
         snippet_builder.places.push(snippet);
@@ -400,6 +393,13 @@ pub enum Snippet {
     Tabstop(TextSize),
     /// A placeholder snippet (e.g. `${0:placeholder}`).
     Placeholder(TextRange),
+    /// A group of placeholder snippets, e.g.
+    ///
+    /// ```no_run
+    /// let ${0:new_var} = 4;
+    /// fun(1, 2, 3, ${0:new_var});
+    /// ```
+    PlaceholderGroup(Vec<TextRange>),
 }
 
 enum PlaceSnippet {
@@ -409,14 +409,20 @@ enum PlaceSnippet {
     After(SyntaxElement),
     /// Place a placeholder snippet in place of the element
     Over(SyntaxElement),
+    /// Place a group of placeholder snippets which are linked together
+    /// in place of the elements
+    OverGroup(Vec<SyntaxElement>),
 }
 
 impl PlaceSnippet {
-    fn finalize_position(self) -> Snippet {
+    fn finalize_position(self) -> Vec<Snippet> {
         match self {
-            PlaceSnippet::Before(it) => Snippet::Tabstop(it.text_range().start()),
-            PlaceSnippet::After(it) => Snippet::Tabstop(it.text_range().end()),
-            PlaceSnippet::Over(it) => Snippet::Placeholder(it.text_range()),
+            PlaceSnippet::Before(it) => vec![Snippet::Tabstop(it.text_range().start())],
+            PlaceSnippet::After(it) => vec![Snippet::Tabstop(it.text_range().end())],
+            PlaceSnippet::Over(it) => vec![Snippet::Placeholder(it.text_range())],
+            PlaceSnippet::OverGroup(it) => {
+                vec![Snippet::PlaceholderGroup(it.into_iter().map(|it| it.text_range()).collect())]
+            }
         }
     }
 }

crates/syntax/src/ast/edit_in_place.rs

@@ -1007,20 +1007,24 @@ impl ast::IdentPat {
 }
 
 pub trait HasVisibilityEdit: ast::HasVisibility {
-    fn set_visibility(&self, visibility: ast::Visibility) {
-        match self.visibility() {
-            Some(current_visibility) => {
-                ted::replace(current_visibility.syntax(), visibility.syntax())
-            }
-            None => {
-                let vis_before = self
-                    .syntax()
-                    .children_with_tokens()
-                    .find(|it| !matches!(it.kind(), WHITESPACE | COMMENT | ATTR))
-                    .unwrap_or_else(|| self.syntax().first_child_or_token().unwrap());
-
-                ted::insert(ted::Position::before(vis_before), visibility.syntax());
+    fn set_visibility(&self, visibility: Option<ast::Visibility>) {
+        if let Some(visibility) = visibility {
+            match self.visibility() {
+                Some(current_visibility) => {
+                    ted::replace(current_visibility.syntax(), visibility.syntax())
+                }
+                None => {
+                    let vis_before = self
+                        .syntax()
+                        .children_with_tokens()
+                        .find(|it| !matches!(it.kind(), WHITESPACE | COMMENT | ATTR))
+                        .unwrap_or_else(|| self.syntax().first_child_or_token().unwrap());
+
+                    ted::insert(ted::Position::before(vis_before), visibility.syntax());
+                }
             }
+        } else if let Some(visibility) = self.visibility() {
+            ted::remove(visibility.syntax());
         }
     }
 }

crates/syntax/src/ast/make.rs

@@ -1147,7 +1147,7 @@ pub mod tokens {
 
     pub(super) static SOURCE_FILE: Lazy<Parse<SourceFile>> = Lazy::new(|| {
         SourceFile::parse(
-            "const C: <()>::Item = ( true && true , true || true , 1 != 1, 2 == 2, 3 < 3, 4 <= 4, 5 > 5, 6 >= 6, !true, *p, &p , &mut p, { let a @ [] })\n;\n\n",
+            "const C: <()>::Item = ( true && true , true || true , 1 != 1, 2 == 2, 3 < 3, 4 <= 4, 5 > 5, 6 >= 6, !true, *p, &p , &mut p, { let a @ [] })\n;\n\nimpl A for B where: {}",
         )
     });