about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2023-11-23 10:00:06 +0000
committerbors <bors@rust-lang.org>2023-11-23 10:00:06 +0000
commit840e227bb6eadb2a01f2344e41ef57bcde6cfa6f (patch)
treea44ca45a2af6fd4442c4bec1c51dde56b2c032c7
parent30c743f8a04d2ad35885eefaa8d29b19a0e3d013 (diff)
parent5d330d08fb35cbed991df29ed94bad69c4f3a4df (diff)
downloadrust-840e227bb6eadb2a01f2344e41ef57bcde6cfa6f.tar.gz
rust-840e227bb6eadb2a01f2344e41ef57bcde6cfa6f.zip
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 https://github.com/rust-lang/rust-clippy/issues/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`
-rw-r--r--clippy_lints/src/methods/mod.rs4
-rw-r--r--clippy_lints/src/methods/result_map_or_else_none.rs46
-rw-r--r--tests/ui/result_map_or_into_option.fixed7
-rw-r--r--tests/ui/result_map_or_into_option.rs7
-rw-r--r--tests/ui/result_map_or_into_option.stderr14
5 files changed, 77 insertions, 1 deletions
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 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 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<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) };
 
@@ -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);
 }
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<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) };
 
@@ -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);
 }
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