Rollup merge of #147171 - Qelxiros:hashmap_diag, r=fee1-dead

recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher

closes rust-lang/rust#147147

~The suggestion span is wrong, but I'm not sure how to find the right one.~ fixed

I'm relatively new to the diagnostics ecosystem, so I'm not sure if `span_help` is the right choice. `span_suggestion_*` might be better, but the output from `x test` looks weird in that case.

compiler/rustc_hir_typeck/src/method/suggest.rs

@@ -1754,6 +1754,20 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 err.note(note);
             }
 
+            if let ty::Adt(adt_def, _) = rcvr_ty.kind() {
+                unsatisfied_predicates.iter().find(|(pred, _parent, _cause)| {
+                    if let ty::PredicateKind::Clause(ty::ClauseKind::Trait(pred)) =
+                        pred.kind().skip_binder()
+                    {
+                        self.suggest_hashmap_on_unsatisfied_hashset_buildhasher(
+                            err, &pred, *adt_def,
+                        )
+                    } else {
+                        false
+                    }
+                });
+            }
+
             *suggested_derive = self.suggest_derive(err, unsatisfied_predicates);
             *unsatisfied_bounds = true;
         }
@@ -2990,7 +3004,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             .filter_map(|e| match e.obligation.predicate.kind().skip_binder() {
                 ty::PredicateKind::Clause(ty::ClauseKind::Trait(pred)) => {
                     match pred.self_ty().kind() {
-                        ty::Adt(_, _) => Some(pred),
+                        ty::Adt(_, _) => Some((e.root_obligation.predicate, pred)),
                         _ => None,
                     }
                 }
@@ -3000,7 +3014,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
 
         // Note for local items and foreign items respectively.
         let (mut local_preds, mut foreign_preds): (Vec<_>, Vec<_>) =
-            preds.iter().partition(|&pred| {
+            preds.iter().partition(|&(_, pred)| {
                 if let ty::Adt(def, _) = pred.self_ty().kind() {
                     def.did().is_local()
                 } else {
@@ -3008,10 +3022,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 }
             });
 
-        local_preds.sort_by_key(|pred: &&ty::TraitPredicate<'_>| pred.trait_ref.to_string());
+        local_preds.sort_by_key(|(_, pred)| pred.trait_ref.to_string());
         let local_def_ids = local_preds
             .iter()
-            .filter_map(|pred| match pred.self_ty().kind() {
+            .filter_map(|(_, pred)| match pred.self_ty().kind() {
                 ty::Adt(def, _) => Some(def.did()),
                 _ => None,
             })
@@ -3024,7 +3038,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             })
             .collect::<Vec<_>>()
             .into();
-        for pred in &local_preds {
+        for (_, pred) in &local_preds {
             if let ty::Adt(def, _) = pred.self_ty().kind() {
                 local_spans.push_span_label(
                     self.tcx.def_span(def.did()),
@@ -3033,7 +3047,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             }
         }
         if local_spans.primary_span().is_some() {
-            let msg = if let [local_pred] = local_preds.as_slice() {
+            let msg = if let [(_, local_pred)] = local_preds.as_slice() {
                 format!(
                     "an implementation of `{}` might be missing for `{}`",
                     local_pred.trait_ref.print_trait_sugared(),
@@ -3051,9 +3065,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             err.span_note(local_spans, msg);
         }
 
-        foreign_preds.sort_by_key(|pred: &&ty::TraitPredicate<'_>| pred.trait_ref.to_string());
+        foreign_preds
+            .sort_by_key(|(_, pred): &(_, ty::TraitPredicate<'_>)| pred.trait_ref.to_string());
 
-        for pred in foreign_preds {
+        for (_, pred) in &foreign_preds {
             let ty = pred.self_ty();
             let ty::Adt(def, _) = ty.kind() else { continue };
             let span = self.tcx.def_span(def.did());
@@ -3066,6 +3081,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 mspan,
                 format!("`{ty}` does not implement `{}`", pred.trait_ref.print_trait_sugared()),
             );
+
+            foreign_preds.iter().find(|&(root_pred, pred)| {
+                if let ty::PredicateKind::Clause(ty::ClauseKind::Trait(root_pred)) =
+                    root_pred.kind().skip_binder()
+                    && let Some(root_adt) = root_pred.self_ty().ty_adt_def()
+                {
+                    self.suggest_hashmap_on_unsatisfied_hashset_buildhasher(err, pred, root_adt)
+                } else {
+                    false
+                }
+            });
         }
 
         let preds: Vec<_> = errors
@@ -4388,6 +4414,22 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
 
         self.autoderef(span, rcvr_ty).silence_errors().any(|(ty, _)| is_local(ty))
     }
+
+    fn suggest_hashmap_on_unsatisfied_hashset_buildhasher(
+        &self,
+        err: &mut Diag<'_>,
+        pred: &ty::TraitPredicate<'_>,
+        adt: ty::AdtDef<'_>,
+    ) -> bool {
+        if self.tcx.is_diagnostic_item(sym::HashSet, adt.did())
+            && self.tcx.is_diagnostic_item(sym::BuildHasher, pred.def_id())
+        {
+            err.help("you might have intended to use a HashMap instead");
+            true
+        } else {
+            false
+        }
+    }
 }
 
 #[derive(Copy, Clone, Debug)]

compiler/rustc_span/src/symbol.rs

@@ -191,6 +191,7 @@ symbols! {
         Borrow,
         BorrowMut,
         Break,
+        BuildHasher,
         C,
         CStr,
         C_dash_unwind: "C-unwind",

library/core/src/hash/mod.rs

@@ -633,6 +633,7 @@ impl<H: Hasher + ?Sized> Hasher for &mut H {
 ///
 /// [`build_hasher`]: BuildHasher::build_hasher
 /// [`HashMap`]: ../../std/collections/struct.HashMap.html
+#[cfg_attr(not(test), rustc_diagnostic_item = "BuildHasher")]
 #[stable(since = "1.7.0", feature = "build_hasher")]
 pub trait BuildHasher {
     /// Type of the hasher that will be created.

tests/ui/hashmap/hashset_generics.rs

@@ -0,0 +1,20 @@
+use std::collections::HashSet;
+
+#[derive(PartialEq)]
+//~^ NOTE in this expansion of
+//~| NOTE in this expansion of
+//~| NOTE in this expansion of
+pub struct MyStruct {
+    pub parameters: HashSet<String, String>,
+    //~^ NOTE `String` does not implement `BuildHasher`
+    //~| ERROR binary operation
+    //~| HELP use a HashMap
+}
+
+fn main() {
+    let h1 = HashSet::<usize, usize>::with_hasher(0);
+    h1.insert(1);
+    //~^ ERROR its trait bounds were not satisfied
+    //~| NOTE the following trait bounds
+    //~| HELP use a HashMap
+}

tests/ui/hashmap/hashset_generics.stderr

@@ -0,0 +1,29 @@
+error[E0369]: binary operation `==` cannot be applied to type `HashSet<String, String>`
+  --> $DIR/hashset_generics.rs:8:5
+   |
+LL | #[derive(PartialEq)]
+   |          --------- in this derive macro expansion
+...
+LL |     pub parameters: HashSet<String, String>,
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+note: `String` does not implement `BuildHasher`
+  --> $SRC_DIR/alloc/src/string.rs:LL:COL
+   |
+   = note: `String` is defined in another crate
+   = help: you might have intended to use a HashMap instead
+
+error[E0599]: the method `insert` exists for struct `HashSet<usize, usize>`, but its trait bounds were not satisfied
+  --> $DIR/hashset_generics.rs:16:8
+   |
+LL |     h1.insert(1);
+   |        ^^^^^^
+   |
+   = note: the following trait bounds were not satisfied:
+           `usize: BuildHasher`
+   = help: you might have intended to use a HashMap instead
+
+error: aborting due to 2 previous errors
+
+Some errors have detailed explanations: E0369, E0599.
+For more information about an error, try `rustc --explain E0369`.