From 05477ff8df2056e7301f28930e72bd13e983504d Mon Sep 17 00:00:00 2001 From: alex-semenyuk Date: Sat, 26 Nov 2022 19:21:07 +0300 Subject: [PATCH] Fix manual_let_else produces a wrong suggestion with or-patterns --- clippy_lints/src/manual_let_else.rs | 7 ++++++- tests/ui/manual_let_else_match.rs | 7 +++++++ tests/ui/manual_let_else_match.stderr | 15 ++++++++++++--- 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/clippy_lints/src/manual_let_else.rs b/clippy_lints/src/manual_let_else.rs index 874d36ca9f4..9c6f8b43c07 100644 --- a/clippy_lints/src/manual_let_else.rs +++ b/clippy_lints/src/manual_let_else.rs @@ -151,7 +151,12 @@ fn emit_manual_let_else(cx: &LateContext<'_>, span: Span, expr: &Expr<'_>, pat: } else { format!("{{ {sn_else} }}") }; - let sugg = format!("let {sn_pat} = {sn_expr} else {else_bl};"); + let sn_bl = if matches!(pat.kind, PatKind::Or(..)) { + format!("({sn_pat})") + } else { + sn_pat.into_owned() + }; + let sugg = format!("let {sn_bl} = {sn_expr} else {else_bl};"); diag.span_suggestion(span, "consider writing", sugg, app); }, ); diff --git a/tests/ui/manual_let_else_match.rs b/tests/ui/manual_let_else_match.rs index 93c86ca24fe..28caed9d79d 100644 --- a/tests/ui/manual_let_else_match.rs +++ b/tests/ui/manual_let_else_match.rs @@ -64,6 +64,13 @@ fn fire() { Ok(v) => v, Err(()) => return, }; + + let f = Variant::Bar(1); + + let _value = match f { + Variant::Bar(_) | Variant::Baz(_) => (), + _ => return, + }; } fn not_fire() { diff --git a/tests/ui/manual_let_else_match.stderr b/tests/ui/manual_let_else_match.stderr index 38be5ac5454..cd5e9a9ac39 100644 --- a/tests/ui/manual_let_else_match.stderr +++ b/tests/ui/manual_let_else_match.stderr @@ -25,7 +25,7 @@ LL | / let v = match h() { LL | | (Some(_), Some(_)) | (None, None) => continue, LL | | (Some(v), None) | (None, Some(v)) => v, LL | | }; - | |__________^ help: consider writing: `let (Some(v), None) | (None, Some(v)) = h() else { continue };` + | |__________^ help: consider writing: `let ((Some(v), None) | (None, Some(v))) = h() else { continue };` error: this could be rewritten as `let...else` --> $DIR/manual_let_else_match.rs:49:9 @@ -34,7 +34,7 @@ LL | / let v = match build_enum() { LL | | _ => continue, LL | | Variant::Bar(v) | Variant::Baz(v) => v, LL | | }; - | |__________^ help: consider writing: `let Variant::Bar(v) | Variant::Baz(v) = build_enum() else { continue };` + | |__________^ help: consider writing: `let (Variant::Bar(v) | Variant::Baz(v)) = build_enum() else { continue };` error: this could be rewritten as `let...else` --> $DIR/manual_let_else_match.rs:57:5 @@ -54,5 +54,14 @@ LL | | Err(()) => return, LL | | }; | |______^ help: consider writing: `let Ok(v) = f().map_err(|_| ()) else { return };` -error: aborting due to 6 previous errors +error: this could be rewritten as `let...else` + --> $DIR/manual_let_else_match.rs:70:5 + | +LL | / let _value = match f { +LL | | Variant::Bar(_) | Variant::Baz(_) => (), +LL | | _ => return, +LL | | }; + | |______^ help: consider writing: `let (Variant::Bar(_) | Variant::Baz(_)) = f else { return };` + +error: aborting due to 7 previous errors