From 2817c5fc14cbf44b07fae0d7fe9cec4c321b4705 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 20 Nov 2023 14:54:20 +0100 Subject: [PATCH 1/2] Extend `result_map_or_into_option` lint to handle `Result::map_or_else(|_| None, Some)` --- clippy_lints/src/methods/mod.rs | 4 ++ .../src/methods/result_map_or_else_none.rs | 46 +++++++++++++++++++ 2 files changed, 50 insertions(+) create mode 100644 clippy_lints/src/methods/result_map_or_else_none.rs diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 82cd3ac04..71de77acf 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -81,6 +81,7 @@ mod read_line_without_trim; 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 @@ impl Methods { 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 000000000..b0e3ca367 --- /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, + ); + } +} From 5d330d08fb35cbed991df29ed94bad69c4f3a4df Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 20 Nov 2023 14:54:43 +0100 Subject: [PATCH 2/2] Add new test for `result_map_or_into_option` extension --- tests/ui/result_map_or_into_option.fixed | 7 +++++++ tests/ui/result_map_or_into_option.rs | 7 +++++++ tests/ui/result_map_or_into_option.stderr | 14 +++++++++++++- 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/tests/ui/result_map_or_into_option.fixed b/tests/ui/result_map_or_into_option.fixed index fb2db6cf5..8c1b2041f 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 06779a699..17bbed02f 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 9396ea4c0..b0fc4a1fb 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