Rollup merge of rust-lang#127987 - estebank:impl-trait-sugg, r=cjgillot

More accurate suggestion for `-> Box<dyn Trait>` or `-> impl Trait` When encountering `-> Trait`, suggest `-> Box<dyn Trait>` (instead of `-> Box<Trait>`. If there's a single returned type within the `fn`, suggest `-> impl Trait`.
rust-lang-ci · Jul 20, 2024 · 89798e9 · 89798e9
2 parents 40cfc88 + 3ff7588
commit 89798e9
Show file tree

Hide file tree

Showing 9 changed files with 71 additions and 64 deletions.
diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs
@@ -1793,25 +1793,42 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
         err.children.clear();
 
         let span = obligation.cause.span;
-        if let Ok(snip) = self.tcx.sess.source_map().span_to_snippet(span)
+        let body = self.tcx.hir().body_owned_by(obligation.cause.body_id);
+
+        let mut visitor = ReturnsVisitor::default();
+        visitor.visit_body(&body);
+
+        let (pre, impl_span) = if let Ok(snip) = self.tcx.sess.source_map().span_to_snippet(span)
             && snip.starts_with("dyn ")
         {
-            err.span_suggestion(
-                span.with_hi(span.lo() + BytePos(4)),
-                "return an `impl Trait` instead of a `dyn Trait`, \
-                if all returned values are the same type",
+            ("", span.with_hi(span.lo() + BytePos(4)))
+        } else {
+            ("dyn ", span.shrink_to_lo())
+        };
+        let alternatively = if visitor
+            .returns
+            .iter()
+            .map(|expr| self.typeck_results.as_ref().unwrap().expr_ty_adjusted_opt(expr))
+            .collect::<FxHashSet<_>>()
+            .len()
+            <= 1
+        {
+            err.span_suggestion_verbose(
+                impl_span,
+                "consider returning an `impl Trait` instead of a `dyn Trait`",
                 "impl ",
                 Applicability::MaybeIncorrect,
             );
-        }
-
-        let body = self.tcx.hir().body_owned_by(obligation.cause.body_id);
-
-        let mut visitor = ReturnsVisitor::default();
-        visitor.visit_body(&body);
+            "alternatively, "
+        } else {
+            err.help("if there were a single returned type, you could use `impl Trait` instead");
+            ""
+        };
 
-        let mut sugg =
-            vec![(span.shrink_to_lo(), "Box<".to_string()), (span.shrink_to_hi(), ">".to_string())];
+        let mut sugg = vec![
+            (span.shrink_to_lo(), format!("Box<{pre}")),
+            (span.shrink_to_hi(), ">".to_string()),
+        ];
         sugg.extend(visitor.returns.into_iter().flat_map(|expr| {
             let span =
                 expr.span.find_ancestor_in_same_ctxt(obligation.cause.span).unwrap_or(expr.span);
@@ -1837,7 +1854,10 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
         }));
 
         err.multipart_suggestion(
-            "box the return type, and wrap all of the returned values in `Box::new`",
+            format!(
+                "{alternatively}box the return type, and wrap all of the returned values in \
+                 `Box::new`",
+            ),
             sugg,
             Applicability::MaybeIncorrect,
         );

diff --git a/tests/ui/error-codes/E0746.stderr b/tests/ui/error-codes/E0746.stderr
@@ -4,11 +4,11 @@ error[E0746]: return type cannot have an unboxed trait object
 LL | fn foo() -> dyn Trait { Struct }
    |             ^^^^^^^^^ doesn't have a size known at compile-time
    |
-help: return an `impl Trait` instead of a `dyn Trait`, if all returned values are the same type
+help: consider returning an `impl Trait` instead of a `dyn Trait`
    |
 LL | fn foo() -> impl Trait { Struct }
    |             ~~~~
-help: box the return type, and wrap all of the returned values in `Box::new`
+help: alternatively, box the return type, and wrap all of the returned values in `Box::new`
    |
 LL | fn foo() -> Box<dyn Trait> { Box::new(Struct) }
    |             ++++         +   +++++++++      +
@@ -19,10 +19,7 @@ error[E0746]: return type cannot have an unboxed trait object
 LL | fn bar() -> dyn Trait {
    |             ^^^^^^^^^ doesn't have a size known at compile-time
    |
-help: return an `impl Trait` instead of a `dyn Trait`, if all returned values are the same type
-   |
-LL | fn bar() -> impl Trait {
-   |             ~~~~
+   = help: if there were a single returned type, you could use `impl Trait` instead
 help: box the return type, and wrap all of the returned values in `Box::new`
    |
 LL ~ fn bar() -> Box<dyn Trait> {

diff --git a/tests/ui/impl-trait/dyn-trait-return-should-be-impl-trait.stderr b/tests/ui/impl-trait/dyn-trait-return-should-be-impl-trait.stderr
@@ -48,22 +48,26 @@ error[E0746]: return type cannot have an unboxed trait object
 LL | fn bap() -> Trait { Struct }
    |             ^^^^^ doesn't have a size known at compile-time
    |
-help: box the return type, and wrap all of the returned values in `Box::new`
+help: consider returning an `impl Trait` instead of a `dyn Trait`
+   |
+LL | fn bap() -> impl Trait { Struct }
+   |             ++++
+help: alternatively, box the return type, and wrap all of the returned values in `Box::new`
    |
-LL | fn bap() -> Box<Trait> { Box::new(Struct) }
-   |             ++++     +   +++++++++      +
+LL | fn bap() -> Box<dyn Trait> { Box::new(Struct) }
+   |             +++++++      +   +++++++++      +
 
 error[E0746]: return type cannot have an unboxed trait object
   --> $DIR/dyn-trait-return-should-be-impl-trait.rs:15:13
    |
 LL | fn ban() -> dyn Trait { Struct }
    |             ^^^^^^^^^ doesn't have a size known at compile-time
    |
-help: return an `impl Trait` instead of a `dyn Trait`, if all returned values are the same type
+help: consider returning an `impl Trait` instead of a `dyn Trait`
    |
 LL | fn ban() -> impl Trait { Struct }
    |             ~~~~
-help: box the return type, and wrap all of the returned values in `Box::new`
+help: alternatively, box the return type, and wrap all of the returned values in `Box::new`
    |
 LL | fn ban() -> Box<dyn Trait> { Box::new(Struct) }
    |             ++++         +   +++++++++      +
@@ -74,11 +78,11 @@ error[E0746]: return type cannot have an unboxed trait object
 LL | fn bak() -> dyn Trait { unimplemented!() }
    |             ^^^^^^^^^ doesn't have a size known at compile-time
    |
-help: return an `impl Trait` instead of a `dyn Trait`, if all returned values are the same type
+help: consider returning an `impl Trait` instead of a `dyn Trait`
    |
 LL | fn bak() -> impl Trait { unimplemented!() }
    |             ~~~~
-help: box the return type, and wrap all of the returned values in `Box::new`
+help: alternatively, box the return type, and wrap all of the returned values in `Box::new`
    |
 LL | fn bak() -> Box<dyn Trait> { Box::new(unimplemented!()) }
    |             ++++         +   +++++++++                +
@@ -89,10 +93,7 @@ error[E0746]: return type cannot have an unboxed trait object
 LL | fn bal() -> dyn Trait {
    |             ^^^^^^^^^ doesn't have a size known at compile-time
    |
-help: return an `impl Trait` instead of a `dyn Trait`, if all returned values are the same type
-   |
-LL | fn bal() -> impl Trait {
-   |             ~~~~
+   = help: if there were a single returned type, you could use `impl Trait` instead
 help: box the return type, and wrap all of the returned values in `Box::new`
    |
 LL ~ fn bal() -> Box<dyn Trait> {
@@ -108,10 +109,7 @@ error[E0746]: return type cannot have an unboxed trait object
 LL | fn bax() -> dyn Trait {
    |             ^^^^^^^^^ doesn't have a size known at compile-time
    |
-help: return an `impl Trait` instead of a `dyn Trait`, if all returned values are the same type
-   |
-LL | fn bax() -> impl Trait {
-   |             ~~~~
+   = help: if there were a single returned type, you could use `impl Trait` instead
 help: box the return type, and wrap all of the returned values in `Box::new`
    |
 LL ~ fn bax() -> Box<dyn Trait> {
@@ -263,10 +261,7 @@ error[E0746]: return type cannot have an unboxed trait object
 LL | fn bat() -> dyn Trait {
    |             ^^^^^^^^^ doesn't have a size known at compile-time
    |
-help: return an `impl Trait` instead of a `dyn Trait`, if all returned values are the same type
-   |
-LL | fn bat() -> impl Trait {
-   |             ~~~~
+   = help: if there were a single returned type, you could use `impl Trait` instead
 help: box the return type, and wrap all of the returned values in `Box::new`
    |
 LL ~ fn bat() -> Box<dyn Trait> {
@@ -282,10 +277,7 @@ error[E0746]: return type cannot have an unboxed trait object
 LL | fn bay() -> dyn Trait {
    |             ^^^^^^^^^ doesn't have a size known at compile-time
    |
-help: return an `impl Trait` instead of a `dyn Trait`, if all returned values are the same type
-   |
-LL | fn bay() -> impl Trait {
-   |             ~~~~
+   = help: if there were a single returned type, you could use `impl Trait` instead
 help: box the return type, and wrap all of the returned values in `Box::new`
    |
 LL ~ fn bay() -> Box<dyn Trait> {

diff --git a/tests/ui/impl-trait/object-unsafe-trait-in-return-position-dyn-trait.stderr b/tests/ui/impl-trait/object-unsafe-trait-in-return-position-dyn-trait.stderr
@@ -54,10 +54,7 @@ error[E0746]: return type cannot have an unboxed trait object
 LL | fn car() -> dyn NotObjectSafe {
    |             ^^^^^^^^^^^^^^^^^ doesn't have a size known at compile-time
    |
-help: return an `impl Trait` instead of a `dyn Trait`, if all returned values are the same type
-   |
-LL | fn car() -> impl NotObjectSafe {
-   |             ~~~~
+   = help: if there were a single returned type, you could use `impl Trait` instead
 help: box the return type, and wrap all of the returned values in `Box::new`
    |
 LL ~ fn car() -> Box<dyn NotObjectSafe> {

diff --git a/tests/ui/impl-trait/point-to-type-err-cause-on-impl-trait-return.stderr b/tests/ui/impl-trait/point-to-type-err-cause-on-impl-trait-return.stderr
@@ -171,11 +171,11 @@ error[E0746]: return type cannot have an unboxed trait object
 LL | fn hat() -> dyn std::fmt::Display {
    |             ^^^^^^^^^^^^^^^^^^^^^ doesn't have a size known at compile-time
    |
-help: return an `impl Trait` instead of a `dyn Trait`, if all returned values are the same type
+help: consider returning an `impl Trait` instead of a `dyn Trait`
    |
 LL | fn hat() -> impl std::fmt::Display {
    |             ~~~~
-help: box the return type, and wrap all of the returned values in `Box::new`
+help: alternatively, box the return type, and wrap all of the returned values in `Box::new`
    |
 LL ~ fn hat() -> Box<dyn std::fmt::Display> {
 LL |     match 13 {
@@ -192,11 +192,11 @@ error[E0746]: return type cannot have an unboxed trait object
 LL | fn pug() -> dyn std::fmt::Display {
    |             ^^^^^^^^^^^^^^^^^^^^^ doesn't have a size known at compile-time
    |
-help: return an `impl Trait` instead of a `dyn Trait`, if all returned values are the same type
+help: consider returning an `impl Trait` instead of a `dyn Trait`
    |
 LL | fn pug() -> impl std::fmt::Display {
    |             ~~~~
-help: box the return type, and wrap all of the returned values in `Box::new`
+help: alternatively, box the return type, and wrap all of the returned values in `Box::new`
    |
 LL ~ fn pug() -> Box<dyn std::fmt::Display> {
 LL |     match 13 {
@@ -211,10 +211,7 @@ error[E0746]: return type cannot have an unboxed trait object
 LL | fn man() -> dyn std::fmt::Display {
    |             ^^^^^^^^^^^^^^^^^^^^^ doesn't have a size known at compile-time
    |
-help: return an `impl Trait` instead of a `dyn Trait`, if all returned values are the same type
-   |
-LL | fn man() -> impl std::fmt::Display {
-   |             ~~~~
+   = help: if there were a single returned type, you could use `impl Trait` instead
 help: box the return type, and wrap all of the returned values in `Box::new`
    |
 LL ~ fn man() -> Box<dyn std::fmt::Display> {

diff --git a/tests/ui/issues/issue-18107.stderr b/tests/ui/issues/issue-18107.stderr
@@ -4,11 +4,11 @@ error[E0746]: return type cannot have an unboxed trait object
 LL |     dyn AbstractRenderer
    |     ^^^^^^^^^^^^^^^^^^^^ doesn't have a size known at compile-time
    |
-help: return an `impl Trait` instead of a `dyn Trait`, if all returned values are the same type
+help: consider returning an `impl Trait` instead of a `dyn Trait`
    |
 LL |     impl AbstractRenderer
    |     ~~~~
-help: box the return type, and wrap all of the returned values in `Box::new`
+help: alternatively, box the return type, and wrap all of the returned values in `Box::new`
    |
 LL ~     Box<dyn AbstractRenderer>
 LL |

diff --git a/tests/ui/unsized/box-instead-of-dyn-fn.stderr b/tests/ui/unsized/box-instead-of-dyn-fn.stderr
@@ -4,11 +4,11 @@ error[E0746]: return type cannot have an unboxed trait object
 LL | fn print_on_or_the_other<'a>(a: i32, b: &'a String) -> dyn Fn() + 'a {
    |                                                        ^^^^^^^^^^^^^ doesn't have a size known at compile-time
    |
-help: return an `impl Trait` instead of a `dyn Trait`, if all returned values are the same type
+help: consider returning an `impl Trait` instead of a `dyn Trait`
    |
 LL | fn print_on_or_the_other<'a>(a: i32, b: &'a String) -> impl Fn() + 'a {
    |                                                        ~~~~
-help: box the return type, and wrap all of the returned values in `Box::new`
+help: alternatively, box the return type, and wrap all of the returned values in `Box::new`
    |
 LL ~ fn print_on_or_the_other<'a>(a: i32, b: &'a String) -> Box<dyn Fn() + 'a> {
 LL |

diff --git a/tests/ui/unsized/issue-91801.stderr b/tests/ui/unsized/issue-91801.stderr
@@ -4,10 +4,14 @@ error[E0746]: return type cannot have an unboxed trait object
 LL | fn or<'a>(first: &'static Validator<'a>, second: &'static Validator<'a>) -> Validator<'a> {
    |                                                                             ^^^^^^^^^^^^^ doesn't have a size known at compile-time
    |
-help: box the return type, and wrap all of the returned values in `Box::new`
+help: consider returning an `impl Trait` instead of a `dyn Trait`
    |
-LL | fn or<'a>(first: &'static Validator<'a>, second: &'static Validator<'a>) -> Box<Validator<'a>> {
-   |                                                                             ++++             +
+LL | fn or<'a>(first: &'static Validator<'a>, second: &'static Validator<'a>) -> impl Validator<'a> {
+   |                                                                             ++++
+help: alternatively, box the return type, and wrap all of the returned values in `Box::new`
+   |
+LL | fn or<'a>(first: &'static Validator<'a>, second: &'static Validator<'a>) -> Box<dyn Validator<'a>> {
+   |                                                                             +++++++              +
 
 error: aborting due to 1 previous error
 

diff --git a/tests/ui/unsized/issue-91803.stderr b/tests/ui/unsized/issue-91803.stderr
@@ -4,11 +4,11 @@ error[E0746]: return type cannot have an unboxed trait object
 LL | fn or<'a>(first: &'static dyn Foo<'a>) -> dyn Foo<'a> {
    |                                           ^^^^^^^^^^^ doesn't have a size known at compile-time
    |
-help: return an `impl Trait` instead of a `dyn Trait`, if all returned values are the same type
+help: consider returning an `impl Trait` instead of a `dyn Trait`
    |
 LL | fn or<'a>(first: &'static dyn Foo<'a>) -> impl Foo<'a> {
    |                                           ~~~~
-help: box the return type, and wrap all of the returned values in `Box::new`
+help: alternatively, box the return type, and wrap all of the returned values in `Box::new`
    |
 LL | fn or<'a>(first: &'static dyn Foo<'a>) -> Box<dyn Foo<'a>> {
    |                                           ++++           +