Use a single multipart suggestion for implicit_hasher

clippy_lints/src/implicit_hasher.rs

@@ -1,7 +1,7 @@
 use std::borrow::Cow;
 use std::collections::BTreeMap;
 
-use rustc_errors::Diag;
+use rustc_errors::{Applicability, Diag};
 use rustc_hir as hir;
 use rustc_hir::intravisit::{walk_body, walk_expr, walk_inf, walk_ty, Visitor};
 use rustc_hir::{Body, Expr, ExprKind, GenericArg, Item, ItemKind, QPath, TyKind};
@@ -13,7 +13,7 @@ use rustc_session::declare_lint_pass;
 use rustc_span::symbol::sym;
 use rustc_span::Span;
 
-use clippy_utils::diagnostics::{multispan_sugg, span_lint_and_then};
+use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::source::{snippet, IntoSpan, SpanRangeExt};
 use clippy_utils::ty::is_type_diagnostic_item;
 
@@ -77,33 +77,32 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitHasher {
                 &generics_snip[1..generics_snip.len() - 1]
             };
 
-            multispan_sugg(
-                diag,
-                "consider adding a type parameter",
-                vec![
-                    (
-                        generics_suggestion_span,
-                        format!(
-                            "<{generics_snip}{}S: ::std::hash::BuildHasher{}>",
-                            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(),),
+            let mut suggestions = vec![
+                (
+                    generics_suggestion_span,
+                    format!(
+                        "<{generics_snip}{}S: ::std::hash::BuildHasher{}>",
+                        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(),),
+                ),
+            ];
+            suggestions.extend(vis.suggestions);
+
+            diag.multipart_suggestion(
+                "add a type parameter for `BuildHasher`",
+                suggestions,
+                Applicability::MaybeIncorrect,
             );
-
-            if !vis.suggestions.is_empty() {
-                multispan_sugg(diag, "...and use generic constructor", vis.suggestions);
-            }
         }
 
         if !cx.effective_visibilities.is_exported(item.owner_id.def_id) {

tests/ui/crashes/ice-3717.stderr

@@ -9,14 +9,13 @@ note: the lint level is defined here
    |
 LL | #![deny(clippy::implicit_hasher)]
    |         ^^^^^^^^^^^^^^^^^^^^^^^
-help: consider adding a type parameter
+help: add a type parameter for `BuildHasher`
    |
-LL | pub fn ice_3717<S: ::std::hash::BuildHasher + Default>(_: &HashSet<usize, S>) {
-   |                +++++++++++++++++++++++++++++++++++++++     ~~~~~~~~~~~~~~~~~
-help: ...and use generic constructor
+LL ~ pub fn ice_3717<S: ::std::hash::BuildHasher + Default>(_: &HashSet<usize, S>) {
+LL |
+LL |     let _ = [0u8; 0];
+LL ~     let _: HashSet<usize> = HashSet::default();
    |
-LL |     let _: HashSet<usize> = HashSet::default();
-   |                             ~~~~~~~~~~~~~~~~~~
 
 error: aborting due to 1 previous error
 

tests/ui/implicit_hasher.fixed

@@ -0,0 +1,102 @@
+//@aux-build:proc_macros.rs
+#![deny(clippy::implicit_hasher)]
+
+#[macro_use]
+extern crate proc_macros;
+
+use std::cmp::Eq;
+use std::collections::{HashMap, HashSet};
+use std::hash::{BuildHasher, Hash};
+
+pub trait Foo<T>: Sized {
+    fn make() -> (Self, Self);
+}
+
+impl<K: Hash + Eq, V, S: ::std::hash::BuildHasher + Default> Foo<i8> for HashMap<K, V, S> {
+    fn make() -> (Self, Self) {
+        // OK, don't suggest to modify these
+        let _: HashMap<i32, i32> = HashMap::new();
+        let _: HashSet<i32> = HashSet::new();
+
+        (HashMap::default(), HashMap::with_capacity_and_hasher(10, Default::default()))
+    }
+}
+impl<K: Hash + Eq, V, S: ::std::hash::BuildHasher + Default> Foo<i8> for (HashMap<K, V, S>,) {
+    fn make() -> (Self, Self) {
+        ((HashMap::default(),), (HashMap::with_capacity_and_hasher(10, Default::default()),))
+    }
+}
+impl<S: ::std::hash::BuildHasher + Default> Foo<i16> for HashMap<String, String, S> {
+    fn make() -> (Self, Self) {
+        (HashMap::default(), HashMap::with_capacity_and_hasher(10, Default::default()))
+    }
+}
+
+impl<K: Hash + Eq, V, S: BuildHasher + Default> Foo<i32> for HashMap<K, V, S> {
+    fn make() -> (Self, Self) {
+        (HashMap::default(), HashMap::with_capacity_and_hasher(10, S::default()))
+    }
+}
+impl<S: BuildHasher + Default> Foo<i64> for HashMap<String, String, S> {
+    fn make() -> (Self, Self) {
+        (HashMap::default(), HashMap::with_capacity_and_hasher(10, S::default()))
+    }
+}
+
+impl<T: Hash + Eq, S: ::std::hash::BuildHasher + Default> Foo<i8> for HashSet<T, S> {
+    fn make() -> (Self, Self) {
+        (HashSet::default(), HashSet::with_capacity_and_hasher(10, Default::default()))
+    }
+}
+impl<S: ::std::hash::BuildHasher + Default> Foo<i16> for HashSet<String, S> {
+    fn make() -> (Self, Self) {
+        (HashSet::default(), HashSet::with_capacity_and_hasher(10, Default::default()))
+    }
+}
+
+impl<T: Hash + Eq, S: BuildHasher + Default> Foo<i32> for HashSet<T, S> {
+    fn make() -> (Self, Self) {
+        (HashSet::default(), HashSet::with_capacity_and_hasher(10, S::default()))
+    }
+}
+impl<S: BuildHasher + Default> Foo<i64> for HashSet<String, S> {
+    fn make() -> (Self, Self) {
+        (HashSet::default(), HashSet::with_capacity_and_hasher(10, S::default()))
+    }
+}
+
+pub fn map<S: ::std::hash::BuildHasher>(map: &mut HashMap<i32, i32, S>) {}
+
+pub fn set<S: ::std::hash::BuildHasher>(set: &mut HashSet<i32, S>) {}
+
+#[inline_macros]
+pub mod gen {
+    use super::*;
+    inline! {
+        impl<K: Hash + Eq, V, S: ::std::hash::BuildHasher + Default> Foo<u8> for HashMap<K, V, S> {
+            fn make() -> (Self, Self) {
+                (HashMap::default(), HashMap::with_capacity_and_hasher(10, Default::default()))
+            }
+        }
+
+        pub fn bar(_map: &mut HashMap<i32, i32>, _set: &mut HashSet<i32>) {}
+    }
+}
+
+// When the macro is in a different file, the suggestion spans can't be combined properly
+// and should not cause an ICE
+// See #2707
+#[macro_use]
+#[path = "auxiliary/test_macro.rs"]
+pub mod test_macro;
+__implicit_hasher_test_macro!(impl<K, V> for HashMap<K, V> where V: test_macro::A);
+
+// #4260
+external! {
+    pub fn f(input: &HashMap<u32, u32>) {}
+}
+
+// #7712
+pub async fn election_vote<S: ::std::hash::BuildHasher>(_data: HashMap<i32, i32, S>) {}
+
+fn main() {}

tests/ui/implicit_hasher.rs

@@ -1,11 +1,8 @@
 //@aux-build:proc_macros.rs
-//@no-rustfix
 #![deny(clippy::implicit_hasher)]
-#![allow(unused)]
 
 #[macro_use]
 extern crate proc_macros;
-use proc_macros::external;
 
 use std::cmp::Eq;
 use std::collections::{HashMap, HashSet};
@@ -68,9 +65,11 @@ impl<S: BuildHasher + Default> Foo<i64> for HashSet<String, S> {
     }
 }
 
-pub fn foo(_map: &mut HashMap<i32, i32>, _set: &mut HashSet<i32>) {}
+pub fn map(map: &mut HashMap<i32, i32>) {}
 
-#[proc_macros::inline_macros]
+pub fn set(set: &mut HashSet<i32>) {}
+
+#[inline_macros]
 pub mod gen {
     use super::*;
     inline! {

tests/ui/implicit_hasher.stderr

@@ -1,40 +1,121 @@
-error: `proc-macro` crate types currently cannot export any items other than functions tagged with `#[proc_macro]`, `#[proc_macro_derive]`, or `#[proc_macro_attribute]`
-  --> tests/ui/implicit_hasher.rs:14:1
+error: impl for `HashMap` should be generalized over different hashers
+  --> tests/ui/implicit_hasher.rs:15:35
+   |
+LL | impl<K: Hash + Eq, V> Foo<i8> for HashMap<K, V> {
+   |                                   ^^^^^^^^^^^^^
+   |
+note: the lint level is defined here
+  --> tests/ui/implicit_hasher.rs:2:9
+   |
+LL | #![deny(clippy::implicit_hasher)]
+   |         ^^^^^^^^^^^^^^^^^^^^^^^
+help: add a type parameter for `BuildHasher`
+   |
+LL ~ impl<K: Hash + Eq, V, S: ::std::hash::BuildHasher + Default> Foo<i8> for HashMap<K, V, S> {
+LL |     fn make() -> (Self, Self) {
+...
+LL |
+LL ~         (HashMap::default(), HashMap::with_capacity_and_hasher(10, Default::default()))
+   |
+
+error: impl for `HashMap` should be generalized over different hashers
+  --> tests/ui/implicit_hasher.rs:24:36
+   |
+LL | impl<K: Hash + Eq, V> Foo<i8> for (HashMap<K, V>,) {
+   |                                    ^^^^^^^^^^^^^
+   |
+help: add a type parameter for `BuildHasher`
+   |
+LL ~ impl<K: Hash + Eq, V, S: ::std::hash::BuildHasher + Default> Foo<i8> for (HashMap<K, V, S>,) {
+LL |     fn make() -> (Self, Self) {
+LL ~         ((HashMap::default(),), (HashMap::with_capacity_and_hasher(10, Default::default()),))
+   |
+
+error: impl for `HashMap` should be generalized over different hashers
+  --> tests/ui/implicit_hasher.rs:29:19
+   |
+LL | impl Foo<i16> for HashMap<String, String> {
+   |                   ^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: add a type parameter for `BuildHasher`
+   |
+LL ~ impl<S: ::std::hash::BuildHasher + Default> Foo<i16> for HashMap<String, String, S> {
+LL |     fn make() -> (Self, Self) {
+LL ~         (HashMap::default(), HashMap::with_capacity_and_hasher(10, Default::default()))
+   |
+
+error: impl for `HashSet` should be generalized over different hashers
+  --> tests/ui/implicit_hasher.rs:46:32
+   |
+LL | impl<T: Hash + Eq> Foo<i8> for HashSet<T> {
+   |                                ^^^^^^^^^^
+   |
+help: add a type parameter for `BuildHasher`
+   |
+LL ~ impl<T: Hash + Eq, S: ::std::hash::BuildHasher + Default> Foo<i8> for HashSet<T, S> {
+LL |     fn make() -> (Self, Self) {
+LL ~         (HashSet::default(), HashSet::with_capacity_and_hasher(10, Default::default()))
    |
-LL | pub trait Foo<T>: Sized {
-   | ^^^^^^^^^^^^^^^^^^^^^^^
 
-error: `proc-macro` crate types currently cannot export any items other than functions tagged with `#[proc_macro]`, `#[proc_macro_derive]`, or `#[proc_macro_attribute]`
-  --> tests/ui/implicit_hasher.rs:71:1
+error: impl for `HashSet` should be generalized over different hashers
+  --> tests/ui/implicit_hasher.rs:51:19
+   |
+LL | impl Foo<i16> for HashSet<String> {
+   |                   ^^^^^^^^^^^^^^^
+   |
+help: add a type parameter for `BuildHasher`
+   |
+LL ~ impl<S: ::std::hash::BuildHasher + Default> Foo<i16> for HashSet<String, S> {
+LL |     fn make() -> (Self, Self) {
+LL ~         (HashSet::default(), HashSet::with_capacity_and_hasher(10, Default::default()))
    |
-LL | pub fn foo(_map: &mut HashMap<i32, i32>, _set: &mut HashSet<i32>) {}
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-error: `proc-macro` crate types currently cannot export any items other than functions tagged with `#[proc_macro]`, `#[proc_macro_derive]`, or `#[proc_macro_attribute]`
-  --> tests/ui/implicit_hasher.rs:74:1
+error: parameter of type `HashMap` should be generalized over different hashers
+  --> tests/ui/implicit_hasher.rs:68:22
+   |
+LL | pub fn map(map: &mut HashMap<i32, i32>) {}
+   |                      ^^^^^^^^^^^^^^^^^
    |
-LL | pub mod gen {
-   | ^^^^^^^^^^^
+help: add a type parameter for `BuildHasher`
+   |
+LL | pub fn map<S: ::std::hash::BuildHasher>(map: &mut HashMap<i32, i32, S>) {}
+   |           +++++++++++++++++++++++++++++           ~~~~~~~~~~~~~~~~~~~~
 
-error: `proc-macro` crate types currently cannot export any items other than functions tagged with `#[proc_macro]`, `#[proc_macro_derive]`, or `#[proc_macro_attribute]`
-  --> tests/ui/implicit_hasher.rs:92:1
+error: parameter of type `HashSet` should be generalized over different hashers
+  --> tests/ui/implicit_hasher.rs:70:22
+   |
+LL | pub fn set(set: &mut HashSet<i32>) {}
+   |                      ^^^^^^^^^^^^
    |
-LL | pub mod test_macro;
-   | ^^^^^^^^^^^^^^^^^^^
+help: add a type parameter for `BuildHasher`
+   |
+LL | pub fn set<S: ::std::hash::BuildHasher>(set: &mut HashSet<i32, S>) {}
+   |           +++++++++++++++++++++++++++++           ~~~~~~~~~~~~~~~
 
-error: `proc-macro` crate types currently cannot export any items other than functions tagged with `#[proc_macro]`, `#[proc_macro_derive]`, or `#[proc_macro_attribute]`
-  --> tests/ui/implicit_hasher.rs:96:1
+error: impl for `HashMap` should be generalized over different hashers
+  --> tests/ui/implicit_hasher.rs:76:43
+   |
+LL |         impl<K: Hash + Eq, V> Foo<u8> for HashMap<K, V> {
+   |                                           ^^^^^^^^^^^^^
    |
-LL | external! {
-   | ^^^^^^^^^
+   = note: this error originates in the macro `__inline_mac_mod_gen` (in Nightly builds, run with -Z macro-backtrace for more info)
+help: add a type parameter for `BuildHasher`
+   |
+LL ~         impl<K: Hash + Eq, V, S: ::std::hash::BuildHasher + Default> Foo<u8> for HashMap<K, V, S> {
+LL |             fn make() -> (Self, Self) {
+LL ~                 (HashMap::default(), HashMap::with_capacity_and_hasher(10, Default::default()))
    |
-   = note: this error originates in the macro `external` (in Nightly builds, run with -Z macro-backtrace for more info)
 
-error: `proc-macro` crate types currently cannot export any items other than functions tagged with `#[proc_macro]`, `#[proc_macro_derive]`, or `#[proc_macro_attribute]`
-  --> tests/ui/implicit_hasher.rs:101:1
+error: parameter of type `HashMap` should be generalized over different hashers
+  --> tests/ui/implicit_hasher.rs:100:35
    |
 LL | pub async fn election_vote(_data: HashMap<i32, i32>) {}
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |                                   ^^^^^^^^^^^^^^^^^
+   |
+help: add a type parameter for `BuildHasher`
+   |
+LL | pub async fn election_vote<S: ::std::hash::BuildHasher>(_data: HashMap<i32, i32, S>) {}
+   |                           +++++++++++++++++++++++++++++        ~~~~~~~~~~~~~~~~~~~~
 
-error: aborting due to 6 previous errors
+error: aborting due to 9 previous errors