about summary refs log tree commit diff
path: root/compiler/rustc_hir_analysis/src
diff options
context:
space:
mode:
authorMatthias Krüger <matthias.krueger@famsik.de>2024-09-03 19:13:23 +0200
committerGitHub <noreply@github.com>2024-09-03 19:13:23 +0200
commitf75a1954eb1c358c5a2ef99736dabac636d40ef7 (patch)
tree051ee3b74a1048981f38b3a8f14e171bfbbbdac8 /compiler/rustc_hir_analysis/src
parentd6c8169c186ab16a3404cd0d0866674018e8a19e (diff)
parent12de141df2d247177e9ca0498ba99bd433cfa500 (diff)
downloadrust-f75a1954eb1c358c5a2ef99736dabac636d40ef7.tar.gz
rust-f75a1954eb1c358c5a2ef99736dabac636d40ef7.zip
Rollup merge of #127692 - veera-sivarajan:bugfix-125139, r=estebank
Suggest `impl Trait` for References to Bare Trait in Function Header

Fixes #125139

This PR suggests `impl Trait` when `&Trait` is found as a function parameter type or return type. This makes use of existing diagnostics by adding `peel_refs()` when checking for type equality.

Additionaly, it makes a few other improvements:
1. Checks if functions inside impl blocks have bare trait in their headers.
2. Introduces a trait `NextLifetimeParamName` similar to the existing `NextTypeParamName` for suggesting a lifetime name. Also, abstracts out the common logic between the two trait impls.

### Related Issues
I ran into a bunch of related diagnostic issues but couldn't fix them within the scope of this PR. So, I have created the following issues:
1. [Misleading Suggestion when Returning a Reference to a Bare Trait from a Function](https://github.com/rust-lang/rust/issues/127689)
2. [Verbose Error When a Function Takes a Bare Trait as Parameter](https://github.com/rust-lang/rust/issues/127690)
3. [Incorrect Suggestion when Returning a Bare Trait from a Function](https://github.com/rust-lang/rust/issues/127691)

r​? ```@estebank``` since you implemented  #119148
Diffstat (limited to 'compiler/rustc_hir_analysis/src')
-rw-r--r--compiler/rustc_hir_analysis/src/hir_ty_lowering/lint.rs78
1 files changed, 55 insertions, 23 deletions
diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/lint.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/lint.rs
index 7be45463f15..149bc6d2698 100644
--- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/lint.rs
+++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/lint.rs
@@ -133,9 +133,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 \
@@ -170,6 +167,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)
@@ -180,6 +181,12 @@ 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 {
@@ -187,6 +194,8 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
         };
         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 {
@@ -202,8 +211,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, ")
@@ -214,14 +230,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 {
@@ -230,24 +258,24 @@ 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 {
@@ -255,14 +283,18 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
                     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!(