Fix needless_match FP on if-lets

rust-lang · Nov 14, 2024 · febe549 · febe549
1 parent 1bdc08a
commit febe549
Show file tree

Hide file tree

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