fix(assist): derive source scope from syntax node to be transformed

3 files changed, 146 insertions, 60 deletions

diff --git a/crates/ide-assists/src/handlers/add_missing_impl_members.rs b/crates/ide-assists/src/handlers/add_missing_impl_members.rs
index 0cf7a848d8b..c7f9fa5bd65 100644
--- a/crates/ide-assists/src/handlers/add_missing_impl_members.rs
+++ b/crates/ide-assists/src/handlers/add_missing_impl_members.rs
@@ -1,5 +1,4 @@
 use hir::HasSource;
-use ide_db::syntax_helpers::insert_whitespace_into_node::insert_ws_into;
 use syntax::ast::{self, make, AstNode};
 
 use crate::{
@@ -129,20 +128,9 @@ fn add_missing_impl_members_inner(
     let target = impl_def.syntax().text_range();
     acc.add(AssistId(assist_id, AssistKind::QuickFix), label, target, |edit| {
         let new_impl_def = edit.make_mut(impl_def.clone());
-        let missing_items = missing_items
-            .into_iter()
-            .map(|it| {
-                if ctx.sema.hir_file_for(it.syntax()).is_macro() {
-                    if let Some(it) = ast::AssocItem::cast(insert_ws_into(it.syntax().clone())) {
-                        return it;
-                    }
-                }
-                it.clone_for_update()
-            })
-            .collect();
         let first_new_item = add_trait_assoc_items_to_impl(
             &ctx.sema,
-            missing_items,
+            &missing_items,
             trait_,
             &new_impl_def,
             target_scope,
@@ -1730,4 +1718,77 @@ impl m::Foo for S {
 }"#,
         )
     }
+
+    #[test]
+    fn nested_macro_should_not_cause_crash() {
+        check_assist(
+            add_missing_impl_members,
+            r#"
+macro_rules! ty { () => { i32 } }
+trait SomeTrait { type Output; }
+impl SomeTrait for i32 { type Output = i64; }
+macro_rules! define_method {
+    () => {
+        fn method(&mut self, params: <ty!() as SomeTrait>::Output);
+    };
+}
+trait AnotherTrait { define_method!(); }
+impl $0AnotherTrait for () {
+}
+"#,
+            r#"
+macro_rules! ty { () => { i32 } }
+trait SomeTrait { type Output; }
+impl SomeTrait for i32 { type Output = i64; }
+macro_rules! define_method {
+    () => {
+        fn method(&mut self, params: <ty!() as SomeTrait>::Output);
+    };
+}
+trait AnotherTrait { define_method!(); }
+impl AnotherTrait for () {
+    $0fn method(&mut self,params: <ty!()as SomeTrait>::Output) {
+        todo!()
+    }
+}
+"#,
+        );
+    }
+
+    // FIXME: `T` in `ty!(T)` should be replaced by `PathTransform`.
+    #[test]
+    fn paths_in_nested_macro_should_get_transformed() {
+        check_assist(
+            add_missing_impl_members,
+            r#"
+macro_rules! ty { ($me:ty) => { $me } }
+trait SomeTrait { type Output; }
+impl SomeTrait for i32 { type Output = i64; }
+macro_rules! define_method {
+    ($t:ty) => {
+        fn method(&mut self, params: <ty!($t) as SomeTrait>::Output);
+    };
+}
+trait AnotherTrait<T: SomeTrait> { define_method!(T); }
+impl $0AnotherTrait<i32> for () {
+}
+"#,
+            r#"
+macro_rules! ty { ($me:ty) => { $me } }
+trait SomeTrait { type Output; }
+impl SomeTrait for i32 { type Output = i64; }
+macro_rules! define_method {
+    ($t:ty) => {
+        fn method(&mut self, params: <ty!($t) as SomeTrait>::Output);
+    };
+}
+trait AnotherTrait<T: SomeTrait> { define_method!(T); }
+impl AnotherTrait<i32> for () {
+    $0fn method(&mut self,params: <ty!(T)as SomeTrait>::Output) {
+        todo!()
+    }
+}
+"#,
+        );
+    }
 }
diff --git a/crates/ide-assists/src/handlers/replace_derive_with_manual_impl.rs b/crates/ide-assists/src/handlers/replace_derive_with_manual_impl.rs
index 1b0a8e0a8da..0469343dbfb 100644
--- a/crates/ide-assists/src/handlers/replace_derive_with_manual_impl.rs
+++ b/crates/ide-assists/src/handlers/replace_derive_with_manual_impl.rs
@@ -1,8 +1,5 @@
 use hir::{InFile, ModuleDef};
-use ide_db::{
-    helpers::mod_path_to_ast, imports::import_assets::NameToImport, items_locator,
-    syntax_helpers::insert_whitespace_into_node::insert_ws_into,
-};
+use ide_db::{helpers::mod_path_to_ast, imports::import_assets::NameToImport, items_locator};
 use itertools::Itertools;
 use syntax::{
     ast::{self, AstNode, HasName},
@@ -202,19 +199,24 @@ fn impl_def_from_trait(
         node
     };
 
-    let trait_items = trait_items
-        .into_iter()
-        .map(|it| {
-            if sema.hir_file_for(it.syntax()).is_macro() {
-                if let Some(it) = ast::AssocItem::cast(insert_ws_into(it.syntax().clone())) {
-                    return it;
-                }
-            }
-            it.clone_for_update()
-        })
-        .collect();
+    // <<<<<<< HEAD
+    //     let trait_items = trait_items
+    //         .into_iter()
+    //         .map(|it| {
+    //             if sema.hir_file_for(it.syntax()).is_macro() {
+    //                 if let Some(it) = ast::AssocItem::cast(insert_ws_into(it.syntax().clone())) {
+    //                     return it;
+    //                 }
+    //             }
+    //             it.clone_for_update()
+    //         })
+    //         .collect();
+    //     let first_assoc_item =
+    //         add_trait_assoc_items_to_impl(sema, trait_items, trait_, &impl_def, target_scope);
+    // =======
     let first_assoc_item =
-        add_trait_assoc_items_to_impl(sema, trait_items, trait_, &impl_def, target_scope);
+        add_trait_assoc_items_to_impl(sema, &trait_items, trait_, &impl_def, target_scope);
+    // >>>>>>> fix(assist): derive source scope from syntax node to be transformed
 
     // Generate a default `impl` function body for the derived trait.
     if let ast::AssocItem::Fn(ref func) = first_assoc_item {
diff --git a/crates/ide-assists/src/utils.rs b/crates/ide-assists/src/utils.rs
index cc4a7f3c0ad..03d8553506f 100644
--- a/crates/ide-assists/src/utils.rs
+++ b/crates/ide-assists/src/utils.rs
@@ -3,8 +3,11 @@
 use std::ops;
 
 pub(crate) use gen_trait_fn_body::gen_trait_fn_body;
-use hir::{db::HirDatabase, HirDisplay, Semantics};
-use ide_db::{famous_defs::FamousDefs, path_transform::PathTransform, RootDatabase, SnippetCap};
+use hir::{db::HirDatabase, HirDisplay, InFile, Semantics};
+use ide_db::{
+    famous_defs::FamousDefs, path_transform::PathTransform,
+    syntax_helpers::insert_whitespace_into_node::insert_ws_into, RootDatabase, SnippetCap,
+};
 use stdx::format_to;
 use syntax::{
     ast::{
@@ -91,30 +94,21 @@ pub fn filter_assoc_items(
     sema: &Semantics<'_, RootDatabase>,
     items: &[hir::AssocItem],
     default_methods: DefaultMethods,
-) -> Vec<ast::AssocItem> {
-    fn has_def_name(item: &ast::AssocItem) -> bool {
-        match item {
-            ast::AssocItem::Fn(def) => def.name(),
-            ast::AssocItem::TypeAlias(def) => def.name(),
-            ast::AssocItem::Const(def) => def.name(),
-            ast::AssocItem::MacroCall(_) => None,
-        }
-        .is_some()
-    }
-
-    items
+) -> Vec<InFile<ast::AssocItem>> {
+    return items
         .iter()
         // Note: This throws away items with no source.
-        .filter_map(|&i| {
-            let item = match i {
-                hir::AssocItem::Function(i) => ast::AssocItem::Fn(sema.source(i)?.value),
-                hir::AssocItem::TypeAlias(i) => ast::AssocItem::TypeAlias(sema.source(i)?.value),
-                hir::AssocItem::Const(i) => ast::AssocItem::Const(sema.source(i)?.value),
+        .copied()
+        .filter_map(|assoc_item| {
+            let item = match assoc_item {
+                hir::AssocItem::Function(it) => sema.source(it)?.map(ast::AssocItem::Fn),
+                hir::AssocItem::TypeAlias(it) => sema.source(it)?.map(ast::AssocItem::TypeAlias),
+                hir::AssocItem::Const(it) => sema.source(it)?.map(ast::AssocItem::Const),
             };
             Some(item)
         })
         .filter(has_def_name)
-        .filter(|it| match it {
+        .filter(|it| match &it.value {
             ast::AssocItem::Fn(def) => matches!(
                 (default_methods, def.body()),
                 (DefaultMethods::Only, Some(_)) | (DefaultMethods::No, None)
@@ -125,26 +119,55 @@ pub fn filter_assoc_items(
             ),
             _ => default_methods == DefaultMethods::No,
         })
-        .collect::<Vec<_>>()
+        .collect();
+
+    fn has_def_name(item: &InFile<ast::AssocItem>) -> bool {
+        match &item.value {
+            ast::AssocItem::Fn(def) => def.name(),
+            ast::AssocItem::TypeAlias(def) => def.name(),
+            ast::AssocItem::Const(def) => def.name(),
+            ast::AssocItem::MacroCall(_) => None,
+        }
+        .is_some()
+    }
 }
 
+/// Given `original_items` retrieved from the trait definition (usually by
+/// [`filter_assoc_items()`]), clones each item for update and applies path transformation to it,
+/// then inserts into `impl_`. Returns the modified `impl_` and the first associated item that got
+/// inserted.
 pub fn add_trait_assoc_items_to_impl(
     sema: &Semantics<'_, RootDatabase>,
-    items: Vec<ast::AssocItem>,
+    original_items: &[InFile<ast::AssocItem>],
     trait_: hir::Trait,
     impl_: &ast::Impl,
     target_scope: hir::SemanticsScope<'_>,
 ) -> ast::AssocItem {
-    let source_scope = sema.scope_for_def(trait_);
-
-    let transform = PathTransform::trait_impl(&target_scope, &source_scope, trait_, impl_.clone());
-
     let new_indent_level = IndentLevel::from_node(impl_.syntax()) + 1;
-    let items = items.into_iter().map(|assoc_item| {
-        transform.apply(assoc_item.syntax());
-        assoc_item.remove_attrs_and_docs();
-        assoc_item.reindent_to(new_indent_level);
-        assoc_item
+    let items = original_items.into_iter().map(|InFile { file_id, value: original_item }| {
+        let cloned_item = {
+            if file_id.is_macro() {
+                if let Some(formatted) =
+                    ast::AssocItem::cast(insert_ws_into(original_item.syntax().clone()))
+                {
+                    return formatted;
+                } else {
+                    stdx::never!("formatted `AssocItem` could not be cast back to `AssocItem`");
+                }
+            }
+            original_item.clone_for_update()
+        };
+
+        if let Some(source_scope) = sema.scope(original_item.syntax()) {
+            // FIXME: Paths in nested macros are not handled well. See
+            // `add_missing_impl_members::paths_in_nested_macro_should_get_transformed` test.
+            let transform =
+                PathTransform::trait_impl(&target_scope, &source_scope, trait_, impl_.clone());
+            transform.apply(cloned_item.syntax());
+        }
+        cloned_item.remove_attrs_and_docs();
+        cloned_item.reindent_to(new_indent_level);
+        cloned_item
     });
 
     let assoc_item_list = impl_.get_or_create_assoc_item_list();