9814: Generate default impl when converting `#[derive(Debug)]` to manual impl r=yoshuawuyts a=yoshuawuyts

This patch makes it so when you convert `#[derive(Debug)]` to a manual impl, a default body is provided that's equivalent to the original output of `#[derive(Debug)]`. This should make it drastically easier to write custom `Debug` impls, especially when all you want to do is quickly omit a single field which is `!Debug`.

This is implemented for enums, record structs, tuple structs, empty structs - and it sets us up to implement variations on this in the future for other traits (like `PartialEq` and `Hash`).

Thanks!

## Codegen diff
This is the difference in codegen for record structs with this patch:
```diff
struct Foo {
    bar: String,
}

impl fmt::Debug for Foo {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        todo!();
+        f.debug_struct("Foo").field("bar", &self.bar).finish()
    }
}
```

Co-authored-by: Irina Shestak <shestak.irina@gmail.com>
Co-authored-by: Yoshua Wuyts <yoshuawuyts@gmail.com>
Co-authored-by: Yoshua Wuyts <yoshuawuyts+github@gmail.com>
This commit is contained in:
bors[bot] 2021-08-08 22:30:37 +00:00 committed by GitHub
commit 5664a2b0b3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 210 additions and 29 deletions

View File

@ -2732,8 +2732,8 @@ fn foo() {
file_id: FileId( file_id: FileId(
1, 1,
), ),
full_range: 251..433, full_range: 252..434,
focus_range: 290..296, focus_range: 291..297,
name: "Future", name: "Future",
kind: Trait, kind: Trait,
description: "pub trait Future", description: "pub trait Future",

View File

@ -2,6 +2,8 @@ use hir::ModuleDef;
use ide_db::helpers::{import_assets::NameToImport, mod_path_to_ast}; use ide_db::helpers::{import_assets::NameToImport, mod_path_to_ast};
use ide_db::items_locator; use ide_db::items_locator;
use itertools::Itertools; use itertools::Itertools;
use syntax::ast::edit::AstNodeEdit;
use syntax::ted;
use syntax::{ use syntax::{
ast::{self, make, AstNode, NameOwner}, ast::{self, make, AstNode, NameOwner},
SyntaxKind::{IDENT, WHITESPACE}, SyntaxKind::{IDENT, WHITESPACE},
@ -32,8 +34,8 @@ use crate::{
// struct S; // struct S;
// //
// impl Debug for S { // impl Debug for S {
// fn fmt(&self, f: &mut Formatter) -> Result<()> { // $0fn fmt(&self, f: &mut Formatter) -> Result<()> {
// ${0:todo!()} // f.debug_struct("S").finish()
// } // }
// } // }
// ``` // ```
@ -111,7 +113,7 @@ fn add_assist(
|builder| { |builder| {
let insert_pos = adt.syntax().text_range().end(); let insert_pos = adt.syntax().text_range().end();
let impl_def_with_items = let impl_def_with_items =
impl_def_from_trait(&ctx.sema, &annotated_name, trait_, trait_path); impl_def_from_trait(&ctx.sema, adt, &annotated_name, trait_, trait_path);
update_attribute(builder, input, &trait_name, attr); update_attribute(builder, input, &trait_name, attr);
let trait_path = format!("{}", trait_path); let trait_path = format!("{}", trait_path);
match (ctx.config.snippet_cap, impl_def_with_items) { match (ctx.config.snippet_cap, impl_def_with_items) {
@ -149,6 +151,7 @@ fn add_assist(
fn impl_def_from_trait( fn impl_def_from_trait(
sema: &hir::Semantics<ide_db::RootDatabase>, sema: &hir::Semantics<ide_db::RootDatabase>,
adt: &ast::Adt,
annotated_name: &ast::Name, annotated_name: &ast::Name,
trait_: Option<hir::Trait>, trait_: Option<hir::Trait>,
trait_path: &ast::Path, trait_path: &ast::Path,
@ -163,9 +166,116 @@ fn impl_def_from_trait(
make::impl_trait(trait_path.clone(), make::ext::ident_path(&annotated_name.text())); make::impl_trait(trait_path.clone(), make::ext::ident_path(&annotated_name.text()));
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);
// Generate a default `impl` function body for the derived trait.
if let ast::AssocItem::Fn(ref func) = first_assoc_item {
let _ = gen_default_impl(func, trait_path, adt, annotated_name);
};
Some((impl_def, first_assoc_item)) Some((impl_def, first_assoc_item))
} }
/// Generate custom trait bodies where possible.
///
/// Returns `Option` so that we can use `?` rather than `if let Some`. Returning
/// `None` means that generating a custom trait body failed, and the body will remain
/// as `todo!` instead.
fn gen_default_impl(
func: &ast::Fn,
trait_path: &ast::Path,
adt: &ast::Adt,
annotated_name: &ast::Name,
) -> Option<()> {
match trait_path.segment()?.name_ref()?.text().as_str() {
"Debug" => gen_debug_impl(adt, func, annotated_name),
_ => Some(()),
}
}
/// Generate a `Debug` impl based on the fields and members of the target type.
fn gen_debug_impl(adt: &ast::Adt, func: &ast::Fn, annotated_name: &ast::Name) -> Option<()> {
match adt {
// `Debug` cannot be derived for unions, so no default impl can be provided.
ast::Adt::Union(_) => Some(()),
// => match self { Self::Variant => write!(f, "Variant") }
ast::Adt::Enum(enum_) => {
let list = enum_.variant_list()?;
let mut arms = vec![];
for variant in list.variants() {
let name = variant.name()?;
let left = make::ext::ident_path("Self");
let right = make::ext::ident_path(&format!("{}", name));
let variant_name = make::path_pat(make::path_concat(left, right));
let target = make::expr_path(make::ext::ident_path("f").into());
let fmt_string = make::expr_literal(&(format!("\"{}\"", name))).into();
let args = make::arg_list(vec![target, fmt_string]);
let macro_name = make::expr_path(make::ext::ident_path("write"));
let macro_call = make::expr_macro_call(macro_name, args);
arms.push(make::match_arm(Some(variant_name.into()), None, macro_call.into()));
}
let match_target = make::expr_path(make::ext::ident_path("self"));
let list = make::match_arm_list(arms).indent(ast::edit::IndentLevel(1));
let match_expr = make::expr_match(match_target, list);
let body = make::block_expr(None, Some(match_expr));
let body = body.indent(ast::edit::IndentLevel(1));
ted::replace(func.body()?.syntax(), body.clone_for_update().syntax());
Some(())
}
ast::Adt::Struct(strukt) => {
let name = format!("\"{}\"", annotated_name);
let args = make::arg_list(Some(make::expr_literal(&name).into()));
let target = make::expr_path(make::ext::ident_path("f"));
let expr = match strukt.field_list() {
// => f.debug_struct("Name").finish()
None => make::expr_method_call(target, make::name_ref("debug_struct"), args),
// => f.debug_struct("Name").field("foo", &self.foo).finish()
Some(ast::FieldList::RecordFieldList(field_list)) => {
let method = make::name_ref("debug_struct");
let mut expr = make::expr_method_call(target, method, args);
for field in field_list.fields() {
let name = field.name()?;
let f_name = make::expr_literal(&(format!("\"{}\"", name))).into();
let f_path = make::expr_path(make::ext::ident_path("self"));
let f_path = make::expr_ref(f_path, false);
let f_path = make::expr_field(f_path, &format!("{}", name)).into();
let args = make::arg_list(vec![f_name, f_path]);
expr = make::expr_method_call(expr, make::name_ref("field"), args);
}
expr
}
// => f.debug_tuple("Name").field(self.0).finish()
Some(ast::FieldList::TupleFieldList(field_list)) => {
let method = make::name_ref("debug_tuple");
let mut expr = make::expr_method_call(target, method, args);
for (idx, _) in field_list.fields().enumerate() {
let f_path = make::expr_path(make::ext::ident_path("self"));
let f_path = make::expr_ref(f_path, false);
let f_path = make::expr_field(f_path, &format!("{}", idx)).into();
let method = make::name_ref("field");
expr = make::expr_method_call(expr, method, make::arg_list(Some(f_path)));
}
expr
}
};
let method = make::name_ref("finish");
let expr = make::expr_method_call(expr, method, make::arg_list(None));
let body = make::block_expr(None, Some(expr)).indent(ast::edit::IndentLevel(1));
ted::replace(func.body()?.syntax(), body.clone_for_update().syntax());
Some(())
}
}
}
fn update_attribute( fn update_attribute(
builder: &mut AssistBuilder, builder: &mut AssistBuilder,
input: &ast::TokenTree, input: &ast::TokenTree,
@ -207,41 +317,92 @@ mod tests {
use super::*; use super::*;
#[test] #[test]
fn add_custom_impl_debug() { fn add_custom_impl_debug_record_struct() {
check_assist( check_assist(
replace_derive_with_manual_impl, replace_derive_with_manual_impl,
r#" r#"
mod fmt { //- minicore: fmt
pub struct Error;
pub type Result = Result<(), Error>;
pub struct Formatter<'a>;
pub trait Debug {
fn fmt(&self, f: &mut Formatter<'_>) -> Result;
}
}
#[derive(Debu$0g)] #[derive(Debu$0g)]
struct Foo { struct Foo {
bar: String, bar: String,
} }
"#, "#,
r#" r#"
mod fmt {
pub struct Error;
pub type Result = Result<(), Error>;
pub struct Formatter<'a>;
pub trait Debug {
fn fmt(&self, f: &mut Formatter<'_>) -> Result;
}
}
struct Foo { struct Foo {
bar: String, bar: String,
} }
impl fmt::Debug for Foo { impl core::fmt::Debug for Foo {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { $0fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
${0:todo!()} f.debug_struct("Foo").field("bar", &self.bar).finish()
}
}
"#,
)
}
#[test]
fn add_custom_impl_debug_tuple_struct() {
check_assist(
replace_derive_with_manual_impl,
r#"
//- minicore: fmt
#[derive(Debu$0g)]
struct Foo(String, usize);
"#,
r#"struct Foo(String, usize);
impl core::fmt::Debug for Foo {
$0fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
f.debug_tuple("Foo").field(&self.0).field(&self.1).finish()
}
}
"#,
)
}
#[test]
fn add_custom_impl_debug_empty_struct() {
check_assist(
replace_derive_with_manual_impl,
r#"
//- minicore: fmt
#[derive(Debu$0g)]
struct Foo;
"#,
r#"
struct Foo;
impl core::fmt::Debug for Foo {
$0fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
f.debug_struct("Foo").finish()
}
}
"#,
)
}
#[test]
fn add_custom_impl_debug_enum() {
check_assist(
replace_derive_with_manual_impl,
r#"
//- minicore: fmt
#[derive(Debu$0g)]
enum Foo {
Bar,
Baz,
}
"#,
r#"
enum Foo {
Bar,
Baz,
}
impl core::fmt::Debug for Foo {
$0fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
match self {
Self::Bar => write!(f, "Bar"),
Self::Baz => write!(f, "Baz"),
}
} }
} }
"#, "#,

View File

@ -1385,8 +1385,8 @@ trait Debug { fn fmt(&self, f: &mut Formatter) -> Result<()>; }
struct S; struct S;
impl Debug for S { impl Debug for S {
fn fmt(&self, f: &mut Formatter) -> Result<()> { $0fn fmt(&self, f: &mut Formatter) -> Result<()> {
${0:todo!()} f.debug_struct("S").finish()
} }
} }
"#####, "#####,

View File

@ -274,11 +274,13 @@ fn check_todo(path: &Path, text: &str) {
"handlers/add_turbo_fish.rs", "handlers/add_turbo_fish.rs",
"handlers/generate_function.rs", "handlers/generate_function.rs",
"handlers/fill_match_arms.rs", "handlers/fill_match_arms.rs",
"handlers/replace_derive_with_manual_impl.rs",
// To support generating `todo!()` in assists, we have `expr_todo()` in // To support generating `todo!()` in assists, we have `expr_todo()` in
// `ast::make`. // `ast::make`.
"ast/make.rs", "ast/make.rs",
// The documentation in string literals may contain anything for its own purposes // The documentation in string literals may contain anything for its own purposes
"ide_db/src/helpers/generated_lints.rs", "ide_db/src/helpers/generated_lints.rs",
"ide_assists/src/tests/generated.rs",
]; ];
if need_todo.iter().any(|p| path.ends_with(p)) { if need_todo.iter().any(|p| path.ends_with(p)) {
return; return;

View File

@ -311,6 +311,9 @@ pub fn expr_method_call(
) -> ast::Expr { ) -> ast::Expr {
expr_from_text(&format!("{}.{}{}", receiver, method, arg_list)) expr_from_text(&format!("{}.{}{}", receiver, method, arg_list))
} }
pub fn expr_macro_call(f: ast::Expr, arg_list: ast::ArgList) -> ast::Expr {
expr_from_text(&format!("{}!{}", f, arg_list))
}
pub fn expr_ref(expr: ast::Expr, exclusive: bool) -> ast::Expr { pub fn expr_ref(expr: ast::Expr, exclusive: bool) -> ast::Expr {
expr_from_text(&if exclusive { format!("&mut {}", expr) } else { format!("&{}", expr) }) expr_from_text(&if exclusive { format!("&mut {}", expr) } else { format!("&{}", expr) })
} }
@ -318,6 +321,9 @@ pub fn expr_closure(pats: impl IntoIterator<Item = ast::Param>, expr: ast::Expr)
let params = pats.into_iter().join(", "); let params = pats.into_iter().join(", ");
expr_from_text(&format!("|{}| {}", params, expr)) expr_from_text(&format!("|{}| {}", params, expr))
} }
pub fn expr_field(receiver: ast::Expr, field: &str) -> ast::Expr {
expr_from_text(&format!("{}.{}", receiver, field))
}
pub fn expr_paren(expr: ast::Expr) -> ast::Expr { pub fn expr_paren(expr: ast::Expr) -> ast::Expr {
expr_from_text(&format!("({})", expr)) expr_from_text(&format!("({})", expr))
} }

View File

@ -31,6 +31,7 @@
//! eq: sized //! eq: sized
//! ord: eq, option //! ord: eq, option
//! derive: //! derive:
//! fmt: result
pub mod marker { pub mod marker {
// region:sized // region:sized
@ -334,6 +335,17 @@ pub mod cmp {
} }
// endregion:eq // endregion:eq
// region:fmt
pub mod fmt {
pub struct Error;
pub type Result = Result<(), Error>;
pub struct Formatter<'a>;
pub trait Debug {
fn fmt(&self, f: &mut Formatter<'_>) -> Result;
}
}
// endregion:fmt
// region:slice // region:slice
pub mod slice { pub mod slice {
#[lang = "slice"] #[lang = "slice"]