Auto merge of rust-lang#5084 - JohnTitor:clean-up-span-lint, r=flip1995

Clean up `span_lint` in `methods/mod.rs`

Uses `span_help_and_lint` instead of `span_lint` and `span_lint_and_sugg` instead of `span_lint_and_then`.

changelog: none

clippy_lints/src/methods/mod.rs

@@ -2133,14 +2133,12 @@ fn lint_iter_nth<'a, 'tcx>(
         return; // caller is not a type that we want to lint
     };
 
-    span_lint(
+    span_help_and_lint(
         cx,
         ITER_NTH,
         expr.span,
-        &format!(
-            "called `.iter{0}().nth()` on a {1}. Calling `.get{0}()` is both faster and more readable",
-            mut_str, caller_type
-        ),
+        &format!("called `.iter{0}().nth()` on a {1}", mut_str, caller_type),
+        &format!("calling `.get{}()` is both faster and more readable", mut_str),
     );
 }
 
@@ -2244,11 +2242,12 @@ fn lint_get_unwrap<'a, 'tcx>(
 fn lint_iter_skip_next(cx: &LateContext<'_, '_>, expr: &hir::Expr<'_>) {
     // lint if caller of skip is an Iterator
     if match_trait_method(cx, expr, &paths::ITERATOR) {
-        span_lint(
+        span_help_and_lint(
             cx,
             ITER_SKIP_NEXT,
             expr.span,
-            "called `skip(x).next()` on an iterator. This is more succinctly expressed by calling `nth(x)`",
+            "called `skip(x).next()` on an iterator",
+            "this is more succinctly expressed by calling `nth(x)`",
         );
     }
 }
@@ -2304,15 +2303,15 @@ fn lint_unwrap(cx: &LateContext<'_, '_>, expr: &hir::Expr<'_>, unwrap_args: &[hi
     };
 
     if let Some((lint, kind, none_value)) = mess {
-        span_lint(
+        span_help_and_lint(
             cx,
             lint,
             expr.span,
+            &format!("used `unwrap()` on `{}` value", kind,),
             &format!(
-                "used `unwrap()` on `{}` value. If you don't want to handle the `{}` case gracefully, consider \
-                 using `expect()` to provide a better panic \
-                 message",
-                kind, none_value
+                "if you don't want to handle the `{}` case gracefully, consider \
+                using `expect()` to provide a better panic message",
+                none_value,
             ),
         );
     }
@@ -2331,14 +2330,12 @@ fn lint_expect(cx: &LateContext<'_, '_>, expr: &hir::Expr<'_>, expect_args: &[hi
     };
 
     if let Some((lint, kind, none_value)) = mess {
-        span_lint(
+        span_help_and_lint(
             cx,
             lint,
             expr.span,
-            &format!(
-                "used `expect()` on `{}` value. If this value is an `{}` it will panic",
-                kind, none_value
-            ),
+            &format!("used `expect()` on `{}` value", kind,),
+            &format!("if this value is an `{}`, it will panic", none_value,),
         );
     }
 }
@@ -2353,11 +2350,12 @@ fn lint_ok_expect(cx: &LateContext<'_, '_>, expr: &hir::Expr<'_>, ok_args: &[hir
         if has_debug_impl(error_type, cx);
 
         then {
-            span_lint(
+            span_help_and_lint(
                 cx,
                 OK_EXPECT,
                 expr.span,
-                "called `ok().expect()` on a `Result` value. You can call `expect()` directly on the `Result`",
+                "called `ok().expect()` on a `Result` value",
+                "you can call `expect()` directly on the `Result`",
             );
         }
     }
@@ -2372,14 +2370,15 @@ fn lint_map_flatten<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr<
         let self_snippet = snippet(cx, map_args[0].span, "..");
         let func_snippet = snippet(cx, map_args[1].span, "..");
         let hint = format!("{0}.flat_map({1})", self_snippet, func_snippet);
-        span_lint_and_then(cx, MAP_FLATTEN, expr.span, msg, |db| {
-            db.span_suggestion(
-                expr.span,
-                "try using `flat_map` instead",
-                hint,
-                Applicability::MachineApplicable,
-            );
-        });
+        span_lint_and_sugg(
+            cx,
+            MAP_FLATTEN,
+            expr.span,
+            msg,
+            "try using `flat_map` instead",
+            hint,
+            Applicability::MachineApplicable,
+        );
     }
 }
 
@@ -2474,14 +2473,15 @@ fn lint_map_or_none<'a, 'tcx>(
             let map_or_self_snippet = snippet(cx, map_or_args[0].span, "..");
             let map_or_func_snippet = snippet(cx, map_or_args[2].span, "..");
             let hint = format!("{0}.and_then({1})", map_or_self_snippet, map_or_func_snippet);
-            span_lint_and_then(cx, OPTION_MAP_OR_NONE, expr.span, msg, |db| {
-                db.span_suggestion(
-                    expr.span,
-                    "try using `and_then` instead",
-                    hint,
-                    Applicability::MachineApplicable, // snippet
-                );
-            });
+            span_lint_and_sugg(
+                cx,
+                OPTION_MAP_OR_NONE,
+                expr.span,
+                msg,
+                "try using `and_then` instead",
+                hint,
+                Applicability::MachineApplicable,
+            );
         }
     }
 }
