From 3ff758877fd994ebeb7a3a0a65fae4de3f66b8d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 19 Jul 2024 19:39:37 +0000 Subject: [PATCH] More accurate suggestion for `-> Box` or `-> impl Trait` When encountering `-> Trait`, suggest `-> Box` (instead of `-> Box`. If there's a single returned type within the `fn`, suggest `-> impl Trait`. --- .../src/error_reporting/traits/suggestions.rs | 50 +++++++++++++------ tests/ui/error-codes/E0746.stderr | 9 ++-- ...n-trait-return-should-be-impl-trait.stderr | 38 ++++++-------- ...-trait-in-return-position-dyn-trait.stderr | 5 +- ...type-err-cause-on-impl-trait-return.stderr | 13 ++--- tests/ui/issues/issue-18107.stderr | 4 +- tests/ui/unsized/box-instead-of-dyn-fn.stderr | 4 +- tests/ui/unsized/issue-91801.stderr | 10 ++-- tests/ui/unsized/issue-91803.stderr | 4 +- 9 files changed, 72 insertions(+), 65 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 index fa2acdd4a54..ffc8839435e 100644 --- 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 @@ fn suggest_impl_trait( err.children.clear(); let span = obligation.cause.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", - "impl ", - Applicability::MaybeIncorrect, - ); - } - let body = self.tcx.hir().body_owned_by(obligation.cause.body_id); let mut visitor = ReturnsVisitor::default(); visitor.visit_body(&body); - let mut sugg = - vec![(span.shrink_to_lo(), "Box<".to_string()), (span.shrink_to_hi(), ">".to_string())]; + let (pre, impl_span) = if let Ok(snip) = self.tcx.sess.source_map().span_to_snippet(span) + && snip.starts_with("dyn ") + { + ("", 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::>() + .len() + <= 1 + { + err.span_suggestion_verbose( + impl_span, + "consider returning an `impl Trait` instead of a `dyn Trait`", + "impl ", + Applicability::MaybeIncorrect, + ); + "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(), 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 @@ fn suggest_impl_trait( })); 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 index 9fe90ab7bec..cfc747cb1e2 100644 --- 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 { 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 { 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 index f25269ca032..9c546659564 100644 --- 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,10 +48,14 @@ 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() -> Box { Box::new(Struct) } - | ++++ + +++++++++ + +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 { Box::new(Struct) } + | +++++++ + +++++++++ + error[E0746]: return type cannot have an unboxed trait object --> $DIR/dyn-trait-return-should-be-impl-trait.rs:15:13 @@ -59,11 +63,11 @@ error[E0746]: return type cannot have an unboxed trait object 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 { 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 { 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 { @@ -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 { @@ -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 { @@ -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 { 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 index fc9c30abf13..09a689e6396 100644 --- 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 { 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 index 9205d74504f..54849c112f5 100644 --- 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 { 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 { 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 { diff --git a/tests/ui/issues/issue-18107.stderr b/tests/ui/issues/issue-18107.stderr index 702207c30f7..705f7d0df12 100644 --- 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 LL | diff --git a/tests/ui/unsized/box-instead-of-dyn-fn.stderr b/tests/ui/unsized/box-instead-of-dyn-fn.stderr index f2828b384b2..1f1845569ef 100644 --- 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 { LL | diff --git a/tests/ui/unsized/issue-91801.stderr b/tests/ui/unsized/issue-91801.stderr index d1d652d1860..e13cabbb81d 100644 --- 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> { - | ++++ + +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> { + | +++++++ + error: aborting due to 1 previous error diff --git a/tests/ui/unsized/issue-91803.stderr b/tests/ui/unsized/issue-91803.stderr index 632af02b4b6..3b89066499d 100644 --- 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> { | ++++ +