Auto merge of rust-lang#133522 - estebank:dont-suggest-unstable-trait, r=compiler-errors

Don't suggest restricting bound with unstable traits on stable and mention it's unstable on nightly

On nightly, we mention the trait is unstable

```
error[E0277]: the trait bound `T: Unstable` is not satisfied
  --> $DIR/unstable-trait-suggestion.rs:13:9
   |
LL |     foo(t)
   |     --- ^ the trait `Unstable` is not implemented for `T`
   |     |
   |     required by a bound introduced by this call
   |
note: required by a bound in `foo`
  --> $DIR/unstable-trait-suggestion.rs:9:11
   |
LL | fn foo<T: Unstable>(_: T) {}
   |           ^^^^^^^^ required by this bound in `foo`
help: consider restricting type parameter `T` but it is an `unstable` trait
   |
LL | pub fn demo<T: Unstable>(t: T) {
   |              ++++++++++
```

On stable, we don't suggest the trait at all

```
error[E0277]: the trait bound `T: Unstable` is not satisfied
  --> $DIR/unstable-trait-suggestion.rs:13:9
   |
LL |     foo(t)
   |     --- ^ the trait `Unstable` is not implemented for `T`
   |     |
   |     required by a bound introduced by this call
   |
note: required by a bound in `foo`
  --> $DIR/unstable-trait-suggestion.rs:9:11
   |
LL | fn foo<T: Unstable>(_: T) {}
   |           ^^^^^^^^ required by this bound in `foo`
```

Fix rust-lang#133511.

Author: bors

compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs

@@ -1450,6 +1450,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
                         ty::Param(param_ty) => Ok((
                             generics.type_param(param_ty, tcx),
                             predicate.trait_ref.print_trait_sugared().to_string(),
+                            Some(predicate.trait_ref.def_id),
                         )),
                         _ => Err(()),
                     }
@@ -1463,9 +1464,9 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
                 tcx,
                 hir_generics,
                 err,
-                predicates
-                    .iter()
-                    .map(|(param, constraint)| (param.name.as_str(), &**constraint, None)),
+                predicates.iter().map(|(param, constraint, def_id)| {
+                    (param.name.as_str(), &**constraint, *def_id)
+                }),
                 None,
             );
         }

compiler/rustc_const_eval/src/check_consts/ops.rs

@@ -140,7 +140,7 @@ impl<'tcx> NonConstOp<'tcx> for FnCallNonConst<'tcx> {
                             err,
                             param_ty.name.as_str(),
                             &constraint,
-                            None,
+                            Some(trait_ref.def_id),
                             None,
                         );
                     }

compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs

