diff --git a/clippy_lints/src/manual_let_else.rs b/clippy_lints/src/manual_let_else.rs index 8de8f7e16f91..6869cfc9cf56 100644 --- a/clippy_lints/src/manual_let_else.rs +++ b/clippy_lints/src/manual_let_else.rs @@ -8,6 +8,7 @@ use rustc_hir::{Expr, ExprKind, MatchSource, Pat, QPath, Stmt, StmtKind}; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::lint::in_external_macro; use rustc_semver::RustcVersion; +use rustc_data_structures::stable_set::FxHashSet; use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::Span; @@ -206,20 +207,33 @@ fn pat_has_no_bindings(pat: &'_ Pat<'_>) -> bool { /// Checks if the passed block is a simple identity referring to bindings created by the pattern fn expr_is_simple_identity(pat: &'_ Pat<'_>, expr: &'_ Expr<'_>) -> bool { - // TODO support patterns with multiple bindings and tuples, like: + // We support patterns with multiple bindings and tuples, like: // let ... = if let (Some(foo), bar) = g() { (foo, bar) } else { ... } - if_chain! { - if let ExprKind::Path(QPath::Resolved(_ty, path)) = &peel_blocks(expr).kind; - if let [path_seg] = path.segments; - then { - let mut pat_bindings = Vec::new(); - pat.each_binding_or_first(&mut |_ann, _hir_id, _sp, ident| { - pat_bindings.push(ident); - }); - if let [binding] = &pat_bindings[..] { - return path_seg.ident == *binding; + let peeled = peel_blocks(expr); + let paths = match peeled.kind { + ExprKind::Tup(exprs) | ExprKind::Array(exprs) => exprs, + ExprKind::Path(_) => std::slice::from_ref(peeled), + _ => return false, + }; + let mut pat_bindings = FxHashSet::default(); + pat.each_binding_or_first(&mut |_ann, _hir_id, _sp, ident| { + pat_bindings.insert(ident); + }); + if pat_bindings.len() < paths.len() { + return false; + } + for path in paths { + if_chain! { + if let ExprKind::Path(QPath::Resolved(_ty, path)) = path.kind; + if let [path_seg] = path.segments; + then { + if !pat_bindings.remove(&path_seg.ident) { + return false; + } + } else { + return false; } } } - false + true } diff --git a/tests/ui/manual_let_else.rs b/tests/ui/manual_let_else.rs index 7296ba24c395..480bd22d7467 100644 --- a/tests/ui/manual_let_else.rs +++ b/tests/ui/manual_let_else.rs @@ -83,18 +83,20 @@ fn fire() { return; }; + // Tuples supported for the identity block and pattern + let v = if let (Some(v_some), w_some) = (g(), 0) { + (w_some, v_some) + } else { + return; + }; + // entirely inside macro lints macro_rules! create_binding_if_some { ($n:ident, $e:expr) => { - let $n = if let Some(v) = $e { - v - } else { - return - }; + let $n = if let Some(v) = $e { v } else { return }; }; } create_binding_if_some!(w, g()); - } fn not_fire() { @@ -164,11 +166,7 @@ fn not_fire() { // Macro boundary inside let macro_rules! some_or_return { ($e:expr) => { - if let Some(v) = $e { - v - } else { - return - } + if let Some(v) = $e { v } else { return } }; } let v = some_or_return!(g()); @@ -180,4 +178,4 @@ fn not_fire() { }; } create_binding_if_some!(v, g()); -} \ No newline at end of file +} diff --git a/tests/ui/manual_let_else.stderr b/tests/ui/manual_let_else.stderr index 22fc7515ce92..63bfaddfda2b 100644 --- a/tests/ui/manual_let_else.stderr +++ b/tests/ui/manual_let_else.stderr @@ -101,19 +101,25 @@ LL | | }; | |______^ error: this could be rewritten as `let else` - --> $DIR/manual_let_else.rs:89:13 + --> $DIR/manual_let_else.rs:87:5 | -LL | / let $n = if let Some(v) = $e { -LL | | v -LL | | } else { -LL | | return -LL | | }; - | |______________^ +LL | / let v = if let (Some(v_some), w_some) = (g(), 0) { +LL | | (w_some, v_some) +LL | | } else { +LL | | return; +LL | | }; + | |______^ + +error: this could be rewritten as `let else` + --> $DIR/manual_let_else.rs:96:13 + | +LL | let $n = if let Some(v) = $e { v } else { return }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ... -LL | create_binding_if_some!(w, g()); - | ------------------------------- in this macro invocation +LL | create_binding_if_some!(w, g()); + | ------------------------------- in this macro invocation | = note: this error originates in the macro `create_binding_if_some` (in Nightly builds, run with -Z macro-backtrace for more info) -error: aborting due to 12 previous errors +error: aborting due to 13 previous errors