From 4f93fa12134e47b12a27bd456e59247e8e9d51df Mon Sep 17 00:00:00 2001 From: TheDoctor314 <64731940+TheDoctor314@users.noreply.github.com> Date: Mon, 1 Nov 2021 22:17:27 +0530 Subject: [PATCH 1/5] Add failing tests for generic struct --- .../replace_derive_with_manual_impl.rs | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) 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 042f31e5c4d..a86fe3fa169 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 @@ -1048,6 +1048,54 @@ impl Debug for Foo { ) } + #[test] + fn add_custom_impl_default_generic_record_struct() { + check_assist( + replace_derive_with_manual_impl, + r#" +//- minicore: default +#[derive(Defau$0lt)] +struct Foo { + foo: T, + bar: U, +} +"#, + r#" +struct Foo { + foo: T, + bar: U, +} + +impl Default for Foo { + $0fn default() -> Self { + Self { foo: Default::default(), bar: Default::default() } + } +} +"#, + ) + } + + #[test] + fn add_custom_impl_clone_generic_tuple_struct() { + check_assist( + replace_derive_with_manual_impl, + r#" +//- minicore: clone +#[derive(Clo$0ne)] +struct Foo(T, usize); +"#, + r#" +struct Foo(T, usize); + +impl Clone for Foo { + $0fn clone(&self) -> Self { + Self(self.0.clone(), self.1.clone()) + } +} +"#, + ) + } + #[test] fn test_ignore_derive_macro_without_input() { check_assist_not_applicable( From 05b368f065cefeff31cbc04283453d6a4027532c Mon Sep 17 00:00:00 2001 From: TheDoctor314 <64731940+TheDoctor314@users.noreply.github.com> Date: Mon, 8 Nov 2021 21:19:22 +0530 Subject: [PATCH 2/5] Add generic parameters for manual impl assist The `impl_trait` function takes an optional `GenericParamList` to create the trait impl. --- .../src/handlers/replace_derive_with_manual_impl.rs | 9 ++++++--- crates/syntax/src/ast/make.rs | 9 +++++++-- 2 files changed, 13 insertions(+), 5 deletions(-) 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 a86fe3fa169..7aeaedf5f05 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 @@ -3,7 +3,7 @@ use ide_db::helpers::{import_assets::NameToImport, mod_path_to_ast}; use ide_db::items_locator; use itertools::Itertools; use syntax::{ - ast::{self, make, AstNode, HasName}, + ast::{self, make, AstNode, HasGenericParams, HasName}, SyntaxKind::{IDENT, WHITESPACE}, }; @@ -160,8 +160,11 @@ fn impl_def_from_trait( if trait_items.is_empty() { return None; } - let impl_def = - make::impl_trait(trait_path.clone(), make::ext::ident_path(&annotated_name.text())); + let impl_def = make::impl_trait( + trait_path.clone(), + make::ext::ident_path(&annotated_name.text()), + adt.generic_param_list(), + ); let (impl_def, first_assoc_item) = add_trait_assoc_items_to_impl(sema, trait_items, trait_, impl_def, target_scope); diff --git a/crates/syntax/src/ast/make.rs b/crates/syntax/src/ast/make.rs index ed69973af92..e1938307cf3 100644 --- a/crates/syntax/src/ast/make.rs +++ b/crates/syntax/src/ast/make.rs @@ -149,8 +149,13 @@ pub fn impl_( ast_from_text(&format!("impl{} {}{} {{}}", params, ty, ty_params)) } -pub fn impl_trait(trait_: ast::Path, ty: ast::Path) -> ast::Impl { - ast_from_text(&format!("impl {} for {} {{}}", trait_, ty)) +pub fn impl_trait( + trait_: ast::Path, + ty: ast::Path, + ty_params: Option, +) -> ast::Impl { + let ty_params = ty_params.map_or_else(String::new, |params| params.to_string()); + ast_from_text(&format!("impl{2} {} for {}{2} {{}}", trait_, ty, ty_params)) } pub(crate) fn generic_arg_list() -> ast::GenericArgList { From e0e8d877c0f4908e877b8ad7ea82acc8cf90be9b Mon Sep 17 00:00:00 2001 From: TheDoctor314 <64731940+TheDoctor314@users.noreply.github.com> Date: Thu, 11 Nov 2021 14:16:29 +0530 Subject: [PATCH 3/5] Add generic bounds in test --- .../src/handlers/replace_derive_with_manual_impl.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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 7aeaedf5f05..1f28ecc755f 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 @@ -1079,18 +1079,18 @@ impl Default for Foo { } #[test] - fn add_custom_impl_clone_generic_tuple_struct() { + fn add_custom_impl_clone_generic_tuple_struct_with_bounds() { check_assist( replace_derive_with_manual_impl, r#" //- minicore: clone #[derive(Clo$0ne)] -struct Foo(T, usize); +struct Foo(T, usize); "#, r#" -struct Foo(T, usize); +struct Foo(T, usize); -impl Clone for Foo { +impl Clone for Foo { $0fn clone(&self) -> Self { Self(self.0.clone(), self.1.clone()) } From 55a4813151a8f36dcdb520c45a461fe5dfbed499 Mon Sep 17 00:00:00 2001 From: TheDoctor314 <64731940+TheDoctor314@users.noreply.github.com> Date: Thu, 11 Nov 2021 14:16:59 +0530 Subject: [PATCH 4/5] Fix `impl_trait` function to emit correct ast `impl_trait` code copied from `generate_impl_text_inner` to properly handle the bounds for the generic parameters. --- .../replace_derive_with_manual_impl.rs | 9 +-- crates/syntax/src/ast/make.rs | 72 ++++++++++++++++--- 2 files changed, 67 insertions(+), 14 deletions(-) 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 1f28ecc755f..060bb17c6d2 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 @@ -3,7 +3,7 @@ use ide_db::helpers::{import_assets::NameToImport, mod_path_to_ast}; use ide_db::items_locator; use itertools::Itertools; use syntax::{ - ast::{self, make, AstNode, HasGenericParams, HasName}, + ast::{self, make, AstNode, HasName}, SyntaxKind::{IDENT, WHITESPACE}, }; @@ -160,11 +160,8 @@ fn impl_def_from_trait( if trait_items.is_empty() { return None; } - let impl_def = make::impl_trait( - trait_path.clone(), - make::ext::ident_path(&annotated_name.text()), - adt.generic_param_list(), - ); + let impl_def = make::impl_trait(&trait_path, &adt, ""); + let (impl_def, first_assoc_item) = add_trait_assoc_items_to_impl(sema, trait_items, trait_, impl_def, target_scope); diff --git a/crates/syntax/src/ast/make.rs b/crates/syntax/src/ast/make.rs index e1938307cf3..fec7c5cfe80 100644 --- a/crates/syntax/src/ast/make.rs +++ b/crates/syntax/src/ast/make.rs @@ -10,9 +10,13 @@ //! `parse(format!())` we use internally is an implementation detail -- long //! term, it will be replaced with direct tree manipulation. use itertools::Itertools; +use smol_str::SmolStr; use stdx::{format_to, never}; -use crate::{ast, AstNode, SourceFile, SyntaxKind, SyntaxToken}; +use crate::{ + ast::{self, HasAttrs, HasGenericParams, HasName, HasTypeBounds}, + AstNode, SourceFile, SyntaxKind, SyntaxToken, +}; /// While the parent module defines basic atomic "constructors", the `ext` /// module defines shortcuts for common things. @@ -149,13 +153,65 @@ pub fn impl_( ast_from_text(&format!("impl{} {}{} {{}}", params, ty, ty_params)) } -pub fn impl_trait( - trait_: ast::Path, - ty: ast::Path, - ty_params: Option, -) -> ast::Impl { - let ty_params = ty_params.map_or_else(String::new, |params| params.to_string()); - ast_from_text(&format!("impl{2} {} for {}{2} {{}}", trait_, ty, ty_params)) +pub fn impl_trait(trait_: &ast::Path, adt: &ast::Adt, code: &str) -> ast::Impl { + let generic_params = adt.generic_param_list(); + let mut buf = String::with_capacity(code.len()); + 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.to_string()).as_str())); + buf.push_str("impl"); + if let Some(generic_params) = &generic_params { + let lifetimes = generic_params.lifetime_params().map(|lt| format!("{}", lt.syntax())); + let type_params = generic_params.type_params().map(|type_param| { + 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 const_params = generic_params.const_params().map(|t| t.syntax().to_string()); + let generics = lifetimes.chain(type_params).chain(const_params).format(", "); + format_to!(buf, "<{}>", generics); + } + buf.push(' '); + let trait_text = trait_.to_string(); + buf.push_str(&trait_text); + buf.push_str(" for "); + + 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 type_params = generic_params + .type_params() + .filter_map(|it| it.name()) + .map(|it| SmolStr::from(it.text())); + let const_params = generic_params + .const_params() + .filter_map(|it| it.name()) + .map(|it| SmolStr::from(it.text())); + format_to!(buf, "<{}>", lifetime_params.chain(type_params).chain(const_params).format(", ")) + } + + match adt.where_clause() { + Some(where_clause) => { + format_to!(buf, "\n{}\n{{\n{}\n}}", where_clause, code); + } + None => { + format_to!(buf, " {{\n{}\n}}", code); + } + } + + ast_from_text(&buf) } pub(crate) fn generic_arg_list() -> ast::GenericArgList { From 58a24de7d85e6427885913e06d7e2587cbfb9a4f Mon Sep 17 00:00:00 2001 From: TheDoctor314 <64731940+TheDoctor314@users.noreply.github.com> Date: Mon, 15 Nov 2021 20:51:48 +0530 Subject: [PATCH 5/5] Fix `impl_def_from_trait` Revert "Fix `impl_trait` function to emit correct ast" This reverts commit 55a4813151a8f36dcdb520c45a461fe5dfbed499. Fix `impl_def_from_trait` It now generates the correct `ast::Impl` using `generate_trait_impl_text` and parses it to form the right node (copied from the private fn 'make::ast_from_text'). --- .../replace_derive_with_manual_impl.rs | 19 ++++- crates/syntax/src/ast/make.rs | 72 +++---------------- 2 files changed, 26 insertions(+), 65 deletions(-) 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 060bb17c6d2..5ba045d3c8f 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 @@ -160,7 +160,24 @@ fn impl_def_from_trait( if trait_items.is_empty() { return None; } - let impl_def = make::impl_trait(&trait_path, &adt, ""); + let impl_def = { + use syntax::ast::Impl; + let text = generate_trait_impl_text(adt, trait_path.to_string().as_str(), ""); + let parse = syntax::SourceFile::parse(&text); + let node = match parse.tree().syntax().descendants().find_map(Impl::cast) { + Some(it) => it, + None => { + panic!( + "Failed to make ast node `{}` from text {}", + std::any::type_name::(), + text + ) + } + }; + let node = node.clone_subtree(); + assert_eq!(node.syntax().text_range().start(), 0.into()); + node + }; let (impl_def, first_assoc_item) = add_trait_assoc_items_to_impl(sema, trait_items, trait_, impl_def, target_scope); diff --git a/crates/syntax/src/ast/make.rs b/crates/syntax/src/ast/make.rs index fec7c5cfe80..e1938307cf3 100644 --- a/crates/syntax/src/ast/make.rs +++ b/crates/syntax/src/ast/make.rs @@ -10,13 +10,9 @@ //! `parse(format!())` we use internally is an implementation detail -- long //! term, it will be replaced with direct tree manipulation. use itertools::Itertools; -use smol_str::SmolStr; use stdx::{format_to, never}; -use crate::{ - ast::{self, HasAttrs, HasGenericParams, HasName, HasTypeBounds}, - AstNode, SourceFile, SyntaxKind, SyntaxToken, -}; +use crate::{ast, AstNode, SourceFile, SyntaxKind, SyntaxToken}; /// While the parent module defines basic atomic "constructors", the `ext` /// module defines shortcuts for common things. @@ -153,65 +149,13 @@ pub fn impl_( ast_from_text(&format!("impl{} {}{} {{}}", params, ty, ty_params)) } -pub fn impl_trait(trait_: &ast::Path, adt: &ast::Adt, code: &str) -> ast::Impl { - let generic_params = adt.generic_param_list(); - let mut buf = String::with_capacity(code.len()); - 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.to_string()).as_str())); - buf.push_str("impl"); - if let Some(generic_params) = &generic_params { - let lifetimes = generic_params.lifetime_params().map(|lt| format!("{}", lt.syntax())); - let type_params = generic_params.type_params().map(|type_param| { - 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 const_params = generic_params.const_params().map(|t| t.syntax().to_string()); - let generics = lifetimes.chain(type_params).chain(const_params).format(", "); - format_to!(buf, "<{}>", generics); - } - buf.push(' '); - let trait_text = trait_.to_string(); - buf.push_str(&trait_text); - buf.push_str(" for "); - - 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 type_params = generic_params - .type_params() - .filter_map(|it| it.name()) - .map(|it| SmolStr::from(it.text())); - let const_params = generic_params - .const_params() - .filter_map(|it| it.name()) - .map(|it| SmolStr::from(it.text())); - format_to!(buf, "<{}>", lifetime_params.chain(type_params).chain(const_params).format(", ")) - } - - match adt.where_clause() { - Some(where_clause) => { - format_to!(buf, "\n{}\n{{\n{}\n}}", where_clause, code); - } - None => { - format_to!(buf, " {{\n{}\n}}", code); - } - } - - ast_from_text(&buf) +pub fn impl_trait( + trait_: ast::Path, + ty: ast::Path, + ty_params: Option, +) -> ast::Impl { + let ty_params = ty_params.map_or_else(String::new, |params| params.to_string()); + ast_from_text(&format!("impl{2} {} for {}{2} {{}}", trait_, ty, ty_params)) } pub(crate) fn generic_arg_list() -> ast::GenericArgList {