Use original variable name in the suggestion

rust-lang · Jan 7, 2023 · aea0082 · aea0082
1 parent ef5a545
commit aea0082
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 48 deletions.
diff --git 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
@@ -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
@@ -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