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
This commit is contained in:
commit
4118738998
@ -6,7 +6,7 @@
|
|||||||
use clippy_utils::{contains_return, higher, is_else_clause, is_res_lang_ctor, path_res, peel_blocks};
|
use clippy_utils::{contains_return, higher, is_else_clause, is_res_lang_ctor, path_res, peel_blocks};
|
||||||
use rustc_errors::Applicability;
|
use rustc_errors::Applicability;
|
||||||
use rustc_hir::LangItem::{OptionNone, OptionSome};
|
use rustc_hir::LangItem::{OptionNone, OptionSome};
|
||||||
use rustc_hir::{Expr, ExprKind, Stmt, StmtKind};
|
use rustc_hir::{Expr, ExprKind};
|
||||||
use rustc_lint::{LateContext, LateLintPass, LintContext};
|
use rustc_lint::{LateContext, LateLintPass, LintContext};
|
||||||
use rustc_middle::lint::in_external_macro;
|
use rustc_middle::lint::in_external_macro;
|
||||||
use rustc_session::{declare_tool_lint, impl_lint_pass};
|
use rustc_session::{declare_tool_lint, impl_lint_pass};
|
||||||
@ -83,7 +83,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
|
|||||||
&& then_expr.span.ctxt() == ctxt
|
&& then_expr.span.ctxt() == ctxt
|
||||||
&& is_res_lang_ctor(cx, path_res(cx, then_call), OptionSome)
|
&& is_res_lang_ctor(cx, path_res(cx, then_call), OptionSome)
|
||||||
&& is_res_lang_ctor(cx, path_res(cx, peel_blocks(els)), OptionNone)
|
&& is_res_lang_ctor(cx, path_res(cx, peel_blocks(els)), OptionNone)
|
||||||
&& !stmts_contains_early_return(then_block.stmts)
|
&& !contains_return(then_block.stmts)
|
||||||
{
|
{
|
||||||
let mut app = Applicability::Unspecified;
|
let mut app = Applicability::Unspecified;
|
||||||
let cond_snip = Sugg::hir_with_context(cx, cond, expr.span.ctxt(), "[condition]", &mut app).maybe_par().to_string();
|
let cond_snip = Sugg::hir_with_context(cx, cond, expr.span.ctxt(), "[condition]", &mut app).maybe_par().to_string();
|
||||||
@ -116,17 +116,3 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
|
|||||||
|
|
||||||
extract_msrv_attr!(LateContext);
|
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)
|
|
||||||
})
|
|
||||||
}
|
|
||||||
|
@ -111,6 +111,7 @@
|
|||||||
use rustc_span::symbol::{kw, Ident, Symbol};
|
use rustc_span::symbol::{kw, Ident, Symbol};
|
||||||
use rustc_span::{sym, Span};
|
use rustc_span::{sym, Span};
|
||||||
use rustc_target::abi::Integer;
|
use rustc_target::abi::Integer;
|
||||||
|
use visitors::Visitable;
|
||||||
|
|
||||||
use crate::consts::{constant, miri_to_const, Constant};
|
use crate::consts::{constant, miri_to_const, Constant};
|
||||||
use crate::higher::Range;
|
use crate::higher::Range;
|
||||||
@ -1287,7 +1288,7 @@ pub fn contains_name<'tcx>(name: Symbol, expr: &'tcx Expr<'_>, cx: &LateContext<
|
|||||||
}
|
}
|
||||||
|
|
||||||
/// Returns `true` if `expr` contains a return expression
|
/// Returns `true` if `expr` contains a return expression
|
||||||
pub fn contains_return(expr: &hir::Expr<'_>) -> bool {
|
pub fn contains_return<'tcx>(expr: impl Visitable<'tcx>) -> bool {
|
||||||
for_each_expr(expr, |e| {
|
for_each_expr(expr, |e| {
|
||||||
if matches!(e.kind, hir::ExprKind::Ret(..)) {
|
if matches!(e.kind, hir::ExprKind::Ret(..)) {
|
||||||
ControlFlow::Break(())
|
ControlFlow::Break(())
|
||||||
|
@ -117,3 +117,15 @@ fn f(b: bool, v: Option<()>) -> Option<()> {
|
|||||||
None
|
None
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn issue11394(b: bool, v: Result<(), ()>) -> Result<(), ()> {
|
||||||
|
let x = if b {
|
||||||
|
#[allow(clippy::let_unit_value)]
|
||||||
|
let _ = v?;
|
||||||
|
Some(())
|
||||||
|
} else {
|
||||||
|
None
|
||||||
|
};
|
||||||
|
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
Loading…
Reference in New Issue
Block a user