diff options
| -rw-r--r-- | clippy_lints/src/manual_let_else.rs | 61 | ||||
| -rw-r--r-- | tests/ui/manual_let_else.rs | 15 | ||||
| -rw-r--r-- | tests/ui/manual_let_else.stderr | 82 | ||||
| -rw-r--r-- | tests/ui/manual_let_else_match.stderr | 4 |
4 files changed, 100 insertions, 62 deletions
diff --git a/clippy_lints/src/manual_let_else.rs b/clippy_lints/src/manual_let_else.rs index 1247370b74a..3f8b42ffe80 100644 --- a/clippy_lints/src/manual_let_else.rs +++ b/clippy_lints/src/manual_let_else.rs @@ -38,7 +38,6 @@ declare_clippy_lint! { /// Could be written: /// /// ```rust - /// # #![feature(let_else)] /// # fn main () { /// # let w = Some(0); /// let Some(v) = w else { return }; @@ -69,29 +68,23 @@ 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) { + return; + } + if 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, let_pat, if_else); } }, IfLetOrMatch::Match(match_expr, arms, source) => { @@ -128,15 +121,23 @@ 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, pat_arm.pat, diverging_arm.body); }, } + }; } extract_msrv_attr!(LateContext); } -fn emit_manual_let_else(cx: &LateContext<'_>, span: Span, expr: &Expr<'_>, pat: &Pat<'_>, else_body: &Expr<'_>) { +fn emit_manual_let_else( + cx: &LateContext<'_>, + span: Span, + expr: &Expr<'_>, + local: &Pat<'_>, + pat: &Pat<'_>, + else_body: &Expr<'_>, +) { span_lint_and_then( cx, MANUAL_LET_ELSE, @@ -145,12 +146,11 @@ fn emit_manual_let_else(cx: &LateContext<'_>, span: Span, expr: &Expr<'_>, pat: |diag| { // This is far from perfect, for example there needs to be: // * mut additions for the bindings - // * renamings of the bindings + // * renamings of the bindings for `PatKind::Or` // * unused binding collision detection with existing ones // * putting patterns with at the top level | inside () // for this to be machine applicable. let mut app = Applicability::HasPlaceholders; - let (sn_pat, _) = snippet_with_context(cx, pat.span, span.ctxt(), "", &mut app); let (sn_expr, _) = snippet_with_context(cx, expr.span, span.ctxt(), "", &mut app); let (sn_else, _) = snippet_with_context(cx, else_body.span, span.ctxt(), "", &mut app); @@ -159,10 +159,21 @@ 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 sn_bl = match pat.kind { + PatKind::Or(..) => { + let (sn_pat, _) = snippet_with_context(cx, pat.span, span.ctxt(), "", &mut app); + format!("({sn_pat})") + }, + // Replace the variable name iff `TupleStruct` has one argument like `Variant(v)`. + PatKind::TupleStruct(ref w, args, ..) if args.len() == 1 => { + let sn_wrapper = cx.sess().source_map().span_to_snippet(w.span()).unwrap_or_default(); + let (sn_inner, _) = snippet_with_context(cx, local.span, span.ctxt(), "", &mut app); + format!("{sn_wrapper}({sn_inner})") + }, + _ => { + let (sn_pat, _) = snippet_with_context(cx, pat.span, span.ctxt(), "", &mut app); + 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.rs b/tests/ui/manual_let_else.rs index d175597a44a..3996d775f55 100644 --- a/tests/ui/manual_let_else.rs +++ b/tests/ui/manual_let_else.rs @@ -8,6 +8,12 @@ )] #![warn(clippy::manual_let_else)] +enum Variant { + A(usize, usize), + B(usize), + C, +} + fn g() -> Option<()> { None } @@ -135,6 +141,15 @@ fn fire() { }; } create_binding_if_some!(w, g()); + + fn e() -> Variant { + Variant::A(0, 0) + } + + // Should not be renamed + let v = if let Variant::A(a, 0) = e() { a } else { return }; + // Should be renamed + let v = if let Variant::B(b) = e() { b } else { return }; } fn not_fire() { diff --git a/tests/ui/manual_let_else.stderr b/tests/ui/manual_let_else.stderr index 52aac6bc673..f6f56f7b00e 100644 --- a/tests/ui/manual_let_else.stderr +++ b/tests/ui/manual_let_else.stderr @@ -1,13 +1,13 @@ error: this could be rewritten as `let...else` - --> $DIR/manual_let_else.rs:18:5 + --> $DIR/manual_let_else.rs:24: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` error: this could be rewritten as `let...else` - --> $DIR/manual_let_else.rs:19:5 + --> $DIR/manual_let_else.rs:25:5 | LL | / let v = if let Some(v_some) = g() { LL | | v_some @@ -18,13 +18,13 @@ LL | | }; | help: consider writing | -LL ~ let Some(v_some) = g() else { +LL ~ let Some(v) = g() else { LL + return; LL + }; | error: this could be rewritten as `let...else` - --> $DIR/manual_let_else.rs:25:5 + --> $DIR/manual_let_else.rs:31:5 | LL | / let v = if let Some(v) = g() { LL | | // Blocks around the identity should have no impact @@ -45,25 +45,25 @@ LL + }; | error: this could be rewritten as `let...else` - --> $DIR/manual_let_else.rs:38:9 + --> $DIR/manual_let_else.rs:44: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 + --> $DIR/manual_let_else.rs:45: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 + --> $DIR/manual_let_else.rs:49: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 + --> $DIR/manual_let_else.rs:52:5 | LL | / let v = if let Some(v_some) = g() { LL | | v_some @@ -74,13 +74,13 @@ LL | | }; | help: consider writing | -LL ~ let Some(v_some) = g() else { +LL ~ let Some(v) = g() else { LL + std::process::abort() LL + }; | error: this could be rewritten as `let...else` - --> $DIR/manual_let_else.rs:53:5 + --> $DIR/manual_let_else.rs:59:5 | LL | / let v = if let Some(v_some) = g() { LL | | v_some @@ -91,13 +91,13 @@ LL | | }; | help: consider writing | -LL ~ let Some(v_some) = g() else { +LL ~ let Some(v) = g() else { LL + if true { return } else { panic!() } LL + }; | error: this could be rewritten as `let...else` - --> $DIR/manual_let_else.rs:60:5 + --> $DIR/manual_let_else.rs:66:5 | LL | / let v = if let Some(v_some) = g() { LL | | v_some @@ -109,14 +109,14 @@ LL | | }; | help: consider writing | -LL ~ let Some(v_some) = g() else { +LL ~ let Some(v) = g() else { LL + if true {} LL + panic!(); LL + }; | error: this could be rewritten as `let...else` - --> $DIR/manual_let_else.rs:70:5 + --> $DIR/manual_let_else.rs:76:5 | LL | / let v = if let Some(v_some) = g() { LL | | v_some @@ -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!(), @@ -138,13 +138,13 @@ LL + }; | error: this could be rewritten as `let...else` - --> $DIR/manual_let_else.rs:80:5 + --> $DIR/manual_let_else.rs:86: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 + --> $DIR/manual_let_else.rs:89:5 | LL | / let v = if let Some(v_some) = g() { LL | | v_some @@ -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 + } @@ -165,7 +165,7 @@ LL + }; | error: this could be rewritten as `let...else` - --> $DIR/manual_let_else.rs:92:5 + --> $DIR/manual_let_else.rs:98:5 | LL | / let v = if let Some(v_some) = g() { LL | | v_some @@ -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"); @@ -186,7 +186,7 @@ LL + } }; | error: this could be rewritten as `let...else` - --> $DIR/manual_let_else.rs:101:5 + --> $DIR/manual_let_else.rs:107:5 | LL | / let v = if let Some(v_some) = g() { LL | | v_some @@ -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(_)) => { @@ -215,7 +215,7 @@ LL + }; | error: this could be rewritten as `let...else` - --> $DIR/manual_let_else.rs:118:5 + --> $DIR/manual_let_else.rs:124:5 | LL | / let (v, w) = if let Some(v_some) = g().map(|v| (v, 42)) { LL | | v_some @@ -226,13 +226,13 @@ 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 + }; | error: this could be rewritten as `let...else` - --> $DIR/manual_let_else.rs:125:5 + --> $DIR/manual_let_else.rs:131:5 | LL | / let v = if let (Some(v_some), w_some) = (g(), 0) { LL | | (w_some, v_some) @@ -249,10 +249,10 @@ LL + }; | error: this could be rewritten as `let...else` - --> $DIR/manual_let_else.rs:134:13 + --> $DIR/manual_let_else.rs:140: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 @@ -260,13 +260,25 @@ LL | create_binding_if_some!(w, g()); = note: this error originates in the macro `create_binding_if_some` (in Nightly builds, run with -Z macro-backtrace for more info) error: this could be rewritten as `let...else` - --> $DIR/manual_let_else.rs:247:5 + --> $DIR/manual_let_else.rs:150:5 + | +LL | let v = if let Variant::A(a, 0) = e() { a } else { return }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Variant::A(a, 0) = e() else { return };` + +error: this could be rewritten as `let...else` + --> $DIR/manual_let_else.rs:152:5 + | +LL | let v = if let Variant::B(b) = e() { b } else { return }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Variant::B(v) = e() else { return };` + +error: this could be rewritten as `let...else` + --> $DIR/manual_let_else.rs:262:5 | 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 +error: aborting due to 20 previous errors diff --git a/tests/ui/manual_let_else_match.stderr b/tests/ui/manual_let_else_match.stderr index 7abaa0b85d2..bacc14dc967 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 |