From ec3d1c8ca31c392e50c3605d461a9ee6f929c97c Mon Sep 17 00:00:00 2001 From: dswij Date: Tue, 16 Nov 2021 13:39:52 +0800 Subject: [PATCH] Fix FP on `if_then_some_else_none` when there is early return --- clippy_lints/src/if_then_some_else_none.rs | 13 +++++++++++-- tests/ui/if_then_some_else_none.rs | 11 +++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/if_then_some_else_none.rs b/clippy_lints/src/if_then_some_else_none.rs index d95f5d4107165..30d222bd7d27d 100644 --- a/clippy_lints/src/if_then_some_else_none.rs +++ b/clippy_lints/src/if_then_some_else_none.rs @@ -1,9 +1,9 @@ use clippy_utils::diagnostics::span_lint_and_help; use clippy_utils::source::snippet_with_macro_callsite; -use clippy_utils::{higher, is_else_clause, is_lang_ctor, meets_msrv, msrvs}; +use clippy_utils::{contains_return, higher, is_else_clause, is_lang_ctor, meets_msrv, msrvs}; use if_chain::if_chain; use rustc_hir::LangItem::{OptionNone, OptionSome}; -use rustc_hir::{Expr, ExprKind}; +use rustc_hir::{Expr, ExprKind, Stmt, StmtKind}; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::lint::in_external_macro; use rustc_semver::RustcVersion; @@ -82,6 +82,7 @@ impl LateLintPass<'_> for IfThenSomeElseNone { if let Some(els_expr) = els_block.expr; if let ExprKind::Path(ref qpath) = els_expr.kind; if is_lang_ctor(cx, qpath, OptionNone); + if !stmts_contains_early_return(then_block.stmts); then { let cond_snip = snippet_with_macro_callsite(cx, cond.span, "[condition]"); let cond_snip = if matches!(cond.kind, ExprKind::Unary(_, _) | ExprKind::Binary(_, _, _)) { @@ -114,3 +115,11 @@ impl LateLintPass<'_> for IfThenSomeElseNone { extract_msrv_attr!(LateContext); } + +fn stmts_contains_early_return(stmts: &[Stmt<'_>]) -> bool { + stmts.iter().any(|stmt| { + let Stmt { kind: StmtKind::Semi(e), .. } = stmt else { return false }; + + contains_return(e) + }) +} diff --git a/tests/ui/if_then_some_else_none.rs b/tests/ui/if_then_some_else_none.rs index 54789bf93209d..3bc3a0395244c 100644 --- a/tests/ui/if_then_some_else_none.rs +++ b/tests/ui/if_then_some_else_none.rs @@ -102,3 +102,14 @@ fn into_some(v: T) -> Option { fn into_none() -> Option { None } + +// Should not warn +fn f(b: bool, v: Option<()>) -> Option<()> { + if b { + v?; // This is a potential early return, is not equivalent with `bool::then` + + Some(()) + } else { + None + } +}