From febe5491133b4b56d6c798552b96f5edc5e09479 Mon Sep 17 00:00:00 2001 From: blyxyas Date: Sat, 2 Nov 2024 18:29:54 +0100 Subject: [PATCH] Fix needless_match FP on if-lets --- clippy_lints/src/matches/needless_match.rs | 6 ++- tests/ui/needless_match.fixed | 53 +++++++++++++++++++ tests/ui/needless_match.rs | 61 ++++++++++++++++++++++ tests/ui/needless_match.stderr | 14 ++++- 4 files changed, 131 insertions(+), 3 deletions(-) diff --git a/clippy_lints/src/matches/needless_match.rs b/clippy_lints/src/matches/needless_match.rs index 6f7d69026404..73822314b4b4 100644 --- a/clippy_lints/src/matches/needless_match.rs +++ b/clippy_lints/src/matches/needless_match.rs @@ -3,7 +3,7 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::snippet_with_applicability; use clippy_utils::ty::{is_type_diagnostic_item, same_type_and_consts}; use clippy_utils::{ - eq_expr_value, get_parent_expr_for_hir, higher, is_else_clause, is_res_lang_ctor, over, path_res, + SpanlessEq, eq_expr_value, get_parent_expr_for_hir, higher, is_else_clause, is_res_lang_ctor, over, path_res, peel_blocks_with_stmt, }; use rustc_errors::Applicability; @@ -90,7 +90,9 @@ fn check_if_let_inner(cx: &LateContext<'_>, if_let: &higher::IfLet<'_>) -> bool } // Recursively check for each `else if let` phrase, - if let Some(ref nested_if_let) = higher::IfLet::hir(cx, if_else) { + if let Some(ref nested_if_let) = higher::IfLet::hir(cx, if_else) + && SpanlessEq::new(cx).eq_expr(nested_if_let.let_expr, if_let.let_expr) + { return check_if_let_inner(cx, nested_if_let); } diff --git a/tests/ui/needless_match.fixed b/tests/ui/needless_match.fixed index a936eb463f96..06c6169d0da0 100644 --- a/tests/ui/needless_match.fixed +++ b/tests/ui/needless_match.fixed @@ -245,4 +245,57 @@ mod issue9084 { } } +fn a() -> Option<()> { + Some(()) +} +fn b() -> Option<()> { + Some(()) +} +fn c() -> Option<()> { + Some(()) +} + +#[allow(clippy::ifs_same_cond)] +pub fn issue13574() -> Option<()> { + // Do not lint. + // The right hand of all these arms are different functions. + let _ = { + if let Some(a) = a() { + Some(a) + } else if let Some(b) = b() { + Some(b) + } else if let Some(c) = c() { + Some(c) + } else { + None + } + }; + + const A: Option<()> = Some(()); + const B: Option<()> = Some(()); + const C: Option<()> = Some(()); + const D: Option<()> = Some(()); + + let _ = { + if let Some(num) = A { + Some(num) + } else if let Some(num) = B { + Some(num) + } else if let Some(num) = C { + Some(num) + } else if let Some(num) = D { + Some(num) + } else { + None + } + }; + + // Same const, should lint + let _ = { + A + }; + + None +} + fn main() {} diff --git a/tests/ui/needless_match.rs b/tests/ui/needless_match.rs index b1dd6ff075d8..6b71de68e1be 100644 --- a/tests/ui/needless_match.rs +++ b/tests/ui/needless_match.rs @@ -289,4 +289,65 @@ mod issue9084 { } } +fn a() -> Option<()> { + Some(()) +} +fn b() -> Option<()> { + Some(()) +} +fn c() -> Option<()> { + Some(()) +} + +#[allow(clippy::ifs_same_cond)] +pub fn issue13574() -> Option<()> { + // Do not lint. + // The right hand of all these arms are different functions. + let _ = { + if let Some(a) = a() { + Some(a) + } else if let Some(b) = b() { + Some(b) + } else if let Some(c) = c() { + Some(c) + } else { + None + } + }; + + const A: Option<()> = Some(()); + const B: Option<()> = Some(()); + const C: Option<()> = Some(()); + const D: Option<()> = Some(()); + + let _ = { + if let Some(num) = A { + Some(num) + } else if let Some(num) = B { + Some(num) + } else if let Some(num) = C { + Some(num) + } else if let Some(num) = D { + Some(num) + } else { + None + } + }; + + // Same const, should lint + let _ = { + if let Some(num) = A { + Some(num) + } else if let Some(num) = A { + Some(num) + } else if let Some(num) = A { + Some(num) + } else { + None + } + }; + + None +} + fn main() {} diff --git a/tests/ui/needless_match.stderr b/tests/ui/needless_match.stderr index 5bcab467aeae..1410585cb2e3 100644 --- a/tests/ui/needless_match.stderr +++ b/tests/ui/needless_match.stderr @@ -131,5 +131,17 @@ LL | | _ => e, LL | | }; | |_________^ help: replace it with: `e` -error: aborting due to 13 previous errors +error: this if-let expression is unnecessary + --> tests/ui/needless_match.rs:339:9 + | +LL | / if let Some(num) = A { +LL | | Some(num) +LL | | } else if let Some(num) = A { +LL | | Some(num) +... | +LL | | None +LL | | } + | |_________^ help: replace it with: `A` + +error: aborting due to 14 previous errors