Skip to content

Commit 4118738

Browse files
committed
Auto merge of #11401 - y21:issue11394, r=xFrednet
[`if_then_some_else_none`]: look into local initializers for early returns Fixes #11394 As the PR title says, problem was that it only looked for early returns in semi statements. Local variables don't count as such, so it didn't count `let _v = x?;` (or even just `let _ = return;`) as a possible early return and didn't realize that it can't lint then. Imo the `stmts_contains_early_return` function that was used before is redundant. `contains_return` could already do that if we just made the parameter a bit more generic, just like `for_each_expr`, which can already accept `&[Stmt]` changelog: [`if_then_some_else_none`]: look into local initializers for early returns
2 parents 8de52e5 + dba7763 commit 4118738

File tree

3 files changed

+16
-17
lines changed

3 files changed

+16
-17
lines changed

clippy_lints/src/if_then_some_else_none.rs

+2-16
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use clippy_utils::sugg::Sugg;
66
use clippy_utils::{contains_return, higher, is_else_clause, is_res_lang_ctor, path_res, peel_blocks};
77
use rustc_errors::Applicability;
88
use rustc_hir::LangItem::{OptionNone, OptionSome};
9-
use rustc_hir::{Expr, ExprKind, Stmt, StmtKind};
9+
use rustc_hir::{Expr, ExprKind};
1010
use rustc_lint::{LateContext, LateLintPass, LintContext};
1111
use rustc_middle::lint::in_external_macro;
1212
use rustc_session::{declare_tool_lint, impl_lint_pass};
@@ -83,7 +83,7 @@ impl<'tcx> LateLintPass<'tcx> for IfThenSomeElseNone {
8383
&& then_expr.span.ctxt() == ctxt
8484
&& is_res_lang_ctor(cx, path_res(cx, then_call), OptionSome)
8585
&& is_res_lang_ctor(cx, path_res(cx, peel_blocks(els)), OptionNone)
86-
&& !stmts_contains_early_return(then_block.stmts)
86+
&& !contains_return(then_block.stmts)
8787
{
8888
let mut app = Applicability::Unspecified;
8989
let cond_snip = Sugg::hir_with_context(cx, cond, expr.span.ctxt(), "[condition]", &mut app).maybe_par().to_string();
@@ -116,17 +116,3 @@ impl<'tcx> LateLintPass<'tcx> for IfThenSomeElseNone {
116116

117117
extract_msrv_attr!(LateContext);
118118
}
119-
120-
fn stmts_contains_early_return(stmts: &[Stmt<'_>]) -> bool {
121-
stmts.iter().any(|stmt| {
122-
let Stmt {
123-
kind: StmtKind::Semi(e),
124-
..
125-
} = stmt
126-
else {
127-
return false;
128-
};
129-
130-
contains_return(e)
131-
})
132-
}

clippy_utils/src/lib.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ use rustc_span::source_map::SourceMap;
111111
use rustc_span::symbol::{kw, Ident, Symbol};
112112
use rustc_span::{sym, Span};
113113
use rustc_target::abi::Integer;
114+
use visitors::Visitable;
114115

115116
use crate::consts::{constant, miri_to_const, Constant};
116117
use crate::higher::Range;
@@ -1287,7 +1288,7 @@ pub fn contains_name<'tcx>(name: Symbol, expr: &'tcx Expr<'_>, cx: &LateContext<
12871288
}
12881289

12891290
/// Returns `true` if `expr` contains a return expression
1290-
pub fn contains_return(expr: &hir::Expr<'_>) -> bool {
1291+
pub fn contains_return<'tcx>(expr: impl Visitable<'tcx>) -> bool {
12911292
for_each_expr(expr, |e| {
12921293
if matches!(e.kind, hir::ExprKind::Ret(..)) {
12931294
ControlFlow::Break(())

tests/ui/if_then_some_else_none.rs

+12
Original file line numberDiff line numberDiff line change
@@ -117,3 +117,15 @@ fn f(b: bool, v: Option<()>) -> Option<()> {
117117
None
118118
}
119119
}
120+
121+
fn issue11394(b: bool, v: Result<(), ()>) -> Result<(), ()> {
122+
let x = if b {
123+
#[allow(clippy::let_unit_value)]
124+
let _ = v?;
125+
Some(())
126+
} else {
127+
None
128+
};
129+
130+
Ok(())
131+
}

0 commit comments

Comments
 (0)