From bc6aee51b0d4902bf93fe31665e802b8d2df0bd8 Mon Sep 17 00:00:00 2001 From: Yoshua Wuyts Date: Mon, 16 Aug 2021 10:07:19 +0200 Subject: [PATCH 1/7] init partialord --- .../src/utils/gen_trait_fn_body.rs | 166 ++++++++++++++++++ 1 file changed, 166 insertions(+) diff --git a/crates/ide_assists/src/utils/gen_trait_fn_body.rs b/crates/ide_assists/src/utils/gen_trait_fn_body.rs index 6915460209b..5ec8adc2d4c 100644 --- a/crates/ide_assists/src/utils/gen_trait_fn_body.rs +++ b/crates/ide_assists/src/utils/gen_trait_fn_body.rs @@ -21,6 +21,7 @@ pub(crate) fn gen_trait_fn_body( "Default" => gen_default_impl(adt, func), "Hash" => gen_hash_impl(adt, func), "PartialEq" => gen_partial_eq(adt, func), + "PartialOrd" => gen_partial_ord(adt, func), _ => None, } } @@ -572,6 +573,171 @@ fn gen_tuple_field(field_name: &String) -> ast::Pat { Some(()) } +fn gen_partial_ord(adt: &ast::Adt, func: &ast::Fn) -> Option<()> { + fn gen_eq_chain(expr: Option, cmp: ast::Expr) -> Option { + match expr { + Some(expr) => Some(make::expr_op(ast::BinOp::BooleanAnd, expr, cmp)), + None => Some(cmp), + } + } + + fn gen_record_pat_field(field_name: &str, pat_name: &str) -> ast::RecordPatField { + let pat = make::ext::simple_ident_pat(make::name(&pat_name)); + let name_ref = make::name_ref(field_name); + make::record_pat_field(name_ref, pat.into()) + } + + fn gen_record_pat(record_name: ast::Path, fields: Vec) -> ast::RecordPat { + let list = make::record_pat_field_list(fields); + make::record_pat_with_fields(record_name, list) + } + + fn gen_variant_path(variant: &ast::Variant) -> Option { + make::ext::path_from_idents(["Self", &variant.name()?.to_string()]) + } + + fn gen_tuple_field(field_name: &String) -> ast::Pat { + ast::Pat::IdentPat(make::ident_pat(false, false, make::name(field_name))) + } + + // FIXME: return `None` if the trait carries a generic type; we can only + // generate this code `Self` for the time being. + + let body = match adt { + // `Hash` cannot be derived for unions, so no default impl can be provided. + ast::Adt::Union(_) => return None, + + ast::Adt::Enum(enum_) => { + // => std::mem::discriminant(self) == std::mem::discriminant(other) + let lhs_name = make::expr_path(make::ext::ident_path("self")); + let lhs = make::expr_call(make_discriminant()?, make::arg_list(Some(lhs_name.clone()))); + let rhs_name = make::expr_path(make::ext::ident_path("other")); + let rhs = make::expr_call(make_discriminant()?, make::arg_list(Some(rhs_name.clone()))); + let eq_check = make::expr_op(ast::BinOp::EqualityTest, lhs, rhs); + + let mut case_count = 0; + let mut arms = vec![]; + for variant in enum_.variant_list()?.variants() { + case_count += 1; + match variant.field_list() { + // => (Self::Bar { bin: l_bin }, Self::Bar { bin: r_bin }) => l_bin == r_bin, + Some(ast::FieldList::RecordFieldList(list)) => { + let mut expr = None; + let mut l_fields = vec![]; + let mut r_fields = vec![]; + + for field in list.fields() { + let field_name = field.name()?.to_string(); + + let l_name = &format!("l_{}", field_name); + l_fields.push(gen_record_pat_field(&field_name, &l_name)); + + let r_name = &format!("r_{}", field_name); + r_fields.push(gen_record_pat_field(&field_name, &r_name)); + + let lhs = make::expr_path(make::ext::ident_path(l_name)); + let rhs = make::expr_path(make::ext::ident_path(r_name)); + let cmp = make::expr_op(ast::BinOp::EqualityTest, lhs, rhs); + expr = gen_eq_chain(expr, cmp); + } + + let left = gen_record_pat(gen_variant_path(&variant)?, l_fields); + let right = gen_record_pat(gen_variant_path(&variant)?, r_fields); + let tuple = make::tuple_pat(vec![left.into(), right.into()]); + + if let Some(expr) = expr { + arms.push(make::match_arm(Some(tuple.into()), None, expr)); + } + } + + Some(ast::FieldList::TupleFieldList(list)) => { + let mut expr = None; + let mut l_fields = vec![]; + let mut r_fields = vec![]; + + for (i, _) in list.fields().enumerate() { + let field_name = format!("{}", i); + + let l_name = format!("l{}", field_name); + l_fields.push(gen_tuple_field(&l_name)); + + let r_name = format!("r{}", field_name); + r_fields.push(gen_tuple_field(&r_name)); + + let lhs = make::expr_path(make::ext::ident_path(&l_name)); + let rhs = make::expr_path(make::ext::ident_path(&r_name)); + let cmp = make::expr_op(ast::BinOp::EqualityTest, lhs, rhs); + expr = gen_eq_chain(expr, cmp); + } + + let left = make::tuple_struct_pat(gen_variant_path(&variant)?, l_fields); + let right = make::tuple_struct_pat(gen_variant_path(&variant)?, r_fields); + let tuple = make::tuple_pat(vec![left.into(), right.into()]); + + if let Some(expr) = expr { + arms.push(make::match_arm(Some(tuple.into()), None, expr)); + } + } + None => continue, + } + } + + let expr = match arms.len() { + 0 => eq_check, + _ => { + if case_count > arms.len() { + let lhs = make::wildcard_pat().into(); + arms.push(make::match_arm(Some(lhs), None, eq_check)); + } + + let match_target = make::expr_tuple(vec![lhs_name, rhs_name]); + let list = make::match_arm_list(arms).indent(ast::edit::IndentLevel(1)); + make::expr_match(match_target, list) + } + }; + + make::block_expr(None, Some(expr)).indent(ast::edit::IndentLevel(1)) + } + ast::Adt::Struct(strukt) => match strukt.field_list() { + Some(ast::FieldList::RecordFieldList(field_list)) => { + let mut expr = None; + for field in field_list.fields() { + let lhs = make::expr_path(make::ext::ident_path("self")); + let lhs = make::expr_field(lhs, &field.name()?.to_string()); + let rhs = make::expr_path(make::ext::ident_path("other")); + let rhs = make::expr_field(rhs, &field.name()?.to_string()); + let cmp = make::expr_op(ast::BinOp::EqualityTest, lhs, rhs); + expr = gen_eq_chain(expr, cmp); + } + make::block_expr(None, expr).indent(ast::edit::IndentLevel(1)) + } + + Some(ast::FieldList::TupleFieldList(field_list)) => { + let mut expr = None; + for (i, _) in field_list.fields().enumerate() { + let idx = format!("{}", i); + let lhs = make::expr_path(make::ext::ident_path("self")); + let lhs = make::expr_field(lhs, &idx); + let rhs = make::expr_path(make::ext::ident_path("other")); + let rhs = make::expr_field(rhs, &idx); + let cmp = make::expr_op(ast::BinOp::EqualityTest, lhs, rhs); + expr = gen_eq_chain(expr, cmp); + } + make::block_expr(None, expr).indent(ast::edit::IndentLevel(1)) + } + + // No fields in the body means there's nothing to hash. + None => { + let expr = make::expr_literal("true").into(); + make::block_expr(None, Some(expr)).indent(ast::edit::IndentLevel(1)) + } + }, + }; + + ted::replace(func.body()?.syntax(), body.clone_for_update().syntax()); + Some(()) +} + fn make_discriminant() -> Option { Some(make::expr_path(make::ext::path_from_idents(["core", "mem", "discriminant"])?)) } From 6941fdc49f0cc68955c07c80f0817b991f4dc495 Mon Sep 17 00:00:00 2001 From: Yoshua Wuyts Date: Tue, 12 Oct 2021 14:18:08 +0200 Subject: [PATCH 2/7] impl PartialOrd codegen for tuple records --- .../replace_derive_with_manual_impl.rs | 37 +++++++++++++++++++ .../src/utils/gen_trait_fn_body.rs | 37 +++++++++++++++++-- 2 files changed, 70 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 c0b7db332e2..1dc8dd95abd 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 @@ -675,6 +675,43 @@ impl Clone for Foo { ) } + #[test] + fn add_custom_impl_partial_ord_record_struct() { + check_assist( + replace_derive_with_manual_impl, + r#" +//- minicore: ord +#[derive(Partial$0Ord)] +struct Foo { + bin: usize, + bar: usize, + baz: usize, +} +"#, + r#" +struct Foo { + bin: usize, + bar: usize, + baz: usize, +} + +impl PartialOrd for Foo { + $0fn partial_cmp(&self, other: &Self) -> Option { + match self.bin.partial_cmp(other.bin) { + Some(core::cmp::Ordering::Eq) => {} + ord => return ord, + } + match self.bar.partial_cmp(other.bar) { + Some(core::cmp::Ordering::Eq) => {} + ord => return ord, + } + self.baz.partial_cmp(other.baz) + } +} +"#, + ) + } + #[test] fn add_custom_impl_partial_eq_record_struct() { check_assist( diff --git a/crates/ide_assists/src/utils/gen_trait_fn_body.rs b/crates/ide_assists/src/utils/gen_trait_fn_body.rs index 5ec8adc2d4c..aa0d9bc76bd 100644 --- a/crates/ide_assists/src/utils/gen_trait_fn_body.rs +++ b/crates/ide_assists/src/utils/gen_trait_fn_body.rs @@ -581,6 +581,29 @@ fn gen_eq_chain(expr: Option, cmp: ast::Expr) -> Option { } } + fn gen_partial_eq_match(match_target: ast::Expr) -> Option { + let mut arms = vec![]; + + let variant_name = + make::path_pat(make::ext::path_from_idents(["core", "cmp", "Ordering", "Eq"])?); + let lhs = make::tuple_struct_pat(make::ext::path_from_idents(["Some"])?, [variant_name]); + arms.push(make::match_arm(Some(lhs.into()), None, make::expr_empty_block())); + + arms.push(make::match_arm( + [make::ident_pat(false, false, make::name("ord")).into()], + None, + make::expr_return(Some(make::expr_path(make::ext::ident_path("ord")))), + )); + // let rhs = make::expr_path(make::ext::ident_path("other")); + let list = make::match_arm_list(arms).indent(ast::edit::IndentLevel(1)); + Some(make::expr_stmt(make::expr_match(match_target, list)).into()) + } + + fn gen_partial_cmp_call(lhs: ast::Expr, rhs: ast::Expr) -> ast::Expr { + let method = make::name_ref("partial_cmp"); + make::expr_method_call(lhs, method, make::arg_list(Some(rhs))) + } + fn gen_record_pat_field(field_name: &str, pat_name: &str) -> ast::RecordPatField { let pat = make::ext::simple_ident_pat(make::name(&pat_name)); let name_ref = make::name_ref(field_name); @@ -700,16 +723,22 @@ fn gen_tuple_field(field_name: &String) -> ast::Pat { } ast::Adt::Struct(strukt) => match strukt.field_list() { Some(ast::FieldList::RecordFieldList(field_list)) => { - let mut expr = None; + let mut exprs = vec![]; for field in field_list.fields() { let lhs = make::expr_path(make::ext::ident_path("self")); let lhs = make::expr_field(lhs, &field.name()?.to_string()); let rhs = make::expr_path(make::ext::ident_path("other")); let rhs = make::expr_field(rhs, &field.name()?.to_string()); - let cmp = make::expr_op(ast::BinOp::EqualityTest, lhs, rhs); - expr = gen_eq_chain(expr, cmp); + let ord = gen_partial_cmp_call(lhs, rhs); + exprs.push(ord); } - make::block_expr(None, expr).indent(ast::edit::IndentLevel(1)) + + let tail = exprs.pop(); + let stmts = exprs + .into_iter() + .map(gen_partial_eq_match) + .collect::>>()?; + make::block_expr(stmts.into_iter(), tail).indent(ast::edit::IndentLevel(1)) } Some(ast::FieldList::TupleFieldList(field_list)) => { From c0263fb07a78e83a76edbf578447d20994c38f8d Mon Sep 17 00:00:00 2001 From: Yoshua Wuyts Date: Tue, 12 Oct 2021 14:24:42 +0200 Subject: [PATCH 3/7] impl PartialOrd codegen for struct records --- .../replace_derive_with_manual_impl.rs | 29 +++++++++++++++++++ .../src/utils/gen_trait_fn_body.rs | 14 +++++---- 2 files changed, 38 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 1dc8dd95abd..27bba8732ff 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 @@ -712,6 +712,35 @@ impl PartialOrd for Foo { ) } + #[test] + fn add_custom_impl_partial_ord_tuple_struct() { + check_assist( + replace_derive_with_manual_impl, + r#" +//- minicore: ord +#[derive(Partial$0Ord)] +struct Foo(usize, usize, usize); +"#, + r#" +struct Foo(usize, usize, usize); + +impl PartialOrd for Foo { + $0fn partial_cmp(&self, other: &Self) -> Option { + match self.0.partial_cmp(other.0) { + Some(core::cmp::Ordering::Eq) => {} + ord => return ord, + } + match self.1.partial_cmp(other.1) { + Some(core::cmp::Ordering::Eq) => {} + ord => return ord, + } + self.2.partial_cmp(other.2) + } +} +"#, + ) + } + #[test] fn add_custom_impl_partial_eq_record_struct() { check_assist( diff --git a/crates/ide_assists/src/utils/gen_trait_fn_body.rs b/crates/ide_assists/src/utils/gen_trait_fn_body.rs index aa0d9bc76bd..d9a3d23eb5c 100644 --- a/crates/ide_assists/src/utils/gen_trait_fn_body.rs +++ b/crates/ide_assists/src/utils/gen_trait_fn_body.rs @@ -594,7 +594,6 @@ fn gen_partial_eq_match(match_target: ast::Expr) -> Option { None, make::expr_return(Some(make::expr_path(make::ext::ident_path("ord")))), )); - // let rhs = make::expr_path(make::ext::ident_path("other")); let list = make::match_arm_list(arms).indent(ast::edit::IndentLevel(1)); Some(make::expr_stmt(make::expr_match(match_target, list)).into()) } @@ -742,17 +741,22 @@ fn gen_tuple_field(field_name: &String) -> ast::Pat { } Some(ast::FieldList::TupleFieldList(field_list)) => { - let mut expr = None; + let mut exprs = vec![]; for (i, _) in field_list.fields().enumerate() { let idx = format!("{}", i); let lhs = make::expr_path(make::ext::ident_path("self")); let lhs = make::expr_field(lhs, &idx); let rhs = make::expr_path(make::ext::ident_path("other")); let rhs = make::expr_field(rhs, &idx); - let cmp = make::expr_op(ast::BinOp::EqualityTest, lhs, rhs); - expr = gen_eq_chain(expr, cmp); + let ord = gen_partial_cmp_call(lhs, rhs); + exprs.push(ord); } - make::block_expr(None, expr).indent(ast::edit::IndentLevel(1)) + let tail = exprs.pop(); + let stmts = exprs + .into_iter() + .map(gen_partial_eq_match) + .collect::>>()?; + make::block_expr(stmts.into_iter(), tail).indent(ast::edit::IndentLevel(1)) } // No fields in the body means there's nothing to hash. From 95eff43cc1af8dd1e0f85354b00d259a51256998 Mon Sep 17 00:00:00 2001 From: Yoshua Wuyts Date: Tue, 12 Oct 2021 14:36:50 +0200 Subject: [PATCH 4/7] impl PartialOrd codegen for C-style enums --- .../replace_derive_with_manual_impl.rs | 29 +++++++++++++++++++ .../src/utils/gen_trait_fn_body.rs | 6 ++-- 2 files changed, 32 insertions(+), 3 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 27bba8732ff..03211f728a8 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 @@ -712,6 +712,35 @@ impl PartialOrd for Foo { ) } + #[test] + fn add_custom_impl_partial_ord_enum() { + check_assist( + replace_derive_with_manual_impl, + r#" +//- minicore: ord +#[derive(Partial$0Ord)] +enum Foo { + Bin, + Bar, + Baz, +} +"#, + r#" +enum Foo { + Bin, + Bar, + Baz, +} + +impl PartialOrd for Foo { + $0fn partial_cmp(&self, other: &Self) -> Option { + core::mem::discriminant(self).partial_cmp(core::mem::discriminant(other)) + } +} +"#, + ) + } + #[test] fn add_custom_impl_partial_ord_tuple_struct() { check_assist( diff --git a/crates/ide_assists/src/utils/gen_trait_fn_body.rs b/crates/ide_assists/src/utils/gen_trait_fn_body.rs index d9a3d23eb5c..9bafae46c5c 100644 --- a/crates/ide_assists/src/utils/gen_trait_fn_body.rs +++ b/crates/ide_assists/src/utils/gen_trait_fn_body.rs @@ -635,7 +635,7 @@ fn gen_tuple_field(field_name: &String) -> ast::Pat { let lhs = make::expr_call(make_discriminant()?, make::arg_list(Some(lhs_name.clone()))); let rhs_name = make::expr_path(make::ext::ident_path("other")); let rhs = make::expr_call(make_discriminant()?, make::arg_list(Some(rhs_name.clone()))); - let eq_check = make::expr_op(ast::BinOp::EqualityTest, lhs, rhs); + let ord_check = gen_partial_cmp_call(lhs, rhs); let mut case_count = 0; let mut arms = vec![]; @@ -705,11 +705,11 @@ fn gen_tuple_field(field_name: &String) -> ast::Pat { } let expr = match arms.len() { - 0 => eq_check, + 0 => ord_check, _ => { if case_count > arms.len() { let lhs = make::wildcard_pat().into(); - arms.push(make::match_arm(Some(lhs), None, eq_check)); + arms.push(make::match_arm(Some(lhs), None, ord_check)); } let match_target = make::expr_tuple(vec![lhs_name, rhs_name]); From 77b5fe6c52bad2f04c552e8a72362ec8864ca6a2 Mon Sep 17 00:00:00 2001 From: Yoshua Wuyts Date: Tue, 12 Oct 2021 14:56:19 +0200 Subject: [PATCH 5/7] impl PartialOrd codegen for record enum --- .../replace_derive_with_manual_impl.rs | 73 ++++++++++++++++--- .../src/utils/gen_trait_fn_body.rs | 20 +++-- 2 files changed, 77 insertions(+), 16 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 03211f728a8..0f5a3843153 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 @@ -712,6 +712,35 @@ impl PartialOrd for Foo { ) } + #[test] + fn add_custom_impl_partial_ord_tuple_struct() { + check_assist( + replace_derive_with_manual_impl, + r#" +//- minicore: ord +#[derive(Partial$0Ord)] +struct Foo(usize, usize, usize); +"#, + r#" +struct Foo(usize, usize, usize); + +impl PartialOrd for Foo { + $0fn partial_cmp(&self, other: &Self) -> Option { + match self.0.partial_cmp(other.0) { + Some(core::cmp::Ordering::Eq) => {} + ord => return ord, + } + match self.1.partial_cmp(other.1) { + Some(core::cmp::Ordering::Eq) => {} + ord => return ord, + } + self.2.partial_cmp(other.2) + } +} +"#, + ) + } + #[test] fn add_custom_impl_partial_ord_enum() { check_assist( @@ -742,28 +771,50 @@ impl PartialOrd for Foo { } #[test] - fn add_custom_impl_partial_ord_tuple_struct() { + fn add_custom_impl_partial_ord_record_enum() { check_assist( replace_derive_with_manual_impl, r#" //- minicore: ord #[derive(Partial$0Ord)] -struct Foo(usize, usize, usize); +enum Foo { + Bar { + bin: String, + }, + Baz { + qux: String, + fez: String, + }, + Qux {}, + Bin, +} "#, r#" -struct Foo(usize, usize, usize); +enum Foo { + Bar { + bin: String, + }, + Baz { + qux: String, + fez: String, + }, + Qux {}, + Bin, +} impl PartialOrd for Foo { $0fn partial_cmp(&self, other: &Self) -> Option { - match self.0.partial_cmp(other.0) { - Some(core::cmp::Ordering::Eq) => {} - ord => return ord, + match (self, other) { + (Self::Bar { bin: l_bin }, Self::Bar { bin: r_bin }) => l_bin.partial_cmp(r_bin), + (Self::Baz { qux: l_qux, fez: l_fez }, Self::Baz { qux: r_qux, fez: r_fez }) => { + match l_qux.partial_cmp(r_qux) { + Some(core::cmp::Ordering::Eq) => {} + ord => return ord, + } + l_fez.partial_cmp(r_fez) + } + _ => core::mem::discriminant(self).partial_cmp(core::mem::discriminant(other)), } - match self.1.partial_cmp(other.1) { - Some(core::cmp::Ordering::Eq) => {} - ord => return ord, - } - self.2.partial_cmp(other.2) } } "#, diff --git a/crates/ide_assists/src/utils/gen_trait_fn_body.rs b/crates/ide_assists/src/utils/gen_trait_fn_body.rs index 9bafae46c5c..9633fd263b6 100644 --- a/crates/ide_assists/src/utils/gen_trait_fn_body.rs +++ b/crates/ide_assists/src/utils/gen_trait_fn_body.rs @@ -644,7 +644,7 @@ fn gen_tuple_field(field_name: &String) -> ast::Pat { match variant.field_list() { // => (Self::Bar { bin: l_bin }, Self::Bar { bin: r_bin }) => l_bin == r_bin, Some(ast::FieldList::RecordFieldList(list)) => { - let mut expr = None; + let mut exprs = vec![]; let mut l_fields = vec![]; let mut r_fields = vec![]; @@ -659,16 +659,26 @@ fn gen_tuple_field(field_name: &String) -> ast::Pat { let lhs = make::expr_path(make::ext::ident_path(l_name)); let rhs = make::expr_path(make::ext::ident_path(r_name)); - let cmp = make::expr_op(ast::BinOp::EqualityTest, lhs, rhs); - expr = gen_eq_chain(expr, cmp); + let ord = gen_partial_cmp_call(lhs, rhs); + exprs.push(ord); } let left = gen_record_pat(gen_variant_path(&variant)?, l_fields); let right = gen_record_pat(gen_variant_path(&variant)?, r_fields); let tuple = make::tuple_pat(vec![left.into(), right.into()]); - if let Some(expr) = expr { - arms.push(make::match_arm(Some(tuple.into()), None, expr)); + if let Some(tail) = exprs.pop() { + let stmts = exprs + .into_iter() + .map(gen_partial_eq_match) + .collect::>>()?; + let expr = match stmts.len() { + 0 => tail, + _ => make::block_expr(stmts.into_iter(), Some(tail)) + .indent(ast::edit::IndentLevel(1)) + .into(), + }; + arms.push(make::match_arm(Some(tuple.into()), None, expr.into())); } } From 5f72bd81a90de0c89c3a154043ea0183d3aaa829 Mon Sep 17 00:00:00 2001 From: Yoshua Wuyts Date: Tue, 12 Oct 2021 15:07:57 +0200 Subject: [PATCH 6/7] impl PartialOrd codegen for tuple enum --- .../replace_derive_with_manual_impl.rs | 41 +++++++++++++++++++ .../src/utils/gen_trait_fn_body.rs | 27 ++++++------ 2 files changed, 56 insertions(+), 12 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 0f5a3843153..4fceefe331d 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 @@ -821,6 +821,47 @@ impl PartialOrd for Foo { ) } + #[test] + fn add_custom_impl_partial_ord_tuple_enum() { + check_assist( + replace_derive_with_manual_impl, + r#" +//- minicore: ord +#[derive(Partial$0Ord)] +enum Foo { + Bar(String), + Baz(String, String), + Qux(), + Bin, +} +"#, + r#" +enum Foo { + Bar(String), + Baz(String, String), + Qux(), + Bin, +} + +impl PartialOrd for Foo { + $0fn partial_cmp(&self, other: &Self) -> Option { + match (self, other) { + (Self::Bar(l0), Self::Bar(r0)) => l0.partial_cmp(r0), + (Self::Baz(l0, l1), Self::Baz(r0, r1)) => { + match l0.partial_cmp(r0) { + Some(core::cmp::Ordering::Eq) => {} + ord => return ord, + } + l1.partial_cmp(r1) + } + _ => core::mem::discriminant(self).partial_cmp(core::mem::discriminant(other)), + } + } +} +"#, + ) + } + #[test] fn add_custom_impl_partial_eq_record_struct() { check_assist( diff --git a/crates/ide_assists/src/utils/gen_trait_fn_body.rs b/crates/ide_assists/src/utils/gen_trait_fn_body.rs index 9633fd263b6..10b781636f4 100644 --- a/crates/ide_assists/src/utils/gen_trait_fn_body.rs +++ b/crates/ide_assists/src/utils/gen_trait_fn_body.rs @@ -574,13 +574,6 @@ fn gen_tuple_field(field_name: &String) -> ast::Pat { } fn gen_partial_ord(adt: &ast::Adt, func: &ast::Fn) -> Option<()> { - fn gen_eq_chain(expr: Option, cmp: ast::Expr) -> Option { - match expr { - Some(expr) => Some(make::expr_op(ast::BinOp::BooleanAnd, expr, cmp)), - None => Some(cmp), - } - } - fn gen_partial_eq_match(match_target: ast::Expr) -> Option { let mut arms = vec![]; @@ -683,7 +676,7 @@ fn gen_tuple_field(field_name: &String) -> ast::Pat { } Some(ast::FieldList::TupleFieldList(list)) => { - let mut expr = None; + let mut exprs = vec![]; let mut l_fields = vec![]; let mut r_fields = vec![]; @@ -698,16 +691,26 @@ fn gen_tuple_field(field_name: &String) -> ast::Pat { let lhs = make::expr_path(make::ext::ident_path(&l_name)); let rhs = make::expr_path(make::ext::ident_path(&r_name)); - let cmp = make::expr_op(ast::BinOp::EqualityTest, lhs, rhs); - expr = gen_eq_chain(expr, cmp); + let ord = gen_partial_cmp_call(lhs, rhs); + exprs.push(ord); } let left = make::tuple_struct_pat(gen_variant_path(&variant)?, l_fields); let right = make::tuple_struct_pat(gen_variant_path(&variant)?, r_fields); let tuple = make::tuple_pat(vec![left.into(), right.into()]); - if let Some(expr) = expr { - arms.push(make::match_arm(Some(tuple.into()), None, expr)); + if let Some(tail) = exprs.pop() { + let stmts = exprs + .into_iter() + .map(gen_partial_eq_match) + .collect::>>()?; + let expr = match stmts.len() { + 0 => tail, + _ => make::block_expr(stmts.into_iter(), Some(tail)) + .indent(ast::edit::IndentLevel(1)) + .into(), + }; + arms.push(make::match_arm(Some(tuple.into()), None, expr.into())); } } None => continue, From 601ed3a10dacc2ba2ee0ca436c23529ae7fde292 Mon Sep 17 00:00:00 2001 From: Yoshua Wuyts Date: Tue, 12 Oct 2021 17:44:57 +0200 Subject: [PATCH 7/7] Simplify generated PartialOrd code --- .../replace_derive_with_manual_impl.rs | 57 ++++---- .../src/utils/gen_trait_fn_body.rs | 129 ++++++++---------- 2 files changed, 85 insertions(+), 101 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 4fceefe331d..b04bd6ba098 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 @@ -682,6 +682,31 @@ fn add_custom_impl_partial_ord_record_struct() { r#" //- minicore: ord #[derive(Partial$0Ord)] +struct Foo { + bin: usize, +} +"#, + r#" +struct Foo { + bin: usize, +} + +impl PartialOrd for Foo { + $0fn partial_cmp(&self, other: &Self) -> Option { + self.bin.partial_cmp(other.bin) + } +} +"#, + ) + } + + #[test] + fn add_custom_impl_partial_ord_record_struct_multi_field() { + check_assist( + replace_derive_with_manual_impl, + r#" +//- minicore: ord +#[derive(Partial$0Ord)] struct Foo { bin: usize, bar: usize, @@ -697,15 +722,7 @@ struct Foo { impl PartialOrd for Foo { $0fn partial_cmp(&self, other: &Self) -> Option { - match self.bin.partial_cmp(other.bin) { - Some(core::cmp::Ordering::Eq) => {} - ord => return ord, - } - match self.bar.partial_cmp(other.bar) { - Some(core::cmp::Ordering::Eq) => {} - ord => return ord, - } - self.baz.partial_cmp(other.baz) + (self.bin, self.bar, self.baz).partial_cmp((other.bin, other.bar, other.baz)) } } "#, @@ -726,15 +743,7 @@ fn add_custom_impl_partial_ord_tuple_struct() { impl PartialOrd for Foo { $0fn partial_cmp(&self, other: &Self) -> Option { - match self.0.partial_cmp(other.0) { - Some(core::cmp::Ordering::Eq) => {} - ord => return ord, - } - match self.1.partial_cmp(other.1) { - Some(core::cmp::Ordering::Eq) => {} - ord => return ord, - } - self.2.partial_cmp(other.2) + (self.0, self.1, self.2).partial_cmp((other.0, other.1, other.2)) } } "#, @@ -807,11 +816,7 @@ impl PartialOrd for Foo { match (self, other) { (Self::Bar { bin: l_bin }, Self::Bar { bin: r_bin }) => l_bin.partial_cmp(r_bin), (Self::Baz { qux: l_qux, fez: l_fez }, Self::Baz { qux: r_qux, fez: r_fez }) => { - match l_qux.partial_cmp(r_qux) { - Some(core::cmp::Ordering::Eq) => {} - ord => return ord, - } - l_fez.partial_cmp(r_fez) + (l_qux, l_fez).partial_cmp((r_qux, r_fez)) } _ => core::mem::discriminant(self).partial_cmp(core::mem::discriminant(other)), } @@ -848,11 +853,7 @@ impl PartialOrd for Foo { match (self, other) { (Self::Bar(l0), Self::Bar(r0)) => l0.partial_cmp(r0), (Self::Baz(l0, l1), Self::Baz(r0, r1)) => { - match l0.partial_cmp(r0) { - Some(core::cmp::Ordering::Eq) => {} - ord => return ord, - } - l1.partial_cmp(r1) + (l0, l1).partial_cmp((r0, r1)) } _ => core::mem::discriminant(self).partial_cmp(core::mem::discriminant(other)), } diff --git a/crates/ide_assists/src/utils/gen_trait_fn_body.rs b/crates/ide_assists/src/utils/gen_trait_fn_body.rs index 10b781636f4..c883e6fb11b 100644 --- a/crates/ide_assists/src/utils/gen_trait_fn_body.rs +++ b/crates/ide_assists/src/utils/gen_trait_fn_body.rs @@ -574,27 +574,18 @@ fn gen_tuple_field(field_name: &String) -> ast::Pat { } fn gen_partial_ord(adt: &ast::Adt, func: &ast::Fn) -> Option<()> { - fn gen_partial_eq_match(match_target: ast::Expr) -> Option { - let mut arms = vec![]; - - let variant_name = - make::path_pat(make::ext::path_from_idents(["core", "cmp", "Ordering", "Eq"])?); - let lhs = make::tuple_struct_pat(make::ext::path_from_idents(["Some"])?, [variant_name]); - arms.push(make::match_arm(Some(lhs.into()), None, make::expr_empty_block())); - - arms.push(make::match_arm( - [make::ident_pat(false, false, make::name("ord")).into()], - None, - make::expr_return(Some(make::expr_path(make::ext::ident_path("ord")))), - )); - let list = make::match_arm_list(arms).indent(ast::edit::IndentLevel(1)); - Some(make::expr_stmt(make::expr_match(match_target, list)).into()) - } - fn gen_partial_cmp_call(lhs: ast::Expr, rhs: ast::Expr) -> ast::Expr { let method = make::name_ref("partial_cmp"); make::expr_method_call(lhs, method, make::arg_list(Some(rhs))) } + fn gen_partial_cmp_call2(mut lhs: Vec, mut rhs: Vec) -> ast::Expr { + let (lhs, rhs) = match (lhs.len(), rhs.len()) { + (1, 1) => (lhs.pop().unwrap(), rhs.pop().unwrap()), + _ => (make::expr_tuple(lhs.into_iter()), make::expr_tuple(rhs.into_iter())), + }; + let method = make::name_ref("partial_cmp"); + make::expr_method_call(lhs, method, make::arg_list(Some(rhs))) + } fn gen_record_pat_field(field_name: &str, pat_name: &str) -> ast::RecordPatField { let pat = make::ext::simple_ident_pat(make::name(&pat_name)); @@ -637,7 +628,8 @@ fn gen_tuple_field(field_name: &String) -> ast::Pat { match variant.field_list() { // => (Self::Bar { bin: l_bin }, Self::Bar { bin: r_bin }) => l_bin == r_bin, Some(ast::FieldList::RecordFieldList(list)) => { - let mut exprs = vec![]; + let mut l_pat_fields = vec![]; + let mut r_pat_fields = vec![]; let mut l_fields = vec![]; let mut r_fields = vec![]; @@ -645,38 +637,36 @@ fn gen_tuple_field(field_name: &String) -> ast::Pat { let field_name = field.name()?.to_string(); let l_name = &format!("l_{}", field_name); - l_fields.push(gen_record_pat_field(&field_name, &l_name)); + l_pat_fields.push(gen_record_pat_field(&field_name, &l_name)); let r_name = &format!("r_{}", field_name); - r_fields.push(gen_record_pat_field(&field_name, &r_name)); + r_pat_fields.push(gen_record_pat_field(&field_name, &r_name)); let lhs = make::expr_path(make::ext::ident_path(l_name)); let rhs = make::expr_path(make::ext::ident_path(r_name)); - let ord = gen_partial_cmp_call(lhs, rhs); - exprs.push(ord); + l_fields.push(lhs); + r_fields.push(rhs); } - let left = gen_record_pat(gen_variant_path(&variant)?, l_fields); - let right = gen_record_pat(gen_variant_path(&variant)?, r_fields); - let tuple = make::tuple_pat(vec![left.into(), right.into()]); + let left_pat = gen_record_pat(gen_variant_path(&variant)?, l_pat_fields); + let right_pat = gen_record_pat(gen_variant_path(&variant)?, r_pat_fields); + let tuple_pat = make::tuple_pat(vec![left_pat.into(), right_pat.into()]); - if let Some(tail) = exprs.pop() { - let stmts = exprs - .into_iter() - .map(gen_partial_eq_match) - .collect::>>()?; - let expr = match stmts.len() { - 0 => tail, - _ => make::block_expr(stmts.into_iter(), Some(tail)) + let len = l_fields.len(); + if len != 0 { + let mut expr = gen_partial_cmp_call2(l_fields, r_fields); + if len >= 2 { + expr = make::block_expr(None, Some(expr)) .indent(ast::edit::IndentLevel(1)) - .into(), - }; - arms.push(make::match_arm(Some(tuple.into()), None, expr.into())); + .into(); + } + arms.push(make::match_arm(Some(tuple_pat.into()), None, expr)); } } Some(ast::FieldList::TupleFieldList(list)) => { - let mut exprs = vec![]; + let mut l_pat_fields = vec![]; + let mut r_pat_fields = vec![]; let mut l_fields = vec![]; let mut r_fields = vec![]; @@ -684,33 +674,32 @@ fn gen_tuple_field(field_name: &String) -> ast::Pat { let field_name = format!("{}", i); let l_name = format!("l{}", field_name); - l_fields.push(gen_tuple_field(&l_name)); + l_pat_fields.push(gen_tuple_field(&l_name)); let r_name = format!("r{}", field_name); - r_fields.push(gen_tuple_field(&r_name)); + r_pat_fields.push(gen_tuple_field(&r_name)); let lhs = make::expr_path(make::ext::ident_path(&l_name)); let rhs = make::expr_path(make::ext::ident_path(&r_name)); - let ord = gen_partial_cmp_call(lhs, rhs); - exprs.push(ord); + l_fields.push(lhs); + r_fields.push(rhs); } - let left = make::tuple_struct_pat(gen_variant_path(&variant)?, l_fields); - let right = make::tuple_struct_pat(gen_variant_path(&variant)?, r_fields); - let tuple = make::tuple_pat(vec![left.into(), right.into()]); + let left_pat = + make::tuple_struct_pat(gen_variant_path(&variant)?, l_pat_fields); + let right_pat = + make::tuple_struct_pat(gen_variant_path(&variant)?, r_pat_fields); + let tuple_pat = make::tuple_pat(vec![left_pat.into(), right_pat.into()]); - if let Some(tail) = exprs.pop() { - let stmts = exprs - .into_iter() - .map(gen_partial_eq_match) - .collect::>>()?; - let expr = match stmts.len() { - 0 => tail, - _ => make::block_expr(stmts.into_iter(), Some(tail)) + let len = l_fields.len(); + if len != 0 { + let mut expr = gen_partial_cmp_call2(l_fields, r_fields); + if len >= 2 { + expr = make::block_expr(None, Some(expr)) .indent(ast::edit::IndentLevel(1)) - .into(), - }; - arms.push(make::match_arm(Some(tuple.into()), None, expr.into())); + .into(); + } + arms.push(make::match_arm(Some(tuple_pat.into()), None, expr)); } } None => continue, @@ -735,41 +724,35 @@ fn gen_tuple_field(field_name: &String) -> ast::Pat { } ast::Adt::Struct(strukt) => match strukt.field_list() { Some(ast::FieldList::RecordFieldList(field_list)) => { - let mut exprs = vec![]; + let mut l_fields = vec![]; + let mut r_fields = vec![]; for field in field_list.fields() { let lhs = make::expr_path(make::ext::ident_path("self")); let lhs = make::expr_field(lhs, &field.name()?.to_string()); let rhs = make::expr_path(make::ext::ident_path("other")); let rhs = make::expr_field(rhs, &field.name()?.to_string()); - let ord = gen_partial_cmp_call(lhs, rhs); - exprs.push(ord); + l_fields.push(lhs); + r_fields.push(rhs); } - let tail = exprs.pop(); - let stmts = exprs - .into_iter() - .map(gen_partial_eq_match) - .collect::>>()?; - make::block_expr(stmts.into_iter(), tail).indent(ast::edit::IndentLevel(1)) + let expr = gen_partial_cmp_call2(l_fields, r_fields); + make::block_expr(None, Some(expr)).indent(ast::edit::IndentLevel(1)) } Some(ast::FieldList::TupleFieldList(field_list)) => { - let mut exprs = vec![]; + let mut l_fields = vec![]; + let mut r_fields = vec![]; for (i, _) in field_list.fields().enumerate() { let idx = format!("{}", i); let lhs = make::expr_path(make::ext::ident_path("self")); let lhs = make::expr_field(lhs, &idx); let rhs = make::expr_path(make::ext::ident_path("other")); let rhs = make::expr_field(rhs, &idx); - let ord = gen_partial_cmp_call(lhs, rhs); - exprs.push(ord); + l_fields.push(lhs); + r_fields.push(rhs); } - let tail = exprs.pop(); - let stmts = exprs - .into_iter() - .map(gen_partial_eq_match) - .collect::>>()?; - make::block_expr(stmts.into_iter(), tail).indent(ast::edit::IndentLevel(1)) + let expr = gen_partial_cmp_call2(l_fields, r_fields); + make::block_expr(None, Some(expr)).indent(ast::edit::IndentLevel(1)) } // No fields in the body means there's nothing to hash.