Suggest impl Trait for References to Bare Trait in Function Header

Author: veera-sivarajan

Diff for: compiler/rustc_hir_analysis/src/hir_ty_lowering/lint.rs

@@ -120,9 +120,6 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
                 return;
             };
             let sugg = self.add_generic_param_suggestion(generics, self_ty.span, &impl_trait_name);
-            if sugg.is_empty() {
-                return;
-            };
             diag.multipart_suggestion(
                 format!(
                     "alternatively use a blanket implementation to implement `{of_trait_name}` for \
@@ -157,6 +154,10 @@ impl<'tcx> dyn HirTyLowerer<'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>`.
+        // Functions are found in three different contexts.
+        // 1. Independent functions
+        // 2. Functions inside trait blocks
+        // 3. Functions inside impl blocks
         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)
@@ -167,13 +168,21 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
                 owner_id,
                 ..
             }) => (sig, generics, Some(tcx.parent(owner_id.to_def_id()))),
+            hir::Node::ImplItem(hir::ImplItem {
+                kind: hir::ImplItemKind::Fn(sig, _),
+                generics,
+                owner_id,
+                ..
+            }) => (sig, generics, Some(tcx.parent(owner_id.to_def_id()))),
             _ => return false,
         };
         let Ok(trait_name) = tcx.sess.source_map().span_to_snippet(self_ty.span) else {
             return false;
         };
         let impl_sugg = vec![(self_ty.span.shrink_to_lo(), "impl ".to_string())];
         let mut is_downgradable = true;
