Auto merge of #6197 - ThibsG:ImproveFilterNext, r=ebroto

Improve suggestions for several lints

This PR is a follow-up of this [Zulip discussion](https://rust-lang.zulipchat.com/#narrow/stream/257328-clippy/topic/filter_next.20lint).

It unifies placeholders for `methods` module and improves several suggestions for `filter_next`, `filter_map_next` and `map_unwrap_or` lints.

changelog: none

Author: bors

clippy_lints/src/methods/mod.rs

@@ -32,8 +32,7 @@ use crate::utils::{
     is_copy, is_expn_of, is_type_diagnostic_item, iter_input_pats, last_path_segment, match_def_path, match_qpath,
     match_trait_method, match_type, match_var, method_calls, method_chain_args, paths, remove_blocks, return_ty,
     single_segment_path, snippet, snippet_with_applicability, snippet_with_macro_callsite, span_lint,
-    span_lint_and_help, span_lint_and_note, span_lint_and_sugg, span_lint_and_then, sugg, walk_ptrs_ty_depth,
-    SpanlessEq,
+    span_lint_and_help, span_lint_and_sugg, span_lint_and_then, sugg, walk_ptrs_ty_depth, SpanlessEq,
 };
 
 declare_clippy_lint! {
@@ -1735,7 +1734,7 @@ fn lint_or_fun_call<'tcx>(
                     "try this",
                     format!(
                         "{}.unwrap_or_default()",
-                        snippet_with_applicability(cx, self_expr.span, "_", &mut applicability)
+                        snippet_with_applicability(cx, self_expr.span, "..", &mut applicability)
                     ),
                     applicability,
                 );
@@ -2142,7 +2141,7 @@ fn lint_clone_on_ref_ptr(cx: &LateContext<'_>, expr: &hir::Expr<'_>, arg: &hir::
             return;
         };
 
-        let snippet = snippet_with_macro_callsite(cx, arg.span, "_");
+        let snippet = snippet_with_macro_callsite(cx, arg.span, "..");
 
         span_lint_and_sugg(
             cx,
@@ -2178,9 +2177,9 @@ fn lint_string_extend(cx: &LateContext<'_>, expr: &hir::Expr<'_>, args: &[hir::E
             "try this",
             format!(
                 "{}.push_str({}{})",
-                snippet_with_applicability(cx, args[0].span, "_", &mut applicability),
+                snippet_with_applicability(cx, args[0].span, "..", &mut applicability),
                 ref_str,
-                snippet_with_applicability(cx, target.span, "_", &mut applicability)
+                snippet_with_applicability(cx, target.span, "..", &mut applicability)
             ),
             applicability,
         );
@@ -2427,7 +2426,7 @@ fn lint_get_unwrap<'tcx>(cx: &LateContext<'tcx>, expr: &hir::Expr<'_>, get_args:
     let mut applicability = Applicability::MachineApplicable;
     let expr_ty = cx.typeck_results().expr_ty(&get_args[0]);
     let get_args_str = if get_args.len() > 1 {
-        snippet_with_applicability(cx, get_args[1].span, "_", &mut applicability)
+        snippet_with_applicability(cx, get_args[1].span, "..", &mut applicability)
     } else {
         return; // not linting on a .get().unwrap() chain or variant
     };
@@ -2487,7 +2486,7 @@ fn lint_get_unwrap<'tcx>(cx: &LateContext<'tcx>, expr: &hir::Expr<'_>, get_args:
         format!(
             "{}{}[{}]",
             borrow_str,
-            snippet_with_applicability(cx, get_args[0].span, "_", &mut applicability),
+            snippet_with_applicability(cx, get_args[0].span, "..", &mut applicability),
             get_args_str
         ),
         applicability,
@@ -2503,7 +2502,7 @@ fn lint_iter_skip_next(cx: &LateContext<'_>, expr: &hir::Expr<'_>, skip_args: &[
                 cx,
                 ITER_SKIP_NEXT,
                 expr.span.trim_start(caller.span).unwrap(),
-                "called `skip(x).next()` on an iterator",
+                "called `skip(..).next()` on an iterator",
                 "use `nth` instead",
                 hint,
                 Applicability::MachineApplicable,
@@ -2706,11 +2705,11 @@ fn lint_map_unwrap_or_else<'tcx>(
 
         // lint message
         let msg = if is_option {
-            "called `map(f).unwrap_or_else(g)` on an `Option` value. This can be done more directly by calling \
-            `map_or_else(g, f)` instead"
+            "called `map(<f>).unwrap_or_else(<g>)` on an `Option` value. This can be done more directly by calling \
+            `map_or_else(<g>, <f>)` instead"
         } else {
-            "called `map(f).unwrap_or_else(g)` on a `Result` value. This can be done more directly by calling \
-            `.map_or_else(g, f)` instead"
+            "called `map(<f>).unwrap_or_else(<g>)` on a `Result` value. This can be done more directly by calling \
+            `.map_or_else(<g>, <f>)` instead"
         };
         // get snippets for args to map() and unwrap_or_else()
         let map_snippet = snippet(cx, map_args[1].span, "..");
@@ -2720,16 +2719,15 @@ fn lint_map_unwrap_or_else<'tcx>(
         let multiline = map_snippet.lines().count() > 1 || unwrap_snippet.lines().count() > 1;
         let same_span = map_args[1].span.ctxt() == unwrap_args[1].span.ctxt();
         if same_span && !multiline {
-            span_lint_and_note(
+            let var_snippet = snippet(cx, map_args[0].span, "..");
+            span_lint_and_sugg(
                 cx,
                 MAP_UNWRAP_OR,
                 expr.span,
                 msg,
-                None,
-                &format!(
-                    "replace `map({0}).unwrap_or_else({1})` with `map_or_else({1}, {0})`",
-                    map_snippet, unwrap_snippet,
-                ),
+                "try this",
+                format!("{}.map_or_else({}, {})", var_snippet, unwrap_snippet, map_snippet),
+                Applicability::MachineApplicable,
             );
             return true;
         } else if same_span && multiline {
@@ -2776,8 +2774,8 @@ fn lint_map_or_none<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, map
         if is_option {
             let self_snippet = snippet(cx, map_or_args[0].span, "..");
             let func_snippet = snippet(cx, map_or_args[2].span, "..");
-            let msg = "called `map_or(None, f)` on an `Option` value. This can be done more directly by calling \
-                       `and_then(f)` instead";
+            let msg = "called `map_or(None, ..)` on an `Option` value. This can be done more directly by calling \
+                       `and_then(..)` instead";
             (
                 OPTION_MAP_OR_NONE,
                 msg,
@@ -2815,18 +2813,20 @@ fn lint_map_or_none<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, map
 fn lint_filter_next<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, filter_args: &'tcx [hir::Expr<'_>]) {
     // lint if caller of `.filter().next()` is an Iterator
     if match_trait_method(cx, expr, &paths::ITERATOR) {
-        let msg = "called `filter(p).next()` on an `Iterator`. This is more succinctly expressed by calling \
-                   `.find(p)` instead.";
+        let msg = "called `filter(..).next()` on an `Iterator`. This is more succinctly expressed by calling \
+                   `.find(..)` instead.";
         let filter_snippet = snippet(cx, filter_args[1].span, "..");
         if filter_snippet.lines().count() <= 1 {
+            let iter_snippet = snippet(cx, filter_args[0].span, "..");
             // add note if not multi-line
-            span_lint_and_note(
+            span_lint_and_sugg(
                 cx,
                 FILTER_NEXT,
                 expr.span,
                 msg,
-                None,
-                &format!("replace `filter({0}).next()` with `find({0})`", filter_snippet),
+                "try this",
+                format!("{}.find({})", iter_snippet, filter_snippet),
+                Applicability::MachineApplicable,
             );
         } else {
             span_lint(cx, FILTER_NEXT, expr.span, msg);
@@ -2846,9 +2846,9 @@ fn lint_skip_while_next<'tcx>(
             cx,
             SKIP_WHILE_NEXT,
             expr.span,
-            "called `skip_while(p).next()` on an `Iterator`",
+            "called `skip_while(<p>).next()` on an `Iterator`",
             None,
-            "this is more succinctly expressed by calling `.find(!p)` instead",
+            "this is more succinctly expressed by calling `.find(!<p>)` instead",
         );
     }
 }
@@ -2862,7 +2862,7 @@ fn lint_filter_map<'tcx>(
 ) {
     // lint if caller of `.filter().map()` is an Iterator
     if match_trait_method(cx, expr, &paths::ITERATOR) {
-        let msg = "called `filter(p).map(q)` on an `Iterator`";
+        let msg = "called `filter(..).map(..)` on an `Iterator`";
         let hint = "this is more succinctly expressed by calling `.filter_map(..)` instead";
         span_lint_and_help(cx, FILTER_MAP, expr.span, msg, None, hint);
     }
@@ -2871,17 +2871,19 @@ fn lint_filter_map<'tcx>(
 /// lint use of `filter_map().next()` for `Iterators`
 fn lint_filter_map_next<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, filter_args: &'tcx [hir::Expr<'_>]) {
     if match_trait_method(cx, expr, &paths::ITERATOR) {
-        let msg = "called `filter_map(p).next()` on an `Iterator`. This is more succinctly expressed by calling \
-                   `.find_map(p)` instead.";
+        let msg = "called `filter_map(..).next()` on an `Iterator`. This is more succinctly expressed by calling \
+                   `.find_map(..)` instead.";
         let filter_snippet = snippet(cx, filter_args[1].span, "..");
         if filter_snippet.lines().count() <= 1 {
-            span_lint_and_note(
+            let iter_snippet = snippet(cx, filter_args[0].span, "..");
+            span_lint_and_sugg(
                 cx,
                 FILTER_MAP_NEXT,
                 expr.span,
                 msg,
-                None,
-                &format!("replace `filter_map({0}).next()` with `find_map({0})`", filter_snippet),
+                "try this",
+                format!("{}.find_map({})", iter_snippet, filter_snippet),
+                Applicability::MachineApplicable,
             );
         } else {
             span_lint(cx, FILTER_MAP_NEXT, expr.span, msg);
@@ -2898,7 +2900,7 @@ fn lint_find_map<'tcx>(
 ) {
     // lint if caller of `.filter().map()` is an Iterator
     if match_trait_method(cx, &map_args[0], &paths::ITERATOR) {
-        let msg = "called `find(p).map(q)` on an `Iterator`";
+        let msg = "called `find(..).map(..)` on an `Iterator`";
         let hint = "this is more succinctly expressed by calling `.find_map(..)` instead";
         span_lint_and_help(cx, FIND_MAP, expr.span, msg, None, hint);
     }
@@ -2913,7 +2915,7 @@ fn lint_filter_map_map<'tcx>(
 ) {
     // lint if caller of `.filter().map()` is an Iterator
     if match_trait_method(cx, expr, &paths::ITERATOR) {
-        let msg = "called `filter_map(p).map(q)` on an `Iterator`";
+        let msg = "called `filter_map(..).map(..)` on an `Iterator`";
         let hint = "this is more succinctly expressed by only calling `.filter_map(..)` instead";
         span_lint_and_help(cx, FILTER_MAP, expr.span, msg, None, hint);
     }
@@ -2928,7 +2930,7 @@ fn lint_filter_flat_map<'tcx>(
 ) {
     // lint if caller of `.filter().flat_map()` is an Iterator
     if match_trait_method(cx, expr, &paths::ITERATOR) {
-        let msg = "called `filter(p).flat_map(q)` on an `Iterator`";
+        let msg = "called `filter(..).flat_map(..)` on an `Iterator`";
         let hint = "this is more succinctly expressed by calling `.flat_map(..)` \
                     and filtering by returning `iter::empty()`";
         span_lint_and_help(cx, FILTER_MAP, expr.span, msg, None, hint);
@@ -2944,7 +2946,7 @@ fn lint_filter_map_flat_map<'tcx>(
 ) {
     // lint if caller of `.filter_map().flat_map()` is an Iterator
     if match_trait_method(cx, expr, &paths::ITERATOR) {
-        let msg = "called `filter_map(p).flat_map(q)` on an `Iterator`";
+        let msg = "called `filter_map(..).flat_map(..)` on an `Iterator`";
         let hint = "this is more succinctly expressed by calling `.flat_map(..)` \
                     and filtering by returning `iter::empty()`";
         span_lint_and_help(cx, FILTER_MAP, expr.span, msg, None, hint);
@@ -3115,9 +3117,9 @@ fn lint_chars_cmp(
                 "like this",
                 format!("{}{}.{}({})",
                         if info.eq { "" } else { "!" },
-                        snippet_with_applicability(cx, args[0][0].span, "_", &mut applicability),
+                        snippet_with_applicability(cx, args[0][0].span, "..", &mut applicability),
                         suggest,
-                        snippet_with_applicability(cx, arg_char[0].span, "_", &mut applicability)),
+                        snippet_with_applicability(cx, arg_char[0].span, "..", &mut applicability)),
                 applicability,
             );
 
@@ -3164,7 +3166,7 @@ fn lint_chars_cmp_with_unwrap<'tcx>(
                 "like this",
                 format!("{}{}.{}('{}')",
                         if info.eq { "" } else { "!" },
-                        snippet_with_applicability(cx, args[0][0].span, "_", &mut applicability),
+                        snippet_with_applicability(cx, args[0][0].span, "..", &mut applicability),
                         suggest,
                         c),
                 applicability,
@@ -3239,7 +3241,7 @@ fn lint_single_char_pattern(cx: &LateContext<'_>, _expr: &hir::Expr<'_>, arg: &h
 fn lint_single_char_push_string(cx: &LateContext<'_>, expr: &hir::Expr<'_>, args: &[hir::Expr<'_>]) {
     let mut applicability = Applicability::MachineApplicable;
     if let Some(extension_string) = get_hint_if_single_char_arg(cx, &args[1], &mut applicability) {
-        let base_string_snippet = snippet_with_applicability(cx, args[0].span, "_", &mut applicability);
+        let base_string_snippet = snippet_with_applicability(cx, args[0].span, "..", &mut applicability);
         let sugg = format!("{}.push({})", base_string_snippet, extension_string);
         span_lint_and_sugg(
             cx,
@@ -3282,7 +3284,7 @@ fn lint_asref(cx: &LateContext<'_>, expr: &hir::Expr<'_>, call_name: &str, as_re
                 expr.span,
                 &format!("this call to `{}` does nothing", call_name),
                 "try this",
-                snippet_with_applicability(cx, recvr.span, "_", &mut applicability).to_string(),
+                snippet_with_applicability(cx, recvr.span, "..", &mut applicability).to_string(),
                 applicability,
             );
         }

clippy_lints/src/methods/option_map_unwrap_or.rs

@@ -53,15 +53,15 @@ pub(super) fn lint<'tcx>(
         // lint message
         // comparing the snippet from source to raw text ("None") below is safe
         // because we already have checked the type.
-        let arg = if unwrap_snippet == "None" { "None" } else { "a" };
+        let arg = if unwrap_snippet == "None" { "None" } else { "<a>" };
         let unwrap_snippet_none = unwrap_snippet == "None";
         let suggest = if unwrap_snippet_none {
-            "and_then(f)"
+            "and_then(<f>)"
         } else {
-            "map_or(a, f)"
+            "map_or(<a>, <f>)"
         };
         let msg = &format!(
-            "called `map(f).unwrap_or({})` on an `Option` value. \
+            "called `map(<f>).unwrap_or({})` on an `Option` value. \
             This can be done more directly by calling `{}` instead",
             arg, suggest
         );

tests/ui/filter_map_next.rs

@@ -3,9 +3,6 @@
 fn main() {
     let a = ["1", "lol", "3", "NaN", "5"];
 
-    let element: Option<i32> = a.iter().filter_map(|s| s.parse().ok()).next();
-    assert_eq!(element, Some(1));
-
     #[rustfmt::skip]
     let _: Option<u32> = vec![1, 2, 3, 4, 5, 6]
         .into_iter()

tests/ui/filter_map_next.stderr

@@ -1,14 +1,5 @@
-error: called `filter_map(p).next()` on an `Iterator`. This is more succinctly expressed by calling `.find_map(p)` instead.
-  --> $DIR/filter_map_next.rs:6:32
-   |
-LL |     let element: Option<i32> = a.iter().filter_map(|s| s.parse().ok()).next();
-   |                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-   |
-   = note: `-D clippy::filter-map-next` implied by `-D warnings`
-   = note: replace `filter_map(|s| s.parse().ok()).next()` with `find_map(|s| s.parse().ok())`
-
-error: called `filter_map(p).next()` on an `Iterator`. This is more succinctly expressed by calling `.find_map(p)` instead.
-  --> $DIR/filter_map_next.rs:10:26
+error: called `filter_map(..).next()` on an `Iterator`. This is more succinctly expressed by calling `.find_map(..)` instead.
+  --> $DIR/filter_map_next.rs:7:26
    |
 LL |       let _: Option<u32> = vec![1, 2, 3, 4, 5, 6]
    |  __________________________^
@@ -19,6 +10,8 @@ LL | |             if x == 2 {
 LL | |         })
 LL | |         .next();
    | |_______________^
+   |
+   = note: `-D clippy::filter-map-next` implied by `-D warnings`
 
-error: aborting due to 2 previous errors
+error: aborting due to previous error
 

tests/ui/filter_map_next_fixable.fixed

@@ -0,0 +1,10 @@
+// run-rustfix
+
+#![warn(clippy::all, clippy::pedantic)]
+
+fn main() {
+    let a = ["1", "lol", "3", "NaN", "5"];
+
+    let element: Option<i32> = a.iter().find_map(|s| s.parse().ok());
+    assert_eq!(element, Some(1));
+}

tests/ui/filter_map_next_fixable.rs

@@ -0,0 +1,10 @@
+// run-rustfix
+
+#![warn(clippy::all, clippy::pedantic)]
+
+fn main() {
+    let a = ["1", "lol", "3", "NaN", "5"];
+
+    let element: Option<i32> = a.iter().filter_map(|s| s.parse().ok()).next();
+    assert_eq!(element, Some(1));
+}

tests/ui/filter_map_next_fixable.stderr

@@ -0,0 +1,10 @@
+error: called `filter_map(..).next()` on an `Iterator`. This is more succinctly expressed by calling `.find_map(..)` instead.
+  --> $DIR/filter_map_next_fixable.rs:8:32
+   |
+LL |     let element: Option<i32> = a.iter().filter_map(|s| s.parse().ok()).next();
+   |                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `a.iter().find_map(|s| s.parse().ok())`
+   |
+   = note: `-D clippy::filter-map-next` implied by `-D warnings`
+
+error: aborting due to previous error
+

tests/ui/filter_methods.stderr

@@ -1,4 +1,4 @@
-error: called `filter(p).map(q)` on an `Iterator`
+error: called `filter(..).map(..)` on an `Iterator`
   --> $DIR/filter_methods.rs:5:21
    |
 LL |     let _: Vec<_> = vec![5; 6].into_iter().filter(|&x| x == 0).map(|x| x * 2).collect();
@@ -7,7 +7,7 @@ LL |     let _: Vec<_> = vec![5; 6].into_iter().filter(|&x| x == 0).map(|x| x *
    = note: `-D clippy::filter-map` implied by `-D warnings`
    = help: this is more succinctly expressed by calling `.filter_map(..)` instead
 
-error: called `filter(p).flat_map(q)` on an `Iterator`
+error: called `filter(..).flat_map(..)` on an `Iterator`
   --> $DIR/filter_methods.rs:7:21
    |
 LL |       let _: Vec<_> = vec![5_i8; 6]
@@ -19,7 +19,7 @@ LL | |         .flat_map(|x| x.checked_mul(2))
    |
    = help: this is more succinctly expressed by calling `.flat_map(..)` and filtering by returning `iter::empty()`
 
-error: called `filter_map(p).flat_map(q)` on an `Iterator`
+error: called `filter_map(..).flat_map(..)` on an `Iterator`
   --> $DIR/filter_methods.rs:13:21
    |
 LL |       let _: Vec<_> = vec![5_i8; 6]
@@ -31,7 +31,7 @@ LL | |         .flat_map(|x| x.checked_mul(2))
    |
    = help: this is more succinctly expressed by calling `.flat_map(..)` and filtering by returning `iter::empty()`
 
-error: called `filter_map(p).map(q)` on an `Iterator`
+error: called `filter_map(..).map(..)` on an `Iterator`
   --> $DIR/filter_methods.rs:19:21
    |
 LL |       let _: Vec<_> = vec![5_i8; 6]