Wrap dyn type with parentheses in suggestion

10 files changed, 286 insertions, 62 deletions

diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs
index e4f8d77dbc2..2268905430a 100644
--- a/compiler/rustc_hir/src/hir.rs
+++ b/compiler/rustc_hir/src/hir.rs
@@ -644,13 +644,49 @@ impl<'hir> Generics<'hir> {
         })
     }
 
-    pub fn bounds_span_for_suggestions(&self, param_def_id: LocalDefId) -> Option<Span> {
+    /// Returns a suggestable empty span right after the "final" bound of the generic parameter.
+    ///
+    /// If that bound needs to be wrapped in parentheses to avoid ambiguity with
+    /// subsequent bounds, it also returns an empty span for an open parenthesis
+    /// as the second component.
+    ///
+    /// E.g., adding `+ 'static` after `Fn() -> dyn Future<Output = ()>` or
+    /// `Fn() -> &'static dyn Debug` requires parentheses:
+    /// `Fn() -> (dyn Future<Output = ()>) + 'static` and
+    /// `Fn() -> &'static (dyn Debug) + 'static`, respectively.
+    pub fn bounds_span_for_suggestions(
+        &self,
+        param_def_id: LocalDefId,
+    ) -> Option<(Span, Option<Span>)> {
         self.bounds_for_param(param_def_id).flat_map(|bp| bp.bounds.iter().rev()).find_map(
             |bound| {
-                // We include bounds that come from a `#[derive(_)]` but point at the user's code,
-                // as we use this method to get a span appropriate for suggestions.
-                let bs = bound.span();
-                bs.can_be_used_for_suggestions().then(|| bs.shrink_to_hi())
+                let span_for_parentheses = if let Some(trait_ref) = bound.trait_ref()
+                    && let [.., segment] = trait_ref.path.segments
+                    && segment.args().parenthesized == GenericArgsParentheses::ParenSugar
+                    && let [binding] = segment.args().bindings
+                    && let TypeBindingKind::Equality { term: Term::Ty(ret_ty) } = binding.kind
+                    && let ret_ty = ret_ty.peel_refs()
+                    && let TyKind::TraitObject(
+                        _,
+                        _,
+                        TraitObjectSyntax::Dyn | TraitObjectSyntax::DynStar,
+                    ) = ret_ty.kind
+                    && ret_ty.span.can_be_used_for_suggestions()
+                {
+                    Some(ret_ty.span)
+                } else {
+                    None
+                };
+
+                span_for_parentheses.map_or_else(
+                    || {
+                        // We include bounds that come from a `#[derive(_)]` but point at the user's code,
+                        // as we use this method to get a span appropriate for suggestions.
+                        let bs = bound.span();
+                        bs.can_be_used_for_suggestions().then(|| (bs.shrink_to_hi(), None))
+                    },
+                    |span| Some((span.shrink_to_hi(), Some(span.shrink_to_lo()))),
+                )
             },
         )
     }
diff --git a/compiler/rustc_hir_typeck/src/method/suggest.rs b/compiler/rustc_hir_typeck/src/method/suggest.rs
index 46227e406a3..be9ac962434 100644
--- a/compiler/rustc_hir_typeck/src/method/suggest.rs
+++ b/compiler/rustc_hir_typeck/src/method/suggest.rs
@@ -3291,14 +3291,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                                 param.name.ident(),
                             ));
                             let bounds_span = hir_generics.bounds_span_for_suggestions(def_id);
