From eea30777ddef56383199ff70fd14b28335afc1e5 Mon Sep 17 00:00:00 2001 From: sinkuu Date: Tue, 17 Oct 2017 21:39:24 +0900 Subject: [PATCH] Type parameter change and type change are now in a multispan suggestion --- clippy_lints/src/types.rs | 47 ++++++++++++++------------ tests/ui/implicit_hasher.stderr | 60 ++++++--------------------------- 2 files changed, 35 insertions(+), 72 deletions(-) diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index 082dce43e097..a284392bfa0b 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -1453,8 +1453,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidUpcastComparisons { } } -/// **What it does:** Checks for public `impl` or `fn` missing generalization over -/// different hashers and implicitly defaulting to the default hashing +/// **What it does:** Checks for public `impl` or `fn` missing generalization +/// over different hashers and implicitly defaulting to the default hashing /// algorithm (SipHash). /// /// **Why is this bad?** `HashMap` or `HashSet` with custom hashers cannot be @@ -1505,26 +1505,29 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ImplicitHasher { &generics_snip[1..generics_snip.len() - 1] }; - db.span_suggestion( - generics_suggestion_span, - "consider adding a type parameter", - format!( - "<{}{}S: ::std::hash::BuildHasher{}>", - generics_snip, - if generics_snip.is_empty() { "" } else { ", " }, - if vis.suggestions.is_empty() { - "" - } else { - // request users to add `Default` bound so that generic constructors can be used - " + Default" - }, - ), - ); - - db.span_suggestion( - target.span(), - "...and change the type to", - format!("{}<{}, S>", target.type_name(), target.type_arguments(),), + multispan_sugg( + db, + "consider adding a type parameter".to_string(), + vec![ + ( + generics_suggestion_span, + format!( + "<{}{}S: ::std::hash::BuildHasher{}>", + generics_snip, + if generics_snip.is_empty() { "" } else { ", " }, + if vis.suggestions.is_empty() { + "" + } else { + // request users to add `Default` bound so that generic constructors can be used + " + Default" + }, + ), + ), + ( + target.span(), + format!("{}<{}, S>", target.type_name(), target.type_arguments(),), + ), + ], ); if !vis.suggestions.is_empty() { diff --git a/tests/ui/implicit_hasher.stderr b/tests/ui/implicit_hasher.stderr index be799f49d005..cc0bdc327b4a 100644 --- a/tests/ui/implicit_hasher.stderr +++ b/tests/ui/implicit_hasher.stderr @@ -7,12 +7,8 @@ error: impl for `HashMap` should be generarized over different hashers = note: `-D implicit-hasher` implied by `-D warnings` help: consider adding a type parameter | -11 | impl Foo for HashMap { +11 | impl Foo for HashMap { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -help: ...and change the type to - | -11 | impl Foo for HashMap { - | ^^^^^^^^^^^^^^^^ help: ...and use generic constructor | 17 | (HashMap::default(), HashMap::with_capacity_and_hasher(10, Default::default())) @@ -26,12 +22,8 @@ error: impl for `HashMap` should be generarized over different hashers | help: consider adding a type parameter | -20 | impl Foo for (HashMap,) { +20 | impl Foo for (HashMap,) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -help: ...and change the type to - | -20 | impl Foo for (HashMap,) { - | ^^^^^^^^^^^^^^^^ help: ...and use generic constructor | 22 | ((HashMap::default(),), (HashMap::with_capacity_and_hasher(10, Default::default()),)) @@ -45,12 +37,8 @@ error: impl for `HashMap` should be generarized over different hashers | help: consider adding a type parameter | -25 | impl Foo for HashMap { +25 | impl Foo for HashMap { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -help: ...and change the type to - | -25 | impl Foo for HashMap { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: ...and use generic constructor | 27 | (HashMap::default(), HashMap::with_capacity_and_hasher(10, Default::default())) @@ -64,12 +52,8 @@ error: impl for `HashSet` should be generarized over different hashers | help: consider adding a type parameter | -43 | impl Foo for HashSet { +43 | impl Foo for HashSet { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -help: ...and change the type to - | -43 | impl Foo for HashSet { - | ^^^^^^^^^^^^^ help: ...and use generic constructor | 45 | (HashSet::default(), HashSet::with_capacity_and_hasher(10, Default::default())) @@ -83,12 +67,8 @@ error: impl for `HashSet` should be generarized over different hashers | help: consider adding a type parameter | -48 | impl Foo for HashSet { +48 | impl Foo for HashSet { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -help: ...and change the type to - | -48 | impl Foo for HashSet { - | ^^^^^^^^^^^^^^^^^^ help: ...and use generic constructor | 50 | (HashSet::default(), HashSet::with_capacity_and_hasher(10, Default::default())) @@ -102,12 +82,8 @@ error: parameter of type `HashMap` should be generarized over different hashers | help: consider adding a type parameter | -65 | pub fn foo(_map: &mut HashMap, _set: &mut HashSet) { +65 | pub fn foo(_map: &mut HashMap, _set: &mut HashSet) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -help: ...and change the type to - | -65 | pub fn foo(_map: &mut HashMap, _set: &mut HashSet) { - | ^^^^^^^^^^^^^^^^^^^^ error: parameter of type `HashSet` should be generarized over different hashers --> $DIR/implicit_hasher.rs:65:53 @@ -117,12 +93,8 @@ error: parameter of type `HashSet` should be generarized over different hashers | help: consider adding a type parameter | -65 | pub fn foo(_map: &mut HashMap, _set: &mut HashSet) { +65 | pub fn foo(_map: &mut HashMap, _set: &mut HashSet) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -help: ...and change the type to - | -65 | pub fn foo(_map: &mut HashMap, _set: &mut HashSet) { - | ^^^^^^^^^^^^^^^ error: impl for `HashMap` should be generarized over different hashers --> $DIR/implicit_hasher.rs:70:43 @@ -135,12 +107,8 @@ error: impl for `HashMap` should be generarized over different hashers | help: consider adding a type parameter | -70 | impl Foo for HashMap { +70 | impl Foo for HashMap { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -help: ...and change the type to - | -70 | impl Foo for HashMap { - | ^^^^^^^^^^^^^^^^ help: ...and use generic constructor | 72 | (HashMap::default(), HashMap::with_capacity_and_hasher(10, Default::default())) @@ -157,12 +125,8 @@ error: parameter of type `HashMap` should be generarized over different hashers | help: consider adding a type parameter | -78 | pub fn $name(_map: &mut HashMap, _set: &mut HashSet) { +78 | pub fn $name(_map: &mut HashMap, _set: &mut HashSet) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -help: ...and change the type to - | -78 | pub fn $name(_map: &mut HashMap, _set: &mut HashSet) { - | ^^^^^^^^^^^^^^^^^^^^ error: parameter of type `HashSet` should be generarized over different hashers --> $DIR/implicit_hasher.rs:78:63 @@ -175,10 +139,6 @@ error: parameter of type `HashSet` should be generarized over different hashers | help: consider adding a type parameter | -78 | pub fn $name(_map: &mut HashMap, _set: &mut HashSet) { +78 | pub fn $name(_map: &mut HashMap, _set: &mut HashSet) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -help: ...and change the type to - | -78 | pub fn $name(_map: &mut HashMap, _set: &mut HashSet) { - | ^^^^^^^^^^^^^^^