Skip to content

Commit

Permalink
Auto merge of #11845 - GuillaumeGomez:result_map_or_into_option-exten…
Browse files Browse the repository at this point in the history
…sion, 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`
  • Loading branch information
bors committed Nov 23, 2023
2 parents 30c743f + 1384ebe commit 6411456
Show file tree
Hide file tree
Showing 5 changed files with 77 additions and 1 deletion.
4 changes: 4 additions & 0 deletions clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
46 changes: 46 additions & 0 deletions clippy_lints/src/methods/result_map_or_else_none.rs
Original file line number Diff line number Diff line change
@@ -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,
);
}
}
7 changes: 7 additions & 0 deletions tests/ui/result_map_or_into_option.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,12 @@
fn main() {
let opt: Result<u32, &str> = 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<u32> { Some(s) };

Expand All @@ -14,4 +20,5 @@ fn main() {
// return should not emit the lint
let opt: Result<u32, &str> = Ok(1);
_ = opt.map_or(None, |_x| Some(1));
let _ = opt.map_or_else(|a| a.parse::<u32>().ok(), Some);
}
7 changes: 7 additions & 0 deletions tests/ui/result_map_or_into_option.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,12 @@
fn main() {
let opt: Result<u32, &str> = 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<u32> { Some(s) };

Expand All @@ -14,4 +20,5 @@ fn main() {
// return should not emit the lint
let opt: Result<u32, &str> = Ok(1);
_ = opt.map_or(None, |_x| Some(1));
let _ = opt.map_or_else(|a| a.parse::<u32>().ok(), Some);
}
14 changes: 13 additions & 1 deletion tests/ui/result_map_or_into_option.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 6411456

Please sign in to comment.