Unnamed repository; edit this file 'description' to name the repository.
Merge #10574
10574: fix: Fix PartialOrd codegen r=lnicola a=yoshuawuyts
Closes https://github.com/rust-analyzer/rust-analyzer/issues/10571#issuecomment-945516462. Thanks!
r? `@lnicola`
Co-authored-by: Yoshua Wuyts <[email protected]>
| -rw-r--r-- | crates/ide_assists/src/handlers/replace_derive_with_manual_impl.rs | 119 | ||||
| -rw-r--r-- | crates/ide_assists/src/utils/gen_trait_fn_body.rs | 144 |
2 files changed, 10 insertions, 253 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 b04bd6ba09..138c694d49 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 @@ -693,7 +693,7 @@ struct Foo { impl PartialOrd for Foo { $0fn partial_cmp(&self, other: &Self) -> Option<core::cmp::Ordering> { - self.bin.partial_cmp(other.bin) + self.bin.partial_cmp(&other.bin) } } "#, @@ -722,7 +722,7 @@ struct Foo { impl PartialOrd for Foo { $0fn partial_cmp(&self, other: &Self) -> Option<core::cmp::Ordering> { - (self.bin, self.bar, self.baz).partial_cmp((other.bin, other.bar, other.baz)) + (self.bin, self.bar, self.baz).partial_cmp(&(other.bin, other.bar, other.baz)) } } "#, @@ -743,120 +743,7 @@ struct Foo(usize, usize, usize); impl PartialOrd for Foo { $0fn partial_cmp(&self, other: &Self) -> Option<core::cmp::Ordering> { - (self.0, self.1, self.2).partial_cmp((other.0, other.1, other.2)) - } -} -"#, - ) - } - - #[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::cmp::Ordering> { - core::mem::discriminant(self).partial_cmp(core::mem::discriminant(other)) - } -} -"#, - ) - } - - #[test] - fn add_custom_impl_partial_ord_record_enum() { - check_assist( - replace_derive_with_manual_impl, - r#" -//- minicore: ord -#[derive(Partial$0Ord)] -enum Foo { - Bar { - bin: String, - }, - Baz { - qux: String, - fez: String, - }, - Qux {}, - Bin, -} -"#, - r#" -enum Foo { - Bar { - bin: String, - }, - Baz { - qux: String, - fez: String, - }, - Qux {}, - Bin, -} - -impl PartialOrd for Foo { - $0fn partial_cmp(&self, other: &Self) -> Option<core::cmp::Ordering> { - 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 }) => { - (l_qux, l_fez).partial_cmp((r_qux, r_fez)) - } - _ => core::mem::discriminant(self).partial_cmp(core::mem::discriminant(other)), - } - } -} -"#, - ) - } - - #[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<core::cmp::Ordering> { - match (self, other) { - (Self::Bar(l0), Self::Bar(r0)) => l0.partial_cmp(r0), - (Self::Baz(l0, l1), Self::Baz(r0, r1)) => { - (l0, l1).partial_cmp((r0, r1)) - } - _ => core::mem::discriminant(self).partial_cmp(core::mem::discriminant(other)), - } + (self.0, self.1, self.2).partial_cmp(&(other.0, other.1, 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 c883e6fb11..40c3734f9d 100644 --- a/crates/ide_assists/src/utils/gen_trait_fn_body.rs +++ b/crates/ide_assists/src/utils/gen_trait_fn_body.rs @@ -574,154 +574,24 @@ fn gen_partial_eq(adt: &ast::Adt, func: &ast::Fn) -> Option<()> { } fn gen_partial_ord(adt: &ast::Adt, func: &ast::Fn) -> Option<()> { - 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<ast::Expr>, mut rhs: Vec<ast::Expr>) -> ast::Expr { + fn gen_partial_cmp_call(mut lhs: Vec<ast::Expr>, mut rhs: Vec<ast::Expr>) -> 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 rhs = make::expr_ref(rhs, false); 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); - make::record_pat_field(name_ref, pat.into()) - } - - fn gen_record_pat(record_name: ast::Path, fields: Vec<ast::RecordPatField>) -> 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<ast::Path> { - 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. + // `PartialOrd` 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 ord_check = gen_partial_cmp_call(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 l_pat_fields = vec![]; - let mut r_pat_fields = vec![]; - 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_pat_fields.push(gen_record_pat_field(&field_name, &l_name)); - - let r_name = &format!("r_{}", field_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)); - l_fields.push(lhs); - r_fields.push(rhs); - } - - 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()]); - - 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_pat.into()), None, expr)); - } - } - - Some(ast::FieldList::TupleFieldList(list)) => { - let mut l_pat_fields = vec![]; - let mut r_pat_fields = vec![]; - 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_pat_fields.push(gen_tuple_field(&l_name)); - - let r_name = format!("r{}", field_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)); - l_fields.push(lhs); - r_fields.push(rhs); - } - - 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()]); - - 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_pat.into()), None, expr)); - } - } - None => continue, - } - } - - let expr = match arms.len() { - 0 => ord_check, - _ => { - if case_count > arms.len() { - let lhs = make::wildcard_pat().into(); - arms.push(make::match_arm(Some(lhs), None, ord_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)) - } + // `core::mem::Discriminant` does not implement `PartialOrd` in stable Rust today. + ast::Adt::Enum(_) => return None, ast::Adt::Struct(strukt) => match strukt.field_list() { Some(ast::FieldList::RecordFieldList(field_list)) => { let mut l_fields = vec![]; @@ -735,7 +605,7 @@ fn gen_partial_ord(adt: &ast::Adt, func: &ast::Fn) -> Option<()> { r_fields.push(rhs); } - let expr = gen_partial_cmp_call2(l_fields, r_fields); + let expr = gen_partial_cmp_call(l_fields, r_fields); make::block_expr(None, Some(expr)).indent(ast::edit::IndentLevel(1)) } @@ -751,7 +621,7 @@ fn gen_partial_ord(adt: &ast::Adt, func: &ast::Fn) -> Option<()> { l_fields.push(lhs); r_fields.push(rhs); } - let expr = gen_partial_cmp_call2(l_fields, r_fields); + let expr = gen_partial_cmp_call(l_fields, r_fields); make::block_expr(None, Some(expr)).indent(ast::edit::IndentLevel(1)) } |