diff options
| author | bors <bors@rust-lang.org> | 2023-08-28 08:48:35 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2023-08-28 08:48:35 +0000 |
| commit | 4118738998993f0c4b0eb30eda502f21ebd28c4e (patch) | |
| tree | dc188349bd09836529ae9670e80ee471551f19d0 | |
| parent | 8de52e5bf4a90f7f0f2f58be3432d860870c39b8 (diff) | |
| parent | dba7763128356f684f31a0a1aff25ac3e9646818 (diff) | |
| download | rust-4118738998993f0c4b0eb30eda502f21ebd28c4e.tar.gz rust-4118738998993f0c4b0eb30eda502f21ebd28c4e.zip | |
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
| -rw-r--r-- | clippy_lints/src/if_then_some_else_none.rs | 18 | ||||
| -rw-r--r-- | clippy_utils/src/lib.rs | 3 | ||||
| -rw-r--r-- | tests/ui/if_then_some_else_none.rs | 12 |
3 files changed, 16 insertions, 17 deletions
diff --git a/clippy_lints/src/if_then_some_else_none.rs b/clippy_lints/src/if_then_some_else_none.rs index ab6ad3f3b3a..e2d19e24557 100644 --- a/clippy_lints/src/if_then_some_else_none.rs +++ b/clippy_lints/src/if_then_some_else_none.rs @@ -6,7 +6,7 @@ use clippy_utils::sugg::Sugg; use clippy_utils::{contains_return, higher, is_else_clause, is_res_lang_ctor, path_res, peel_blocks}; use rustc_errors::Applicability; 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_middle::lint::in_external_macro; use rustc_session::{declare_tool_lint, impl_lint_pass}; @@ -83,7 +83,7 @@ impl<'tcx> LateLintPass<'tcx> for IfThenSomeElseNone { && then_expr.span.ctxt() == ctxt && is_res_lang_ctor(cx, path_res(cx, then_call), OptionSome) && 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 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 { 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/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index d53cafde45d..4ef3ec19647 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -111,6 +111,7 @@ use rustc_span::source_map::SourceMap; use rustc_span::symbol::{kw, Ident, Symbol}; use rustc_span::{sym, Span}; use rustc_target::abi::Integer; +use visitors::Visitable; use crate::consts::{constant, miri_to_const, Constant}; 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 -pub fn contains_return(expr: &hir::Expr<'_>) -> bool { +pub fn contains_return<'tcx>(expr: impl Visitable<'tcx>) -> bool { for_each_expr(expr, |e| { if matches!(e.kind, hir::ExprKind::Ret(..)) { ControlFlow::Break(()) diff --git a/tests/ui/if_then_some_else_none.rs b/tests/ui/if_then_some_else_none.rs index 8fa0f34a6c4..77abd663e0a 100644 --- a/tests/ui/if_then_some_else_none.rs +++ b/tests/ui/if_then_some_else_none.rs @@ -117,3 +117,15 @@ fn f(b: bool, v: Option<()>) -> Option<()> { None } } + +fn issue11394(b: bool, v: Result<(), ()>) -> Result<(), ()> { + let x = if b { + #[allow(clippy::let_unit_value)] + let _ = v?; + Some(()) + } else { + None + }; + + Ok(()) +} |
