Rollup merge of rust-lang#122120 - fmease:sugg-assoc-ty-bound-on-eq-b…

…ound, r=compiler-errors

Suggest associated type bounds on problematic associated equality bounds

Fixes rust-lang#105056. TL;DR: Suggest `Trait<Ty: Bound>` on `Trait<Ty = Bound>` in Rust >=2021.

~~Blocked on rust-lang#122055 (stabilization of `associated_type_bounds`), I'd say.~~ (merged)

compiler/rustc_hir/src/hir.rs

@@ -2289,21 +2289,15 @@ pub enum ImplItemKind<'hir> {
     Type(&'hir Ty<'hir>),
 }
 
-/// Bind a type to an associated type (i.e., `A = Foo`).
+/// An associated item binding.
 ///
-/// Bindings like `A: Debug` are represented as a special type `A =
-/// $::Debug` that is understood by the HIR ty lowering code.
+/// ### Examples
 ///
-/// FIXME(alexreg): why have a separate type for the binding case,
-/// wouldn't it be better to make the `ty` field an enum like the
-/// following?
-///
-/// ```ignore (pseudo-rust)
-/// enum TypeBindingKind {
-///    Equals(...),
-///    Binding(...),
-/// }
-/// ```
+/// * `Trait<A = Ty, B = Ty>`
+/// * `Trait<G<Ty> = Ty>`
+/// * `Trait<A: Bound>`
+/// * `Trait<C = { Ct }>` (under feature `associated_const_equality`)
+/// * `Trait<f(): Bound>` (under feature `return_type_notation`)
 #[derive(Debug, Clone, Copy, HashStable_Generic)]
 pub struct TypeBinding<'hir> {
     pub hir_id: HirId,
@@ -2336,7 +2330,7 @@ impl<'hir> From<AnonConst> for Term<'hir> {
 pub enum TypeBindingKind<'hir> {
     /// E.g., `Foo<Bar: Send>`.
     Constraint { bounds: &'hir [GenericBound<'hir>] },
-    /// E.g., `Foo<Bar = ()>`, `Foo<Bar = ()>`
+    /// E.g., `Foo<Bar = ()>`.
     Equality { term: Term<'hir> },
 }
 

compiler/rustc_hir_analysis/src/hir_ty_lowering/lint.rs

@@ -9,8 +9,85 @@ use rustc_trait_selection::traits::error_reporting::suggestions::NextTypeParamNa
 use super::HirTyLowerer;
 
 impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
+    /// Prohibit or lint against *bare* trait object types depending on the edition.
+    ///
+    /// *Bare* trait object types are ones that aren't preceeded by the keyword `dyn`.
+    /// In edition 2021 and onward we emit a hard error for them.
+    pub(super) fn prohibit_or_lint_bare_trait_object_ty(
+        &self,
+        self_ty: &hir::Ty<'_>,
+        in_path: bool,
+    ) {
+        let tcx = self.tcx();
+
+        let hir::TyKind::TraitObject([poly_trait_ref, ..], _, TraitObjectSyntax::None) =
+            self_ty.kind
+        else {
+            return;
+        };
+
+        let needs_bracket = in_path
+            && !tcx
+                .sess
+                .source_map()
+                .span_to_prev_source(self_ty.span)
+                .ok()
+                .is_some_and(|s| s.trim_end().ends_with('<'));
+
+        let is_global = poly_trait_ref.trait_ref.path.is_global();
+
+        let mut sugg = vec![(
+            self_ty.span.shrink_to_lo(),
+            format!(
+                "{}dyn {}",
+                if needs_bracket { "<" } else { "" },
+                if is_global { "(" } else { "" },
+            ),
+        )];
+
+        if is_global || needs_bracket {
+            sugg.push((
+                self_ty.span.shrink_to_hi(),
+                format!(
+                    "{}{}",
+                    if is_global { ")" } else { "" },
+                    if needs_bracket { ">" } else { "" },
+                ),
+            ));
+        }
+
+        if self_ty.span.edition().at_least_rust_2021() {
+            let msg = "trait objects must include the `dyn` keyword";
+            let label = "add `dyn` keyword before this trait";
+            let mut diag =
+                rustc_errors::struct_span_code_err!(tcx.dcx(), self_ty.span, E0782, "{}", msg);
+            if self_ty.span.can_be_used_for_suggestions()
+                && !self.maybe_suggest_impl_trait(self_ty, &mut diag)
+            {
+                // FIXME: Only emit this suggestion if the trait is object safe.
+                diag.multipart_suggestion_verbose(label, sugg, Applicability::MachineApplicable);
+            }
+            // Check if the impl trait that we are considering is an impl of a local trait.
+            self.maybe_suggest_blanket_trait_impl(self_ty, &mut diag);
+            self.maybe_suggest_assoc_ty_bound(self_ty, &mut diag);
+            diag.stash(self_ty.span, StashKey::TraitMissingMethod);
+        } else {
+            let msg = "trait objects without an explicit `dyn` are deprecated";
+            tcx.node_span_lint(BARE_TRAIT_OBJECTS, self_ty.hir_id, self_ty.span, msg, |lint| {
+                if self_ty.span.can_be_used_for_suggestions() {
+                    lint.multipart_suggestion_verbose(
+                        "if this is an object-safe trait, use `dyn`",
+                        sugg,
+                        Applicability::MachineApplicable,
+                    );
+                }
+                self.maybe_suggest_blanket_trait_impl(self_ty, lint);
+            });
+        }
+    }
+
     /// Make sure that we are in the condition to suggest the blanket implementation.
