Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix needless_match FP on if-lets #13646

Merged
merged 1 commit into from
Dec 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions clippy_lints/src/matches/needless_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}

Expand Down
53 changes: 53 additions & 0 deletions tests/ui/needless_match.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -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() {}
61 changes: 61 additions & 0 deletions tests/ui/needless_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {}
14 changes: 13 additions & 1 deletion tests/ui/needless_match.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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