Use clearer multipart suggestions for `unnecessary_map_or` lint (#13998)

A multipart suggestion will be used whenever the method call can be
replaced by another one with the first argument removed. It helps place
the new method call in context, especially when it is part of a larger
expression.

This fixes #13995 by applying a suggestion made by @y21.

r? @y21

changelog: [`unnecessary_map_or`]: better representation of suggestions
by placing them in context

3 files changed, 147 insertions, 46 deletions

diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index b416d7d5814..9bfa5947990 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -5070,7 +5070,7 @@ impl Methods {
                     option_map_or_none::check(cx, expr, recv, def, map);
                     manual_ok_or::check(cx, expr, recv, def, map);
                     option_map_or_err_ok::check(cx, expr, recv, def, map);
-                    unnecessary_map_or::check(cx, expr, recv, def, map, &self.msrv);
+                    unnecessary_map_or::check(cx, expr, recv, def, map, span, &self.msrv);
                 },
                 ("map_or_else", [def, map]) => {
                     result_map_or_else_none::check(cx, expr, recv, def, map);
diff --git a/clippy_lints/src/methods/unnecessary_map_or.rs b/clippy_lints/src/methods/unnecessary_map_or.rs
index b7dbebe60a4..210e7f77930 100644
--- a/clippy_lints/src/methods/unnecessary_map_or.rs
+++ b/clippy_lints/src/methods/unnecessary_map_or.rs
@@ -1,9 +1,8 @@
 use std::borrow::Cow;
 
-use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::eager_or_lazy::switch_to_eager_eval;
 use clippy_utils::msrvs::{self, Msrv};
-use clippy_utils::source::snippet_opt;
 use clippy_utils::sugg::{Sugg, make_binop};
 use clippy_utils::ty::{get_type_diagnostic_name, implements_trait};
 use clippy_utils::visitors::is_local_used;
@@ -12,7 +11,7 @@ use rustc_ast::LitKind::Bool;
 use rustc_errors::Applicability;
 use rustc_hir::{BinOpKind, Expr, ExprKind, PatKind};
 use rustc_lint::LateContext;
-use rustc_span::sym;
+use rustc_span::{Span, sym};
 
 use super::UNNECESSARY_MAP_OR;
 
@@ -42,6 +41,7 @@ pub(super) fn check<'a>(
     recv: &Expr<'_>,
     def: &Expr<'_>,
     map: &Expr<'_>,
+    method_span: Span,
     msrv: &Msrv,
 ) {
     let ExprKind::Lit(def_kind) = def.kind else {
@@ -60,6 +60,8 @@ pub(super) fn check<'a>(
         Some(_) | None => return,
     };
 
+    let ext_def_span = def.span.until(map.span);
+
     let (sugg, method, applicability) = if let ExprKind::Closure(map_closure) = map.kind
             && let closure_body = cx.tcx.hir().body(map_closure.body)
             && let closure_body_value = closure_body.value.peel_blocks()
@@ -114,26 +116,17 @@ pub(super) fn check<'a>(
         }
         .into_string();
 
-        (sugg, "a standard comparison", app)
-    } else if !def_bool
-        && msrv.meets(msrvs::OPTION_RESULT_IS_VARIANT_AND)
-        && let Some(recv_callsite) = snippet_opt(cx, recv.span.source_callsite())
-        && let Some(span_callsite) = snippet_opt(cx, map.span.source_callsite())
-    {
+        (vec![(expr.span, sugg)], "a standard comparison", app)
+    } else if !def_bool && msrv.meets(msrvs::OPTION_RESULT_IS_VARIANT_AND) {
         let suggested_name = variant.method_name();
         (
-            format!("{recv_callsite}.{suggested_name}({span_callsite})",),
+            vec![(method_span, suggested_name.into()), (ext_def_span, String::default())],
             suggested_name,
             Applicability::MachineApplicable,
         )
-    } else if def_bool
-        && matches!(variant, Variant::Some)
-        && msrv.meets(msrvs::IS_NONE_OR)
-        && let Some(recv_callsite) = snippet_opt(cx, recv.span.source_callsite())
-        && let Some(span_callsite) = snippet_opt(cx, map.span.source_callsite())
-    {
+    } else if def_bool && matches!(variant, Variant::Some) && msrv.meets(msrvs::IS_NONE_OR) {
         (
-            format!("{recv_callsite}.is_none_or({span_callsite})"),
+            vec![(method_span, "is_none_or".into()), (ext_def_span, String::default())],
             "is_none_or",
             Applicability::MachineApplicable,
         )
@@ -145,13 +138,13 @@ pub(super) fn check<'a>(
         return;
     }
 
-    span_lint_and_sugg(
+    span_lint_and_then(
         cx,
         UNNECESSARY_MAP_OR,
         expr.span,
         "this `map_or` can be simplified",
-        format!("use {method} instead"),
-        sugg,
-        applicability,
+        |diag| {
+            diag.multipart_suggestion_verbose(format!("use {method} instead"), sugg, applicability);
+        },
     );
 }
