Merge #6043

6043: Allow missing trait members assist without needing braces r=matklad a=M-J-Hooper

Assist to complete missing items when implementing a trait does not appear without impl def braces (see #5144 ).

The reason behind this was that this assist is based on `ast::AssocItemList` which only appears in the AST after the braces are added to the impl def.

Instead of relying on and replacing the item list, we now instead replace the entire `ast::Impl` and add the item list if its missing.

Co-authored-by: Matt Hooper <[email protected]>

crates/assists/src/handlers/add_missing_impl_members.rs

@@ -111,8 +111,6 @@ fn add_missing_impl_members_inner(
 ) -> Option<()> {
     let _p = profile::span("add_missing_impl_members_inner");
     let impl_def = ctx.find_node_at_offset::<ast::Impl>()?;
-    let impl_item_list = impl_def.assoc_item_list()?;
-
     let trait_ = resolve_target_trait(&ctx.sema, &impl_def)?;
 
     let def_name = |item: &ast::AssocItem| -> Option<SmolStr> {
@@ -148,11 +146,14 @@ fn add_missing_impl_members_inner(
 
     let target = impl_def.syntax().text_range();
     acc.add(AssistId(assist_id, AssistKind::QuickFix), label, target, |builder| {
+        let impl_item_list = impl_def.assoc_item_list().unwrap_or(make::assoc_item_list());
+
         let n_existing_items = impl_item_list.assoc_items().count();
         let source_scope = ctx.sema.scope_for_def(trait_);
-        let target_scope = ctx.sema.scope(impl_item_list.syntax());
+        let target_scope = ctx.sema.scope(impl_def.syntax());
         let ast_transform = QualifyPaths::new(&target_scope, &source_scope)
-            .or(SubstituteTypeParams::for_trait_impl(&source_scope, trait_, impl_def));
+            .or(SubstituteTypeParams::for_trait_impl(&source_scope, trait_, impl_def.clone()));
+
         let items = missing_items
             .into_iter()
             .map(|it| ast_transform::apply(&*ast_transform, it))
@@ -162,12 +163,14 @@ fn add_missing_impl_members_inner(
                 _ => it,
             })
             .map(|it| edit::remove_attrs_and_docs(&it));
+
         let new_impl_item_list = impl_item_list.append_items(items);
-        let first_new_item = new_impl_item_list.assoc_items().nth(n_existing_items).unwrap();
+        let new_impl_def = impl_def.with_assoc_item_list(new_impl_item_list);
+        let first_new_item =
+            new_impl_def.assoc_item_list().unwrap().assoc_items().nth(n_existing_items).unwrap();
 
-        let original_range = impl_item_list.syntax().text_range();
         match ctx.config.snippet_cap {
-            None => builder.replace(original_range, new_impl_item_list.to_string()),
+            None => builder.replace(target, new_impl_def.to_string()),
             Some(cap) => {
                 let mut cursor = Cursor::Before(first_new_item.syntax());
                 let placeholder;
@@ -181,8 +184,8 @@ fn add_missing_impl_members_inner(
                 }
                 builder.replace_snippet(
                     cap,
-                    original_range,
-                    render_snippet(cap, new_impl_item_list.syntax(), cursor),
+                    target,
+                    render_snippet(cap, new_impl_def.syntax(), cursor),
                 )
             }
         };
@@ -310,6 +313,25 @@ impl Foo for S {
         );
     }
 
+    #[test]
+    fn test_impl_def_without_braces() {
+        check_assist(
+            add_missing_impl_members,
+            r#"
+trait Foo { fn foo(&self); }
+struct S;
+impl Foo for S<|>"#,
+            r#"
+trait Foo { fn foo(&self); }
+struct S;
+impl Foo for S {
+    fn foo(&self) {
+        ${0:todo!()}
+    }
+}"#,
+        );
+    }
+
     #[test]
     fn fill_in_type_params_1() {
         check_assist(

crates/syntax/src/ast/edit.rs

@@ -93,6 +93,22 @@ where
     }
 }
 
+impl ast::Impl {
+    #[must_use]
+    pub fn with_assoc_item_list(&self, items: ast::AssocItemList) -> ast::Impl {
+        let mut to_insert: ArrayVec<[SyntaxElement; 2]> = ArrayVec::new();
+        if let Some(old_items) = self.assoc_item_list() {
+            let to_replace: SyntaxElement = old_items.syntax().clone().into();
+            to_insert.push(items.syntax().clone().into());
+            self.replace_children(single_node(to_replace), to_insert)
+        } else {
+            to_insert.push(make::tokens::single_space().into());
+            to_insert.push(items.syntax().clone().into());
+            self.insert_children(InsertPosition::Last, to_insert)
+        }
+    }
+}
+
 impl ast::AssocItemList {
     #[must_use]
     pub fn append_items(

crates/syntax/src/ast/make.rs

@@ -21,6 +21,10 @@ pub fn ty(text: &str) -> ast::Type {
     ast_from_text(&format!("impl {} for D {{}};", text))
 }
 
+pub fn assoc_item_list() -> ast::AssocItemList {
+    ast_from_text("impl C for D {};")
+}
+
 pub fn path_segment(name_ref: ast::NameRef) -> ast::PathSegment {
     ast_from_text(&format!("use {};", name_ref))
 }