Skip to content

Commit 840e227

Browse files
committed
Auto merge of #11845 - GuillaumeGomez:result_map_or_into_option-extension, r=flip1995
Extend `result_map_or_into_option` lint to handle `Result::map_or_else(|_| None, Some)` Fixes #10365. As indicated in the title, it extends the `result_map_or_into_option` lint to handle `Result::map_or_else(|_| None, Some)`. changelog: extension of the `result_map_or_into_option` lint to handle `Result::map_or_else(|_| None, Some)` r? `@blyxyas`
2 parents 30c743f + 5d330d0 commit 840e227

5 files changed

+77
-1
lines changed

clippy_lints/src/methods/mod.rs

+4
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ mod read_line_without_trim;
8181
mod readonly_write_lock;
8282
mod redundant_as_str;
8383
mod repeat_once;
84+
mod result_map_or_else_none;
8485
mod search_is_some;
8586
mod seek_from_current;
8687
mod seek_to_start_instead_of_rewind;
@@ -4335,6 +4336,9 @@ impl Methods {
43354336
option_map_or_none::check(cx, expr, recv, def, map);
43364337
manual_ok_or::check(cx, expr, recv, def, map);
43374338
},
4339+
("map_or_else", [def, map]) => {
4340+
result_map_or_else_none::check(cx, expr, recv, def, map);
4341+
},
43384342
("next", []) => {
43394343
if let Some((name2, recv2, args2, _, _)) = method_call(recv) {
43404344
match (name2, args2) {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
use clippy_utils::diagnostics::span_lint_and_sugg;
2+
use clippy_utils::source::snippet;
3+
use clippy_utils::ty::is_type_diagnostic_item;
4+
use clippy_utils::{is_res_lang_ctor, path_res, peel_blocks};
5+
use rustc_errors::Applicability;
6+
use rustc_hir as hir;
7+
use rustc_hir::LangItem::{OptionNone, OptionSome};
8+
use rustc_lint::LateContext;
9+
use rustc_span::symbol::sym;
10+
11+
use super::RESULT_MAP_OR_INTO_OPTION;
12+
13+
/// lint use of `_.map_or_else(|_| None, Some)` for `Result`s
14+
pub(super) fn check<'tcx>(
15+
cx: &LateContext<'tcx>,
16+
expr: &'tcx hir::Expr<'_>,
17+
recv: &'tcx hir::Expr<'_>,
18+
def_arg: &'tcx hir::Expr<'_>,
19+
map_arg: &'tcx hir::Expr<'_>,
20+
) {
21+
let is_result = is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(recv), sym::Result);
22+
23+
if !is_result {
24+
return;
25+
}
26+
27+
let f_arg_is_some = is_res_lang_ctor(cx, path_res(cx, map_arg), OptionSome);
28+
29+
if f_arg_is_some
30+
&& let hir::ExprKind::Closure(&hir::Closure { body, .. }) = def_arg.kind
31+
&& let body = cx.tcx.hir().body(body)
32+
&& is_res_lang_ctor(cx, path_res(cx, peel_blocks(body.value)), OptionNone)
33+
{
34+
let msg = "called `map_or_else(|_| None, Some)` on a `Result` value";
35+
let self_snippet = snippet(cx, recv.span, "..");
36+
span_lint_and_sugg(
37+
cx,
38+
RESULT_MAP_OR_INTO_OPTION,
39+
expr.span,
40+
msg,
41+
"try using `ok` instead",
42+
format!("{self_snippet}.ok()"),
43+
Applicability::MachineApplicable,
44+
);
45+
}
46+
}

tests/ui/result_map_or_into_option.fixed

+7
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,12 @@
33
fn main() {
44
let opt: Result<u32, &str> = Ok(1);
55
let _ = opt.ok();
6+
//~^ ERROR: called `map_or(None, Some)` on a `Result` value.
7+
let _ = opt.ok();
8+
//~^ ERROR: called `map_or_else(|_| None, Some)` on a `Result` value
9+
#[rustfmt::skip]
10+
let _ = opt.ok();
11+
//~^ ERROR: called `map_or_else(|_| None, Some)` on a `Result` value
612

713
let rewrap = |s: u32| -> Option<u32> { Some(s) };
814

@@ -14,4 +20,5 @@ fn main() {
1420
// return should not emit the lint
1521
let opt: Result<u32, &str> = Ok(1);
1622
_ = opt.map_or(None, |_x| Some(1));
23+
let _ = opt.map_or_else(|a| a.parse::<u32>().ok(), Some);
1724
}

tests/ui/result_map_or_into_option.rs

+7
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,12 @@
33
fn main() {
44
let opt: Result<u32, &str> = Ok(1);
55
let _ = opt.map_or(None, Some);
6+
//~^ ERROR: called `map_or(None, Some)` on a `Result` value.
7+
let _ = opt.map_or_else(|_| None, Some);
8+
//~^ ERROR: called `map_or_else(|_| None, Some)` on a `Result` value
9+
#[rustfmt::skip]
10+
let _ = opt.map_or_else(|_| { None }, Some);
11+
//~^ ERROR: called `map_or_else(|_| None, Some)` on a `Result` value
612

713
let rewrap = |s: u32| -> Option<u32> { Some(s) };
814

@@ -14,4 +20,5 @@ fn main() {
1420
// return should not emit the lint
1521
let opt: Result<u32, &str> = Ok(1);
1622
_ = opt.map_or(None, |_x| Some(1));
23+
let _ = opt.map_or_else(|a| a.parse::<u32>().ok(), Some);
1724
}

tests/ui/result_map_or_into_option.stderr

+13-1
Original file line numberDiff line numberDiff line change
@@ -7,5 +7,17 @@ LL | let _ = opt.map_or(None, Some);
77
= note: `-D clippy::result-map-or-into-option` implied by `-D warnings`
88
= help: to override `-D warnings` add `#[allow(clippy::result_map_or_into_option)]`
99

10-
error: aborting due to previous error
10+
error: called `map_or_else(|_| None, Some)` on a `Result` value
11+
--> $DIR/result_map_or_into_option.rs:7:13
12+
|
13+
LL | let _ = opt.map_or_else(|_| None, Some);
14+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `ok` instead: `opt.ok()`
15+
16+
error: called `map_or_else(|_| None, Some)` on a `Result` value
17+
--> $DIR/result_map_or_into_option.rs:10:13
18+
|
19+
LL | let _ = opt.map_or_else(|_| { None }, Some);
20+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `ok` instead: `opt.ok()`
21+
22+
error: aborting due to 3 previous errors
1123

0 commit comments

Comments
 (0)