+
+        // Check if trait object is safe for suggesting dynamic dispatch.
         let is_object_safe = match self_ty.kind {
             hir::TyKind::TraitObject(objects, ..) => {
                 objects.iter().all(|o| match o.trait_ref.path.res {
@@ -189,8 +198,15 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
             }
             _ => false,
         };
+
+        let borrowed = matches!(
+            tcx.parent_hir_node(self_ty.hir_id),
+            hir::Node::Ty(hir::Ty { kind: hir::TyKind::Ref(..), .. })
+        );
+
+        // Suggestions for function return type.
         if let hir::FnRetTy::Return(ty) = sig.decl.output
-            && ty.hir_id == self_ty.hir_id
+            && ty.peel_refs().hir_id == self_ty.hir_id
         {
             let pre = if !is_object_safe {
                 format!("`{trait_name}` is not object safe, ")
@@ -201,14 +217,26 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
                 "{pre}use `impl {trait_name}` to return an opaque type, as long as you return a \
                  single underlying type",
             );
+
             diag.multipart_suggestion_verbose(msg, impl_sugg, Applicability::MachineApplicable);
+
+            // Suggest `Box<dyn Trait>` for return type
             if is_object_safe {
-                diag.multipart_suggestion_verbose(
-                    "alternatively, you can return an owned trait object",
+                // If the return type is `&Trait`, we don't want
+                // the ampersand to be displayed in the `Box<dyn Trait>`
+                // suggestion.
+                let suggestion = if borrowed {
+                    vec![(ty.span, format!("Box<dyn {trait_name}>"))]
+                } else {
                     vec![
                         (ty.span.shrink_to_lo(), "Box<dyn ".to_string()),
                         (ty.span.shrink_to_hi(), ">".to_string()),
-                    ],
+                    ]
+                };
+
+                diag.multipart_suggestion_verbose(
+                    "alternatively, you can return an owned trait object",
+                    suggestion,
                     Applicability::MachineApplicable,
                 );
             } else if is_downgradable {
@@ -217,39 +245,43 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
             }
             return true;
         }
+
+        // Suggestions for function parameters.
         for ty in sig.decl.inputs {
-            if ty.hir_id != self_ty.hir_id {
+            if ty.peel_refs().hir_id != self_ty.hir_id {
                 continue;
             }
             let sugg = self.add_generic_param_suggestion(generics, self_ty.span, &trait_name);
-            if !sugg.is_empty() {
-                diag.multipart_suggestion_verbose(
-                    format!("use a new generic type parameter, constrained by `{trait_name}`"),
-                    sugg,
-                    Applicability::MachineApplicable,
-                );
-                diag.multipart_suggestion_verbose(
-                    "you can also use an opaque type, but users won't be able to specify the type \
-                     parameter when calling the `fn`, having to rely exclusively on type inference",
-                    impl_sugg,
-                    Applicability::MachineApplicable,
-                );
-            }
+            diag.multipart_suggestion_verbose(
+                format!("use a new generic type parameter, constrained by `{trait_name}`"),
+                sugg,
+                Applicability::MachineApplicable,
+            );
+            diag.multipart_suggestion_verbose(
+                "you can also use an opaque type, but users won't be able to specify the type \
+                 parameter when calling the `fn`, having to rely exclusively on type inference",
+                impl_sugg,
+                Applicability::MachineApplicable,
+            );
             if !is_object_safe {
                 diag.note(format!("`{trait_name}` it is not object safe, so it can't be `dyn`"));
                 if is_downgradable {
                     // We'll emit the object safety error already, with a structured suggestion.
                     diag.downgrade_to_delayed_bug();
                 }
             } else {
+                // No ampersand in suggestion if it's borrowed already
+                let (dyn_str, paren_dyn_str) =
+                    if borrowed { ("dyn ", "(dyn ") } else { ("&dyn ", "&(dyn ") };
+
                 let sugg = if let hir::TyKind::TraitObject([_, _, ..], _, _) = self_ty.kind {
                     // There are more than one trait bound, we need surrounding parentheses.
                     vec![
-                        (self_ty.span.shrink_to_lo(), "&(dyn ".to_string()),
+                        (self_ty.span.shrink_to_lo(), paren_dyn_str.to_string()),
                         (self_ty.span.shrink_to_hi(), ")".to_string()),
                     ]
                 } else {
-                    vec![(self_ty.span.shrink_to_lo(), "&dyn ".to_string())]
+                    vec![(self_ty.span.shrink_to_lo(), dyn_str.to_string())]
                 };
                 diag.multipart_suggestion_verbose(
                     format!(

Diff for: compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs

@@ -4925,24 +4925,32 @@ impl<'v> Visitor<'v> for AwaitsVisitor {
     }
 }
 
+/// Suggest a new type parameter name for diagnostic purposes.
+///
+/// `name` is the preferred name you'd like to suggest if it's not in use already.
 pub trait NextTypeParamName {
     fn next_type_param_name(&self, name: Option<&str>) -> String;
 }
 
 impl NextTypeParamName for &[hir::GenericParam<'_>] {
     fn next_type_param_name(&self, name: Option<&str>) -> String {
-        // This is the list of possible parameter names that we might suggest.
+        // Type names are usually single letters in uppercase. So convert the first letter of input string to uppercase.
         let name = name.and_then(|n| n.chars().next()).map(|c| c.to_uppercase().to_string());
         let name = name.as_deref();
+
+        // This is the list of possible parameter names that we might suggest.
         let possible_names = [name.unwrap_or("T"), "T", "U", "V", "X", "Y", "Z", "A", "B", "C"];
-        let used_names = self
+
+        // Filter out used names based on `filter_fn`.
+        let used_names: Vec<Symbol> = self
             .iter()
-            .filter_map(|p| match p.name {
+            .filter_map(|param| match param.name {
                 hir::ParamName::Plain(ident) => Some(ident.name),
                 _ => None,
             })
-            .collect::<Vec<_>>();
+            .collect();
 
+        // Find a name from `possible_names` that is not in `used_names`.
         possible_names
             .iter()
             .find(|n| !used_names.contains(&Symbol::intern(n)))

Diff for: tests/ui/object-safety/reference-to-bare-trait-in-fn-inputs-and-outputs-issue-125139.rs

@@ -43,7 +43,7 @@ impl IceCream {
 
     fn parrot() -> &mut Trait {
         //~^ ERROR: missing lifetime specifier
-        //~| ERROR: cannot return a mutable reference to a bare trait
+        //~| ERROR: trait objects must include the `dyn` keyword
         &mut Type
         //~^ ERROR: cannot return reference to temporary value
     }
@@ -86,12 +86,12 @@ trait Sing {
 
     fn parrot() -> &mut Trait {
         //~^ ERROR: missing lifetime specifier
-        //~| ERROR: cannot return a mutable reference to a bare trait
+        //~| ERROR: trait objects must include the `dyn` keyword
         &mut Type
         //~^ ERROR: cannot return reference to temporary value
     }
 }
-    
+
 fn foo(_: &Trait) {}
 //~^ ERROR: trait objects must include the `dyn` keyword
 
@@ -134,7 +134,7 @@ fn puppy<'a>() -> &'a Trait {
 
 fn parrot() -> &mut Trait {
     //~^ ERROR: missing lifetime specifier
-    //~| ERROR: cannot return a mutable reference to a bare trait
+    //~| ERROR: trait objects must include the `dyn` keyword
     &mut Type
     //~^ ERROR: cannot return reference to temporary value
 }

Diff for: tests/ui/object-safety/reference-to-bare-trait-in-fn-inputs-and-outputs-issue-125139.stderr

@@ -298,8 +298,8 @@ LL |     fn cat() -> &Trait;
    |
 help: use `impl Trait` to return an opaque type, as long as you return a single underlying type
    |
-LL |     fn cat<'a>() -> &'a impl Trait;
-   |           ++++       +++++++
+LL |     fn cat() -> &impl Trait;
+   |                  ++++
 help: alternatively, you can return an owned trait object
    |
 LL |     fn cat() -> Box<dyn Trait>;
@@ -313,8 +313,8 @@ LL |     fn dog<'a>() -> &Trait {
    |
 help: use `impl Trait` to return an opaque type, as long as you return a single underlying type
    |
-LL |     fn dog<'b, 'a>() -> &'b impl Trait {
-   |            +++           +++++++
+LL |     fn dog<'a>() -> &impl Trait {
+   |                      ++++
 help: alternatively, you can return an owned trait object
    |
 LL |     fn dog<'a>() -> Box<dyn Trait> {
@@ -350,16 +350,16 @@ help: alternatively, you can return an owned trait object
 LL |     fn puppy<'a>() -> Box<dyn Trait> {
    |                       ~~~~~~~~~~~~~~
 
-error[E0782]: cannot return a mutable reference to a bare trait
+error[E0782]: trait objects must include the `dyn` keyword
   --> $DIR/reference-to-bare-trait-in-fn-inputs-and-outputs-issue-125139.rs:87:25
    |
 LL |     fn parrot() -> &mut Trait {
    |                         ^^^^^
    |
 help: use `impl Trait` to return an opaque type, as long as you return a single underlying type
    |
-LL |     fn parrot() -> impl Trait {
-   |                    ~~~~~~~~~~
+LL |     fn parrot() -> &mut impl Trait {
+   |                         ++++
 help: alternatively, you can return an owned trait object
    |
 LL |     fn parrot() -> Box<dyn Trait> {
@@ -449,8 +449,8 @@ LL | fn cat() -> &Trait {
    |
 help: use `impl Trait` to return an opaque type, as long as you return a single underlying type
    |
-LL | fn cat<'a>() -> &'a impl Trait {
-   |       ++++       +++++++
+LL | fn cat() -> &impl Trait {
+   |              ++++
 help: alternatively, you can return an owned trait object
    |
 LL | fn cat() -> Box<dyn Trait> {
@@ -464,8 +464,8 @@ LL | fn dog<'a>() -> &Trait {
    |
 help: use `impl Trait` to return an opaque type, as long as you return a single underlying type
    |
-LL | fn dog<'b, 'a>() -> &'b impl Trait {
-   |        +++           +++++++
+LL | fn dog<'a>() -> &impl Trait {
+   |                  ++++
 help: alternatively, you can return an owned trait object
    |
 LL | fn dog<'a>() -> Box<dyn Trait> {
@@ -501,16 +501,16 @@ help: alternatively, you can return an owned trait object
 LL | fn puppy<'a>() -> Box<dyn Trait> {
    |                   ~~~~~~~~~~~~~~
 
-error[E0782]: cannot return a mutable reference to a bare trait
+error[E0782]: trait objects must include the `dyn` keyword
   --> $DIR/reference-to-bare-trait-in-fn-inputs-and-outputs-issue-125139.rs:135:21
    |
 LL | fn parrot() -> &mut Trait {
    |                     ^^^^^
    |
 help: use `impl Trait` to return an opaque type, as long as you return a single underlying type
    |
-LL | fn parrot() -> impl Trait {
-   |                ~~~~~~~~~~
+LL | fn parrot() -> &mut impl Trait {
+   |                     ++++
 help: alternatively, you can return an owned trait object
    |
 LL | fn parrot() -> Box<dyn Trait> {
@@ -600,8 +600,8 @@ LL |     fn cat() -> &Trait {
    |
 help: use `impl Trait` to return an opaque type, as long as you return a single underlying type
    |
-LL |     fn cat<'a>() -> &'a impl Trait {
-   |           ++++       +++++++
+LL |     fn cat() -> &impl Trait {
+   |                  ++++
 help: alternatively, you can return an owned trait object
    |
 LL |     fn cat() -> Box<dyn Trait> {
@@ -615,8 +615,8 @@ LL |     fn dog<'a>() -> &Trait {
    |
 help: use `impl Trait` to return an opaque type, as long as you return a single underlying type
    |
-LL |     fn dog<'b, 'a>() -> &'b impl Trait {
-   |            +++           +++++++
+LL |     fn dog<'a>() -> &impl Trait {
+   |                      ++++
 help: alternatively, you can return an owned trait object
    |
 LL |     fn dog<'a>() -> Box<dyn Trait> {
@@ -652,16 +652,16 @@ help: alternatively, you can return an owned trait object
 LL |     fn puppy<'a>() -> Box<dyn Trait> {
    |                       ~~~~~~~~~~~~~~
 
-error[E0782]: cannot return a mutable reference to a bare trait
+error[E0782]: trait objects must include the `dyn` keyword
   --> $DIR/reference-to-bare-trait-in-fn-inputs-and-outputs-issue-125139.rs:44:25
    |
 LL |     fn parrot() -> &mut Trait {
    |                         ^^^^^
    |
 help: use `impl Trait` to return an opaque type, as long as you return a single underlying type
    |
-LL |     fn parrot() -> impl Trait {
-   |                    ~~~~~~~~~~
+LL |     fn parrot() -> &mut impl Trait {
+   |                         ++++
 help: alternatively, you can return an owned trait object
    |
 LL |     fn parrot() -> Box<dyn Trait> {