-    pub(super) fn maybe_lint_blanket_trait_impl<G: EmissionGuarantee>(
+    fn maybe_suggest_blanket_trait_impl<G: EmissionGuarantee>(
         &self,
         self_ty: &hir::Ty<'_>,
         diag: &mut Diag<'_, G>,
@@ -75,9 +152,11 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
     }
 
     /// Make sure that we are in the condition to suggest `impl Trait`.
-    fn maybe_lint_impl_trait(&self, self_ty: &hir::Ty<'_>, diag: &mut Diag<'_>) -> bool {
+    fn maybe_suggest_impl_trait(&self, self_ty: &hir::Ty<'_>, diag: &mut Diag<'_>) -> bool {
         let tcx = self.tcx();
         let parent_id = tcx.hir().get_parent_item(self_ty.hir_id).def_id;
+        // FIXME: If `type_alias_impl_trait` is enabled, also look for `Trait0<Ty = Trait1>`
+        //        and suggest `Trait0<Ty = impl Trait1>`.
         let (sig, generics, owner) = match tcx.hir_node_by_def_id(parent_id) {
             hir::Node::Item(hir::Item { kind: hir::ItemKind::Fn(sig, generics, _), .. }) => {
                 (sig, generics, None)
@@ -186,71 +265,37 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
         false
     }
 
-    pub(super) fn maybe_lint_bare_trait(&self, self_ty: &hir::Ty<'_>, in_path: bool) {
-        let tcx = self.tcx();
-        if let hir::TyKind::TraitObject([poly_trait_ref, ..], _, TraitObjectSyntax::None) =
-            self_ty.kind
-        {
-            let needs_bracket = in_path
-                && !tcx
-                    .sess
-                    .source_map()
-                    .span_to_prev_source(self_ty.span)
-                    .ok()
-                    .is_some_and(|s| s.trim_end().ends_with('<'));
-
-            let is_global = poly_trait_ref.trait_ref.path.is_global();
-
-            let mut sugg = Vec::from_iter([(
-                self_ty.span.shrink_to_lo(),
-                format!(
-                    "{}dyn {}",
-                    if needs_bracket { "<" } else { "" },
-                    if is_global { "(" } else { "" },
-                ),
-            )]);
+    fn maybe_suggest_assoc_ty_bound(&self, self_ty: &hir::Ty<'_>, diag: &mut Diag<'_>) {
+        let mut parents = self.tcx().hir().parent_iter(self_ty.hir_id);
 
-            if is_global || needs_bracket {
-                sugg.push((
-                    self_ty.span.shrink_to_hi(),
-                    format!(
-                        "{}{}",
-                        if is_global { ")" } else { "" },
-                        if needs_bracket { ">" } else { "" },
-                    ),
-                ));
+        if let Some((_, hir::Node::TypeBinding(binding))) = parents.next()
+            && let hir::TypeBindingKind::Equality { term: hir::Term::Ty(obj_ty) } = binding.kind
+        {
+            if let Some((_, hir::Node::TraitRef(..))) = parents.next()
+                && let Some((_, hir::Node::Ty(ty))) = parents.next()
+                && let hir::TyKind::TraitObject(..) = ty.kind
+            {
+                // Assoc ty bounds aren't permitted inside trait object types.
+                return;
             }
 
-            if self_ty.span.edition().at_least_rust_2021() {
-                let msg = "trait objects must include the `dyn` keyword";
-                let label = "add `dyn` keyword before this trait";
-                let mut diag =
-                    rustc_errors::struct_span_code_err!(tcx.dcx(), self_ty.span, E0782, "{}", msg);
-                if self_ty.span.can_be_used_for_suggestions()
-                    && !self.maybe_lint_impl_trait(self_ty, &mut diag)
-                {
-                    diag.multipart_suggestion_verbose(
-                        label,
-                        sugg,
-                        Applicability::MachineApplicable,
-                    );
-                }
-                // check if the impl trait that we are considering is a impl of a local trait
-                self.maybe_lint_blanket_trait_impl(self_ty, &mut diag);
-                diag.stash(self_ty.span, StashKey::TraitMissingMethod);
+            let lo = if binding.gen_args.span_ext.is_dummy() {
+                binding.ident.span
             } else {
-                let msg = "trait objects without an explicit `dyn` are deprecated";
-                tcx.node_span_lint(BARE_TRAIT_OBJECTS, self_ty.hir_id, self_ty.span, msg, |lint| {
-                    if self_ty.span.can_be_used_for_suggestions() {
-                        lint.multipart_suggestion_verbose(
-                            "if this is an object-safe trait, use `dyn`",
-                            sugg,
-                            Applicability::MachineApplicable,
-                        );
-                    }
-                    self.maybe_lint_blanket_trait_impl(self_ty, lint);
-                });
+                binding.gen_args.span_ext
+            };
+            let hi = obj_ty.span;
+
+            if !lo.eq_ctxt(hi) {
+                return;
             }
+
+            diag.span_suggestion_verbose(
+                lo.between(hi),
+                "you might have meant to write a bound here",
+                ": ",
+                Applicability::MaybeIncorrect,
+            );
         }
     }
 }

compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs

@@ -2339,12 +2339,12 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
                 )
             }
             hir::TyKind::TraitObject(bounds, lifetime, repr) => {
-                self.maybe_lint_bare_trait(hir_ty, in_path);
+                self.prohibit_or_lint_bare_trait_object_ty(hir_ty, in_path);
+
                 let repr = match repr {
                     TraitObjectSyntax::Dyn | TraitObjectSyntax::None => ty::Dyn,
                     TraitObjectSyntax::DynStar => ty::DynStar,
                 };
-
                 self.lower_trait_object_ty(
                     hir_ty.span,
                     hir_ty.hir_id,

compiler/rustc_parse/messages.ftl

@@ -14,10 +14,6 @@ parse_array_index_offset_of = array indexing not supported in offset_of
 
 parse_assignment_else_not_allowed = <assignment> ... else {"{"} ... {"}"} is not allowed
 
-parse_assoc_lifetime = associated lifetimes are not supported
-    .label = the lifetime is given here
-    .help = if you meant to specify a trait object, write `dyn Trait + 'lifetime`
-
 parse_associated_static_item_not_allowed = associated `static` items are not allowed
 
 parse_async_block_in_2015 = `async` blocks are only allowed in Rust 2018 or later
@@ -445,6 +441,12 @@ parse_lifetime_in_borrow_expression = borrow expressions cannot be annotated wit
     .suggestion = remove the lifetime annotation
     .label = annotated with lifetime here
 
+parse_lifetime_in_eq_constraint = lifetimes are not permitted in this context
+    .label = lifetime is not allowed here
+    .context_label = this introduces an associated item binding
+    .help = if you meant to specify a trait object, write `dyn /* Trait */ + {$lifetime}`
+    .colon_sugg = you might have meant to write a bound here
+
 parse_lone_slash = invalid trailing slash in literal
     .label = {parse_lone_slash}
 

compiler/rustc_parse/src/errors.rs

@@ -2631,13 +2631,22 @@ pub(crate) struct GenericsInPath {
 }
 
 #[derive(Diagnostic)]
-#[diag(parse_assoc_lifetime)]
+#[diag(parse_lifetime_in_eq_constraint)]
 #[help]
-pub(crate) struct AssocLifetime {
+pub(crate) struct LifetimeInEqConstraint {
     #[primary_span]
-    pub span: Span,
     #[label]
-    pub lifetime: Span,
+    pub span: Span,
+    pub lifetime: Ident,
+    #[label(parse_context_label)]
+    pub binding_label: Span,
+    #[suggestion(
+        parse_colon_sugg,
+        style = "verbose",
+        applicability = "maybe-incorrect",
+        code = ": "
+    )]
+    pub colon_sugg: Span,
 }
 
 #[derive(Diagnostic)]

compiler/rustc_parse/src/parser/path.rs

@@ -718,7 +718,11 @@ impl<'a> Parser<'a> {
                         let bounds = self.parse_generic_bounds()?;
                         AssocConstraintKind::Bound { bounds }
                     } else if self.eat(&token::Eq) {
-                        self.parse_assoc_equality_term(ident, self.prev_token.span)?
+                        self.parse_assoc_equality_term(
+                            ident,
+                            gen_args.as_ref(),
+                            self.prev_token.span,
+                        )?
                     } else {
                         unreachable!();
                     };
@@ -753,11 +757,13 @@ impl<'a> Parser<'a> {
     }
 
     /// Parse the term to the right of an associated item equality constraint.
-    /// That is, parse `<term>` in `Item = <term>`.
-    /// Right now, this only admits types in `<term>`.
+    ///
+    /// That is, parse `$term` in `Item = $term` where `$term` is a type or
+    /// a const expression (wrapped in curly braces if complex).
     fn parse_assoc_equality_term(
         &mut self,
         ident: Ident,
+        gen_args: Option<&GenericArgs>,
         eq: Span,
     ) -> PResult<'a, AssocConstraintKind> {
         let arg = self.parse_generic_arg(None)?;
@@ -769,9 +775,15 @@ impl<'a> Parser<'a> {
                 c.into()
             }
             Some(GenericArg::Lifetime(lt)) => {
-                let guar =
-                    self.dcx().emit_err(errors::AssocLifetime { span, lifetime: lt.ident.span });
-                self.mk_ty(span, ast::TyKind::Err(guar)).into()
+                let guar = self.dcx().emit_err(errors::LifetimeInEqConstraint {
+                    span: lt.ident.span,
+                    lifetime: lt.ident,
+                    binding_label: span,
+                    colon_sugg: gen_args
+                        .map_or(ident.span, |args| args.span())
+                        .between(lt.ident.span),
+                });
+                self.mk_ty(lt.ident.span, ast::TyKind::Err(guar)).into()
             }
             None => {
                 let after_eq = eq.shrink_to_hi();

tests/ui/associated-type-bounds/suggest-assoc-ty-bound-on-eq-bound.rs

@@ -0,0 +1,30 @@
+// Regression test for issue #105056.
+//@ edition: 2021
+
+fn f(_: impl Trait<T = Copy>) {}
+//~^ ERROR trait objects must include the `dyn` keyword
+//~| HELP add `dyn` keyword before this trait
+//~| HELP you might have meant to write a bound here
+//~| ERROR the trait `Copy` cannot be made into an object
+
+fn g(_: impl Trait<T = std::fmt::Debug + Eq>) {}
+//~^ ERROR trait objects must include the `dyn` keyword
+//~| HELP add `dyn` keyword before this trait
+//~| HELP you might have meant to write a bound here
+//~| ERROR only auto traits can be used as additional traits in a trait object
+//~| HELP consider creating a new trait
+//~| ERROR the trait `Eq` cannot be made into an object
+
+fn h(_: impl Trait<T<> = 'static + for<'a> Fn(&'a ())>) {}
+//~^ ERROR trait objects must include the `dyn` keyword
+//~| HELP add `dyn` keyword before this trait
+//~| HELP you might have meant to write a bound here
+
+// Don't suggest assoc ty bound in trait object types, that's not valid:
+type Obj = dyn Trait<T = Clone>;
+//~^ ERROR trait objects must include the `dyn` keyword
+//~| HELP add `dyn` keyword before this trait
+
+trait Trait { type T; }
+
+fn main() {}