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.
| -rw-r--r-- | crates/ide-assists/src/handlers/replace_derive_with_manual_impl.rs | 60 | ||||
| -rw-r--r-- | crates/ide-assists/src/utils/gen_trait_fn_body.rs | 14 |
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]); |