diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index d7e822382ef..07fe3a23147 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -973,6 +973,7 @@ managed_boxes, manually_drop, map, + map_err, marker, marker_trait_attr, masked, @@ -1137,6 +1138,7 @@ offset, offset_of, offset_of_enum, + ok_or_else, omit_gdb_pretty_printer_section, on, on_unimplemented, diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs index 58ea7a0edd5..5f522fc29eb 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs @@ -41,7 +41,7 @@ use rustc_session::Limit; use rustc_span::def_id::LOCAL_CRATE; use rustc_span::symbol::sym; -use rustc_span::{BytePos, ExpnKind, Span, DUMMY_SP}; +use rustc_span::{BytePos, ExpnKind, Span, Symbol, DUMMY_SP}; use std::borrow::Cow; use std::fmt; use std::iter; @@ -1045,6 +1045,7 @@ fn visit_expr(&mut self, ex: &'v hir::Expr<'v>) { return; } let self_ty = trait_ref.self_ty(); + let found_ty = trait_ref.args.get(1).and_then(|a| a.as_type()); let mut prev_ty = self.resolve_vars_if_possible( typeck.expr_ty_adjusted_opt(expr).unwrap_or(Ty::new_misc_error(self.tcx)), @@ -1070,17 +1071,80 @@ fn visit_expr(&mut self, ex: &'v hir::Expr<'v>) { // The following logic is simlar to `point_at_chain`, but that's focused on associated types let mut expr = expr; - while let hir::ExprKind::MethodCall(_path_segment, rcvr_expr, _args, span) = expr.kind { + while let hir::ExprKind::MethodCall(path_segment, rcvr_expr, args, span) = expr.kind { // Point at every method call in the chain with the `Result` type. // let foo = bar.iter().map(mapper)?; // ------ ----------- expr = rcvr_expr; chain.push((span, prev_ty)); - prev_ty = self.resolve_vars_if_possible( + let next_ty = self.resolve_vars_if_possible( typeck.expr_ty_adjusted_opt(expr).unwrap_or(Ty::new_misc_error(self.tcx)), ); + let is_diagnostic_item = |symbol: Symbol, ty: Ty<'tcx>| { + let ty::Adt(def, _) = ty.kind() else { + return false; + }; + self.tcx.is_diagnostic_item(symbol, def.did()) + }; + // For each method in the chain, see if this is `Result::map_err` or + // `Option::ok_or_else` and if it is, see if the closure passed to it has an incorrect + // trailing `;`. + if let Some(ty) = get_e_type(prev_ty) + && let Some(found_ty) = found_ty + // Ideally we would instead use `FnCtxt::lookup_method_for_diagnostic` for 100% + // accurate check, but we are in the wrong stage to do that and looking for + // `Result::map_err` by checking the Self type and the path segment is enough. + // sym::ok_or_else + && ( + ( // Result::map_err + path_segment.ident.name == sym::map_err + && is_diagnostic_item(sym::Result, next_ty) + ) || ( // Option::ok_or_else + path_segment.ident.name == sym::ok_or_else + && is_diagnostic_item(sym::Option, next_ty) + ) + ) + // Found `Result<_, ()>?` + && let ty::Tuple(tys) = found_ty.kind() + && tys.is_empty() + // The current method call returns `Result<_, ()>` + && self.can_eq(obligation.param_env, ty, found_ty) + // There's a single argument in the method call and it is a closure + && args.len() == 1 + && let Some(arg) = args.get(0) + && let hir::ExprKind::Closure(closure) = arg.kind + // The closure has a block for its body with no tail expression + && let body = self.tcx.hir().body(closure.body) + && let hir::ExprKind::Block(block, _) = body.value.kind + && let None = block.expr + // The last statement is of a type that can be converted to the return error type + && let [.., stmt] = block.stmts + && let hir::StmtKind::Semi(expr) = stmt.kind + && let expr_ty = self.resolve_vars_if_possible( + typeck.expr_ty_adjusted_opt(expr) + .unwrap_or(Ty::new_misc_error(self.tcx)), + ) + && self + .infcx + .type_implements_trait( + self.tcx.get_diagnostic_item(sym::From).unwrap(), + [self_ty, expr_ty], + obligation.param_env, + ) + .must_apply_modulo_regions() + { + err.span_suggestion_short( + stmt.span.with_lo(expr.span.hi()), + "remove this semicolon", + String::new(), + Applicability::MachineApplicable, + ); + } + + prev_ty = next_ty; + if let hir::ExprKind::Path(hir::QPath::Resolved(None, path)) = expr.kind && let hir::Path { res: hir::def::Res::Local(hir_id), .. } = path && let Some(hir::Node::Pat(binding)) = self.tcx.hir().find(*hir_id) diff --git a/tests/ui/traits/question-mark-result-err-mismatch.rs b/tests/ui/traits/question-mark-result-err-mismatch.rs index e5ccca2e5f7..317029e0046 100644 --- a/tests/ui/traits/question-mark-result-err-mismatch.rs +++ b/tests/ui/traits/question-mark-result-err-mismatch.rs @@ -8,7 +8,9 @@ fn foo() -> Result { //~ NOTE expected `String` because of this }); let one = x .map(|s| ()) - .map_err(|_| ()) //~ NOTE this can't be annotated with `?` because it has type `Result<_, ()>` + .map_err(|e| { //~ NOTE this can't be annotated with `?` because it has type `Result<_, ()>` + e; //~ HELP remove this semicolon + }) .map(|()| "")?; //~ ERROR `?` couldn't convert the error to `String` //~^ NOTE in this expansion of desugaring of operator `?` //~| NOTE in this expansion of desugaring of operator `?` @@ -17,6 +19,7 @@ fn foo() -> Result { //~ NOTE expected `String` because of this //~| NOTE the trait `From<()>` is not implemented for `String` //~| NOTE the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait //~| NOTE required for `Result` to implement `FromResidual>` + //~| HELP the following other types implement trait `From`: Ok(one.to_string()) } @@ -33,6 +36,7 @@ fn bar() -> Result<(), String> { //~ NOTE expected `String` because of this //~| NOTE the trait `From<()>` is not implemented for `String` //~| NOTE the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait //~| NOTE required for `Result<(), String>` to implement `FromResidual>` + //~| HELP the following other types implement trait `From`: Ok(one) } @@ -42,7 +46,7 @@ fn baz() -> Result { //~ NOTE expected `String` because of this .split_whitespace() .next() .ok_or_else(|| { //~ NOTE this can't be annotated with `?` because it has type `Result<_, ()>` - "Couldn't split the test string"; + "Couldn't split the test string"; //~ HELP remove this semicolon })?; //~^ ERROR `?` couldn't convert the error to `String` //~| NOTE in this expansion of desugaring of operator `?` @@ -52,6 +56,7 @@ fn baz() -> Result { //~ NOTE expected `String` because of this //~| NOTE the trait `From<()>` is not implemented for `String` //~| NOTE the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait //~| NOTE required for `Result` to implement `FromResidual>` + //~| HELP the following other types implement trait `From`: Ok(one.to_string()) } diff --git a/tests/ui/traits/question-mark-result-err-mismatch.stderr b/tests/ui/traits/question-mark-result-err-mismatch.stderr index fc3b2e6b46b..1f9495a505a 100644 --- a/tests/ui/traits/question-mark-result-err-mismatch.stderr +++ b/tests/ui/traits/question-mark-result-err-mismatch.stderr @@ -1,5 +1,5 @@ error[E0277]: `?` couldn't convert the error to `String` - --> $DIR/question-mark-result-err-mismatch.rs:12:22 + --> $DIR/question-mark-result-err-mismatch.rs:14:22 | LL | fn foo() -> Result { | ---------------------- expected `String` because of this @@ -10,8 +10,12 @@ LL | | "Couldn't split the test string" LL | | }); | |__________- this has type `Result<_, &str>` ... -LL | .map_err(|_| ()) - | --------------- this can't be annotated with `?` because it has type `Result<_, ()>` +LL | .map_err(|e| { + | __________- +LL | | e; + | | - help: remove this semicolon +LL | | }) + | |__________- this can't be annotated with `?` because it has type `Result<_, ()>` LL | .map(|()| "")?; | ^ the trait `From<()>` is not implemented for `String` | @@ -26,7 +30,7 @@ LL | .map(|()| "")?; = note: required for `Result` to implement `FromResidual>` error[E0277]: `?` couldn't convert the error to `String` - --> $DIR/question-mark-result-err-mismatch.rs:27:25 + --> $DIR/question-mark-result-err-mismatch.rs:30:25 | LL | fn bar() -> Result<(), String> { | ------------------ expected `String` because of this @@ -49,7 +53,7 @@ LL | .map_err(|_| ())?; = note: required for `Result<(), String>` to implement `FromResidual>` error[E0277]: `?` couldn't convert the error to `String` - --> $DIR/question-mark-result-err-mismatch.rs:46:11 + --> $DIR/question-mark-result-err-mismatch.rs:50:11 | LL | fn baz() -> Result { | ---------------------- expected `String` because of this @@ -57,6 +61,7 @@ LL | fn baz() -> Result { LL | .ok_or_else(|| { | __________- LL | | "Couldn't split the test string"; + | | - help: remove this semicolon LL | | })?; | | -^ the trait `From<()>` is not implemented for `String` | |__________|