Use multispan_sugg

clippy_lints/src/loops.rs

@@ -549,8 +549,8 @@ fn check_for_loop_range<'a, 'tcx>(
                                        |db| {
                         multispan_sugg(db,
                                        "consider using an iterator".to_string(),
-                                       &[(pat.span, &format!("({}, <item>)", ident.node)),
-                                         (arg.span, &format!("{}.iter().enumerate(){}{}", indexed, take, skip))]);
+                                       vec![(pat.span, format!("({}, <item>)", ident.node)),
+                                            (arg.span, format!("{}.iter().enumerate(){}{}", indexed, take, skip))]);
                     });
                 } else {
                     let repl = if starts_at_zero && take.is_empty() {
@@ -568,7 +568,7 @@ fn check_for_loop_range<'a, 'tcx>(
                                        |db| {
                         multispan_sugg(db,
                                        "consider using an iterator".to_string(),
-                                       &[(pat.span, "<item>"), (arg.span, &repl)]);
+                                       vec![(pat.span, "<item>".to_string()), (arg.span, repl)]);
                     });
                 }
             }
@@ -816,8 +816,8 @@ fn check_for_loop_over_map_kv<'a, 'tcx>(
                     let map = sugg::Sugg::hir(cx, arg, "map");
                     multispan_sugg(db,
                                    "use the corresponding method".into(),
-                                   &[(pat_span, &snippet(cx, new_pat_span, kind)),
-                                     (arg_span, &format!("{}.{}s{}()", map.maybe_par(), kind, mutbl))]);
+                                   vec![(pat_span, snippet(cx, new_pat_span, kind).into_owned()),
+                                        (arg_span, format!("{}.{}s{}()", map.maybe_par(), kind, mutbl))]);
                 });
             }
         }

clippy_lints/src/needless_pass_by_value.rs

@@ -10,7 +10,7 @@ use syntax::ast::NodeId;
 use syntax_pos::Span;
 use syntax::errors::DiagnosticBuilder;
 use utils::{in_macro, is_self, is_copy, implements_trait, get_trait_def_id, match_type, snippet, span_lint_and_then,
-            paths};
+            multispan_sugg, paths};
 use std::collections::{HashSet, HashMap};
 
 /// **What it does:** Checks for functions taking arguments by value, but not consuming them in its
@@ -55,8 +55,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessPassByValue {
             return;
         }
 
-        if let FnKind::ItemFn(..) = kind {
-        } else {
+        if !matches!(kind, FnKind::ItemFn(..)) {
             return;
         }
 
@@ -146,19 +145,13 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessPassByValue {
                                            format!("&{}", snippet(cx, input.span, "_")));
                     }
 
-                    // Suggests adding `*` to dereference the added reference.
+                    // Suggests adding `*` to dereference the added reference if needed.
                     if let Some(spans) = spans_need_deref.get(&defid) {
-                        let mut spans: Vec<_> = spans.iter().cloned().collect();
-                        spans.sort();
-                        for (i, span) in spans.into_iter().enumerate() {
-                            db.span_suggestion(span,
-                                                if i == 0 {
-                                                    "...and dereference it here"
-                                                } else {
-                                                    "...and here"
-                                                },
-                                                format!("*{}", snippet(cx, span, "<expr>")));
-                        }
+                        let mut spans: Vec<_> = spans.iter().cloned()
+                            .map(|span| (span, format!("*{}", snippet(cx, span, "<expr>"))))
+                            .collect();
+                        spans.sort_by_key(|&(span, _)| span);
+                        multispan_sugg(db, "...and dereference it here".to_string(), spans);
                     }
                 };
 

clippy_lints/src/utils/mod.rs

@@ -573,10 +573,10 @@ pub fn span_lint_and_then<'a, 'tcx: 'a, T: LintContext<'tcx>, F>(
 ///
 /// Note: in the JSON format (used by `compiletest_rs`), the help message will appear once per
 /// replacement. In human-readable format though, it only appears once before the whole suggestion.
-pub fn multispan_sugg(db: &mut DiagnosticBuilder, help_msg: String, sugg: &[(Span, &str)]) {
+pub fn multispan_sugg(db: &mut DiagnosticBuilder, help_msg: String, sugg: Vec<(Span, String)>) {
     let sugg = rustc_errors::RenderSpan::Suggestion(rustc_errors::CodeSuggestion {
         msp: MultiSpan::from_spans(sugg.iter().map(|&(span, _)| span).collect()),
-        substitutes: sugg.iter().map(|&(_, subs)| subs.to_owned()).collect(),
+        substitutes: sugg.into_iter().map(|(_, subs)| subs).collect(),
     });
 
     let sub = rustc_errors::SubDiagnostic {

tests/ui/needless_pass_by_value.rs

@@ -53,6 +53,8 @@ fn test_match(x: Option<Option<String>>, y: Option<Option<String>>) {
 fn test_destructure(x: Wrapper, y: Wrapper, z: Wrapper) {
     let Wrapper(s) = z; // moved
     let Wrapper(ref t) = y; // not moved
+    let Wrapper(_) = y; // still not moved
+
     assert_eq!(x.0.len(), s.len());
     println!("{}", t);
 }

tests/ui/needless_pass_by_value.stderr

@@ -69,6 +69,7 @@ help: consider taking a reference instead
    | fn test_destructure(x: Wrapper, y: &Wrapper, z: Wrapper) {
 help: ...and dereference it here
    |     let Wrapper(ref t) = *y; // not moved
+   |     let Wrapper(_) = *y; // still not moved
 
 error: aborting due to 7 previous errors