@@ -2607,9 +2607,9 @@ fn lint_filter_map<'a, '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`. \
-                   This is more succinctly expressed by calling `.filter_map(..)` instead.";
-        span_lint(cx, FILTER_MAP, expr.span, msg);
+        let msg = "called `filter(p).map(q)` on an `Iterator`";
+        let hint = "this is more succinctly expressed by calling `.filter_map(..)` instead";
+        span_help_and_lint(cx, FILTER_MAP, expr.span, msg, hint);
     }
 }
 
@@ -2647,9 +2647,9 @@ fn lint_find_map<'a, '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`. \
-                   This is more succinctly expressed by calling `.find_map(..)` instead.";
-        span_lint(cx, FIND_MAP, expr.span, msg);
+        let msg = "called `find(p).map(q)` on an `Iterator`";
+        let hint = "this is more succinctly expressed by calling `.find_map(..)` instead";
+        span_help_and_lint(cx, FIND_MAP, expr.span, msg, hint);
     }
 }
 
@@ -2662,9 +2662,9 @@ fn lint_filter_map_map<'a, '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`. \
-                   This is more succinctly expressed by only calling `.filter_map(..)` instead.";
-        span_lint(cx, FILTER_MAP, expr.span, msg);
+        let msg = "called `filter_map(p).map(q)` on an `Iterator`";
+        let hint = "this is more succinctly expressed by only calling `.filter_map(..)` instead";
+        span_help_and_lint(cx, FILTER_MAP, expr.span, msg, hint);
     }
 }
 
@@ -2677,10 +2677,10 @@ fn lint_filter_flat_map<'a, '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`. \
-                   This is more succinctly expressed by calling `.flat_map(..)` \
-                   and filtering by returning an empty Iterator.";
-        span_lint(cx, FILTER_MAP, expr.span, msg);
+        let msg = "called `filter(p).flat_map(q)` on an `Iterator`";
+        let hint = "this is more succinctly expressed by calling `.flat_map(..)` \
+            and filtering by returning `iter::empty()`";
+        span_help_and_lint(cx, FILTER_MAP, expr.span, msg, hint);
     }
 }
 
@@ -2693,10 +2693,10 @@ fn lint_filter_map_flat_map<'a, '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`. \
-                   This is more succinctly expressed by calling `.flat_map(..)` \
-                   and filtering by returning an empty Iterator.";
-        span_lint(cx, FILTER_MAP, expr.span, msg);
+        let msg = "called `filter_map(p).flat_map(q)` on an `Iterator`";
+        let hint = "this is more succinctly expressed by calling `.flat_map(..)` \
+            and filtering by returning `iter::empty()`";
+        span_help_and_lint(cx, FILTER_MAP, expr.span, msg, hint);
     }
 }
 

tests/ui/expect.stderr

@@ -1,18 +1,20 @@
-error: used `expect()` on `an Option` value. If this value is an `None` it will panic
+error: used `expect()` on `an Option` value
   --> $DIR/expect.rs:5:13
    |
 LL |     let _ = opt.expect("");
    |             ^^^^^^^^^^^^^^
    |
    = note: `-D clippy::option-expect-used` implied by `-D warnings`
+   = help: if this value is an `None`, it will panic
 
-error: used `expect()` on `a Result` value. If this value is an `Err` it will panic
+error: used `expect()` on `a Result` value
   --> $DIR/expect.rs:10:13
    |
 LL |     let _ = res.expect("");
    |             ^^^^^^^^^^^^^^
    |
    = note: `-D clippy::result-expect-used` implied by `-D warnings`
+   = help: if this value is an `Err`, it will panic
 
 error: aborting due to 2 previous errors
 

tests/ui/filter_methods.stderr

@@ -1,12 +1,13 @@
-error: called `filter(p).map(q)` on an `Iterator`. This is more succinctly expressed by calling `.filter_map(..)` instead.
+error: called `filter(p).map(q)` 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();
    |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = 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`. This is more succinctly expressed by calling `.flat_map(..)` and filtering by returning an empty Iterator.
+error: called `filter(p).flat_map(q)` on an `Iterator`
   --> $DIR/filter_methods.rs:7:21
    |
 LL |       let _: Vec<_> = vec![5_i8; 6]
