Wrap dyn type with parentheses in suggestion

Author: long-long-float

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()))),
+                )
             },
         )
     }

compiler/rustc_hir_typeck/src/method/suggest.rs

@@ -7,6 +7,7 @@ use crate::errors::{self, CandidateTraitNote, NoAssociatedItem};
 use crate::Expectation;
 use crate::FnCtxt;
 use core::ops::ControlFlow;
+use itertools::Itertools;
 use rustc_ast::ast::Mutability;
 use rustc_attr::parse_confusables;
 use rustc_data_structures::fx::{FxIndexMap, FxIndexSet};
@@ -3291,14 +3292,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,20 +3312,22 @@ 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,
-                                msg,
-                                candidates.iter().map(|t| {
+                            let mut suggs = vec![];
+
+                            let suggestion = candidates
+                                .iter()
+                                .map(|t| {
                                     format!(
                                         "{} {}",
                                         match introducer {
@@ -3331,9 +3337,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                                         },
                                         self.tcx.def_path_str(t.def_id)
                                     )
-                                }),
-                                Applicability::MaybeIncorrect,
-                            );
+                                })
+                                .join("");
+                            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));
+                            }
+
+                            err.multipart_suggestion(msg, suggs, Applicability::MaybeIncorrect);
+
                             return;
                         }
                         Node::Item(hir::Item {

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)

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;
         }
 

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,
         );
     }

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() {

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() {

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`.

tests/ui/suggestions/wrap-dyn-in-suggestion-issue-120223.rs

@@ -0,0 +1,29 @@
+#![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
+}
+
+pub fn in_ty_param<F: FnOnce() -> &'static dyn std::fmt::Debug>(f: F) {
+    f();
+    f(); //~ ERROR use of moved value
+}
+
+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() {}