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

This commit is contained in:
Lukas Wirth 2021-12-13 16:25:54 +01:00
parent 328419534d
commit 749eeef3e7
8 changed files with 84 additions and 26 deletions

View File

@ -1163,10 +1163,6 @@ impl<'a> SemanticsScope<'a> {
Some(Crate { id: self.resolver.krate()? }) 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 /// Note: `FxHashSet<TraitId>` should be treated as an opaque type, passed into `Type
pub fn visible_traits(&self) -> FxHashSet<TraitId> { pub fn visible_traits(&self) -> FxHashSet<TraitId> {
let resolver = &self.resolver; let resolver = &self.resolver;

View File

@ -1,6 +1,6 @@
use hir::Semantics; use hir::Semantics;
use ide_db::{ 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, RootDatabase,
}; };
use itertools::Itertools; use itertools::Itertools;
@ -50,7 +50,7 @@ pub(crate) fn expand_macro(db: &RootDatabase, position: FilePosition) -> Option<
let expansions = sema.expand_derive_macro(&attr)?; let expansions = sema.expand_derive_macro(&attr)?;
Some(ExpandedMacro { Some(ExpandedMacro {
name: tt, name: tt,
expansion: expansions.into_iter().map(render_with_ws_inserted).join(""), expansion: expansions.into_iter().map(insert_ws_into).join(""),
}) })
} else { } else {
None None
@ -83,7 +83,7 @@ pub(crate) fn expand_macro(db: &RootDatabase, position: FilePosition) -> Option<
// FIXME: // FIXME:
// macro expansion may lose all white space information // macro expansion may lose all white space information
// But we hope someday we can use ra_fmt for that // 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 }) Some(ExpandedMacro { name: name.unwrap_or_else(|| "???".to_owned()), expansion })
} }

View File

