From d3c96f03b5f6b4457cbabf4f3e1ec6c1aed23385 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 10 Apr 2020 10:19:47 -0700 Subject: [PATCH] Suggest `-> impl Trait` and `-> Box` on fn that doesn't return During development, a function could have a return type set that is a bare trait object by accident. We already suggest using either a boxed trait object or `impl Trait` if the return paths will allow it. We now do so too when there are *no* return paths or they all resolve to `!`. We still don't handle cases where the trait object is *not* the entirety of the return type gracefully. --- .../traits/error_reporting/suggestions.rs | 85 +++++++++++++------ src/test/ui/error-codes/E0746.stderr | 4 +- .../dyn-trait-return-should-be-impl-trait.rs | 2 +- ...n-trait-return-should-be-impl-trait.stderr | 28 ++++-- src/test/ui/issues/issue-18107.rs | 2 +- src/test/ui/issues/issue-18107.stderr | 22 +++-- 6 files changed, 101 insertions(+), 42 deletions(-) diff --git a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs index 2b19699d6ec..abebbe9e903 100644 --- a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs +++ b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs @@ -826,12 +826,12 @@ fn suggest_impl_trait( .iter() .filter_map(|expr| tables.node_type_opt(expr.hir_id)) .map(|ty| self.resolve_vars_if_possible(&ty)); - let (last_ty, all_returns_have_same_type) = ret_types.clone().fold( - (None, true), - |(last_ty, mut same): (std::option::Option>, bool), ty| { + let (last_ty, all_returns_have_same_type, count) = ret_types.clone().fold( + (None, true, 0), + |(last_ty, mut same, count): (std::option::Option>, bool, usize), ty| { let ty = self.resolve_vars_if_possible(&ty); same &= last_ty.map_or(true, |last_ty| last_ty == ty) && ty.kind != ty::Error; - (Some(ty), same) + (Some(ty), same, count + 1) }, ); let all_returns_conform_to_trait = @@ -846,7 +846,7 @@ fn suggest_impl_trait( let obl = Obligation::new(cause.clone(), param_env, pred); self.predicate_may_hold(&obl) }) - }) + }) || count == 0 } _ => false, } @@ -855,21 +855,19 @@ fn suggest_impl_trait( }; let sm = self.tcx.sess.source_map(); - let (snippet, last_ty) = - if let (true, hir::TyKind::TraitObject(..), Ok(snippet), true, Some(last_ty)) = ( - // Verify that we're dealing with a return `dyn Trait` - ret_ty.span.overlaps(span), - &ret_ty.kind, - sm.span_to_snippet(ret_ty.span), - // If any of the return types does not conform to the trait, then we can't - // suggest `impl Trait` nor trait objects, it is a type mismatch error. - all_returns_conform_to_trait, - last_ty, - ) { - (snippet, last_ty) - } else { - return false; - }; + let snippet = if let (true, hir::TyKind::TraitObject(..), Ok(snippet), true) = ( + // Verify that we're dealing with a return `dyn Trait` + ret_ty.span.overlaps(span), + &ret_ty.kind, + sm.span_to_snippet(ret_ty.span), + // If any of the return types does not conform to the trait, then we can't + // suggest `impl Trait` nor trait objects, it is a type mismatch error. + all_returns_conform_to_trait, + ) { + snippet + } else { + return false; + }; err.code(error_code!(E0746)); err.set_primary_message("return type cannot have an unboxed trait object"); err.children.clear(); @@ -881,13 +879,50 @@ fn suggest_impl_trait( #using-trait-objects-that-allow-for-values-of-different-types>"; let has_dyn = snippet.split_whitespace().next().map_or(false, |s| s == "dyn"); let trait_obj = if has_dyn { &snippet[4..] } else { &snippet[..] }; - if all_returns_have_same_type { + if count == 0 { + // No return paths. Suggest `-> T`, `-> impl Trait`, and if `Trait` is object safe, + // `-> Box`. + err.note( + "currently nothing is being returned, depending on the final implementation \ + you could change the return type in different ways", + ); + err.span_suggestion( + ret_ty.span, + "you could use some type `T` that is `T: Sized` as the return type if all return \ + paths will have the same type", + "T".to_string(), + Applicability::MaybeIncorrect, + ); + err.span_suggestion( + ret_ty.span, + &format!( + "you could use `impl {}` as the return type if all return paths will have the \ + same type but you want to expose only the trait in the signature", + trait_obj, + ), + format!("impl {}", trait_obj), + Applicability::MaybeIncorrect, + ); + err.note(impl_trait_msg); + if is_object_safe { + err.span_suggestion( + ret_ty.span, + &format!( + "you could use a boxed trait object if all return paths `impl` trait `{}`", + trait_obj, + ), + format!("Box", trait_obj), + Applicability::MaybeIncorrect, + ); + err.note(trait_obj_msg); + } + } else if let (Some(last_ty), true) = (last_ty, all_returns_have_same_type) { // Suggest `-> impl Trait`. err.span_suggestion( ret_ty.span, &format!( - "return `impl {1}` instead, as all return paths are of type `{}`, \ - which implements `{1}`", + "use `impl {1}` as the return type, as all return paths are of type `{}`, \ + which implements `{1}`", last_ty, trait_obj, ), format!("impl {}", trait_obj), @@ -925,8 +960,8 @@ fn suggest_impl_trait( } err.note(trait_obj_msg); err.note(&format!( - "if all the returned values were of the same type you could use \ - `impl {}` as the return type", + "if all the returned values were of the same type you could use `impl {}` as the \ + return type", trait_obj, )); err.note(impl_trait_msg); diff --git a/src/test/ui/error-codes/E0746.stderr b/src/test/ui/error-codes/E0746.stderr index e7a8fd304ca..3757ed6d092 100644 --- a/src/test/ui/error-codes/E0746.stderr +++ b/src/test/ui/error-codes/E0746.stderr @@ -5,7 +5,7 @@ LL | fn foo() -> dyn Trait { Struct } | ^^^^^^^^^ doesn't have a size known at compile-time | = note: for information on `impl Trait`, see -help: return `impl Trait` instead, as all return paths are of type `Struct`, which implements `Trait` +help: use `impl Trait` as the return type, as all return paths are of type `Struct`, which implements `Trait` | LL | fn foo() -> impl Trait { Struct } | ^^^^^^^^^^ @@ -17,7 +17,7 @@ LL | fn bar() -> dyn Trait { | ^^^^^^^^^ doesn't have a size known at compile-time | = note: for information on `impl Trait`, see -help: return `impl Trait` instead, as all return paths are of type `{integer}`, which implements `Trait` +help: use `impl Trait` as the return type, as all return paths are of type `{integer}`, which implements `Trait` | LL | fn bar() -> impl Trait { | ^^^^^^^^^^ diff --git a/src/test/ui/impl-trait/dyn-trait-return-should-be-impl-trait.rs b/src/test/ui/impl-trait/dyn-trait-return-should-be-impl-trait.rs index 08bab5734fd..af368203de0 100644 --- a/src/test/ui/impl-trait/dyn-trait-return-should-be-impl-trait.rs +++ b/src/test/ui/impl-trait/dyn-trait-return-should-be-impl-trait.rs @@ -14,7 +14,7 @@ fn bap() -> Trait { Struct } //~^ ERROR E0746 fn ban() -> dyn Trait { Struct } //~^ ERROR E0746 -fn bak() -> dyn Trait { unimplemented!() } //~ ERROR E0277 +fn bak() -> dyn Trait { unimplemented!() } //~ ERROR E0746 // Suggest using `Box` fn bal() -> dyn Trait { //~ ERROR E0746 if true { diff --git a/src/test/ui/impl-trait/dyn-trait-return-should-be-impl-trait.stderr b/src/test/ui/impl-trait/dyn-trait-return-should-be-impl-trait.stderr index 664a897c593..a01bfc4a81f 100644 --- a/src/test/ui/impl-trait/dyn-trait-return-should-be-impl-trait.stderr +++ b/src/test/ui/impl-trait/dyn-trait-return-should-be-impl-trait.stderr @@ -49,7 +49,7 @@ LL | fn bap() -> Trait { Struct } | ^^^^^ doesn't have a size known at compile-time | = note: for information on `impl Trait`, see -help: return `impl Trait` instead, as all return paths are of type `Struct`, which implements `Trait` +help: use `impl Trait` as the return type, as all return paths are of type `Struct`, which implements `Trait` | LL | fn bap() -> impl Trait { Struct } | ^^^^^^^^^^ @@ -61,20 +61,32 @@ LL | fn ban() -> dyn Trait { Struct } | ^^^^^^^^^ doesn't have a size known at compile-time | = note: for information on `impl Trait`, see -help: return `impl Trait` instead, as all return paths are of type `Struct`, which implements `Trait` +help: use `impl Trait` as the return type, as all return paths are of type `Struct`, which implements `Trait` | LL | fn ban() -> impl Trait { Struct } | ^^^^^^^^^^ -error[E0277]: the size for values of type `(dyn Trait + 'static)` cannot be known at compilation time +error[E0746]: return type cannot have an unboxed trait object --> $DIR/dyn-trait-return-should-be-impl-trait.rs:17:13 | LL | fn bak() -> dyn Trait { unimplemented!() } | ^^^^^^^^^ doesn't have a size known at compile-time | - = help: the trait `std::marker::Sized` is not implemented for `(dyn Trait + 'static)` - = note: to learn more, visit - = note: the return type of a function must have a statically known size + = note: currently nothing is being returned, depending on the final implementation you could change the return type in different ways + = note: for information on `impl Trait`, see + = note: for information on trait objects, see +help: you could use some type `T` that is `T: Sized` as the return type if all return paths will have the same type + | +LL | fn bak() -> T { unimplemented!() } + | ^ +help: you could use `impl Trait` as the return type if all return paths will have the same type but you want to expose only the trait in the signature + | +LL | fn bak() -> impl Trait { unimplemented!() } + | ^^^^^^^^^^ +help: you could use a boxed trait object if all return paths `impl` trait `Trait` + | +LL | fn bak() -> Box { unimplemented!() } + | ^^^^^^^^^^^^^^ error[E0746]: return type cannot have an unboxed trait object --> $DIR/dyn-trait-return-should-be-impl-trait.rs:19:13 @@ -249,7 +261,7 @@ LL | fn bat() -> dyn Trait { | ^^^^^^^^^ doesn't have a size known at compile-time | = note: for information on `impl Trait`, see -help: return `impl Trait` instead, as all return paths are of type `{integer}`, which implements `Trait` +help: use `impl Trait` as the return type, as all return paths are of type `{integer}`, which implements `Trait` | LL | fn bat() -> impl Trait { | ^^^^^^^^^^ @@ -261,7 +273,7 @@ LL | fn bay() -> dyn Trait { | ^^^^^^^^^ doesn't have a size known at compile-time | = note: for information on `impl Trait`, see -help: return `impl Trait` instead, as all return paths are of type `{integer}`, which implements `Trait` +help: use `impl Trait` as the return type, as all return paths are of type `{integer}`, which implements `Trait` | LL | fn bay() -> impl Trait { | ^^^^^^^^^^ diff --git a/src/test/ui/issues/issue-18107.rs b/src/test/ui/issues/issue-18107.rs index 122940f1c17..4bf5b6c0f30 100644 --- a/src/test/ui/issues/issue-18107.rs +++ b/src/test/ui/issues/issue-18107.rs @@ -2,7 +2,7 @@ pub trait AbstractRenderer {} fn _create_render(_: &()) -> dyn AbstractRenderer -//~^ ERROR the size for values of type +//~^ ERROR return type cannot have an unboxed trait object { match 0 { _ => unimplemented!() diff --git a/src/test/ui/issues/issue-18107.stderr b/src/test/ui/issues/issue-18107.stderr index 9bdf470413b..08785ffa73a 100644 --- a/src/test/ui/issues/issue-18107.stderr +++ b/src/test/ui/issues/issue-18107.stderr @@ -1,13 +1,25 @@ -error[E0277]: the size for values of type `(dyn AbstractRenderer + 'static)` cannot be known at compilation time +error[E0746]: return type cannot have an unboxed trait object --> $DIR/issue-18107.rs:4:5 | LL | dyn AbstractRenderer | ^^^^^^^^^^^^^^^^^^^^ doesn't have a size known at compile-time | - = help: the trait `std::marker::Sized` is not implemented for `(dyn AbstractRenderer + 'static)` - = note: to learn more, visit - = note: the return type of a function must have a statically known size + = note: currently nothing is being returned, depending on the final implementation you could change the return type in different ways + = note: for information on `impl Trait`, see + = note: for information on trait objects, see +help: you could use some type `T` that is `T: Sized` as the return type if all return paths will have the same type + | +LL | T + | +help: you could use `impl AbstractRenderer` as the return type if all return paths will have the same type but you want to expose only the trait in the signature + | +LL | impl AbstractRenderer + | +help: you could use a boxed trait object if all return paths `impl` trait `AbstractRenderer` + | +LL | Box + | error: aborting due to previous error -For more information about this error, try `rustc --explain E0277`. +For more information about this error, try `rustc --explain E0746`. -- GitLab