diff --git a/tests/ui/unnecessary_map_or.stderr b/tests/ui/unnecessary_map_or.stderr
index 2b78996d5f3..baa490871e3 100644
--- a/tests/ui/unnecessary_map_or.stderr
+++ b/tests/ui/unnecessary_map_or.stderr
@@ -2,16 +2,25 @@ error: this `map_or` can be simplified
   --> tests/ui/unnecessary_map_or.rs:13:13
    |
 LL |     let _ = Some(5).map_or(false, |n| n == 5);
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use a standard comparison instead: `Some(5) == Some(5)`
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: `-D clippy::unnecessary-map-or` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::unnecessary_map_or)]`
+help: use a standard comparison instead
+   |
+LL |     let _ = Some(5) == Some(5);
+   |             ~~~~~~~~~~~~~~~~~~
 
 error: this `map_or` can be simplified
   --> tests/ui/unnecessary_map_or.rs:14:13
    |
 LL |     let _ = Some(5).map_or(true, |n| n != 5);
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use a standard comparison instead: `Some(5) != Some(5)`
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: use a standard comparison instead
+   |
+LL |     let _ = Some(5) != Some(5);
+   |             ~~~~~~~~~~~~~~~~~~
 
 error: this `map_or` can be simplified
   --> tests/ui/unnecessary_map_or.rs:15:13
@@ -21,7 +30,12 @@ LL |       let _ = Some(5).map_or(false, |n| {
 LL | |         let _ = 1;
 LL | |         n == 5
 LL | |     });
-   | |______^ help: use a standard comparison instead: `Some(5) == Some(5)`
+   | |______^
+   |
+help: use a standard comparison instead
+   |
+LL |     let _ = Some(5) == Some(5);
+   |             ~~~~~~~~~~~~~~~~~~
 
 error: this `map_or` can be simplified
   --> tests/ui/unnecessary_map_or.rs:19:13
@@ -35,113 +49,207 @@ LL | |     });
    |
 help: use is_some_and instead
    |
-LL ~     let _ = Some(5).is_some_and(|n| {
-LL +         let _ = n;
-LL +         6 >= 5
-LL ~     });
+LL -     let _ = Some(5).map_or(false, |n| {
+LL +     let _ = Some(5).is_some_and(|n| {
    |
 
 error: this `map_or` can be simplified
   --> tests/ui/unnecessary_map_or.rs:23:13
    |
 LL |     let _ = Some(vec![5]).map_or(false, |n| n == [5]);
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use is_some_and instead: `Some(vec![5]).is_some_and(|n| n == [5])`
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: use is_some_and instead
+   |
+LL -     let _ = Some(vec![5]).map_or(false, |n| n == [5]);
+LL +     let _ = Some(vec![5]).is_some_and(|n| n == [5]);
+   |
 
 error: this `map_or` can be simplified
   --> tests/ui/unnecessary_map_or.rs:24:13
    |
 LL |     let _ = Some(vec![1]).map_or(false, |n| vec![2] == n);
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use is_some_and instead: `Some(vec![1]).is_some_and(|n| vec![2] == n)`
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: use is_some_and instead
+   |
+LL -     let _ = Some(vec![1]).map_or(false, |n| vec![2] == n);
+LL +     let _ = Some(vec![1]).is_some_and(|n| vec![2] == n);
+   |
 
 error: this `map_or` can be simplified
   --> tests/ui/unnecessary_map_or.rs:25:13
    |
 LL |     let _ = Some(5).map_or(false, |n| n == n);
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use is_some_and instead: `Some(5).is_some_and(|n| n == n)`
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: use is_some_and instead
+   |
+LL -     let _ = Some(5).map_or(false, |n| n == n);
+LL +     let _ = Some(5).is_some_and(|n| n == n);
+   |
 
 error: this `map_or` can be simplified
   --> tests/ui/unnecessary_map_or.rs:26:13
    |
 LL |     let _ = Some(5).map_or(false, |n| n == if 2 > 1 { n } else { 0 });
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use is_some_and instead: `Some(5).is_some_and(|n| n == if 2 > 1 { n } else { 0 })`
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: use is_some_and instead
+   |
+LL -     let _ = Some(5).map_or(false, |n| n == if 2 > 1 { n } else { 0 });
+LL +     let _ = Some(5).is_some_and(|n| n == if 2 > 1 { n } else { 0 });
+   |
 
 error: this `map_or` can be simplified
   --> tests/ui/unnecessary_map_or.rs:27:13
    |
 LL |     let _ = Ok::<Vec<i32>, i32>(vec![5]).map_or(false, |n| n == [5]);
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use is_ok_and instead: `Ok::<Vec<i32>, i32>(vec![5]).is_ok_and(|n| n == [5])`
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: use is_ok_and instead
+   |
+LL -     let _ = Ok::<Vec<i32>, i32>(vec![5]).map_or(false, |n| n == [5]);
+LL +     let _ = Ok::<Vec<i32>, i32>(vec![5]).is_ok_and(|n| n == [5]);
+   |
 
 error: this `map_or` can be simplified
   --> tests/ui/unnecessary_map_or.rs:28:13
    |
 LL |     let _ = Ok::<i32, i32>(5).map_or(false, |n| n == 5);
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use a standard comparison instead: `Ok::<i32, i32>(5) == Ok(5)`
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: use a standard comparison instead
+   |
+LL |     let _ = Ok::<i32, i32>(5) == Ok(5);
+   |             ~~~~~~~~~~~~~~~~~~~~~~~~~~
 
 error: this `map_or` can be simplified
   --> tests/ui/unnecessary_map_or.rs:29:13
    |
 LL |     let _ = Some(5).map_or(false, |n| n == 5).then(|| 1);
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use a standard comparison instead: `(Some(5) == Some(5))`
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: use a standard comparison instead
+   |
+LL |     let _ = (Some(5) == Some(5)).then(|| 1);
+   |             ~~~~~~~~~~~~~~~~~~~~
 
 error: this `map_or` can be simplified
   --> tests/ui/unnecessary_map_or.rs:30:13
    |
 LL |     let _ = Some(5).map_or(true, |n| n == 5);
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use is_none_or instead: `Some(5).is_none_or(|n| n == 5)`
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: use is_none_or instead
+   |
+LL -     let _ = Some(5).map_or(true, |n| n == 5);
+LL +     let _ = Some(5).is_none_or(|n| n == 5);
+   |
 
 error: this `map_or` can be simplified
   --> tests/ui/unnecessary_map_or.rs:31:13
    |
 LL |     let _ = Some(5).map_or(true, |n| 5 == n);
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use is_none_or instead: `Some(5).is_none_or(|n| 5 == n)`
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: use is_none_or instead
+   |
+LL -     let _ = Some(5).map_or(true, |n| 5 == n);
+LL +     let _ = Some(5).is_none_or(|n| 5 == n);
+   |
 
 error: this `map_or` can be simplified
   --> tests/ui/unnecessary_map_or.rs:32:14
    |
 LL |     let _ = !Some(5).map_or(false, |n| n == 5);
-   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use a standard comparison instead: `(Some(5) == Some(5))`
+   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: use a standard comparison instead
+   |
+LL |     let _ = !(Some(5) == Some(5));
+   |              ~~~~~~~~~~~~~~~~~~~~
 
 error: this `map_or` can be simplified
   --> tests/ui/unnecessary_map_or.rs:33:13
    |
 LL |     let _ = Some(5).map_or(false, |n| n == 5) || false;
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use a standard comparison instead: `(Some(5) == Some(5))`
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: use a standard comparison instead
+   |
+LL |     let _ = (Some(5) == Some(5)) || false;
+   |             ~~~~~~~~~~~~~~~~~~~~
 
 error: this `map_or` can be simplified
   --> tests/ui/unnecessary_map_or.rs:34:13
    |
 LL |     let _ = Some(5).map_or(false, |n| n == 5) as usize;
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use a standard comparison instead: `(Some(5) == Some(5))`
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: use a standard comparison instead
+   |
+LL |     let _ = (Some(5) == Some(5)) as usize;
+   |             ~~~~~~~~~~~~~~~~~~~~
 
 error: this `map_or` can be simplified
   --> tests/ui/unnecessary_map_or.rs:58:13
    |
 LL |     let _ = r.map_or(false, |x| x == 7);
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use is_ok_and instead: `r.is_ok_and(|x| x == 7)`
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: use is_ok_and instead
+   |
+LL -     let _ = r.map_or(false, |x| x == 7);
+LL +     let _ = r.is_ok_and(|x| x == 7);
+   |
 
 error: this `map_or` can be simplified
   --> tests/ui/unnecessary_map_or.rs:63:13
    |
 LL |     let _ = r.map_or(false, func);
-   |             ^^^^^^^^^^^^^^^^^^^^^ help: use is_ok_and instead: `r.is_ok_and(func)`
+   |             ^^^^^^^^^^^^^^^^^^^^^
+   |
+help: use is_ok_and instead
+   |
+LL -     let _ = r.map_or(false, func);
+LL +     let _ = r.is_ok_and(func);
+   |
 
 error: this `map_or` can be simplified
   --> tests/ui/unnecessary_map_or.rs:64:13
    |
 LL |     let _ = Some(5).map_or(false, func);
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use is_some_and instead: `Some(5).is_some_and(func)`
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: use is_some_and instead
+   |
+LL -     let _ = Some(5).map_or(false, func);
+LL +     let _ = Some(5).is_some_and(func);
+   |
 
 error: this `map_or` can be simplified
   --> tests/ui/unnecessary_map_or.rs:65:13
    |
 LL |     let _ = Some(5).map_or(true, func);
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use is_none_or instead: `Some(5).is_none_or(func)`
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: use is_none_or instead
+   |
+LL -     let _ = Some(5).map_or(true, func);
+LL +     let _ = Some(5).is_none_or(func);
+   |
 
 error: this `map_or` can be simplified
   --> tests/ui/unnecessary_map_or.rs:70:13
    |
 LL |     let _ = r.map_or(false, |x| x == 8);
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use a standard comparison instead: `r == Ok(8)`
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: use a standard comparison instead
+   |
+LL |     let _ = r == Ok(8);
+   |             ~~~~~~~~~~
 
 error: aborting due to 21 previous errors