fix: insert whitespaces into assoc items for assist when macro generated

8 files changed, 84 insertions, 26 deletions

diff --git a/crates/hir/src/semantics.rs b/crates/hir/src/semantics.rs
index 9302eaf913b..75f6b025779 100644
--- a/crates/hir/src/semantics.rs
+++ b/crates/hir/src/semantics.rs
@@ -1163,10 +1163,6 @@ impl<'a> SemanticsScope<'a> {
         Some(Crate { id: self.resolver.krate()? })
     }
 
-    pub fn in_macro_file(&self) -> bool {
-        self.file_id.is_macro()
-    }
-
     /// Note: `FxHashSet<TraitId>` should be treated as an opaque type, passed into `Type
     pub fn visible_traits(&self) -> FxHashSet<TraitId> {
         let resolver = &self.resolver;
diff --git a/crates/ide/src/expand_macro.rs b/crates/ide/src/expand_macro.rs
index 7f57d1d6389..949744c01b2 100644
--- a/crates/ide/src/expand_macro.rs
+++ b/crates/ide/src/expand_macro.rs
@@ -1,6 +1,6 @@
 use hir::Semantics;
 use ide_db::{
-    helpers::{pick_best_token, render_macro_node::render_with_ws_inserted},
+    helpers::{insert_whitespace_into_node::insert_ws_into, pick_best_token},
     RootDatabase,
 };
 use itertools::Itertools;
@@ -50,7 +50,7 @@ pub(crate) fn expand_macro(db: &RootDatabase, position: FilePosition) -> Option<
             let expansions = sema.expand_derive_macro(&attr)?;
             Some(ExpandedMacro {
                 name: tt,
-                expansion: expansions.into_iter().map(render_with_ws_inserted).join(""),
+                expansion: expansions.into_iter().map(insert_ws_into).join(""),
             })
         } else {
             None
@@ -83,7 +83,7 @@ pub(crate) fn expand_macro(db: &RootDatabase, position: FilePosition) -> Option<
     // FIXME:
     // macro expansion may lose all white space information
     // But we hope someday we can use ra_fmt for that
-    let expansion = render_with_ws_inserted(expanded?).to_string();
+    let expansion = insert_ws_into(expanded?).to_string();
     Some(ExpandedMacro { name: name.unwrap_or_else(|| "???".to_owned()), expansion })
 }
 
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 6b4640ce7d7..a10eca10d11 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,5 @@
 use hir::HasSource;