@@ -279,7 +279,13 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
                     } else {
                         let mut err = self.dcx().create_err(err);
                         if suggest_constraining_type_param(
-                            tcx, generics, &mut err, &qself_str, &trait_ref, None, None,
+                            tcx,
+                            generics,
+                            &mut err,
+                            &qself_str,
+                            &trait_ref,
+                            Some(best_trait),
+                            None,
                         ) && !identically_named
                         {
                             // We suggested constraining a type parameter, but the associated item on it

compiler/rustc_middle/src/ty/diagnostics.rs

@@ -1,11 +1,12 @@
 //! Diagnostics related methods for `Ty`.
 
-use std::borrow::Cow;
 use std::fmt::Write;
 use std::ops::ControlFlow;
 
 use rustc_data_structures::fx::FxHashMap;
-use rustc_errors::{Applicability, Diag, DiagArgValue, IntoDiagArg, into_diag_arg_using_display};
+use rustc_errors::{
+    Applicability, Diag, DiagArgValue, IntoDiagArg, into_diag_arg_using_display, pluralize,
+};
 use rustc_hir::def::DefKind;
 use rustc_hir::def_id::DefId;
 use rustc_hir::{self as hir, LangItem, PredicateOrigin, WherePredicateKind};
@@ -161,7 +162,7 @@ pub fn suggest_arbitrary_trait_bound<'tcx>(
     true
 }
 
-#[derive(Debug)]
+#[derive(Debug, Clone, Copy)]
 enum SuggestChangingConstraintsMessage<'a> {
     RestrictBoundFurther,
     RestrictType { ty: &'a str },
@@ -172,7 +173,7 @@ enum SuggestChangingConstraintsMessage<'a> {
 
 fn suggest_changing_unsized_bound(
     generics: &hir::Generics<'_>,
-    suggestions: &mut Vec<(Span, String, SuggestChangingConstraintsMessage<'_>)>,
+    suggestions: &mut Vec<(Span, String, String, SuggestChangingConstraintsMessage<'_>)>,
     param: &hir::GenericParam<'_>,
     def_id: Option<DefId>,
 ) {
@@ -207,7 +208,8 @@ fn suggest_changing_unsized_bound(
             continue;
         }
 
-        let mut push_suggestion = |sp, msg| suggestions.push((sp, String::new(), msg));
+        let mut push_suggestion =
+            |sp, msg| suggestions.push((sp, "Sized".to_string(), String::new(), msg));
 
         if predicate.bounds.len() == unsized_bounds.len() {
             // All the bounds are unsized bounds, e.g.
@@ -278,8 +280,25 @@ pub fn suggest_constraining_type_params<'a>(
     span_to_replace: Option<Span>,
 ) -> bool {
     let mut grouped = FxHashMap::default();
+    let mut unstable_suggestion = false;
     param_names_and_constraints.for_each(|(param_name, constraint, def_id)| {
-        grouped.entry(param_name).or_insert(Vec::new()).push((constraint, def_id))
+        let stable = match def_id {
+            Some(def_id) => match tcx.lookup_stability(def_id) {
+                Some(s) => s.level.is_stable(),
+                None => true,
+            },
+            None => true,
+        };
+        if stable || tcx.sess.is_nightly_build() {
+            grouped.entry(param_name).or_insert(Vec::new()).push((
+                constraint,
+                def_id,
+                if stable { "" } else { "unstable " },
+            ));
+            if !stable {
+                unstable_suggestion = true;
+            }
+        }
     });
 
     let mut applicability = Applicability::MachineApplicable;
@@ -290,16 +309,21 @@ pub fn suggest_constraining_type_params<'a>(
         let Some(param) = param else { return false };
 
         {
-            let mut sized_constraints = constraints.extract_if(|(_, def_id)| {
+            let mut sized_constraints = constraints.extract_if(|(_, def_id, _)| {
                 def_id.is_some_and(|def_id| tcx.is_lang_item(def_id, LangItem::Sized))
             });
-            if let Some((_, def_id)) = sized_constraints.next() {
+            if let Some((_, def_id, _)) = sized_constraints.next() {
                 applicability = Applicability::MaybeIncorrect;
 
                 err.span_label(param.span, "this type parameter needs to be `Sized`");
                 suggest_changing_unsized_bound(generics, &mut suggestions, param, def_id);
             }
         }
+        let bound_message = if constraints.iter().any(|(_, def_id, _)| def_id.is_none()) {
+            SuggestChangingConstraintsMessage::RestrictBoundFurther
+        } else {
+            SuggestChangingConstraintsMessage::RestrictTypeFurther { ty: param_name }
+        };
 
         // in the scenario like impl has stricter requirements than trait,
         // we should not suggest restrict bound on the impl, here we double check
@@ -312,15 +336,54 @@ pub fn suggest_constraining_type_params<'a>(
             .collect();
 
         constraints
-            .retain(|(_, def_id)| def_id.map_or(true, |def| !bound_trait_defs.contains(&def)));
+            .retain(|(_, def_id, _)| def_id.map_or(true, |def| !bound_trait_defs.contains(&def)));
 
         if constraints.is_empty() {
             continue;
         }
 
-        let mut constraint = constraints.iter().map(|&(c, _)| c).collect::<Vec<_>>();
+        let mut constraint = constraints.iter().map(|&(c, _, _)| c).collect::<Vec<_>>();
         constraint.sort();
         constraint.dedup();
+        let all_known = constraints.iter().all(|&(_, def_id, _)| def_id.is_some());
+        let all_stable = constraints.iter().all(|&(_, _, stable)| stable.is_empty());
+        let all_unstable = constraints.iter().all(|&(_, _, stable)| !stable.is_empty());
+        let post = if all_stable || all_unstable {
+            // Don't redundantly say "trait `X`, trait `Y`", instead "traits `X` and `Y`"
+            let mut trait_names = constraints
+                .iter()
+                .map(|&(c, def_id, _)| match def_id {
+                    None => format!("`{c}`"),
+                    Some(def_id) => format!("`{}`", tcx.item_name(def_id)),
+                })
+                .collect::<Vec<_>>();
+            trait_names.sort();
+            trait_names.dedup();
+            let n = trait_names.len();
+            let stable = if all_stable { "" } else { "unstable " };
+            let trait_ = if all_known { format!("trait{}", pluralize!(n)) } else { String::new() };
+            format!("{stable}{trait_}{}", match &trait_names[..] {
+                [t] => format!(" {t}"),
+                [ts @ .., last] => format!(" {} and {last}", ts.join(", ")),
+                [] => return false,
+            },)
+        } else {
+            // We're more explicit when there's a mix of stable and unstable traits.
+            let mut trait_names = constraints
+                .iter()
+                .map(|&(c, def_id, stable)| match def_id {
+                    None => format!("`{c}`"),
+                    Some(def_id) => format!("{stable}trait `{}`", tcx.item_name(def_id)),
+                })
+                .collect::<Vec<_>>();
+            trait_names.sort();
+            trait_names.dedup();
+            match &trait_names[..] {
+                [t] => t.to_string(),
+                [ts @ .., last] => format!("{} and {last}", ts.join(", ")),
+                [] => return false,
+            }
+        };
         let constraint = constraint.join(" + ");
         let mut suggest_restrict = |span, bound_list_non_empty, open_paren_sp| {
             let suggestion = if span_to_replace.is_some() {
@@ -333,13 +396,11 @@ pub fn suggest_constraining_type_params<'a>(
                 format!(" {constraint}")
             };
 
-            use SuggestChangingConstraintsMessage::RestrictBoundFurther;
-
             if let Some(open_paren_sp) = open_paren_sp {
-                suggestions.push((open_paren_sp, "(".to_string(), RestrictBoundFurther));
-                suggestions.push((span, format!("){suggestion}"), RestrictBoundFurther));
+                suggestions.push((open_paren_sp, post.clone(), "(".to_string(), bound_message));
+                suggestions.push((span, post.clone(), format!("){suggestion}"), bound_message));
             } else {
-                suggestions.push((span, suggestion, RestrictBoundFurther));
+                suggestions.push((span, post.clone(), suggestion, bound_message));
             }
         };
 
@@ -397,7 +458,8 @@ pub fn suggest_constraining_type_params<'a>(
             //                                           - insert: `, X: Bar`
             suggestions.push((
                 generics.tail_span_for_predicate_suggestion(),
-                constraints.iter().fold(String::new(), |mut string, &(constraint, _)| {
+                post,
+                constraints.iter().fold(String::new(), |mut string, &(constraint, _, _)| {
                     write!(string, ", {param_name}: {constraint}").unwrap();
                     string
                 }),
@@ -426,6 +488,7 @@ pub fn suggest_constraining_type_params<'a>(
             // default (`<T=Foo>`), so we suggest adding `where T: Bar`.
             suggestions.push((
                 generics.tail_span_for_predicate_suggestion(),
+                post,
                 format!("{where_prefix} {param_name}: {constraint}"),
                 SuggestChangingConstraintsMessage::RestrictTypeFurther { ty: param_name },
             ));
@@ -439,6 +502,7 @@ pub fn suggest_constraining_type_params<'a>(
         if let Some(colon_span) = param.colon_span {
             suggestions.push((
                 colon_span.shrink_to_hi(),
+                post,
                 format!(" {constraint}"),
                 SuggestChangingConstraintsMessage::RestrictType { ty: param_name },
             ));
@@ -451,6 +515,7 @@ pub fn suggest_constraining_type_params<'a>(
         //          - help: consider restricting this type parameter with `T: Foo`
         suggestions.push((
             param.span.shrink_to_hi(),
+            post,
             format!(": {constraint}"),
             SuggestChangingConstraintsMessage::RestrictType { ty: param_name },
         ));
@@ -459,39 +524,46 @@ pub fn suggest_constraining_type_params<'a>(
     // FIXME: remove the suggestions that are from derive, as the span is not correct
     suggestions = suggestions
         .into_iter()
-        .filter(|(span, _, _)| !span.in_derive_expansion())
+        .filter(|(span, _, _, _)| !span.in_derive_expansion())
         .collect::<Vec<_>>();
-
+    let suggested = !suggestions.is_empty();
     if suggestions.len() == 1 {
-        let (span, suggestion, msg) = suggestions.pop().unwrap();
+        let (span, post, suggestion, msg) = suggestions.pop().unwrap();
         let msg = match msg {
             SuggestChangingConstraintsMessage::RestrictBoundFurther => {
-                Cow::from("consider further restricting this bound")
+                format!("consider further restricting this bound")
+            }
+            SuggestChangingConstraintsMessage::RestrictTypeFurther { ty }
+            | SuggestChangingConstraintsMessage::RestrictType { ty }
+                if ty.starts_with("impl ") =>
+            {
+                format!("consider restricting opaque type `{ty}` with {post}")
             }
             SuggestChangingConstraintsMessage::RestrictType { ty } => {
-                Cow::from(format!("consider restricting type parameter `{ty}`"))
+                format!("consider restricting type parameter `{ty}` with {post}")
             }
             SuggestChangingConstraintsMessage::RestrictTypeFurther { ty } => {
-                Cow::from(format!("consider further restricting type parameter `{ty}`"))
+                format!("consider further restricting type parameter `{ty}` with {post}")
             }
             SuggestChangingConstraintsMessage::RemoveMaybeUnsized => {
-                Cow::from("consider removing the `?Sized` bound to make the type parameter `Sized`")
+                format!("consider removing the `?Sized` bound to make the type parameter `Sized`")
             }
             SuggestChangingConstraintsMessage::ReplaceMaybeUnsizedWithSized => {
-                Cow::from("consider replacing `?Sized` with `Sized`")
+                format!("consider replacing `?Sized` with `Sized`")
             }
         };
 
         err.span_suggestion_verbose(span, msg, suggestion, applicability);
     } else if suggestions.len() > 1 {
+        let post = if unstable_suggestion { " (some of them are unstable traits)" } else { "" };
         err.multipart_suggestion_verbose(
-            "consider restricting type parameters",
-            suggestions.into_iter().map(|(span, suggestion, _)| (span, suggestion)).collect(),
+            format!("consider restricting type parameters{post}"),
+            suggestions.into_iter().map(|(span, _, suggestion, _)| (span, suggestion)).collect(),
             applicability,
         );
     }
 
-    true
+    suggested
 }
 
 /// Collect al types that have an implicit `'static` obligation that we could suggest `'_` for.

tests/run-make/missing-unstable-trait-bound/missing-bound.rs

@@ -0,0 +1,4 @@
+pub fn baz<T>(t: std::ops::Range<T>) {
+    for _ in t {}
+}
+fn main() {}

tests/run-make/missing-unstable-trait-bound/missing-bound.stderr

@@ -0,0 +1,12 @@
+error[E0277]: the trait bound `T: Step` is not satisfied
+ --> missing-bound.rs:2:14
+  |
+2 |     for _ in t {}
+  |              ^ the trait `Step` is not implemented for `T`
+  |
+  = note: required for `std::ops::Range<T>` to implement `Iterator`
+  = note: required for `std::ops::Range<T>` to implement `IntoIterator`
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0277`.

tests/run-make/missing-unstable-trait-bound/rmake.rs

@@ -0,0 +1,24 @@
+//@ only-linux
+//@ ignore-wasm32
+//@ ignore-wasm64
+// ignore-tidy-linelength
+
+// Ensure that on stable we don't suggest restricting with an unsafe trait and we continue
+// mentioning the rest of the obligation chain.
+
+use run_make_support::{diff, rust_lib_name, rustc};
+
+fn main() {
+    let out = rustc()
+        .env("RUSTC_BOOTSTRAP", "-1")
+        .input("missing-bound.rs")
+        .run_fail()
+        .assert_stderr_not_contains("help: consider restricting type parameter `T`")
+        .assert_stderr_contains(
+            r#"
+  = note: required for `std::ops::Range<T>` to implement `Iterator`
+  = note: required for `std::ops::Range<T>` to implement `IntoIterator`"#,
+        )
+        .stderr_utf8();
+    diff().expected_file("missing-bound.stderr").actual_text("(stable rustc)", &out).run()
+}

tests/rustdoc-ui/issues/issue-96287.stderr

@@ -4,7 +4,7 @@ error[E0220]: associated type `Assoc` not found for `V`
 LL | pub type Foo<V> = impl Trait<V::Assoc>;
    |                                 ^^^^^ there is an associated type `Assoc` in the trait `TraitWithAssoc`
    |
-help: consider restricting type parameter `V`
+help: consider restricting type parameter `V` with trait `TraitWithAssoc`
    |
 LL | pub type Foo<V: TraitWithAssoc> = impl Trait<V::Assoc>;
    |               ++++++++++++++++
@@ -16,7 +16,7 @@ LL | pub type Foo<V> = impl Trait<V::Assoc>;
    |                                 ^^^^^ there is an associated type `Assoc` in the trait `TraitWithAssoc`
    |
    = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
-help: consider restricting type parameter `V`
+help: consider restricting type parameter `V` with trait `TraitWithAssoc`
    |
 LL | pub type Foo<V: TraitWithAssoc> = impl Trait<V::Assoc>;
    |               ++++++++++++++++

tests/rustdoc-ui/synthetic-auto-trait-impls/projections-in-super-trait-bound-unsatisfied.stderr

@@ -4,7 +4,7 @@ error[E0277]: the trait bound `C: Bar<5>` is not satisfied
 LL | pub struct Structure<C: Tec> {
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Bar<5>` is not implemented for `C`
    |
-help: consider further restricting this bound
+help: consider further restricting type parameter `C` with trait `Bar`
    |
 LL | pub struct Structure<C: Tec + Bar<5>> {
    |                             ++++++++
@@ -15,7 +15,7 @@ error[E0277]: the trait bound `C: Bar<5>` is not satisfied
 LL |     _field: C::BarType,
    |             ^^^^^^^^^^ the trait `Bar<5>` is not implemented for `C`
    |
-help: consider further restricting this bound
+help: consider further restricting type parameter `C` with trait `Bar`
    |
 LL | pub struct Structure<C: Tec + Bar<5>> {
    |                             ++++++++