From aea008276eb51e55d9ad76b52486dba1b6ca9b2c Mon Sep 17 00:00:00 2001 From: koka Date: Sun, 8 Jan 2023 02:35:18 +0900 Subject: [PATCH] Use original variable name in the suggestion --- clippy_lints/src/manual_let_else.rs | 36 +++++++++------------------ tests/ui/manual_let_else.stderr | 34 ++++++++++++------------- tests/ui/manual_let_else_match.stderr | 14 +++++------ 3 files changed, 36 insertions(+), 48 deletions(-) diff --git a/clippy_lints/src/manual_let_else.rs b/clippy_lints/src/manual_let_else.rs index 9c6f8b43c078..4cc1e20a8956 100644 --- a/clippy_lints/src/manual_let_else.rs +++ b/clippy_lints/src/manual_let_else.rs @@ -68,29 +68,21 @@ impl_lint_pass!(ManualLetElse => [MANUAL_LET_ELSE]); impl<'tcx> LateLintPass<'tcx> for ManualLetElse { fn check_stmt(&mut self, cx: &LateContext<'_>, stmt: &'tcx Stmt<'tcx>) { - let if_let_or_match = if_chain! { - if self.msrv.meets(msrvs::LET_ELSE); - if !in_external_macro(cx.sess(), stmt.span); - if let StmtKind::Local(local) = stmt.kind; - if let Some(init) = local.init; - if local.els.is_none(); - if local.ty.is_none(); - if init.span.ctxt() == stmt.span.ctxt(); - if let Some(if_let_or_match) = IfLetOrMatch::parse(cx, init); - then { - if_let_or_match - } else { - return; - } - }; - + if self.msrv.meets(msrvs::LET_ELSE) && + !in_external_macro(cx.sess(), stmt.span) && + let StmtKind::Local(local) = stmt.kind && + let Some(init) = local.init && + local.els.is_none() && + local.ty.is_none() && + init.span.ctxt() == stmt.span.ctxt() && + let Some(if_let_or_match) = IfLetOrMatch::parse(cx, init) { match if_let_or_match { IfLetOrMatch::IfLet(if_let_expr, let_pat, if_then, if_else) => if_chain! { if expr_is_simple_identity(let_pat, if_then); if let Some(if_else) = if_else; if expr_diverges(cx, if_else); then { - emit_manual_let_else(cx, stmt.span, if_let_expr, let_pat, if_else); + emit_manual_let_else(cx, stmt.span, if_let_expr, local.pat, if_else); } }, IfLetOrMatch::Match(match_expr, arms, source) => { @@ -120,9 +112,10 @@ impl<'tcx> LateLintPass<'tcx> for ManualLetElse { return; } - emit_manual_let_else(cx, stmt.span, match_expr, pat_arm.pat, diverging_arm.body); + emit_manual_let_else(cx, stmt.span, match_expr, local.pat, diverging_arm.body); }, } + }; } extract_msrv_attr!(LateContext); @@ -151,12 +144,7 @@ fn emit_manual_let_else(cx: &LateContext<'_>, span: Span, expr: &Expr<'_>, pat: } else { format!("{{ {sn_else} }}") }; - 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};"); + let sugg = format!("let Some({sn_pat}) = {sn_expr} else {else_bl};"); diag.span_suggestion(span, "consider writing", sugg, app); }, ); diff --git a/tests/ui/manual_let_else.stderr b/tests/ui/manual_let_else.stderr index 52aac6bc673d..07135ee67708 100644 --- a/tests/ui/manual_let_else.stderr +++ b/tests/ui/manual_let_else.stderr @@ -2,7 +2,7 @@ error: this could be rewritten as `let...else` --> $DIR/manual_let_else.rs:18:5 | LL | let v = if let Some(v_some) = g() { v_some } else { return }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some(v_some) = g() else { return };` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some(v) = g() else { return };` | = note: `-D clippy::manual-let-else` implied by `-D warnings` @@ -18,7 +18,7 @@ LL | | }; | help: consider writing | -LL ~ let Some(v_some) = g() else { +LL ~ let Some(v) = g() else { LL + return; LL + }; | @@ -48,19 +48,19 @@ error: this could be rewritten as `let...else` --> $DIR/manual_let_else.rs:38:9 | LL | let v = if let Some(v_some) = g() { v_some } else { continue }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some(v_some) = g() else { continue };` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some(v) = g() else { continue };` error: this could be rewritten as `let...else` --> $DIR/manual_let_else.rs:39:9 | LL | let v = if let Some(v_some) = g() { v_some } else { break }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some(v_some) = g() else { break };` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some(v) = g() else { break };` error: this could be rewritten as `let...else` --> $DIR/manual_let_else.rs:43:5 | LL | let v = if let Some(v_some) = g() { v_some } else { panic!() }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some(v_some) = g() else { panic!() };` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some(v) = g() else { panic!() };` error: this could be rewritten as `let...else` --> $DIR/manual_let_else.rs:46:5 @@ -74,7 +74,7 @@ LL | | }; | help: consider writing | -LL ~ let Some(v_some) = g() else { +LL ~ let Some(v) = g() else { LL + std::process::abort() LL + }; | @@ -91,7 +91,7 @@ LL | | }; | help: consider writing | -LL ~ let Some(v_some) = g() else { +LL ~ let Some(v) = g() else { LL + if true { return } else { panic!() } LL + }; | @@ -109,7 +109,7 @@ LL | | }; | help: consider writing | -LL ~ let Some(v_some) = g() else { +LL ~ let Some(v) = g() else { LL + if true {} LL + panic!(); LL + }; @@ -129,7 +129,7 @@ LL | | }; | help: consider writing | -LL ~ let Some(v_some) = g() else { +LL ~ let Some(v) = g() else { LL + match () { LL + _ if panic!() => {}, LL + _ => panic!(), @@ -141,7 +141,7 @@ error: this could be rewritten as `let...else` --> $DIR/manual_let_else.rs:80:5 | LL | let v = if let Some(v_some) = g() { v_some } else { if panic!() {} }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some(v_some) = g() else { if panic!() {} };` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some(v) = g() else { if panic!() {} };` error: this could be rewritten as `let...else` --> $DIR/manual_let_else.rs:83:5 @@ -157,7 +157,7 @@ LL | | }; | help: consider writing | -LL ~ let Some(v_some) = g() else { +LL ~ let Some(v) = g() else { LL + match panic!() { LL + _ => {}, LL + } @@ -178,7 +178,7 @@ LL | | }; | help: consider writing | -LL ~ let Some(v_some) = g() else { if true { +LL ~ let Some(v) = g() else { if true { LL + return; LL + } else { LL + panic!("diverge"); @@ -199,7 +199,7 @@ LL | | }; | help: consider writing | -LL ~ let Some(v_some) = g() else { +LL ~ let Some(v) = g() else { LL + match (g(), g()) { LL + (Some(_), None) => return, LL + (None, Some(_)) => { @@ -226,7 +226,7 @@ LL | | }; | help: consider writing | -LL ~ let Some(v_some) = g().map(|v| (v, 42)) else { +LL ~ let Some((v, w)) = g().map(|v| (v, 42)) else { LL + return; LL + }; | @@ -243,7 +243,7 @@ LL | | }; | help: consider writing | -LL ~ let (Some(v_some), w_some) = (g(), 0) else { +LL ~ let Some(v) = (g(), 0) else { LL + return; LL + }; | @@ -252,7 +252,7 @@ error: this could be rewritten as `let...else` --> $DIR/manual_let_else.rs:134:13 | LL | let $n = if let Some(v) = $e { v } else { return }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some(v) = g() else { return };` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some($n) = g() else { return };` ... LL | create_binding_if_some!(w, g()); | ------------------------------- in this macro invocation @@ -266,7 +266,7 @@ LL | / let _ = match ff { LL | | Some(value) => value, LL | | _ => macro_call!(), LL | | }; - | |______^ help: consider writing: `let Some(value) = ff else { macro_call!() };` + | |______^ help: consider writing: `let Some(_) = ff else { macro_call!() };` error: aborting due to 18 previous errors diff --git a/tests/ui/manual_let_else_match.stderr b/tests/ui/manual_let_else_match.stderr index cd5e9a9ac39c..3fa1a8d8f8f3 100644 --- a/tests/ui/manual_let_else_match.stderr +++ b/tests/ui/manual_let_else_match.stderr @@ -5,7 +5,7 @@ LL | / let v = match g() { LL | | Some(v_some) => v_some, LL | | None => return, LL | | }; - | |______^ help: consider writing: `let Some(v_some) = g() else { return };` + | |______^ help: consider writing: `let Some(v) = g() else { return };` | = note: `-D clippy::manual-let-else` implied by `-D warnings` @@ -16,7 +16,7 @@ LL | / let v = match g() { LL | | Some(v_some) => v_some, LL | | _ => return, LL | | }; - | |______^ help: consider writing: `let Some(v_some) = g() else { return };` + | |______^ help: consider writing: `let Some(v) = g() else { return };` error: this could be rewritten as `let...else` --> $DIR/manual_let_else_match.rs:44:9 @@ -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) = 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 Some(v) = build_enum() else { continue };` error: this could be rewritten as `let...else` --> $DIR/manual_let_else_match.rs:57:5 @@ -43,7 +43,7 @@ LL | / let v = match f() { LL | | Ok(v) => v, LL | | Err(_) => return, LL | | }; - | |______^ help: consider writing: `let Ok(v) = f() else { return };` + | |______^ help: consider writing: `let Some(v) = f() else { return };` error: this could be rewritten as `let...else` --> $DIR/manual_let_else_match.rs:63:5 @@ -52,7 +52,7 @@ LL | / let v = match f().map_err(|_| ()) { LL | | Ok(v) => v, LL | | Err(()) => return, LL | | }; - | |______^ help: consider writing: `let Ok(v) = f().map_err(|_| ()) else { return };` + | |______^ help: consider writing: `let Some(v) = f().map_err(|_| ()) else { return };` error: this could be rewritten as `let...else` --> $DIR/manual_let_else_match.rs:70:5 @@ -61,7 +61,7 @@ LL | / let _value = match f { LL | | Variant::Bar(_) | Variant::Baz(_) => (), LL | | _ => return, LL | | }; - | |______^ help: consider writing: `let (Variant::Bar(_) | Variant::Baz(_)) = f else { return };` + | |______^ help: consider writing: `let Some(_value) = f else { return };` error: aborting due to 7 previous errors