-use ide_db::traits::resolve_target_trait;
+use ide_db::{helpers::insert_whitespace_into_node::insert_ws_into, traits::resolve_target_trait};
 use syntax::ast::{self, make, AstNode};
 
 use crate::{
@@ -105,7 +105,7 @@ fn add_missing_impl_members_inner(
     let trait_ = resolve_target_trait(&ctx.sema, &impl_def)?;
 
     let missing_items = filter_assoc_items(
-        ctx.db(),
+        &ctx.sema,
         &ide_db::traits::get_missing_assoc_items(&ctx.sema, &impl_def),
         mode,
     );
@@ -117,6 +117,17 @@ fn add_missing_impl_members_inner(
     let target = impl_def.syntax().text_range();
     acc.add(AssistId(assist_id, AssistKind::QuickFix), label, target, |builder| {
         let target_scope = ctx.sema.scope(impl_def.syntax());
+        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 (new_impl_def, first_new_item) = add_trait_assoc_items_to_impl(
             &ctx.sema,
             missing_items,
@@ -124,9 +135,6 @@ fn add_missing_impl_members_inner(
             impl_def.clone(),
             target_scope,
         );
-        // if target_scope.in_macro_file() {
-
-        // }
         match ctx.config.snippet_cap {
             None => builder.replace(target, new_impl_def.to_string()),
             Some(cap) => {
@@ -896,4 +904,42 @@ impl Default for Foo {
 "#,
         )
     }
+
+    #[test]
+    fn test_from_macro() {
+        check_assist(
+            add_missing_default_members,
+            r#"
+macro_rules! foo {
+    () => {
+        trait FooB {
+            fn foo<'lt>(&'lt self) {}
+        }
+    }
+}
+foo!();
+struct Foo(usize);
+
+impl FooB for Foo {
+    $0
+}
+"#,
+            r#"
+macro_rules! foo {
+    () => {
+        trait FooB {
+            fn foo<'lt>(&'lt self) {}
+        }
+    }
+}
+foo!();
+struct Foo(usize);
+
+impl FooB for Foo {
+    $0fn foo< 'lt>(& 'lt self){}
+
+}
+"#,
+        )
+    }
 }
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 3e33c62144e..250f772fe15 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,4 +1,5 @@
 use hir::ModuleDef;
+use ide_db::helpers::insert_whitespace_into_node::insert_ws_into;
 use ide_db::helpers::{
     get_path_at_cursor_in_tt, import_assets::NameToImport, mod_path_to_ast,
     parse_tt_as_comma_sep_paths,
@@ -170,7 +171,7 @@ fn impl_def_from_trait(
 ) -> Option<(ast::Impl, ast::AssocItem)> {
     let trait_ = trait_?;
     let target_scope = sema.scope(annotated_name.syntax());
-    let trait_items = filter_assoc_items(sema.db, &trait_.items(sema.db), DefaultMethods::No);
+    let trait_items = filter_assoc_items(sema, &trait_.items(sema.db), DefaultMethods::No);
     if trait_items.is_empty() {
         return None;
     }
@@ -193,6 +194,17 @@ 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();
     let (impl_def, first_assoc_item) =
         add_trait_assoc_items_to_impl(sema, trait_items, trait_, impl_def, target_scope);
 
diff --git a/crates/ide_assists/src/utils.rs b/crates/ide_assists/src/utils.rs
index 03433fc42af..90ec710c8e9 100644
--- a/crates/ide_assists/src/utils.rs
+++ b/crates/ide_assists/src/utils.rs
@@ -5,7 +5,7 @@ use std::ops;
 use itertools::Itertools;
 
 pub(crate) use gen_trait_fn_body::gen_trait_fn_body;
-use hir::{db::HirDatabase, HasSource, HirDisplay};
+use hir::{db::HirDatabase, HirDisplay, Semantics};
 use ide_db::{
     helpers::FamousDefs, helpers::SnippetCap, path_transform::PathTransform, RootDatabase,
 };
@@ -92,7 +92,7 @@ pub enum DefaultMethods {
 }
 
 pub fn filter_assoc_items(
-    db: &RootDatabase,
+    sema: &Semantics<RootDatabase>,
     items: &[hir::AssocItem],
     default_methods: DefaultMethods,
 ) -> Vec<ast::AssocItem> {
@@ -109,11 +109,11 @@ pub fn filter_assoc_items(
     items
         .iter()
         // Note: This throws away items with no source.
-        .filter_map(|i| {
+        .filter_map(|&i| {
             let item = match i {
-                hir::AssocItem::Function(i) => ast::AssocItem::Fn(i.source(db)?.value),
-                hir::AssocItem::TypeAlias(i) => ast::AssocItem::TypeAlias(i.source(db)?.value),
-                hir::AssocItem::Const(i) => ast::AssocItem::Const(i.source(db)?.value),
+                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),
             };
             Some(item)
         })
@@ -129,7 +129,7 @@ pub fn filter_assoc_items(
 }
 
 pub fn add_trait_assoc_items_to_impl(
-    sema: &hir::Semantics<ide_db::RootDatabase>,
+    sema: &Semantics<RootDatabase>,
     items: Vec<ast::AssocItem>,
     trait_: hir::Trait,
     impl_: ast::Impl,
@@ -140,7 +140,6 @@ pub fn add_trait_assoc_items_to_impl(
     let transform = PathTransform::trait_impl(&target_scope, &source_scope, trait_, impl_.clone());
 
     let items = items.into_iter().map(|assoc_item| {
-        let assoc_item = assoc_item.clone_for_update();
         transform.apply(assoc_item.syntax());
         assoc_item.remove_attrs_and_docs();
         assoc_item
diff --git a/crates/ide_completion/src/completions/qualified_path.rs b/crates/ide_completion/src/completions/qualified_path.rs
index 782615a8848..a0d6d5cdc6b 100644
--- a/crates/ide_completion/src/completions/qualified_path.rs
+++ b/crates/ide_completion/src/completions/qualified_path.rs
@@ -152,7 +152,9 @@ pub(crate) fn complete_qualified_path(acc: &mut Completions, ctx: &CompletionCon
             }
         }
         hir::PathResolution::Def(
-            def @ (hir::ModuleDef::Adt(_)
+            def
+            @
+            (hir::ModuleDef::Adt(_)
             | hir::ModuleDef::TypeAlias(_)
             | hir::ModuleDef::BuiltinType(_)),
         ) => {
diff --git a/crates/ide_db/src/helpers.rs b/crates/ide_db/src/helpers.rs
index b8a8723a2e5..8118e4fcbbb 100644
--- a/crates/ide_db/src/helpers.rs
+++ b/crates/ide_db/src/helpers.rs
@@ -4,7 +4,7 @@ pub mod generated_lints;
 pub mod import_assets;
 pub mod insert_use;
 pub mod merge_imports;
-pub mod render_macro_node;
+pub mod insert_whitespace_into_node;
 pub mod node_ext;
 pub mod rust_doc;
 
diff --git a/crates/ide_db/src/helpers/render_macro_node.rs b/crates/ide_db/src/helpers/insert_whitespace_into_node.rs
index 7c45d281535..251a4caa132 100644
--- a/crates/ide_db/src/helpers/render_macro_node.rs
+++ b/crates/ide_db/src/helpers/insert_whitespace_into_node.rs
@@ -1,3 +1,4 @@
+//! Utilities for formatting macro expanded nodes until we get a proper formatter.
 use syntax::{
     ast::make,
     ted::{self, Position},
@@ -9,7 +10,7 @@ use syntax::{
 // FIXME: It would also be cool to share logic here and in the mbe tests,
 // which are pretty unreadable at the moment.
 /// Renders a [`SyntaxNode`] with whitespace inserted between tokens that require them.
-pub fn render_with_ws_inserted(syn: SyntaxNode) -> SyntaxNode {
+pub fn insert_ws_into(syn: SyntaxNode) -> SyntaxNode {
     let mut indent = 0;
     let mut last: Option<SyntaxKind> = None;
     let mut mods = Vec::new();
@@ -40,7 +41,9 @@ pub fn render_with_ws_inserted(syn: SyntaxNode) -> SyntaxNode {
                         make::tokens::whitespace(&" ".repeat(2 * indent)),
                     ));
                 }
-                mods.push((Position::after(node), make::tokens::single_newline()));
+                if node.parent().is_some() {
+                    mods.push((Position::after(node), make::tokens::single_newline()));
+                }
                 continue;
             }
             _ => continue,
@@ -82,7 +85,7 @@ pub fn render_with_ws_inserted(syn: SyntaxNode) -> SyntaxNode {
                 }
                 mods.push(do_nl(after, tok));
             }
-            LIFETIME_IDENT if is_next(|it| it == IDENT || it == MUT_KW, true) => {
+            LIFETIME_IDENT if is_next(|it| is_text(it), true) => {
                 mods.push(do_ws(after, tok));
             }
             AS_KW => {