@ -1,5 +1,5 @@
use hir::HasSource; 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 syntax::ast::{self, make, AstNode};
use crate::{ use crate::{
@ -105,7 +105,7 @@ fn add_missing_impl_members_inner(
let trait_ = resolve_target_trait(&ctx.sema, &impl_def)?; let trait_ = resolve_target_trait(&ctx.sema, &impl_def)?;
let missing_items = filter_assoc_items( let missing_items = filter_assoc_items(
ctx.db(), &ctx.sema,
&ide_db::traits::get_missing_assoc_items(&ctx.sema, &impl_def), &ide_db::traits::get_missing_assoc_items(&ctx.sema, &impl_def),
mode, mode,
); );
@ -117,6 +117,17 @@ fn add_missing_impl_members_inner(
let target = impl_def.syntax().text_range(); let target = impl_def.syntax().text_range();
acc.add(AssistId(assist_id, AssistKind::QuickFix), label, target, |builder| { acc.add(AssistId(assist_id, AssistKind::QuickFix), label, target, |builder| {
let target_scope = ctx.sema.scope(impl_def.syntax()); 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( let (new_impl_def, first_new_item) = add_trait_assoc_items_to_impl(
&ctx.sema, &ctx.sema,
missing_items, missing_items,
@ -124,9 +135,6 @@ fn add_missing_impl_members_inner(
impl_def.clone(), impl_def.clone(),
target_scope, target_scope,
); );
// if target_scope.in_macro_file() {
// }
match ctx.config.snippet_cap { match ctx.config.snippet_cap {
None => builder.replace(target, new_impl_def.to_string()), None => builder.replace(target, new_impl_def.to_string()),
Some(cap) => { Some(cap) => {
@ -893,6 +901,44 @@ impl Default for Foo {
Self(Default::default()) 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){}
}
"#, "#,
) )
} }

View File

@ -1,4 +1,5 @@
use hir::ModuleDef; use hir::ModuleDef;
use ide_db::helpers::insert_whitespace_into_node::insert_ws_into;
use ide_db::helpers::{ use ide_db::helpers::{
get_path_at_cursor_in_tt, import_assets::NameToImport, mod_path_to_ast, get_path_at_cursor_in_tt, import_assets::NameToImport, mod_path_to_ast,
parse_tt_as_comma_sep_paths, parse_tt_as_comma_sep_paths,
@ -170,7 +171,7 @@ fn impl_def_from_trait(
) -> Option<(ast::Impl, ast::AssocItem)> { ) -> Option<(ast::Impl, ast::AssocItem)> {
let trait_ = trait_?; let trait_ = trait_?;
let target_scope = sema.scope(annotated_name.syntax()); 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() { if trait_items.is_empty() {
return None; return None;
} }
@ -193,6 +194,17 @@ fn impl_def_from_trait(
node 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) = let (impl_def, 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);

View File

@ -5,7 +5,7 @@ use std::ops;
use itertools::Itertools; use itertools::Itertools;
pub(crate) use gen_trait_fn_body::gen_trait_fn_body; 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::{ use ide_db::{
helpers::FamousDefs, helpers::SnippetCap, path_transform::PathTransform, RootDatabase, helpers::FamousDefs, helpers::SnippetCap, path_transform::PathTransform, RootDatabase,
}; };
@ -92,7 +92,7 @@ pub enum DefaultMethods {
} }
pub fn filter_assoc_items( pub fn filter_assoc_items(
db: &RootDatabase, sema: &Semantics<RootDatabase>,
items: &[hir::AssocItem], items: &[hir::AssocItem],
default_methods: DefaultMethods, default_methods: DefaultMethods,
) -> Vec<ast::AssocItem> { ) -> Vec<ast::AssocItem> {
@ -109,11 +109,11 @@ pub fn filter_assoc_items(
items items
.iter() .iter()
// Note: This throws away items with no source. // Note: This throws away items with no source.
.filter_map(|i| { .filter_map(|&i| {
let item = match i { let item = match i {
hir::AssocItem::Function(i) => ast::AssocItem::Fn(i.source(db)?.value), hir::AssocItem::Function(i) => ast::AssocItem::Fn(sema.source(i)?.value),
hir::AssocItem::TypeAlias(i) => ast::AssocItem::TypeAlias(i.source(db)?.value), hir::AssocItem::TypeAlias(i) => ast::AssocItem::TypeAlias(sema.source(i)?.value),
hir::AssocItem::Const(i) => ast::AssocItem::Const(i.source(db)?.value), hir::AssocItem::Const(i) => ast::AssocItem::Const(sema.source(i)?.value),
}; };
Some(item) Some(item)
}) })
@ -129,7 +129,7 @@ pub fn filter_assoc_items(
} }
pub fn add_trait_assoc_items_to_impl( pub fn add_trait_assoc_items_to_impl(
sema: &hir::Semantics<ide_db::RootDatabase>, sema: &Semantics<RootDatabase>,
items: Vec<ast::AssocItem>, items: Vec<ast::AssocItem>,
trait_: hir::Trait, trait_: hir::Trait,
impl_: ast::Impl, 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 transform = PathTransform::trait_impl(&target_scope, &source_scope, trait_, impl_.clone());
let items = items.into_iter().map(|assoc_item| { let items = items.into_iter().map(|assoc_item| {
let assoc_item = assoc_item.clone_for_update();
transform.apply(assoc_item.syntax()); transform.apply(assoc_item.syntax());
assoc_item.remove_attrs_and_docs(); assoc_item.remove_attrs_and_docs();
assoc_item assoc_item

View File

@ -152,7 +152,9 @@ pub(crate) fn complete_qualified_path(acc: &mut Completions, ctx: &CompletionCon
} }
} }
hir::PathResolution::Def( hir::PathResolution::Def(
def @ (hir::ModuleDef::Adt(_) def
@
(hir::ModuleDef::Adt(_)
| hir::ModuleDef::TypeAlias(_) | hir::ModuleDef::TypeAlias(_)
| hir::ModuleDef::BuiltinType(_)), | hir::ModuleDef::BuiltinType(_)),
) => { ) => {

View File

@ -4,7 +4,7 @@ pub mod generated_lints;
pub mod import_assets; pub mod import_assets;
pub mod insert_use; pub mod insert_use;
pub mod merge_imports; pub mod merge_imports;
pub mod render_macro_node; pub mod insert_whitespace_into_node;
pub mod node_ext; pub mod node_ext;
pub mod rust_doc; pub mod rust_doc;

View File

@ -1,3 +1,4 @@
//! Utilities for formatting macro expanded nodes until we get a proper formatter.
use syntax::{ use syntax::{
ast::make, ast::make,
ted::{self, Position}, ted::{self, Position},
@ -9,7 +10,7 @@ use syntax::{
// FIXME: It would also be cool to share logic here and in the mbe tests, // FIXME: It would also be cool to share logic here and in the mbe tests,
// which are pretty unreadable at the moment. // which are pretty unreadable at the moment.
/// Renders a [`SyntaxNode`] with whitespace inserted between tokens that require them. /// 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 indent = 0;
let mut last: Option<SyntaxKind> = None; let mut last: Option<SyntaxKind> = None;
let mut mods = Vec::new(); let mut mods = Vec::new();
@ -40,7 +41,9 @@ pub fn render_with_ws_inserted(syn: SyntaxNode) -> SyntaxNode {
make::tokens::whitespace(&" ".repeat(2 * indent)), 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;
} }
_ => continue, _ => continue,
@ -82,7 +85,7 @@ pub fn render_with_ws_inserted(syn: SyntaxNode) -> SyntaxNode {
} }
mods.push(do_nl(after, tok)); 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)); mods.push(do_ws(after, tok));
} }
AS_KW => { AS_KW => {