Skip to content

Commit

Permalink
Fix FP on if_then_some_else_none when there is early return
Browse files Browse the repository at this point in the history
  • Loading branch information
dswij committed Nov 16, 2021
1 parent 5b1b65b commit 4151fe3
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 4 deletions.
17 changes: 13 additions & 4 deletions clippy_lints/src/if_then_some_else_none.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -73,6 +73,7 @@ impl LateLintPass<'_> for IfThenSomeElseNone {
if_chain! {
if let Some(higher::If { cond, then, r#else: Some(els) }) = higher::If::hir(expr);
if let ExprKind::Block(then_block, _) = then.kind;
if !stmts_contains_early_return(then_block.stmts);
if let Some(then_expr) = then_block.expr;
if let ExprKind::Call(then_call, [then_arg]) = then_expr.kind;
if let ExprKind::Path(ref then_call_qpath) = then_call.kind;
Expand All @@ -99,18 +100,26 @@ impl LateLintPass<'_> for IfThenSomeElseNone {
"consider using `bool::then` like: `{}.then(|| {})`",
cond_snip,
closure_body,
);
);
span_lint_and_help(
cx,
IF_THEN_SOME_ELSE_NONE,
expr.span,
"this could be simplified with `bool::then`",
None,
&help,
);
);
}
}
}

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)
})
}
11 changes: 11 additions & 0 deletions tests/ui/if_then_some_else_none.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,3 +102,14 @@ fn into_some<T>(v: T) -> Option<T> {
fn into_none<T>() -> Option<T> {
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
}
}

0 comments on commit 4151fe3

Please sign in to comment.