From 749eeef3e75a3acc993fdd454ebadaa7e319509a Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Mon, 13 Dec 2021 16:25:54 +0100 Subject: [PATCH] fix: insert whitespaces into assoc items for assist when macro generated --- crates/hir/src/semantics.rs | 4 -- crates/ide/src/expand_macro.rs | 6 +- .../src/handlers/add_missing_impl_members.rs | 56 +++++++++++++++++-- .../replace_derive_with_manual_impl.rs | 14 ++++- crates/ide_assists/src/utils.rs | 15 +++-- .../src/completions/qualified_path.rs | 4 +- crates/ide_db/src/helpers.rs | 2 +- ...node.rs => insert_whitespace_into_node.rs} | 9 ++- 8 files changed, 84 insertions(+), 26 deletions(-) rename crates/ide_db/src/helpers/{render_macro_node.rs => insert_whitespace_into_node.rs} (91%) 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 @@ pub fn krate(&self) -> Option { Some(Crate { id: self.resolver.krate()? }) } - pub fn in_macro_file(&self) -> bool { - self.file_id.is_macro() - } - /// Note: `FxHashSet` should be treated as an opaque type, passed into `Type pub fn visible_traits(&self) -> FxHashSet { 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) => { @@ -893,6 +901,44 @@ impl Default for Foo { Self(Default::default()) } } +"#, + ) + } + + #[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 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, items: &[hir::AssocItem], default_methods: DefaultMethods, ) -> Vec { @@ -109,11 +109,11 @@ fn has_def_name(item: &ast::AssocItem) -> bool { 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 @@ fn has_def_name(item: &ast::AssocItem) -> bool { } pub fn add_trait_assoc_items_to_impl( - sema: &hir::Semantics, + sema: &Semantics, items: Vec, 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 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 similarity index 91% rename from crates/ide_db/src/helpers/render_macro_node.rs rename to 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 @@ // 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 = 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 => { -- 2.44.0