-                            if rcvr_ty.is_ref() && param.is_impl_trait() && bounds_span.is_some() {
+                            if rcvr_ty.is_ref()
+                                && param.is_impl_trait()
+                                && let Some((bounds_span, _)) = bounds_span
+                            {
                                 err.multipart_suggestions(
                                     msg,
                                     candidates.iter().map(|t| {
                                         vec![
                                             (param.span.shrink_to_lo(), "(".to_string()),
                                             (
-                                                bounds_span.unwrap(),
+                                                bounds_span,
                                                 format!(" + {})", self.tcx.def_path_str(t.def_id)),
                                             ),
                                         ]
@@ -3308,32 +3311,46 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                                 return;
                             }
 
-                            let (sp, introducer) = if let Some(span) = bounds_span {
-                                (span, Introducer::Plus)
-                            } else if let Some(colon_span) = param.colon_span {
-                                (colon_span.shrink_to_hi(), Introducer::Nothing)
-                            } else if param.is_impl_trait() {
-                                (param.span.shrink_to_hi(), Introducer::Plus)
-                            } else {
-                                (param.span.shrink_to_hi(), Introducer::Colon)
-                            };
+                            let (sp, introducer, open_paren_sp) =
+                                if let Some((span, open_paren_sp)) = bounds_span {
+                                    (span, Introducer::Plus, open_paren_sp)
+                                } else if let Some(colon_span) = param.colon_span {
+                                    (colon_span.shrink_to_hi(), Introducer::Nothing, None)
+                                } else if param.is_impl_trait() {
+                                    (param.span.shrink_to_hi(), Introducer::Plus, None)
+                                } else {
+                                    (param.span.shrink_to_hi(), Introducer::Colon, None)
+                                };
 
-                            err.span_suggestions(
-                                sp,
+                            let all_suggs = candidates.iter().map(|cand| {
+                                let suggestion = format!(
+                                    "{} {}",
+                                    match introducer {
+                                        Introducer::Plus => " +",
+                                        Introducer::Colon => ":",
+                                        Introducer::Nothing => "",
+                                    },
+                                    self.tcx.def_path_str(cand.def_id)
+                                );
+
+                                let mut suggs = vec![];
+
+                                if let Some(open_paren_sp) = open_paren_sp {
+                                    suggs.push((open_paren_sp, "(".to_string()));
+                                    suggs.push((sp, format!("){suggestion}")));
+                                } else {
+                                    suggs.push((sp, suggestion));
+                                }
+
+                                suggs
+                            });
+
+                            err.multipart_suggestions(
                                 msg,
-                                candidates.iter().map(|t| {
-                                    format!(
-                                        "{} {}",
-                                        match introducer {
-                                            Introducer::Plus => " +",
-                                            Introducer::Colon => ":",
-                                            Introducer::Nothing => "",
-                                        },
-                                        self.tcx.def_path_str(t.def_id)
-                                    )
-                                }),
+                                all_suggs,
                                 Applicability::MaybeIncorrect,
                             );
+
                             return;
                         }
                         Node::Item(hir::Item {
diff --git a/compiler/rustc_infer/src/infer/error_reporting/mod.rs b/compiler/rustc_infer/src/infer/error_reporting/mod.rs
index fdae9d84b5f..c8e6580f4b7 100644
--- a/compiler/rustc_infer/src/infer/error_reporting/mod.rs
+++ b/compiler/rustc_infer/src/infer/error_reporting/mod.rs
@@ -2369,7 +2369,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
                 generic_param_scope = self.tcx.local_parent(generic_param_scope);
             }
 
-            // type_param_sugg_span is (span, has_bounds)
+            // type_param_sugg_span is (span, has_bounds, needs_parentheses)
             let (type_scope, type_param_sugg_span) = match bound_kind {
                 GenericKind::Param(ref param) => {
                     let generics = self.tcx.generics_of(generic_param_scope);
@@ -2380,10 +2380,10 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
                     // instead we suggest `T: 'a + 'b` in that case.
                     let hir_generics = self.tcx.hir().get_generics(scope).unwrap();
                     let sugg_span = match hir_generics.bounds_span_for_suggestions(def_id) {
-                        Some(span) => Some((span, true)),
+                        Some((span, open_paren_sp)) => Some((span, true, open_paren_sp)),
                         // If `param` corresponds to `Self`, no usable suggestion span.
                         None if generics.has_self && param.index == 0 => None,
-                        None => Some((self.tcx.def_span(def_id).shrink_to_hi(), false)),
+                        None => Some((self.tcx.def_span(def_id).shrink_to_hi(), false, None)),
                     };
                     (scope, sugg_span)
                 }
@@ -2406,12 +2406,18 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
             let mut suggs = vec![];
             let lt_name = self.suggest_name_region(sub, &mut suggs);
 
-            if let Some((sp, has_lifetimes)) = type_param_sugg_span
+            if let Some((sp, has_lifetimes, open_paren_sp)) = type_param_sugg_span
                 && suggestion_scope == type_scope
             {
                 let suggestion =
                     if has_lifetimes { format!(" + {lt_name}") } else { format!(": {lt_name}") };
-                suggs.push((sp, suggestion))
+
+                if let Some(open_paren_sp) = open_paren_sp {
+                    suggs.push((open_paren_sp, "(".to_string()));
+                    suggs.push((sp, format!("){suggestion}")));
+                } else {
+                    suggs.push((sp, suggestion))
+                }
             } else if let GenericKind::Alias(ref p) = bound_kind
                 && let ty::Projection = p.kind(self.tcx)
                 && let DefKind::AssocTy = self.tcx.def_kind(p.def_id)
diff --git a/compiler/rustc_middle/src/ty/diagnostics.rs b/compiler/rustc_middle/src/ty/diagnostics.rs
index cc1d6e50f6d..8c3ee6955f5 100644
--- a/compiler/rustc_middle/src/ty/diagnostics.rs
+++ b/compiler/rustc_middle/src/ty/diagnostics.rs
@@ -279,24 +279,29 @@ pub fn suggest_constraining_type_params<'a>(
         constraint.sort();
         constraint.dedup();
         let constraint = constraint.join(" + ");
-        let mut suggest_restrict = |span, bound_list_non_empty| {
-            suggestions.push((
-                span,
-                if span_to_replace.is_some() {
-                    constraint.clone()
-                } else if constraint.starts_with('<') {
-                    constraint.to_string()
-                } else if bound_list_non_empty {
-                    format!(" + {constraint}")
-                } else {
-                    format!(" {constraint}")
-                },
-                SuggestChangingConstraintsMessage::RestrictBoundFurther,
-            ))
+        let mut suggest_restrict = |span, bound_list_non_empty, open_paren_sp| {
+            let suggestion = if span_to_replace.is_some() {
+                constraint.clone()
+            } else if constraint.starts_with('<') {
+                constraint.to_string()
+            } else if bound_list_non_empty {
+                format!(" + {constraint}")
+            } else {
+                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));
+            } else {
+                suggestions.push((span, suggestion, RestrictBoundFurther));
+            }
         };
 
         if let Some(span) = span_to_replace {
-            suggest_restrict(span, true);
+            suggest_restrict(span, true, None);
             continue;
         }
 
@@ -327,8 +332,8 @@ pub fn suggest_constraining_type_params<'a>(
         //          --
         //          |
         //          replace with: `T: Bar +`
-        if let Some(span) = generics.bounds_span_for_suggestions(param.def_id) {
-            suggest_restrict(span, true);
+        if let Some((span, open_paren_sp)) = generics.bounds_span_for_suggestions(param.def_id) {
+            suggest_restrict(span, true, open_paren_sp);
             continue;
         }
 
diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs
index 96596de32aa..0e309689680 100644
--- a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs
+++ b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs
@@ -2938,17 +2938,28 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
             }
             _ => {}
         };
+
         // Didn't add an indirection suggestion, so add a general suggestion to relax `Sized`.
-        let (span, separator) = if let Some(s) = generics.bounds_span_for_suggestions(param.def_id)
-        {
-            (s, " +")
+        let (span, separator, open_paren_sp) =
+            if let Some((s, open_paren_sp)) = generics.bounds_span_for_suggestions(param.def_id) {
+                (s, " +", open_paren_sp)
+            } else {
+                (param.name.ident().span.shrink_to_hi(), ":", None)
+            };
+
+        let mut suggs = vec![];
+        let suggestion = format!("{separator} ?Sized");
+
+        if let Some(open_paren_sp) = open_paren_sp {
+            suggs.push((open_paren_sp, "(".to_string()));
+            suggs.push((span, format!("){suggestion}")));
         } else {
-            (param.name.ident().span.shrink_to_hi(), ":")
-        };
-        err.span_suggestion_verbose(
-            span,
+            suggs.push((span, suggestion));
+        }
+
+        err.multipart_suggestion_verbose(
             "consider relaxing the implicit `Sized` restriction",
-            format!("{separator} ?Sized"),
+            suggs,
             Applicability::MachineApplicable,
         );
     }
diff --git a/tests/ui/suggestions/impl-trait-with-missing-trait-bounds-in-arg.fixed b/tests/ui/suggestions/impl-trait-with-missing-trait-bounds-in-arg.fixed
index 232d1dd8138..69780648ab6 100644
--- a/tests/ui/suggestions/impl-trait-with-missing-trait-bounds-in-arg.fixed
+++ b/tests/ui/suggestions/impl-trait-with-missing-trait-bounds-in-arg.fixed
@@ -12,7 +12,18 @@ impl Foo for S {}
 impl Bar for S {}
 
 fn test(foo: impl Foo + Bar) {
-    foo.hello(); //~ ERROR E0599
+    foo.hello(); //~ ERROR no method named `hello` found
+}
+
+trait Trait {
+    fn method(&self) {}
+}
+
+impl Trait for fn() {}
+
+#[allow(dead_code)]
+fn test2(f: impl Fn() -> (dyn std::fmt::Debug) + Trait) {
+    f.method(); //~ ERROR no method named `method` found
 }
 
 fn main() {
diff --git a/tests/ui/suggestions/impl-trait-with-missing-trait-bounds-in-arg.rs b/tests/ui/suggestions/impl-trait-with-missing-trait-bounds-in-arg.rs
index ab25b362fed..f49512bdd62 100644
--- a/tests/ui/suggestions/impl-trait-with-missing-trait-bounds-in-arg.rs
+++ b/tests/ui/suggestions/impl-trait-with-missing-trait-bounds-in-arg.rs
@@ -12,7 +12,18 @@ impl Foo for S {}
 impl Bar for S {}
 
 fn test(foo: impl Foo) {
-    foo.hello(); //~ ERROR E0599
+    foo.hello(); //~ ERROR no method named `hello` found
+}
+
+trait Trait {
+    fn method(&self) {}
+}
+
+impl Trait for fn() {}
+
+#[allow(dead_code)]
+fn test2(f: impl Fn() -> dyn std::fmt::Debug) {
+    f.method(); //~ ERROR no method named `method` found
 }
 
 fn main() {
diff --git a/tests/ui/suggestions/impl-trait-with-missing-trait-bounds-in-arg.stderr b/tests/ui/suggestions/impl-trait-with-missing-trait-bounds-in-arg.stderr
index 3a6052448cb..0aced78ddac 100644
--- a/tests/ui/suggestions/impl-trait-with-missing-trait-bounds-in-arg.stderr
+++ b/tests/ui/suggestions/impl-trait-with-missing-trait-bounds-in-arg.stderr
@@ -12,6 +12,20 @@ help: the following trait defines an item `hello`, perhaps you need to restrict
 LL | fn test(foo: impl Foo + Bar) {
    |                       +++++
 
-error: aborting due to 1 previous error
+error[E0599]: no method named `method` found for type parameter `impl Fn() -> dyn std::fmt::Debug` in the current scope
+  --> $DIR/impl-trait-with-missing-trait-bounds-in-arg.rs:26:7
+   |
+LL | fn test2(f: impl Fn() -> dyn std::fmt::Debug) {
+   |             -------------------------------- method `method` not found for this type parameter
+LL |     f.method();
+   |       ^^^^^^ method not found in `impl Fn() -> dyn std::fmt::Debug`
+   |
+   = help: items from traits can only be used if the type parameter is bounded by the trait
+help: the following trait defines an item `method`, perhaps you need to restrict type parameter `impl Fn() -> dyn std::fmt::Debug` with it:
+   |
+LL | fn test2(f: impl Fn() -> (dyn std::fmt::Debug) + Trait) {
+   |                          +                   +++++++++
+
+error: aborting due to 2 previous errors
 
 For more information about this error, try `rustc --explain E0599`.
diff --git a/tests/ui/suggestions/wrap-dyn-in-suggestion-issue-120223.rs b/tests/ui/suggestions/wrap-dyn-in-suggestion-issue-120223.rs
new file mode 100644
index 00000000000..6a273997ee6
--- /dev/null
+++ b/tests/ui/suggestions/wrap-dyn-in-suggestion-issue-120223.rs
@@ -0,0 +1,35 @@
+#![feature(dyn_star)] //~ WARNING the feature `dyn_star` is incomplete
+
+use std::future::Future;
+
+pub fn dyn_func<T>(
+    executor: impl FnOnce(T) -> dyn Future<Output = ()>,
+) -> Box<dyn FnOnce(T) -> dyn Future<Output = ()>> {
+    Box::new(executor) //~ ERROR may not live long enough
+}
+
+pub fn dyn_star_func<T>(
+    executor: impl FnOnce(T) -> dyn* Future<Output = ()>,
+) -> Box<dyn FnOnce(T) -> dyn* Future<Output = ()>> {
+    Box::new(executor) //~ ERROR may not live long enough
+}
+
+trait Trait {
+    fn method(&self) {}
+}
+
+impl Trait for fn() {}
+
+pub fn in_ty_param<T: Fn() -> dyn std::fmt::Debug> (t: T) {
+    t.method();
+    //~^ ERROR no method named `method` found for type parameter `T`
+}
+
+fn with_sized<T: Fn() -> &'static (dyn std::fmt::Debug) + ?Sized>() {
+    without_sized::<T>();
+    //~^ ERROR the size for values of type `T` cannot be known at compilation time
+}
+
+fn without_sized<T: Fn() -> &'static dyn std::fmt::Debug>() {}
+
+fn main() {}
diff --git a/tests/ui/suggestions/wrap-dyn-in-suggestion-issue-120223.stderr b/tests/ui/suggestions/wrap-dyn-in-suggestion-issue-120223.stderr
new file mode 100644
index 00000000000..f7fc17ea24f
--- /dev/null
+++ b/tests/ui/suggestions/wrap-dyn-in-suggestion-issue-120223.stderr
@@ -0,0 +1,78 @@
+warning: the feature `dyn_star` is incomplete and may not be safe to use and/or cause compiler crashes
+  --> $DIR/wrap-dyn-in-suggestion-issue-120223.rs:1:12
+   |
+LL | #![feature(dyn_star)]
+   |            ^^^^^^^^
+   |
+   = note: see issue #102425 <https://github.com/rust-lang/rust/issues/102425> for more information
+   = note: `#[warn(incomplete_features)]` on by default
+
+error[E0599]: no method named `method` found for type parameter `T` in the current scope
+  --> $DIR/wrap-dyn-in-suggestion-issue-120223.rs:24:7
+   |
+LL | pub fn in_ty_param<T: Fn() -> dyn std::fmt::Debug> (t: T) {
+   |                    - method `method` not found for this type parameter
+LL |     t.method();
+   |       ^^^^^^ method not found in `T`
+   |
+   = help: items from traits can only be used if the type parameter is bounded by the trait
+help: the following trait defines an item `method`, perhaps you need to restrict type parameter `T` with it:
+   |
+LL | pub fn in_ty_param<T: Fn() -> (dyn std::fmt::Debug) + Trait> (t: T) {
+   |                               +                   +++++++++
+
+error[E0277]: the size for values of type `T` cannot be known at compilation time
+  --> $DIR/wrap-dyn-in-suggestion-issue-120223.rs:29:21
+   |
+LL | fn with_sized<T: Fn() -> &'static (dyn std::fmt::Debug) + ?Sized>() {
+   |               - this type parameter needs to be `Sized`
+LL |     without_sized::<T>();
+   |                     ^ doesn't have a size known at compile-time
+   |
+note: required by an implicit `Sized` bound in `without_sized`
+  --> $DIR/wrap-dyn-in-suggestion-issue-120223.rs:33:18
+   |
+LL | fn without_sized<T: Fn() -> &'static dyn std::fmt::Debug>() {}
+   |                  ^ required by the implicit `Sized` requirement on this type parameter in `without_sized`
+help: consider removing the `?Sized` bound to make the type parameter `Sized`
+   |
+LL - fn with_sized<T: Fn() -> &'static (dyn std::fmt::Debug) + ?Sized>() {
+LL + fn with_sized<T: Fn() -> &'static (dyn std::fmt::Debug)>() {
+   |
+help: consider relaxing the implicit `Sized` restriction
+   |
+LL | fn without_sized<T: Fn() -> &'static (dyn std::fmt::Debug) + ?Sized>() {}
+   |                                      +                   ++++++++++
+
+error[E0310]: the parameter type `impl FnOnce(T) -> dyn Future<Output = ()>` may not live long enough
+  --> $DIR/wrap-dyn-in-suggestion-issue-120223.rs:8:5
+   |
+LL |     Box::new(executor)
+   |     ^^^^^^^^^^^^^^^^^^
+   |     |
+   |     the parameter type `impl FnOnce(T) -> dyn Future<Output = ()>` must be valid for the static lifetime...
+   |     ...so that the type `impl FnOnce(T) -> dyn Future<Output = ()>` will meet its required lifetime bounds
+   |
+help: consider adding an explicit lifetime bound
+   |
+LL |     executor: impl FnOnce(T) -> (dyn Future<Output = ()>) + 'static,
+   |                                 +                       +++++++++++
+
+error[E0310]: the parameter type `impl FnOnce(T) -> Future<Output = ()>` may not live long enough
+  --> $DIR/wrap-dyn-in-suggestion-issue-120223.rs:14:5
+   |
+LL |     Box::new(executor)
+   |     ^^^^^^^^^^^^^^^^^^
+   |     |
+   |     the parameter type `impl FnOnce(T) -> Future<Output = ()>` must be valid for the static lifetime...
+   |     ...so that the type `impl FnOnce(T) -> Future<Output = ()>` will meet its required lifetime bounds
+   |
+help: consider adding an explicit lifetime bound
+   |
+LL |     executor: impl FnOnce(T) -> (dyn* Future<Output = ()>) + 'static,
+   |                                 +                        +++++++++++
+
+error: aborting due to 4 previous errors; 1 warning emitted
+
+Some errors have detailed explanations: E0277, E0310, E0599.
+For more information about an error, try `rustc --explain E0277`.