From 75f641799effae2c0138215eca968b33429d23fe Mon Sep 17 00:00:00 2001 From: DropDemBits Date: Sun, 9 Oct 2022 17:58:55 -0400 Subject: [PATCH 1/3] Add `GenericParamList::to_generic_args` --- .../extract_struct_from_enum_variant.rs | 34 ++----------------- crates/syntax/src/ast/edit_in_place.rs | 17 ++++++++++ 2 files changed, 19 insertions(+), 32 deletions(-) diff --git a/crates/ide-assists/src/handlers/extract_struct_from_enum_variant.rs b/crates/ide-assists/src/handlers/extract_struct_from_enum_variant.rs index 8d5cab283d0..970e948dfd9 100644 --- a/crates/ide-assists/src/handlers/extract_struct_from_enum_variant.rs +++ b/crates/ide-assists/src/handlers/extract_struct_from_enum_variant.rs @@ -9,7 +9,7 @@ use ide_db::{ search::FileReference, FxHashSet, RootDatabase, }; -use itertools::{Itertools, Position}; +use itertools::Itertools; use syntax::{ ast::{ self, edit::IndentLevel, edit_in_place::Indent, make, AstNode, HasAttrs, HasGenericParams, @@ -298,37 +298,7 @@ fn update_variant(variant: &ast::Variant, generics: Option 0) - .map(|generics| { - let mut generic_str = String::with_capacity(8); - - for (p, more) in generics.generic_params().with_position().map(|p| match p { - Position::First(p) | Position::Middle(p) => (p, true), - Position::Last(p) | Position::Only(p) => (p, false), - }) { - match p { - ast::GenericParam::ConstParam(konst) => { - if let Some(name) = konst.name() { - generic_str.push_str(name.text().as_str()); - } - } - ast::GenericParam::LifetimeParam(lt) => { - if let Some(lt) = lt.lifetime() { - generic_str.push_str(lt.text().as_str()); - } - } - ast::GenericParam::TypeParam(ty) => { - if let Some(name) = ty.name() { - generic_str.push_str(name.text().as_str()); - } - } - } - if more { - generic_str.push_str(", "); - } - } - - make::ty(&format!("{}<{}>", &name.text(), &generic_str)) - }) + .map(|generics| make::ty(&format!("{}{}", &name.text(), generics.to_generic_args()))) .unwrap_or_else(|| make::ty(&name.text())); // change from a record to a tuple field list diff --git a/crates/syntax/src/ast/edit_in_place.rs b/crates/syntax/src/ast/edit_in_place.rs index eadebbe8a21..7d632352828 100644 --- a/crates/syntax/src/ast/edit_in_place.rs +++ b/crates/syntax/src/ast/edit_in_place.rs @@ -235,6 +235,23 @@ impl ast::GenericParamList { } } } + + /// Extracts the const, type, and lifetime names into a new [`ast::GenericParamList`] + pub fn to_generic_args(&self) -> ast::GenericParamList { + let params = self.generic_params().filter_map(|param| match param { + ast::GenericParam::ConstParam(it) => { + Some(ast::GenericParam::TypeParam(make::type_param(it.name()?, None))) + } + ast::GenericParam::LifetimeParam(it) => { + Some(ast::GenericParam::LifetimeParam(make::lifetime_param(it.lifetime()?))) + } + ast::GenericParam::TypeParam(it) => { + Some(ast::GenericParam::TypeParam(make::type_param(it.name()?, None))) + } + }); + + make::generic_param_list(params) + } } impl ast::WhereClause { From bfe6ec9b778d12287fce1fdf2df7e7b0d58ee775 Mon Sep 17 00:00:00 2001 From: DropDemBits Date: Sun, 9 Oct 2022 18:12:08 -0400 Subject: [PATCH 2/3] Add `{TypeParam, ConstParam}::remove_default` Also includes a drive-by refactor of `utils::generate_impl_text_inner`, since that's what drove this change --- .../ide-assists/src/handlers/generate_impl.rs | 13 ++++ crates/ide-assists/src/utils.rs | 76 +++++++++---------- crates/syntax/src/ast/edit_in_place.rs | 36 +++++++++ 3 files changed, 87 insertions(+), 38 deletions(-) diff --git a/crates/ide-assists/src/handlers/generate_impl.rs b/crates/ide-assists/src/handlers/generate_impl.rs index 68287a20bf8..307cea3d0a4 100644 --- a/crates/ide-assists/src/handlers/generate_impl.rs +++ b/crates/ide-assists/src/handlers/generate_impl.rs @@ -52,6 +52,7 @@ mod tests { use super::*; + // FIXME: break up into separate test fns #[test] fn test_add_impl() { check_assist( @@ -134,6 +135,18 @@ mod tests { }"#, ); + check_assist( + generate_impl, + r#" + struct Defaulted {}$0"#, + r#" + struct Defaulted {} + + impl Defaulted { + $0 + }"#, + ); + check_assist( generate_impl, r#"pub trait Trait {} diff --git a/crates/ide-assists/src/utils.rs b/crates/ide-assists/src/utils.rs index 4ab6e2627fa..38396cd7d7b 100644 --- a/crates/ide-assists/src/utils.rs +++ b/crates/ide-assists/src/utils.rs @@ -2,8 +2,6 @@ use std::ops; -use itertools::Itertools; - 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}; @@ -15,7 +13,7 @@ use syntax::{ edit_in_place::{AttrsOwnerEdit, Removable}, make, HasArgList, HasAttrs, HasGenericParams, HasName, HasTypeBounds, Whitespace, }, - ted, AstNode, AstToken, Direction, SmolStr, SourceFile, + ted, AstNode, AstToken, Direction, SourceFile, SyntaxKind::*, SyntaxNode, TextRange, TextSize, T, }; @@ -424,34 +422,44 @@ pub(crate) fn generate_trait_impl_text(adt: &ast::Adt, trait_text: &str, code: & } fn generate_impl_text_inner(adt: &ast::Adt, trait_text: Option<&str>, code: &str) -> String { - let generic_params = adt.generic_param_list(); + // Ensure lifetime params are before type & const params + let generic_params = adt.generic_param_list().map(|generic_params| { + let lifetime_params = + generic_params.lifetime_params().map(ast::GenericParam::LifetimeParam); + let ty_or_const_params = generic_params.type_or_const_params().filter_map(|param| { + // remove defaults since they can't be specified in impls + match param { + ast::TypeOrConstParam::Type(param) => { + let param = param.clone_for_update(); + param.remove_default(); + Some(ast::GenericParam::TypeParam(param)) + } + ast::TypeOrConstParam::Const(param) => { + let param = param.clone_for_update(); + param.remove_default(); + Some(ast::GenericParam::ConstParam(param)) + } + } + }); + + make::generic_param_list(itertools::chain(lifetime_params, ty_or_const_params)) + }); + + // FIXME: use syntax::make & mutable AST apis instead + // `trait_text` and `code` can't be opaque blobs of text let mut buf = String::with_capacity(code.len()); + + // Copy any cfg attrs from the original adt buf.push_str("\n\n"); - adt.attrs() - .filter(|attr| attr.as_simple_call().map(|(name, _arg)| name == "cfg").unwrap_or(false)) - .for_each(|attr| buf.push_str(format!("{}\n", attr).as_str())); + let cfg_attrs = adt + .attrs() + .filter(|attr| attr.as_simple_call().map(|(name, _arg)| name == "cfg").unwrap_or(false)); + cfg_attrs.for_each(|attr| buf.push_str(&format!("{attr}\n"))); + + // `impl{generic_params} {trait_text} for {name}{generic_params.to_generic_args()}` buf.push_str("impl"); if let Some(generic_params) = &generic_params { - let lifetimes = generic_params.lifetime_params().map(|lt| format!("{}", lt.syntax())); - let toc_params = generic_params.type_or_const_params().map(|toc_param| { - let type_param = match toc_param { - ast::TypeOrConstParam::Type(x) => x, - ast::TypeOrConstParam::Const(x) => return x.syntax().to_string(), - }; - let mut buf = String::new(); - if let Some(it) = type_param.name() { - format_to!(buf, "{}", it.syntax()); - } - if let Some(it) = type_param.colon_token() { - format_to!(buf, "{} ", it); - } - if let Some(it) = type_param.type_bound_list() { - format_to!(buf, "{}", it.syntax()); - } - buf - }); - let generics = lifetimes.chain(toc_params).format(", "); - format_to!(buf, "<{}>", generics); + format_to!(buf, "{generic_params}"); } buf.push(' '); if let Some(trait_text) = trait_text { @@ -460,23 +468,15 @@ fn generate_impl_text_inner(adt: &ast::Adt, trait_text: Option<&str>, code: &str } buf.push_str(&adt.name().unwrap().text()); if let Some(generic_params) = generic_params { - let lifetime_params = generic_params - .lifetime_params() - .filter_map(|it| it.lifetime()) - .map(|it| SmolStr::from(it.text())); - let toc_params = generic_params - .type_or_const_params() - .filter_map(|it| it.name()) - .map(|it| SmolStr::from(it.text())); - format_to!(buf, "<{}>", lifetime_params.chain(toc_params).format(", ")) + format_to!(buf, "{}", generic_params.to_generic_args()); } match adt.where_clause() { Some(where_clause) => { - format_to!(buf, "\n{}\n{{\n{}\n}}", where_clause, code); + format_to!(buf, "\n{where_clause}\n{{\n{code}\n}}"); } None => { - format_to!(buf, " {{\n{}\n}}", code); + format_to!(buf, " {{\n{code}\n}}"); } } diff --git a/crates/syntax/src/ast/edit_in_place.rs b/crates/syntax/src/ast/edit_in_place.rs index 7d632352828..173c064ccc9 100644 --- a/crates/syntax/src/ast/edit_in_place.rs +++ b/crates/syntax/src/ast/edit_in_place.rs @@ -265,6 +265,42 @@ impl ast::WhereClause { } } +impl ast::TypeParam { + pub fn remove_default(&self) { + if let Some((eq, last)) = self + .syntax() + .children_with_tokens() + .find(|it| it.kind() == T![=]) + .zip(self.syntax().last_child_or_token()) + { + ted::remove_all(eq..=last); + + // remove any trailing ws + if let Some(last) = self.syntax().last_token().filter(|it| it.kind() == WHITESPACE) { + last.detach(); + } + } + } +} + +impl ast::ConstParam { + pub fn remove_default(&self) { + if let Some((eq, last)) = self + .syntax() + .children_with_tokens() + .find(|it| it.kind() == T![=]) + .zip(self.syntax().last_child_or_token()) + { + ted::remove_all(eq..=last); + + // remove any trailing ws + if let Some(last) = self.syntax().last_token().filter(|it| it.kind() == WHITESPACE) { + last.detach(); + } + } + } +} + pub trait Removable: AstNode { fn remove(&self); } From 1015a177d4a9f1c8ed853dee68dc2bf13e0cb02d Mon Sep 17 00:00:00 2001 From: DropDemBits Date: Sun, 9 Oct 2022 20:45:20 -0400 Subject: [PATCH 3/3] Have `to_generic_args` return `ast::GenericArgList` --- crates/syntax/src/ast/edit_in_place.rs | 21 +++++++++++---------- crates/syntax/src/ast/make.rs | 23 +++++++++++++++++++---- 2 files changed, 30 insertions(+), 14 deletions(-) diff --git a/crates/syntax/src/ast/edit_in_place.rs b/crates/syntax/src/ast/edit_in_place.rs index 173c064ccc9..229e7419b73 100644 --- a/crates/syntax/src/ast/edit_in_place.rs +++ b/crates/syntax/src/ast/edit_in_place.rs @@ -236,21 +236,22 @@ impl ast::GenericParamList { } } - /// Extracts the const, type, and lifetime names into a new [`ast::GenericParamList`] - pub fn to_generic_args(&self) -> ast::GenericParamList { - let params = self.generic_params().filter_map(|param| match param { - ast::GenericParam::ConstParam(it) => { - Some(ast::GenericParam::TypeParam(make::type_param(it.name()?, None))) - } + /// Constructs a matching [`ast::GenericArgList`] + pub fn to_generic_args(&self) -> ast::GenericArgList { + let args = self.generic_params().filter_map(|param| match param { ast::GenericParam::LifetimeParam(it) => { - Some(ast::GenericParam::LifetimeParam(make::lifetime_param(it.lifetime()?))) + Some(ast::GenericArg::LifetimeArg(make::lifetime_arg(it.lifetime()?))) } ast::GenericParam::TypeParam(it) => { - Some(ast::GenericParam::TypeParam(make::type_param(it.name()?, None))) + Some(ast::GenericArg::TypeArg(make::type_arg(make::ext::ty_name(it.name()?)))) + } + ast::GenericParam::ConstParam(it) => { + // Name-only const params get parsed as `TypeArg`s + Some(ast::GenericArg::TypeArg(make::type_arg(make::ext::ty_name(it.name()?)))) } }); - make::generic_param_list(params) + make::generic_arg_list(args) } } @@ -317,7 +318,7 @@ impl Removable for ast::TypeBoundList { impl ast::PathSegment { pub fn get_or_create_generic_arg_list(&self) -> ast::GenericArgList { if self.generic_arg_list().is_none() { - let arg_list = make::generic_arg_list().clone_for_update(); + let arg_list = make::generic_arg_list(empty()).clone_for_update(); ted::append_child(self.syntax(), arg_list.syntax()); } self.generic_arg_list().unwrap() diff --git a/crates/syntax/src/ast/make.rs b/crates/syntax/src/ast/make.rs index 83f8bbac588..c9a21e12c08 100644 --- a/crates/syntax/src/ast/make.rs +++ b/crates/syntax/src/ast/make.rs @@ -88,6 +88,9 @@ pub mod ext { block_expr(None, None) } + pub fn ty_name(name: ast::Name) -> ast::Type { + ty_path(ident_path(&format!("{name}"))) + } pub fn ty_bool() -> ast::Type { ty_path(ident_path("bool")) } @@ -160,6 +163,7 @@ pub fn assoc_item_list() -> ast::AssocItemList { ast_from_text("impl C for D {}") } +// FIXME: `ty_params` should be `ast::GenericArgList` pub fn impl_( ty: ast::Path, params: Option, @@ -185,10 +189,6 @@ pub fn impl_trait( ast_from_text(&format!("impl{ty_params} {trait_} for {ty}{ty_params} {{}}")) } -pub(crate) fn generic_arg_list() -> ast::GenericArgList { - ast_from_text("const S: T<> = ();") -} - pub fn path_segment(name_ref: ast::NameRef) -> ast::PathSegment { ast_from_text(&format!("type __ = {name_ref};")) } @@ -718,6 +718,21 @@ pub fn generic_param_list( ast_from_text(&format!("fn f<{args}>() {{ }}")) } +pub fn type_arg(ty: ast::Type) -> ast::TypeArg { + ast_from_text(&format!("const S: T<{ty}> = ();")) +} + +pub fn lifetime_arg(lifetime: ast::Lifetime) -> ast::LifetimeArg { + ast_from_text(&format!("const S: T<{lifetime}> = ();")) +} + +pub(crate) fn generic_arg_list( + args: impl IntoIterator, +) -> ast::GenericArgList { + let args = args.into_iter().join(", "); + ast_from_text(&format!("const S: T<{args}> = ();")) +} + pub fn visibility_pub_crate() -> ast::Visibility { ast_from_text("pub(crate) struct S") }