From 3516d45d7c03bbecb67a468bf06fa217484b6ecd Mon Sep 17 00:00:00 2001 From: sinkuu Date: Tue, 21 Feb 2017 18:44:31 +0900 Subject: [PATCH] Use `multispan_sugg` --- clippy_lints/src/loops.rs | 10 +++++----- clippy_lints/src/needless_pass_by_value.rs | 23 ++++++++-------------- clippy_lints/src/utils/mod.rs | 4 ++-- tests/ui/needless_pass_by_value.rs | 2 ++ tests/ui/needless_pass_by_value.stderr | 1 + 5 files changed, 18 insertions(+), 22 deletions(-) diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index d7f952255fe..5fb16cc05eb 100644 --- a/clippy_lints/src/loops.rs +++ b/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!("({}, )", ident.node)), - (arg.span, &format!("{}.iter().enumerate(){}{}", indexed, take, skip))]); + vec![(pat.span, format!("({}, )", 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, ""), (arg.span, &repl)]); + vec![(pat.span, "".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))]); }); } } diff --git a/clippy_lints/src/needless_pass_by_value.rs b/clippy_lints/src/needless_pass_by_value.rs index ff19700a41f..4666cb575ef 100644 --- a/clippy_lints/src/needless_pass_by_value.rs +++ b/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, ""))); - } + let mut spans: Vec<_> = spans.iter().cloned() + .map(|span| (span, format!("*{}", snippet(cx, span, "")))) + .collect(); + spans.sort_by_key(|&(span, _)| span); + multispan_sugg(db, "...and dereference it here".to_string(), spans); } }; diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index ce581febfe4..44d4963a98d 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/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 { diff --git a/tests/ui/needless_pass_by_value.rs b/tests/ui/needless_pass_by_value.rs index fbfd3c517f9..5caf8017f96 100644 --- a/tests/ui/needless_pass_by_value.rs +++ b/tests/ui/needless_pass_by_value.rs @@ -53,6 +53,8 @@ fn test_match(x: Option>, y: Option>) { 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); } diff --git a/tests/ui/needless_pass_by_value.stderr b/tests/ui/needless_pass_by_value.stderr index 08df6b976ae..ded9e0aa731 100644 --- a/tests/ui/needless_pass_by_value.stderr +++ b/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