diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs b/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs index 41c38f558b6ef..e6fa95b91e9b3 100644 --- a/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs +++ b/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs @@ -31,9 +31,7 @@ use rustc_middle::ty::{self, DefIdTree, IsSuggestable, Ty}; use rustc_session::Session; use rustc_span::symbol::Ident; use rustc_span::{self, Span}; -use rustc_trait_selection::traits::{ - self, ObligationCauseCode, SelectionContext, StatementAsExpression, -}; +use rustc_trait_selection::traits::{self, ObligationCauseCode, SelectionContext}; use std::iter; use std::slice; @@ -1410,7 +1408,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { &self.misc(sp), &mut |err| { if let Some(expected_ty) = expected.only_has_type(self) { - self.consider_hint_about_removing_semicolon(blk, expected_ty, err); + if !self.consider_removing_semicolon(blk, expected_ty, err) { + self.consider_returning_binding(blk, expected_ty, err); + } if expected_ty == self.tcx.types.bool { // If this is caused by a missing `let` in a `while let`, // silence this redundant error, as we already emit E0070. @@ -1478,42 +1478,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ty } - /// A common error is to add an extra semicolon: - /// - /// ```compile_fail,E0308 - /// fn foo() -> usize { - /// 22; - /// } - /// ``` - /// - /// This routine checks if the final statement in a block is an - /// expression with an explicit semicolon whose type is compatible - /// with `expected_ty`. If so, it suggests removing the semicolon. - fn consider_hint_about_removing_semicolon( - &self, - blk: &'tcx hir::Block<'tcx>, - expected_ty: Ty<'tcx>, - err: &mut Diagnostic, - ) { - if let Some((span_semi, boxed)) = self.could_remove_semicolon(blk, expected_ty) { - if let StatementAsExpression::NeedsBoxing = boxed { - err.span_suggestion_verbose( - span_semi, - "consider removing this semicolon and boxing the expression", - "", - Applicability::HasPlaceholders, - ); - } else { - err.span_suggestion_short( - span_semi, - "remove this semicolon", - "", - Applicability::MachineApplicable, - ); - } - } - } - fn parent_item_span(&self, id: hir::HirId) -> Option { let node = self.tcx.hir().get_by_def_id(self.tcx.hir().get_parent_item(id)); match node { diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs b/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs index 80feac184125c..d5ee299c0f98d 100644 --- a/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs +++ b/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs @@ -3,6 +3,7 @@ use crate::astconv::AstConv; use crate::errors::{AddReturnTypeSuggestion, ExpectedReturnTypeLabel}; use rustc_ast::util::parser::ExprPrecedence; +use rustc_data_structures::stable_set::FxHashSet; use rustc_errors::{Applicability, Diagnostic, MultiSpan}; use rustc_hir as hir; use rustc_hir::def::{CtorOf, DefKind}; @@ -11,12 +12,12 @@ use rustc_hir::{ Expr, ExprKind, GenericBound, Node, Path, QPath, Stmt, StmtKind, TyKind, WherePredicate, }; use rustc_infer::infer::{self, TyCtxtInferExt}; -use rustc_infer::traits; +use rustc_infer::traits::{self, StatementAsExpression}; use rustc_middle::lint::in_external_macro; -use rustc_middle::ty::{self, Binder, IsSuggestable, Subst, ToPredicate, Ty}; +use rustc_middle::ty::{self, Binder, IsSuggestable, Subst, ToPredicate, Ty, TypeVisitable}; use rustc_span::symbol::sym; use rustc_span::Span; -use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt; +use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt as _; impl<'a, 'tcx> FnCtxt<'a, 'tcx> { pub(in super::super) fn suggest_semicolon_at_end(&self, span: Span, err: &mut Diagnostic) { @@ -864,4 +865,156 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ); } } + + /// A common error is to add an extra semicolon: + /// + /// ```compile_fail,E0308 + /// fn foo() -> usize { + /// 22; + /// } + /// ``` + /// + /// This routine checks if the final statement in a block is an + /// expression with an explicit semicolon whose type is compatible + /// with `expected_ty`. If so, it suggests removing the semicolon. + pub(crate) fn consider_removing_semicolon( + &self, + blk: &'tcx hir::Block<'tcx>, + expected_ty: Ty<'tcx>, + err: &mut Diagnostic, + ) -> bool { + if let Some((span_semi, boxed)) = self.could_remove_semicolon(blk, expected_ty) { + if let StatementAsExpression::NeedsBoxing = boxed { + err.span_suggestion_verbose( + span_semi, + "consider removing this semicolon and boxing the expression", + "", + Applicability::HasPlaceholders, + ); + } else { + err.span_suggestion_short( + span_semi, + "remove this semicolon", + "", + Applicability::MachineApplicable, + ); + } + true + } else { + false + } + } + + pub(crate) fn consider_returning_binding( + &self, + blk: &'tcx hir::Block<'tcx>, + expected_ty: Ty<'tcx>, + err: &mut Diagnostic, + ) { + let mut shadowed = FxHashSet::default(); + let mut candidate_idents = vec![]; + let mut find_compatible_candidates = |pat: &hir::Pat<'_>| { + if let hir::PatKind::Binding(_, hir_id, ident, _) = &pat.kind + && let Some(pat_ty) = self.typeck_results.borrow().node_type_opt(*hir_id) + { + let pat_ty = self.resolve_vars_if_possible(pat_ty); + if self.can_coerce(pat_ty, expected_ty) + && !(pat_ty, expected_ty).references_error() + && shadowed.insert(ident.name) + { + candidate_idents.push((*ident, pat_ty)); + } + } + true + }; + + let hir = self.tcx.hir(); + for stmt in blk.stmts.iter().rev() { + let StmtKind::Local(local) = &stmt.kind else { continue; }; + local.pat.walk(&mut find_compatible_candidates); + } + match hir.find(hir.get_parent_node(blk.hir_id)) { + Some(hir::Node::Expr(hir::Expr { hir_id, .. })) => { + match hir.find(hir.get_parent_node(*hir_id)) { + Some(hir::Node::Arm(hir::Arm { pat, .. })) => { + pat.walk(&mut find_compatible_candidates); + } + Some( + hir::Node::Item(hir::Item { kind: hir::ItemKind::Fn(_, _, body), .. }) + | hir::Node::ImplItem(hir::ImplItem { + kind: hir::ImplItemKind::Fn(_, body), + .. + }) + | hir::Node::TraitItem(hir::TraitItem { + kind: hir::TraitItemKind::Fn(_, hir::TraitFn::Provided(body)), + .. + }) + | hir::Node::Expr(hir::Expr { + kind: hir::ExprKind::Closure(hir::Closure { body, .. }), + .. + }), + ) => { + for param in hir.body(*body).params { + param.pat.walk(&mut find_compatible_candidates); + } + } + Some(hir::Node::Expr(hir::Expr { + kind: + hir::ExprKind::If( + hir::Expr { kind: hir::ExprKind::Let(let_), .. }, + then_block, + _, + ), + .. + })) if then_block.hir_id == *hir_id => { + let_.pat.walk(&mut find_compatible_candidates); + } + _ => {} + } + } + _ => {} + } + + match &candidate_idents[..] { + [(ident, _ty)] => { + let sm = self.tcx.sess.source_map(); + if let Some(stmt) = blk.stmts.last() { + let stmt_span = sm.stmt_span(stmt.span, blk.span); + let sugg = if sm.is_multiline(blk.span) + && let Some(spacing) = sm.indentation_before(stmt_span) + { + format!("\n{spacing}{ident}") + } else { + format!(" {ident}") + }; + err.span_suggestion_verbose( + stmt_span.shrink_to_hi(), + format!("consider returning the local binding `{ident}`"), + sugg, + Applicability::MachineApplicable, + ); + } else { + let sugg = if sm.is_multiline(blk.span) + && let Some(spacing) = sm.indentation_before(blk.span.shrink_to_lo()) + { + format!("\n{spacing} {ident}\n{spacing}") + } else { + format!(" {ident} ") + }; + let left_span = sm.span_through_char(blk.span, '{').shrink_to_hi(); + err.span_suggestion_verbose( + sm.span_extend_while(left_span, |c| c.is_whitespace()).unwrap_or(left_span), + format!("consider returning the local binding `{ident}`"), + sugg, + Applicability::MachineApplicable, + ); + } + } + values if (1..3).contains(&values.len()) => { + let spans = values.iter().map(|(ident, _)| ident.span).collect::>(); + err.span_note(spans, "consider returning one of these bindings"); + } + _ => {} + } + } } diff --git a/src/test/ui/async-await/async-block-control-flow-static-semantics.stderr b/src/test/ui/async-await/async-block-control-flow-static-semantics.stderr index e5887689690e7..ada6e357aea5e 100644 --- a/src/test/ui/async-await/async-block-control-flow-static-semantics.stderr +++ b/src/test/ui/async-await/async-block-control-flow-static-semantics.stderr @@ -18,6 +18,14 @@ LL | | break 0u8; LL | | }; | |_________- enclosing `async` block +error[E0271]: type mismatch resolving ` as Future>::Output == ()` + --> $DIR/async-block-control-flow-static-semantics.rs:26:39 + | +LL | let _: &dyn Future = █ + | ^^^^^^ expected `()`, found `u8` + | + = note: required for the cast from `impl Future` to the object type `dyn Future` + error[E0308]: mismatched types --> $DIR/async-block-control-flow-static-semantics.rs:21:58 | @@ -32,7 +40,7 @@ LL | | } | |_^ expected `u8`, found `()` error[E0271]: type mismatch resolving ` as Future>::Output == ()` - --> $DIR/async-block-control-flow-static-semantics.rs:26:39 + --> $DIR/async-block-control-flow-static-semantics.rs:17:39 | LL | let _: &dyn Future = █ | ^^^^^^ expected `()`, found `u8` @@ -47,14 +55,6 @@ LL | fn return_targets_async_block_not_fn() -> u8 { | | | implicitly returns `()` as its body has no tail or `return` expression -error[E0271]: type mismatch resolving ` as Future>::Output == ()` - --> $DIR/async-block-control-flow-static-semantics.rs:17:39 - | -LL | let _: &dyn Future = █ - | ^^^^^^ expected `()`, found `u8` - | - = note: required for the cast from `impl Future` to the object type `dyn Future` - error[E0308]: mismatched types --> $DIR/async-block-control-flow-static-semantics.rs:47:44 | diff --git a/src/test/ui/liveness/liveness-forgot-ret.stderr b/src/test/ui/liveness/liveness-forgot-ret.stderr index 95070322bddef..ddbdbdb0fd043 100644 --- a/src/test/ui/liveness/liveness-forgot-ret.stderr +++ b/src/test/ui/liveness/liveness-forgot-ret.stderr @@ -5,6 +5,11 @@ LL | fn f(a: isize) -> isize { if god_exists(a) { return 5; }; } | - ^^^^^ expected `isize`, found `()` | | | implicitly returns `()` as its body has no tail or `return` expression + | +help: consider returning the local binding `a` + | +LL | fn f(a: isize) -> isize { if god_exists(a) { return 5; }; a } + | + error: aborting due to previous error diff --git a/src/test/ui/parser/issues/issue-33413.stderr b/src/test/ui/parser/issues/issue-33413.stderr index ac320f095a238..b7250f3b0b539 100644 --- a/src/test/ui/parser/issues/issue-33413.stderr +++ b/src/test/ui/parser/issues/issue-33413.stderr @@ -11,6 +11,11 @@ LL | fn f(*, a: u8) -> u8 {} | - ^^ expected `u8`, found `()` | | | implicitly returns `()` as its body has no tail or `return` expression + | +help: consider returning the local binding `a` + | +LL | fn f(*, a: u8) -> u8 { a } + | + error: aborting due to 2 previous errors diff --git a/src/test/ui/suggestions/return-bindings-multi.rs b/src/test/ui/suggestions/return-bindings-multi.rs new file mode 100644 index 0000000000000..8c3bd641e9784 --- /dev/null +++ b/src/test/ui/suggestions/return-bindings-multi.rs @@ -0,0 +1,9 @@ +fn a(i: i32) -> i32 { + //~^ ERROR mismatched types + let j = 2i32; +} + +fn b(i: i32, j: i32) -> i32 {} +//~^ ERROR mismatched types + +fn main() {} diff --git a/src/test/ui/suggestions/return-bindings-multi.stderr b/src/test/ui/suggestions/return-bindings-multi.stderr new file mode 100644 index 0000000000000..738e3f2f4beb8 --- /dev/null +++ b/src/test/ui/suggestions/return-bindings-multi.stderr @@ -0,0 +1,34 @@ +error[E0308]: mismatched types + --> $DIR/return-bindings-multi.rs:1:17 + | +LL | fn a(i: i32) -> i32 { + | - ^^^ expected `i32`, found `()` + | | + | implicitly returns `()` as its body has no tail or `return` expression + | +note: consider returning one of these bindings + --> $DIR/return-bindings-multi.rs:1:6 + | +LL | fn a(i: i32) -> i32 { + | ^ +LL | +LL | let j = 2i32; + | ^ + +error[E0308]: mismatched types + --> $DIR/return-bindings-multi.rs:6:25 + | +LL | fn b(i: i32, j: i32) -> i32 {} + | - ^^^ expected `i32`, found `()` + | | + | implicitly returns `()` as its body has no tail or `return` expression + | +note: consider returning one of these bindings + --> $DIR/return-bindings-multi.rs:6:6 + | +LL | fn b(i: i32, j: i32) -> i32 {} + | ^ ^ + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0308`. diff --git a/src/test/ui/suggestions/return-bindings.fixed b/src/test/ui/suggestions/return-bindings.fixed new file mode 100644 index 0000000000000..4fabc411abcbe --- /dev/null +++ b/src/test/ui/suggestions/return-bindings.fixed @@ -0,0 +1,23 @@ +// run-rustfix + +#![allow(unused)] + +fn a(i: i32) -> i32 { i } +//~^ ERROR mismatched types + +fn b(opt_str: Option) { + let s: String = if let Some(s) = opt_str { + s + //~^ ERROR mismatched types + } else { + String::new() + }; +} + +fn c() -> Option { + //~^ ERROR mismatched types + let x = Some(1); + x +} + +fn main() {} diff --git a/src/test/ui/suggestions/return-bindings.rs b/src/test/ui/suggestions/return-bindings.rs new file mode 100644 index 0000000000000..d05b4ba27d6e8 --- /dev/null +++ b/src/test/ui/suggestions/return-bindings.rs @@ -0,0 +1,21 @@ +// run-rustfix + +#![allow(unused)] + +fn a(i: i32) -> i32 {} +//~^ ERROR mismatched types + +fn b(opt_str: Option) { + let s: String = if let Some(s) = opt_str { + //~^ ERROR mismatched types + } else { + String::new() + }; +} + +fn c() -> Option { + //~^ ERROR mismatched types + let x = Some(1); +} + +fn main() {} diff --git a/src/test/ui/suggestions/return-bindings.stderr b/src/test/ui/suggestions/return-bindings.stderr new file mode 100644 index 0000000000000..e5d4925500556 --- /dev/null +++ b/src/test/ui/suggestions/return-bindings.stderr @@ -0,0 +1,48 @@ +error[E0308]: mismatched types + --> $DIR/return-bindings.rs:5:17 + | +LL | fn a(i: i32) -> i32 {} + | - ^^^ expected `i32`, found `()` + | | + | implicitly returns `()` as its body has no tail or `return` expression + | +help: consider returning the local binding `i` + | +LL | fn a(i: i32) -> i32 { i } + | + + +error[E0308]: mismatched types + --> $DIR/return-bindings.rs:9:46 + | +LL | let s: String = if let Some(s) = opt_str { + | ______________________________________________^ +LL | | +LL | | } else { + | |_____^ expected struct `String`, found `()` + | +help: consider returning the local binding `s` + | +LL ~ let s: String = if let Some(s) = opt_str { +LL + s +LL ~ + | + +error[E0308]: mismatched types + --> $DIR/return-bindings.rs:16:11 + | +LL | fn c() -> Option { + | - ^^^^^^^^^^^ expected enum `Option`, found `()` + | | + | implicitly returns `()` as its body has no tail or `return` expression + | + = note: expected enum `Option` + found unit type `()` +help: consider returning the local binding `x` + | +LL ~ let x = Some(1); +LL + x + | + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0308`.