diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 82cd3ac0486..71de77acfc1 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -81,6 +81,7 @@ mod readonly_write_lock; mod redundant_as_str; mod repeat_once; +mod result_map_or_else_none; mod search_is_some; mod seek_from_current; mod seek_to_start_instead_of_rewind; @@ -4335,6 +4336,9 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { option_map_or_none::check(cx, expr, recv, def, map); manual_ok_or::check(cx, expr, recv, def, map); }, + ("map_or_else", [def, map]) => { + result_map_or_else_none::check(cx, expr, recv, def, map); + }, ("next", []) => { if let Some((name2, recv2, args2, _, _)) = method_call(recv) { match (name2, args2) { diff --git a/clippy_lints/src/methods/result_map_or_else_none.rs b/clippy_lints/src/methods/result_map_or_else_none.rs new file mode 100644 index 00000000000..b0e3ca367b4 --- /dev/null +++ b/clippy_lints/src/methods/result_map_or_else_none.rs @@ -0,0 +1,46 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::source::snippet; +use clippy_utils::ty::is_type_diagnostic_item; +use clippy_utils::{is_res_lang_ctor, path_res, peel_blocks}; +use rustc_errors::Applicability; +use rustc_hir as hir; +use rustc_hir::LangItem::{OptionNone, OptionSome}; +use rustc_lint::LateContext; +use rustc_span::symbol::sym; + +use super::RESULT_MAP_OR_INTO_OPTION; + +/// lint use of `_.map_or_else(|_| None, Some)` for `Result`s +pub(super) fn check<'tcx>( + cx: &LateContext<'tcx>, + expr: &'tcx hir::Expr<'_>, + recv: &'tcx hir::Expr<'_>, + def_arg: &'tcx hir::Expr<'_>, + map_arg: &'tcx hir::Expr<'_>, +) { + let is_result = is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(recv), sym::Result); + + if !is_result { + return; + } + + let f_arg_is_some = is_res_lang_ctor(cx, path_res(cx, map_arg), OptionSome); + + if f_arg_is_some + && let hir::ExprKind::Closure(&hir::Closure { body, .. }) = def_arg.kind + && let body = cx.tcx.hir().body(body) + && is_res_lang_ctor(cx, path_res(cx, peel_blocks(body.value)), OptionNone) + { + let msg = "called `map_or_else(|_| None, Some)` on a `Result` value"; + let self_snippet = snippet(cx, recv.span, ".."); + span_lint_and_sugg( + cx, + RESULT_MAP_OR_INTO_OPTION, + expr.span, + msg, + "try using `ok` instead", + format!("{self_snippet}.ok()"), + Applicability::MachineApplicable, + ); + } +} diff --git a/tests/ui/result_map_or_into_option.fixed b/tests/ui/result_map_or_into_option.fixed index fb2db6cf5ec..8c1b2041ff8 100644 --- a/tests/ui/result_map_or_into_option.fixed +++ b/tests/ui/result_map_or_into_option.fixed @@ -3,6 +3,12 @@ fn main() { let opt: Result = Ok(1); let _ = opt.ok(); + //~^ ERROR: called `map_or(None, Some)` on a `Result` value. + let _ = opt.ok(); + //~^ ERROR: called `map_or_else(|_| None, Some)` on a `Result` value + #[rustfmt::skip] + let _ = opt.ok(); + //~^ ERROR: called `map_or_else(|_| None, Some)` on a `Result` value let rewrap = |s: u32| -> Option { Some(s) }; @@ -14,4 +20,5 @@ fn main() { // return should not emit the lint let opt: Result = Ok(1); _ = opt.map_or(None, |_x| Some(1)); + let _ = opt.map_or_else(|a| a.parse::().ok(), Some); } diff --git a/tests/ui/result_map_or_into_option.rs b/tests/ui/result_map_or_into_option.rs index 06779a69925..17bbed02fa4 100644 --- a/tests/ui/result_map_or_into_option.rs +++ b/tests/ui/result_map_or_into_option.rs @@ -3,6 +3,12 @@ fn main() { let opt: Result = Ok(1); let _ = opt.map_or(None, Some); + //~^ ERROR: called `map_or(None, Some)` on a `Result` value. + let _ = opt.map_or_else(|_| None, Some); + //~^ ERROR: called `map_or_else(|_| None, Some)` on a `Result` value + #[rustfmt::skip] + let _ = opt.map_or_else(|_| { None }, Some); + //~^ ERROR: called `map_or_else(|_| None, Some)` on a `Result` value let rewrap = |s: u32| -> Option { Some(s) }; @@ -14,4 +20,5 @@ fn main() { // return should not emit the lint let opt: Result = Ok(1); _ = opt.map_or(None, |_x| Some(1)); + let _ = opt.map_or_else(|a| a.parse::().ok(), Some); } diff --git a/tests/ui/result_map_or_into_option.stderr b/tests/ui/result_map_or_into_option.stderr index 9396ea4c064..b0fc4a1fbca 100644 --- a/tests/ui/result_map_or_into_option.stderr +++ b/tests/ui/result_map_or_into_option.stderr @@ -7,5 +7,17 @@ LL | let _ = opt.map_or(None, Some); = note: `-D clippy::result-map-or-into-option` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::result_map_or_into_option)]` -error: aborting due to previous error +error: called `map_or_else(|_| None, Some)` on a `Result` value + --> $DIR/result_map_or_into_option.rs:7:13 + | +LL | let _ = opt.map_or_else(|_| None, Some); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `ok` instead: `opt.ok()` + +error: called `map_or_else(|_| None, Some)` on a `Result` value + --> $DIR/result_map_or_into_option.rs:10:13 + | +LL | let _ = opt.map_or_else(|_| { None }, Some); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `ok` instead: `opt.ok()` + +error: aborting due to 3 previous errors