From 796ce9fcb7e35767e6e57ad79099dcf730a0a94b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 4 Feb 2021 12:22:01 -0800 Subject: [PATCH] Suggest `return`ing tail expressions that match return type Some newcomers are confused by the behavior of tail expressions, interpreting that "leaving out the `;` makes it the return value". To help them go in the right direction, suggest using `return` instead when applicable. --- compiler/rustc_typeck/src/check/coercion.rs | 12 +++++++- .../src/check/fn_ctxt/suggestions.rs | 28 +++++++++++++++++++ src/test/ui/macros/empty-trailing-stmt.stderr | 5 ++++ src/test/ui/parser/expr-as-stmt-2.stderr | 22 +++++++++++++-- src/test/ui/parser/expr-as-stmt.stderr | 20 +++++++++++++ .../return/tail-expr-as-potential-return.rs | 11 ++++++++ .../tail-expr-as-potential-return.stderr | 27 ++++++++++++++++++ 7 files changed, 122 insertions(+), 3 deletions(-) create mode 100644 src/test/ui/return/tail-expr-as-potential-return.rs create mode 100644 src/test/ui/return/tail-expr-as-potential-return.stderr diff --git a/compiler/rustc_typeck/src/check/coercion.rs b/compiler/rustc_typeck/src/check/coercion.rs index b2395b7bb25..f95627cfdee 100644 --- a/compiler/rustc_typeck/src/check/coercion.rs +++ b/compiler/rustc_typeck/src/check/coercion.rs @@ -1458,7 +1458,7 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> { fcx.get_fn_decl(parent_id) }; - if let (Some((fn_decl, can_suggest)), _) = (fn_decl, pointing_at_return_type) { + if let Some((fn_decl, can_suggest)) = fn_decl { if expression.is_none() { pointing_at_return_type |= fcx.suggest_missing_return_type( &mut err, @@ -1472,6 +1472,16 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> { fn_output = Some(&fn_decl.output); // `impl Trait` return type } } + + let parent_id = fcx.tcx.hir().get_parent_item(id); + let parent_item = fcx.tcx.hir().get(parent_id); + + if let (Some((expr, _)), Some((fn_decl, _, _))) = + (expression, fcx.get_node_fn_decl(parent_item)) + { + fcx.suggest_missing_return_expr(&mut err, expr, fn_decl, expected, found); + } + if let (Some(sp), Some(fn_output)) = (fcx.ret_coercion_span.get(), fn_output) { self.add_impl_trait_explanation(&mut err, cause, fcx, expected, sp, fn_output); } diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs b/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs index a0465ca6aef..9d816e76c00 100644 --- a/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs +++ b/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs @@ -464,6 +464,34 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } + pub(in super::super) fn suggest_missing_return_expr( + &self, + err: &mut DiagnosticBuilder<'_>, + expr: &'tcx hir::Expr<'tcx>, + fn_decl: &hir::FnDecl<'_>, + expected: Ty<'tcx>, + found: Ty<'tcx>, + ) { + if !expected.is_unit() { + return; + } + let found = self.resolve_vars_with_obligations(found); + if let hir::FnRetTy::Return(ty) = fn_decl.output { + let ty = AstConv::ast_ty_to_ty(self, ty); + let ty = self.normalize_associated_types_in(expr.span, ty); + if self.can_coerce(found, ty) { + err.multipart_suggestion( + "you might have meant to return this value", + vec![ + (expr.span.shrink_to_lo(), "return ".to_string()), + (expr.span.shrink_to_hi(), ";".to_string()), + ], + Applicability::MaybeIncorrect, + ); + } + } + } + pub(in super::super) fn suggest_missing_parentheses( &self, err: &mut DiagnosticBuilder<'_>, diff --git a/src/test/ui/macros/empty-trailing-stmt.stderr b/src/test/ui/macros/empty-trailing-stmt.stderr index e88b12712fb..1db759a2181 100644 --- a/src/test/ui/macros/empty-trailing-stmt.stderr +++ b/src/test/ui/macros/empty-trailing-stmt.stderr @@ -3,6 +3,11 @@ error[E0308]: mismatched types | LL | { true } | ^^^^ expected `()`, found `bool` + | +help: you might have meant to return this value + | +LL | { return true; } + | ^^^^^^ ^ error[E0308]: mismatched types --> $DIR/empty-trailing-stmt.rs:5:13 diff --git a/src/test/ui/parser/expr-as-stmt-2.stderr b/src/test/ui/parser/expr-as-stmt-2.stderr index ee07c367633..75c0e7bb475 100644 --- a/src/test/ui/parser/expr-as-stmt-2.stderr +++ b/src/test/ui/parser/expr-as-stmt-2.stderr @@ -2,19 +2,37 @@ error[E0308]: mismatched types --> $DIR/expr-as-stmt-2.rs:3:26 | LL | if let Some(x) = a { true } else { false } - | ---------------------^^^^------------------ help: consider using a semicolon here + | ---------------------^^^^----------------- | | | | | expected `()`, found `bool` | expected this to be `()` + | +help: consider using a semicolon here + | +LL | if let Some(x) = a { true } else { false }; + | ^ +help: you might have meant to return this value + | +LL | if let Some(x) = a { return true; } else { false } + | ^^^^^^ ^ error[E0308]: mismatched types --> $DIR/expr-as-stmt-2.rs:3:40 | LL | if let Some(x) = a { true } else { false } - | -----------------------------------^^^^^--- help: consider using a semicolon here + | -----------------------------------^^^^^-- | | | | | expected `()`, found `bool` | expected this to be `()` + | +help: consider using a semicolon here + | +LL | if let Some(x) = a { true } else { false }; + | ^ +help: you might have meant to return this value + | +LL | if let Some(x) = a { true } else { return false; } + | ^^^^^^ ^ error[E0308]: mismatched types --> $DIR/expr-as-stmt-2.rs:6:5 diff --git a/src/test/ui/parser/expr-as-stmt.stderr b/src/test/ui/parser/expr-as-stmt.stderr index 324aed0ad7c..09a6d7cbeb1 100644 --- a/src/test/ui/parser/expr-as-stmt.stderr +++ b/src/test/ui/parser/expr-as-stmt.stderr @@ -40,24 +40,44 @@ error[E0308]: mismatched types | LL | {2} + {2} | ^ expected `()`, found integer + | +help: you might have meant to return this value + | +LL | {return 2;} + {2} + | ^^^^^^ ^ error[E0308]: mismatched types --> $DIR/expr-as-stmt.rs:12:6 | LL | {2} + 2 | ^ expected `()`, found integer + | +help: you might have meant to return this value + | +LL | {return 2;} + 2 + | ^^^^^^ ^ error[E0308]: mismatched types --> $DIR/expr-as-stmt.rs:18:7 | LL | { 42 } + foo; | ^^ expected `()`, found integer + | +help: you might have meant to return this value + | +LL | { return 42; } + foo; + | ^^^^^^ ^ error[E0308]: mismatched types --> $DIR/expr-as-stmt.rs:24:7 | LL | { 3 } * 3 | ^ expected `()`, found integer + | +help: you might have meant to return this value + | +LL | { return 3; } * 3 + | ^^^^^^ ^ error[E0614]: type `{integer}` cannot be dereferenced --> $DIR/expr-as-stmt.rs:24:11 diff --git a/src/test/ui/return/tail-expr-as-potential-return.rs b/src/test/ui/return/tail-expr-as-potential-return.rs new file mode 100644 index 00000000000..72798c720f3 --- /dev/null +++ b/src/test/ui/return/tail-expr-as-potential-return.rs @@ -0,0 +1,11 @@ +fn main() { + let _ = foo(true); +} + +fn foo(x: bool) -> Result { + if x { + Err(42) //~ ERROR mismatched types + } + Ok(42.0) +} + diff --git a/src/test/ui/return/tail-expr-as-potential-return.stderr b/src/test/ui/return/tail-expr-as-potential-return.stderr new file mode 100644 index 00000000000..d079e0b080d --- /dev/null +++ b/src/test/ui/return/tail-expr-as-potential-return.stderr @@ -0,0 +1,27 @@ +error[E0308]: mismatched types + --> $DIR/tail-expr-as-potential-return.rs:7:9 + | +LL | / if x { +LL | | Err(42) + | | ^^^^^^^ expected `()`, found enum `std::result::Result` +LL | | } + | |_____- expected this to be `()` + | + = note: expected unit type `()` + found enum `std::result::Result<_, {integer}>` +help: try adding a semicolon + | +LL | Err(42); + | ^ +help: consider using a semicolon here + | +LL | }; + | ^ +help: you might have meant to return this value + | +LL | return Err(42); + | ^^^^^^ ^ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0308`.