Unnamed repository; edit this file 'description' to name the repository.
Auto merge of #13732 - rami3l:fix/gen-partial-eq, r=jonas-schievink
fix: add fallback case in generated `PartialEq` impl Partially fixes #13727. When generating `PartialEq` implementations for enums, the original code can already generate the following fallback case: ```rs _ => std::mem::discriminant(self) == std::mem::discriminant(other), ``` However, it has been suppressed in the following example for no good reason: ```rs enum Either<T, U> { Left(T), Right(U), } impl<T, U> PartialEq for Either<T, U> { fn eq(&self, other: &Self) -> bool { match (self, other) { (Self::Left(l0), Self::Left(r0)) => l0 == r0, (Self::Right(l0), Self::Right(r0)) => l0 == r0, // _ => std::mem::discriminant(self) == std::mem::discriminant(other), // ^ this completes the match arms! } } } ``` This PR has removed that suppression logic. ~~Of course, the PR could have suppressed the fallback case generation for single-variant enums instead, but I believe that this case is quite rare and should be caught by `#[warn(unreachable_patterns)]` anyway.~~ After this fix, when the enum has >1 variants, the following fallback arm will be generated : * `_ => false,` if we've already gone through every case where the variants of `self` and `other` match; * The original one (as stated above) in other cases. --- Note: The code example is still wrong after the fix due to incorrect trait bounds.
bors 2022-12-12
parent 16c70fe · parent 57fb18e · commit 3a7215b
-rw-r--r--crates/ide-assists/src/handlers/replace_derive_with_manual_impl.rs60
-rw-r--r--crates/ide-assists/src/utils/gen_trait_fn_body.rs14
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 6fa15b28e4..2854701c08 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
@@ -907,7 +907,34 @@ impl PartialEq for Foo {
}
#[test]
- fn add_custom_impl_partial_eq_tuple_enum() {
+ fn add_custom_impl_partial_eq_single_variant_tuple_enum() {
+ check_assist(
+ replace_derive_with_manual_impl,
+ r#"
+//- minicore: eq, derive
+#[derive(Partial$0Eq)]
+enum Foo {
+ Bar(String),
+}
+"#,
+ r#"
+enum Foo {
+ Bar(String),
+}
+
+impl PartialEq for Foo {
+ $0fn eq(&self, other: &Self) -> bool {
+ match (self, other) {
+ (Self::Bar(l0), Self::Bar(r0)) => l0 == r0,
+ }
+ }
+}
+"#,
+ )
+ }
+
+ #[test]
+ fn add_custom_impl_partial_eq_partial_tuple_enum() {
check_assist(
replace_derive_with_manual_impl,
r#"
@@ -937,6 +964,37 @@ impl PartialEq for Foo {
}
#[test]
+ fn add_custom_impl_partial_eq_tuple_enum() {
+ check_assist(
+ replace_derive_with_manual_impl,
+ r#"
+//- minicore: eq, derive
+#[derive(Partial$0Eq)]
+enum Foo {
+ Bar(String),
+ Baz(i32),
+}
+"#,
+ r#"
+enum Foo {
+ Bar(String),
+ Baz(i32),
+}
+
+impl PartialEq for Foo {
+ $0fn eq(&self, other: &Self) -> bool {
+ match (self, other) {
+ (Self::Bar(l0), Self::Bar(r0)) => l0 == r0,
+ (Self::Baz(l0), Self::Baz(r0)) => l0 == r0,
+ _ => false,
+ }
+ }
+}
+"#,
+ )
+ }
+
+ #[test]
fn add_custom_impl_partial_eq_record_enum() {
check_assist(
replace_derive_with_manual_impl,
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 6c87e66c13..f32e5ce97d 100644
--- a/crates/ide-assists/src/utils/gen_trait_fn_body.rs
+++ b/crates/ide-assists/src/utils/gen_trait_fn_body.rs
@@ -516,10 +516,18 @@ fn gen_partial_eq(adt: &ast::Adt, func: &ast::Fn) -> Option<()> {
let expr = match arms.len() {
0 => eq_check,
- _ => {
- if n_cases > arms.len() {
+ arms_len => {
+ // Generate the fallback arm when this enum has >1 variants.
+ // The fallback arm will be `_ => false,` if we've already gone through every case where the variants of self and other match,
+ // and `_ => std::mem::discriminant(self) == std::mem::discriminant(other),` otherwise.
+ if n_cases > 1 {
let lhs = make::wildcard_pat().into();
- arms.push(make::match_arm(Some(lhs), None, eq_check));
+ let rhs = if arms_len == n_cases {
+ make::expr_literal("false").into()
+ } else {
+ eq_check
+ };
+ arms.push(make::match_arm(Some(lhs), None, rhs));
}
let match_target = make::expr_tuple(vec![lhs_name, rhs_name]);