@@ -15,8 +16,10 @@ LL | |         .into_iter()
 LL | |         .filter(|&x| x == 0)
 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`. This is more succinctly expressed by calling `.flat_map(..)` and filtering by returning an empty Iterator.
+error: called `filter_map(p).flat_map(q)` on an `Iterator`
   --> $DIR/filter_methods.rs:13:21
    |
 LL |       let _: Vec<_> = vec![5_i8; 6]
@@ -25,8 +28,10 @@ LL | |         .into_iter()
 LL | |         .filter_map(|x| x.checked_mul(2))
 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`. This is more succinctly expressed by only calling `.filter_map(..)` instead.
+error: called `filter_map(p).map(q)` on an `Iterator`
   --> $DIR/filter_methods.rs:19:21
    |
 LL |       let _: Vec<_> = vec![5_i8; 6]
@@ -35,6 +40,8 @@ LL | |         .into_iter()
 LL | |         .filter_map(|x| x.checked_mul(2))
 LL | |         .map(|x| x.checked_mul(2))
    | |__________________________________^
+   |
+   = help: this is more succinctly expressed by only calling `.filter_map(..)` instead
 
 error: aborting due to 4 previous errors
 

tests/ui/find_map.stderr

@@ -1,12 +1,13 @@
-error: called `find(p).map(q)` on an `Iterator`. This is more succinctly expressed by calling `.find_map(..)` instead.
+error: called `find(p).map(q)` on an `Iterator`
   --> $DIR/find_map.rs:20:26
    |
 LL |     let _: Option<i32> = a.iter().find(|s| s.parse::<i32>().is_ok()).map(|s| s.parse().unwrap());
    |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: `-D clippy::find-map` implied by `-D warnings`
+   = help: this is more succinctly expressed by calling `.find_map(..)` instead
 
-error: called `find(p).map(q)` on an `Iterator`. This is more succinctly expressed by calling `.find_map(..)` instead.
+error: called `find(p).map(q)` on an `Iterator`
   --> $DIR/find_map.rs:22:29
    |
 LL |       let _: Option<Flavor> = desserts_of_the_week
@@ -18,6 +19,8 @@ LL | |             Dessert::Cake(_) => true,
 LL | |             _ => unreachable!(),
 LL | |         });
    | |__________^
+   |
+   = help: this is more succinctly expressed by calling `.find_map(..)` instead
 
 error: aborting due to 2 previous errors
 

tests/ui/iter_nth.stderr

@@ -1,46 +1,59 @@
-error: called `.iter().nth()` on a Vec. Calling `.get()` is both faster and more readable
+error: called `.iter().nth()` on a Vec
   --> $DIR/iter_nth.rs:33:23
    |
 LL |         let bad_vec = some_vec.iter().nth(3);
    |                       ^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: `-D clippy::iter-nth` implied by `-D warnings`
+   = help: calling `.get()` is both faster and more readable
 
-error: called `.iter().nth()` on a slice. Calling `.get()` is both faster and more readable
+error: called `.iter().nth()` on a slice
   --> $DIR/iter_nth.rs:34:26
    |
 LL |         let bad_slice = &some_vec[..].iter().nth(3);
    |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: calling `.get()` is both faster and more readable
 
-error: called `.iter().nth()` on a slice. Calling `.get()` is both faster and more readable
+error: called `.iter().nth()` on a slice
   --> $DIR/iter_nth.rs:35:31
    |
 LL |         let bad_boxed_slice = boxed_slice.iter().nth(3);
    |                               ^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: calling `.get()` is both faster and more readable
 
-error: called `.iter().nth()` on a VecDeque. Calling `.get()` is both faster and more readable
+error: called `.iter().nth()` on a VecDeque
   --> $DIR/iter_nth.rs:36:29
    |
 LL |         let bad_vec_deque = some_vec_deque.iter().nth(3);
    |                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: calling `.get()` is both faster and more readable
 
-error: called `.iter_mut().nth()` on a Vec. Calling `.get_mut()` is both faster and more readable
+error: called `.iter_mut().nth()` on a Vec
   --> $DIR/iter_nth.rs:41:23
    |
 LL |         let bad_vec = some_vec.iter_mut().nth(3);
    |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: calling `.get_mut()` is both faster and more readable
 
-error: called `.iter_mut().nth()` on a slice. Calling `.get_mut()` is both faster and more readable
+error: called `.iter_mut().nth()` on a slice
   --> $DIR/iter_nth.rs:44:26
    |
 LL |         let bad_slice = &some_vec[..].iter_mut().nth(3);
    |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: calling `.get_mut()` is both faster and more readable
 
-error: called `.iter_mut().nth()` on a VecDeque. Calling `.get_mut()` is both faster and more readable
+error: called `.iter_mut().nth()` on a VecDeque
   --> $DIR/iter_nth.rs:47:29
    |
 LL |         let bad_vec_deque = some_vec_deque.iter_mut().nth(3);
    |                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: calling `.get_mut()` is both faster and more readable
 
 error: aborting due to 7 previous errors