diff options
| author | Samuel Tardieu <sam@rfc1149.net> | 2025-02-17 15:22:46 +0100 |
|---|---|---|
| committer | Samuel Tardieu <sam@rfc1149.net> | 2025-02-17 15:49:44 +0100 |
| commit | 66d19d84ae1a83bba0317e2c991cadb2ab28a064 (patch) | |
| tree | 866ec5eb82739c547bac657e3c5366d09652bd6f | |
| parent | 735bed7aa5085c14f96a6e887c1d091c5a69ef85 (diff) | |
| download | rust-66d19d84ae1a83bba0317e2c991cadb2ab28a064.tar.gz rust-66d19d84ae1a83bba0317e2c991cadb2ab28a064.zip | |
`manual_ok_err`: blockify the replacement of an `else if …`
If the part being replaced is an `if` expression following an `else`, the replacement expression must be blockified.
| -rw-r--r-- | clippy_lints/src/matches/manual_ok_err.rs | 15 | ||||
| -rw-r--r-- | tests/ui/manual_ok_err.fixed | 9 | ||||
| -rw-r--r-- | tests/ui/manual_ok_err.rs | 11 | ||||
| -rw-r--r-- | tests/ui/manual_ok_err.stderr | 20 |
4 files changed, 52 insertions, 3 deletions
diff --git a/clippy_lints/src/matches/manual_ok_err.rs b/clippy_lints/src/matches/manual_ok_err.rs index 3deaaf96c1e..576e42a564c 100644 --- a/clippy_lints/src/matches/manual_ok_err.rs +++ b/clippy_lints/src/matches/manual_ok_err.rs @@ -1,7 +1,8 @@ use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::source::{indent_of, reindent_multiline}; use clippy_utils::sugg::Sugg; use clippy_utils::ty::option_arg_ty; -use clippy_utils::{is_res_lang_ctor, path_res, peel_blocks, span_contains_comment}; +use clippy_utils::{get_parent_expr, is_res_lang_ctor, path_res, peel_blocks, span_contains_comment}; use rustc_ast::BindingMode; use rustc_errors::Applicability; use rustc_hir::LangItem::{OptionNone, OptionSome, ResultErr}; @@ -132,13 +133,23 @@ fn apply_lint(cx: &LateContext<'_>, expr: &Expr<'_>, scrutinee: &Expr<'_>, is_ok Applicability::MachineApplicable }; let scrut = Sugg::hir_with_applicability(cx, scrutinee, "..", &mut app).maybe_par(); + let sugg = format!("{scrut}.{method}()"); + // If the expression being expanded is the `if …` part of an `else if …`, it must be blockified. + let sugg = if let Some(parent_expr) = get_parent_expr(cx, expr) + && let ExprKind::If(_, _, Some(else_part)) = parent_expr.kind + && else_part.hir_id == expr.hir_id + { + reindent_multiline(&format!("{{\n {sugg}\n}}"), true, indent_of(cx, parent_expr.span)) + } else { + sugg + }; span_lint_and_sugg( cx, MANUAL_OK_ERR, expr.span, format!("manual implementation of `{method}`"), "replace with", - format!("{scrut}.{method}()"), + sugg, app, ); } diff --git a/tests/ui/manual_ok_err.fixed b/tests/ui/manual_ok_err.fixed index e7e0464c478..bc169b64be9 100644 --- a/tests/ui/manual_ok_err.fixed +++ b/tests/ui/manual_ok_err.fixed @@ -89,3 +89,12 @@ const fn cf(x: Result<u32, &'static str>) -> Option<u32> { Err(_) => None, } } + +fn issue14239() { + let _ = if false { + None + } else { + "1".parse::<u8>().ok() + }; + //~^^^^^ manual_ok_err +} diff --git a/tests/ui/manual_ok_err.rs b/tests/ui/manual_ok_err.rs index 77300b7af53..03c730d4b4e 100644 --- a/tests/ui/manual_ok_err.rs +++ b/tests/ui/manual_ok_err.rs @@ -125,3 +125,14 @@ const fn cf(x: Result<u32, &'static str>) -> Option<u32> { Err(_) => None, } } + +fn issue14239() { + let _ = if false { + None + } else if let Ok(n) = "1".parse::<u8>() { + Some(n) + } else { + None + }; + //~^^^^^ manual_ok_err +} diff --git a/tests/ui/manual_ok_err.stderr b/tests/ui/manual_ok_err.stderr index f10f52cc4c9..13fceacda10 100644 --- a/tests/ui/manual_ok_err.stderr +++ b/tests/ui/manual_ok_err.stderr @@ -93,5 +93,23 @@ LL | | _ => None, LL | | }; | |_____^ help: replace with: `(-S).ok()` -error: aborting due to 8 previous errors +error: manual implementation of `ok` + --> tests/ui/manual_ok_err.rs:132:12 + | +LL | } else if let Ok(n) = "1".parse::<u8>() { + | ____________^ +LL | | Some(n) +LL | | } else { +LL | | None +LL | | }; + | |_____^ + | +help: replace with + | +LL ~ } else { +LL + "1".parse::<u8>().ok() +LL ~ }; + | + +error: aborting due to 9 previous errors |
