From 6c3879d1f154bb6f18562d29aa30fbc03239c66f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Esteban=20K=C3=BCber?= <esteban@kuber.com.ar>
Date: Fri, 6 Oct 2023 19:07:05 +0000
Subject: [PATCH 1/4] Provide context when `?` can't be called because of
 `Result<_, E>`

When a method chain ending in `?` causes an E0277 because the
expression's `Result::Err` variant doesn't have a type that can be
converted to the `Result<_, E>` type parameter in the return type,
provide additional context of which parts of the chain can and can't
support the `?` operator.

```
error[E0277]: `?` couldn't convert the error to `String`
  --> $DIR/question-mark-result-err-mismatch.rs:28:25
   |
LL | fn bar() -> Result<(), String> {
   |             ------------------ expected `String` because of this
LL |     let x = foo();
   |             ----- this can be annotated with `?` because it has type `Result<String, String>`
LL |     let one = x
LL |         .map(|s| ())
   |          ----------- this can be annotated with `?` because it has type `Result<(), String>`
LL |         .map_err(|_| ())?;
   |          ---------------^ the trait `From<()>` is not implemented for `String`
   |          |
   |          this can't be annotated with `?` because it has type `Result<(), ()>`
   |
   = note: the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait
   = help: the following other types implement trait `From<T>`:
             <String as From<char>>
             <String as From<Box<str>>>
             <String as From<Cow<'a, str>>>
             <String as From<&str>>
             <String as From<&mut str>>
             <String as From<&String>>
   = note: required for `Result<(), String>` to implement `FromResidual<Result<Infallible, ()>>`
```

Fix #72124.
---
 .../error_reporting/type_err_ctxt_ext.rs      | 144 +++++++++++++++++-
 .../question-mark-result-err-mismatch.rs      |  59 +++++++
 .../question-mark-result-err-mismatch.stderr  |  83 ++++++++++
 tests/ui/try-block/try-block-bad-type.stderr  |   4 +-
 tests/ui/try-trait/bad-interconversion.stderr |   4 +-
 tests/ui/try-trait/issue-32709.stderr         |   4 +-
 6 files changed, 294 insertions(+), 4 deletions(-)
 create mode 100644 tests/ui/traits/question-mark-result-err-mismatch.rs
 create mode 100644 tests/ui/traits/question-mark-result-err-mismatch.stderr

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 b3910a2770b3a..bad74588d0cc8 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
@@ -3,6 +3,7 @@ use super::suggestions::{get_explanation_based_on_obligation, TypeErrCtxtExt as
 use crate::errors::{ClosureFnMutLabel, ClosureFnOnceLabel, ClosureKindMismatch};
 use crate::infer::error_reporting::{TyCategory, TypeAnnotationNeeded as ErrorCode};
 use crate::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
+use crate::infer::InferCtxtExt as _;
 use crate::infer::{self, InferCtxt};
 use crate::traits::error_reporting::infer_ctxt_ext::InferCtxtExt;
 use crate::traits::error_reporting::{ambiguity, ambiguity::Ambiguity::*};
@@ -40,7 +41,7 @@ use rustc_session::config::{DumpSolverProofTree, TraitSolver};
 use rustc_session::Limit;
 use rustc_span::def_id::LOCAL_CRATE;
 use rustc_span::symbol::sym;
-use rustc_span::{ExpnKind, Span, DUMMY_SP};
+use rustc_span::{BytePos, ExpnKind, Span, DUMMY_SP};
 use std::borrow::Cow;
 use std::fmt;
 use std::iter;
@@ -106,6 +107,13 @@ pub trait TypeErrCtxtExt<'tcx> {
 
     fn fn_arg_obligation(&self, obligation: &PredicateObligation<'tcx>) -> bool;
 
+    fn try_conversion_context(
+        &self,
+        obligation: &PredicateObligation<'tcx>,
+        trait_ref: ty::TraitRef<'tcx>,
+        err: &mut Diagnostic,
+    );
+
     fn report_const_param_not_wf(
         &self,
         ty: Ty<'tcx>,
@@ -509,6 +517,10 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
 
                         let mut err = struct_span_err!(self.tcx.sess, span, E0277, "{}", err_msg);
 
+                        if is_try_conversion {
+                            self.try_conversion_context(&obligation, trait_ref.skip_binder(), &mut err);
+                        }
+
                         if is_try_conversion && let Some(ret_span) = self.return_type_span(&obligation) {
                             err.span_label(
                                 ret_span,
@@ -982,6 +994,136 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
         false
     }
 
+    /// When the `E` of the resulting `Result<T, E>` in an expression `foo().bar().baz()?`,
+    /// identify thoe method chain sub-expressions that could or could not have been annotated
+    /// with `?`.
+    fn try_conversion_context(
+        &self,
+        obligation: &PredicateObligation<'tcx>,
+        trait_ref: ty::TraitRef<'tcx>,
+        err: &mut Diagnostic,
+    ) {
+        let span = obligation.cause.span;
+        struct V<'v> {
+            search_span: Span,
+            found: Option<&'v hir::Expr<'v>>,
+        }
+        impl<'v> Visitor<'v> for V<'v> {
+            fn visit_expr(&mut self, ex: &'v hir::Expr<'v>) {
+                if let hir::ExprKind::Match(expr, _arms, hir::MatchSource::TryDesugar(_)) = ex.kind
+                {
+                    if ex.span.with_lo(ex.span.hi() - BytePos(1)).source_equal(self.search_span) {
+                        if let hir::ExprKind::Call(_, [expr, ..]) = expr.kind {
+                            self.found = Some(expr);
+                            return;
+                        }
+                    }
+                }
+                hir::intravisit::walk_expr(self, ex);
+            }
+        }
+        let hir_id = self.tcx.local_def_id_to_hir_id(obligation.cause.body_id);
+        let body_id = match self.tcx.hir().find(hir_id) {
+            Some(hir::Node::Item(hir::Item { kind: hir::ItemKind::Fn(_, _, body_id), .. })) => {
+                body_id
+            }
+            _ => return,
+        };
+        let mut v = V { search_span: span, found: None };
+        v.visit_body(self.tcx.hir().body(*body_id));
+        let Some(expr) = v.found else {
+            return;
+        };
+        let Some(typeck) = &self.typeck_results else {
+            return;
+        };
+        let Some((ObligationCauseCode::QuestionMark, Some(y))) = obligation.cause.code().parent()
+        else {
+            return;
+        };
+        if !self.tcx.is_diagnostic_item(sym::FromResidual, y.def_id()) {
+            return;
+        }
+        let self_ty = trait_ref.self_ty();
+
+        let mut prev_ty = self.resolve_vars_if_possible(
+            typeck.expr_ty_adjusted_opt(expr).unwrap_or(Ty::new_misc_error(self.tcx)),
+        );
+        let mut annotate_expr = |span: Span, prev_ty: Ty<'tcx>, self_ty: Ty<'tcx>| -> bool {
+            // We always look at the `E` type, because that's the only one affected by `?`. If the
+            // incorrect `Result<T, E>` is because of the `T`, we'll get an E0308 on the whole
+            // expression, after the `?` has "unwrapped" the `T`.
+            let ty::Adt(def, args) = prev_ty.kind() else {
+                return false;
+            };
+            let Some(arg) = args.get(1) else {
+                return false;
+            };
+            if !self.tcx.is_diagnostic_item(sym::Result, def.did()) {
+                return false;
+            }
+            let can = if self
+                .infcx
+                .type_implements_trait(
+                    self.tcx.get_diagnostic_item(sym::From).unwrap(),
+                    [self_ty.into(), *arg],
+                    obligation.param_env,
+                )
+                .must_apply_modulo_regions()
+            {
+                "can"
+            } else {
+                "can't"
+            };
+            err.span_label(
+                span,
+                format!("this {can} be annotated with `?` because it has type `{prev_ty}`"),
+            );
+            true
+        };
+
+        // 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 {
+            // Point at every method call in the chain with the `Result` type.
+            // let foo = bar.iter().map(mapper)?;
+            //               ------ -----------
+            expr = rcvr_expr;
+            if !annotate_expr(span, prev_ty, self_ty) {
+                break;
+            }
+
+            prev_ty = self.resolve_vars_if_possible(
+                typeck.expr_ty_adjusted_opt(expr).unwrap_or(Ty::new_misc_error(self.tcx)),
+            );
+
+            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)
+                && let Some(parent) = self.tcx.hir().find_parent(binding.hir_id)
+            {
+                // We've reached the root of the method call chain...
+                if let hir::Node::Local(local) = parent
+                    && let Some(binding_expr) = local.init
+                {
+                    // ...and it is a binding. Get the binding creation and continue the chain.
+                    expr = binding_expr;
+                }
+                if let hir::Node::Param(_param) = parent {
+                    // ...and it is a an fn argument.
+                    break;
+                }
+            }
+        }
+        // `expr` is now the "root" expression of the method call chain, which can be any
+        // expression kind, like a method call or a path. If this expression is `Result<T, E>` as
+        // well, then we also point at it.
+        prev_ty = self.resolve_vars_if_possible(
+            typeck.expr_ty_adjusted_opt(expr).unwrap_or(Ty::new_misc_error(self.tcx)),
+        );
+        annotate_expr(expr.span, prev_ty, self_ty);
+    }
+
     fn report_const_param_not_wf(
         &self,
         ty: Ty<'tcx>,
diff --git a/tests/ui/traits/question-mark-result-err-mismatch.rs b/tests/ui/traits/question-mark-result-err-mismatch.rs
new file mode 100644
index 0000000000000..7b364580858be
--- /dev/null
+++ b/tests/ui/traits/question-mark-result-err-mismatch.rs
@@ -0,0 +1,59 @@
+fn foo() -> Result<String, String> { //~ NOTE expected `String` because of this
+    let test = String::from("one,two");
+    let x = test
+        .split_whitespace()
+        .next()
+        .ok_or_else(|| { //~ NOTE this can be annotated with `?` because it has type `Result<&str, &str>`
+            "Couldn't split the test string"
+        });
+    let one = x
+        .map(|s| ()) //~ NOTE this can be annotated with `?` because it has type `Result<(), &str>`
+        .map_err(|_| ()) //~ NOTE this can't be annotated with `?` because it has type `Result<(), ()>`
+        .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 `?`
+    //~| NOTE in this expansion of desugaring of operator `?`
+    //~| NOTE in this expansion of desugaring of operator `?`
+    //~| NOTE this can't be annotated with `?` because it has type `Result<&str, ()>`
+    //~| 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, String>` to implement `FromResidual<Result<Infallible, ()>>`
+    Ok(one.to_string())
+}
+
+fn bar() -> Result<(), String> { //~ NOTE expected `String` because of this
+    let x = foo(); //~ NOTE this can be annotated with `?` because it has type `Result<String, String>`
+    let one = x
+        .map(|s| ()) //~ NOTE this can be annotated with `?` because it has type `Result<(), String>`
+        .map_err(|_| ())?; //~ ERROR `?` couldn't convert the error to `String`
+    //~^ NOTE in this expansion of desugaring of operator `?`
+    //~| NOTE in this expansion of desugaring of operator `?`
+    //~| NOTE in this expansion of desugaring of operator `?`
+    //~| NOTE in this expansion of desugaring of operator `?`
+    //~| NOTE this can't be annotated with `?` because it has type `Result<(), ()>`
+    //~| 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<Result<Infallible, ()>>`
+    Ok(one)
+}
+
+fn baz() -> Result<String, String> { //~ NOTE expected `String` because of this
+    let test = String::from("one,two");
+    let one = test
+        .split_whitespace()
+        .next()
+        .ok_or_else(|| { //~ NOTE this can't be annotated with `?` because it has type `Result<&str, ()>`
+            "Couldn't split the test string";
+        })?;
+    //~^ ERROR `?` couldn't convert the error to `String`
+    //~| NOTE in this expansion of desugaring of operator `?`
+    //~| NOTE in this expansion of desugaring of operator `?`
+    //~| NOTE in this expansion of desugaring of operator `?`
+    //~| NOTE in this expansion of desugaring of operator `?`
+    //~| 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, String>` to implement `FromResidual<Result<Infallible, ()>>`
+    Ok(one.to_string())
+}
+
+fn main() {}
diff --git a/tests/ui/traits/question-mark-result-err-mismatch.stderr b/tests/ui/traits/question-mark-result-err-mismatch.stderr
new file mode 100644
index 0000000000000..f6acbc6dd0785
--- /dev/null
+++ b/tests/ui/traits/question-mark-result-err-mismatch.stderr
@@ -0,0 +1,83 @@
+error[E0277]: `?` couldn't convert the error to `String`
+  --> $DIR/question-mark-result-err-mismatch.rs:12:22
+   |
+LL |   fn foo() -> Result<String, String> {
+   |               ---------------------- expected `String` because of this
+...
+LL |           .ok_or_else(|| {
+   |  __________-
+LL | |             "Couldn't split the test string"
+LL | |         });
+   | |__________- this can be annotated with `?` because it has type `Result<&str, &str>`
+LL |       let one = x
+LL |           .map(|s| ())
+   |            ----------- this can be annotated with `?` because it has type `Result<(), &str>`
+LL |           .map_err(|_| ())
+   |            --------------- this can't be annotated with `?` because it has type `Result<(), ()>`
+LL |           .map(|()| "")?;
+   |            ------------^ the trait `From<()>` is not implemented for `String`
+   |            |
+   |            this can't be annotated with `?` because it has type `Result<&str, ()>`
+   |
+   = note: the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait
+   = help: the following other types implement trait `From<T>`:
+             <String as From<char>>
+             <String as From<Box<str>>>
+             <String as From<Cow<'a, str>>>
+             <String as From<&str>>
+             <String as From<&mut str>>
+             <String as From<&String>>
+   = note: required for `Result<String, String>` to implement `FromResidual<Result<Infallible, ()>>`
+
+error[E0277]: `?` couldn't convert the error to `String`
+  --> $DIR/question-mark-result-err-mismatch.rs:28:25
+   |
+LL | fn bar() -> Result<(), String> {
+   |             ------------------ expected `String` because of this
+LL |     let x = foo();
+   |             ----- this can be annotated with `?` because it has type `Result<String, String>`
+LL |     let one = x
+LL |         .map(|s| ())
+   |          ----------- this can be annotated with `?` because it has type `Result<(), String>`
+LL |         .map_err(|_| ())?;
+   |          ---------------^ the trait `From<()>` is not implemented for `String`
+   |          |
+   |          this can't be annotated with `?` because it has type `Result<(), ()>`
+   |
+   = note: the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait
+   = help: the following other types implement trait `From<T>`:
+             <String as From<char>>
+             <String as From<Box<str>>>
+             <String as From<Cow<'a, str>>>
+             <String as From<&str>>
+             <String as From<&mut str>>
+             <String as From<&String>>
+   = note: required for `Result<(), String>` to implement `FromResidual<Result<Infallible, ()>>`
+
+error[E0277]: `?` couldn't convert the error to `String`
+  --> $DIR/question-mark-result-err-mismatch.rs:47:11
+   |
+LL |   fn baz() -> Result<String, String> {
+   |               ---------------------- expected `String` because of this
+...
+LL |           .ok_or_else(|| {
+   |  __________-
+LL | |             "Couldn't split the test string";
+LL | |         })?;
+   | |          -^ the trait `From<()>` is not implemented for `String`
+   | |__________|
+   |            this can't be annotated with `?` because it has type `Result<&str, ()>`
+   |
+   = note: the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait
+   = help: the following other types implement trait `From<T>`:
+             <String as From<char>>
+             <String as From<Box<str>>>
+             <String as From<Cow<'a, str>>>
+             <String as From<&str>>
+             <String as From<&mut str>>
+             <String as From<&String>>
+   = note: required for `Result<String, String>` to implement `FromResidual<Result<Infallible, ()>>`
+
+error: aborting due to 3 previous errors
+
+For more information about this error, try `rustc --explain E0277`.
diff --git a/tests/ui/try-block/try-block-bad-type.stderr b/tests/ui/try-block/try-block-bad-type.stderr
index b41bf86d3d911..d58a011ff5501 100644
--- a/tests/ui/try-block/try-block-bad-type.stderr
+++ b/tests/ui/try-block/try-block-bad-type.stderr
@@ -2,7 +2,9 @@ error[E0277]: `?` couldn't convert the error to `TryFromSliceError`
   --> $DIR/try-block-bad-type.rs:7:16
    |
 LL |         Err("")?;
-   |                ^ the trait `From<&str>` is not implemented for `TryFromSliceError`
+   |         -------^ the trait `From<&str>` is not implemented for `TryFromSliceError`
+   |         |
+   |         this can't be annotated with `?` because it has type `Result<_, &str>`
    |
    = note: the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait
    = help: the trait `From<Infallible>` is implemented for `TryFromSliceError`
diff --git a/tests/ui/try-trait/bad-interconversion.stderr b/tests/ui/try-trait/bad-interconversion.stderr
index d8b9431becc6b..97fbbdbf8f8a8 100644
--- a/tests/ui/try-trait/bad-interconversion.stderr
+++ b/tests/ui/try-trait/bad-interconversion.stderr
@@ -4,7 +4,9 @@ error[E0277]: `?` couldn't convert the error to `u8`
 LL | fn result_to_result() -> Result<u64, u8> {
    |                          --------------- expected `u8` because of this
 LL |     Ok(Err(123_i32)?)
-   |                    ^ the trait `From<i32>` is not implemented for `u8`
+   |        ------------^ the trait `From<i32>` is not implemented for `u8`
+   |        |
+   |        this can't be annotated with `?` because it has type `Result<_, i32>`
    |
    = note: the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait
    = help: the following other types implement trait `From<T>`:
diff --git a/tests/ui/try-trait/issue-32709.stderr b/tests/ui/try-trait/issue-32709.stderr
index 46798f5dcfda8..b155b3ff66313 100644
--- a/tests/ui/try-trait/issue-32709.stderr
+++ b/tests/ui/try-trait/issue-32709.stderr
@@ -4,7 +4,9 @@ error[E0277]: `?` couldn't convert the error to `()`
 LL | fn a() -> Result<i32, ()> {
    |           --------------- expected `()` because of this
 LL |     Err(5)?;
-   |           ^ the trait `From<{integer}>` is not implemented for `()`
+   |     ------^ the trait `From<{integer}>` is not implemented for `()`
+   |     |
+   |     this can't be annotated with `?` because it has type `Result<_, {integer}>`
    |
    = note: the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait
    = help: the following other types implement trait `From<T>`:

From 53817963ed8796b35f802d87866aea8de5d4caf6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Esteban=20K=C3=BCber?= <esteban@kuber.com.ar>
Date: Sat, 7 Oct 2023 01:14:43 +0000
Subject: [PATCH 2/4] Point at fewer methods in the chain, only those that
 change the E type

---
 .../error_reporting/type_err_ctxt_ext.rs      | 76 ++++++++++++-------
 .../question-mark-result-err-mismatch.rs      | 15 ++--
 .../question-mark-result-err-mismatch.stderr  | 26 +++----
 3 files changed, 64 insertions(+), 53 deletions(-)

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 bad74588d0cc8..58ea7a0edd521 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
@@ -1049,39 +1049,25 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
         let mut prev_ty = self.resolve_vars_if_possible(
             typeck.expr_ty_adjusted_opt(expr).unwrap_or(Ty::new_misc_error(self.tcx)),
         );
-        let mut annotate_expr = |span: Span, prev_ty: Ty<'tcx>, self_ty: Ty<'tcx>| -> bool {
-            // We always look at the `E` type, because that's the only one affected by `?`. If the
-            // incorrect `Result<T, E>` is because of the `T`, we'll get an E0308 on the whole
-            // expression, after the `?` has "unwrapped" the `T`.
+
+        // We always look at the `E` type, because that's the only one affected by `?`. If the
+        // incorrect `Result<T, E>` is because of the `T`, we'll get an E0308 on the whole
+        // expression, after the `?` has "unwrapped" the `T`.
+        let get_e_type = |prev_ty: Ty<'tcx>| -> Option<Ty<'tcx>> {
             let ty::Adt(def, args) = prev_ty.kind() else {
-                return false;
+                return None;
             };
             let Some(arg) = args.get(1) else {
-                return false;
+                return None;
             };
             if !self.tcx.is_diagnostic_item(sym::Result, def.did()) {
-                return false;
+                return None;
             }
-            let can = if self
-                .infcx
-                .type_implements_trait(
-                    self.tcx.get_diagnostic_item(sym::From).unwrap(),
-                    [self_ty.into(), *arg],
-                    obligation.param_env,
-                )
-                .must_apply_modulo_regions()
-            {
-                "can"
-            } else {
-                "can't"
-            };
-            err.span_label(
-                span,
-                format!("this {can} be annotated with `?` because it has type `{prev_ty}`"),
-            );
-            true
+            Some(arg.as_type()?)
         };
 
+        let mut chain = vec![];
+
         // 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 {
@@ -1089,9 +1075,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
             // let foo = bar.iter().map(mapper)?;
             //               ------ -----------
             expr = rcvr_expr;
-            if !annotate_expr(span, prev_ty, self_ty) {
-                break;
-            }
+            chain.push((span, prev_ty));
 
             prev_ty = self.resolve_vars_if_possible(
                 typeck.expr_ty_adjusted_opt(expr).unwrap_or(Ty::new_misc_error(self.tcx)),
@@ -1121,7 +1105,41 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
         prev_ty = self.resolve_vars_if_possible(
             typeck.expr_ty_adjusted_opt(expr).unwrap_or(Ty::new_misc_error(self.tcx)),
         );
-        annotate_expr(expr.span, prev_ty, self_ty);
+        chain.push((expr.span, prev_ty));
+
+        let mut prev = None;
+        for (span, err_ty) in chain.into_iter().rev() {
+            let err_ty = get_e_type(err_ty);
+            let err_ty = match (err_ty, prev) {
+                (Some(err_ty), Some(prev)) if !self.can_eq(obligation.param_env, err_ty, prev) => {
+                    err_ty
+                }
+                (Some(err_ty), None) => err_ty,
+                _ => {
+                    prev = err_ty;
+                    continue;
+                }
+            };
+            if self
+                .infcx
+                .type_implements_trait(
+                    self.tcx.get_diagnostic_item(sym::From).unwrap(),
+                    [self_ty, err_ty],
+                    obligation.param_env,
+                )
+                .must_apply_modulo_regions()
+            {
+                err.span_label(span, format!("this has type `Result<_, {err_ty}>`"));
+            } else {
+                err.span_label(
+                span,
+                format!(
+                    "this can't be annotated with `?` because it has type `Result<_, {err_ty}>`",
+                ),
+            );
+            }
+            prev = Some(err_ty);
+        }
     }
 
     fn report_const_param_not_wf(
diff --git a/tests/ui/traits/question-mark-result-err-mismatch.rs b/tests/ui/traits/question-mark-result-err-mismatch.rs
index 7b364580858be..e5ccca2e5f7c7 100644
--- a/tests/ui/traits/question-mark-result-err-mismatch.rs
+++ b/tests/ui/traits/question-mark-result-err-mismatch.rs
@@ -3,18 +3,17 @@ fn foo() -> Result<String, String> { //~ NOTE expected `String` because of this
     let x = test
         .split_whitespace()
         .next()
-        .ok_or_else(|| { //~ NOTE this can be annotated with `?` because it has type `Result<&str, &str>`
+        .ok_or_else(|| { //~ NOTE this has type `Result<_, &str>`
             "Couldn't split the test string"
         });
     let one = x
-        .map(|s| ()) //~ NOTE this can be annotated with `?` because it has type `Result<(), &str>`
-        .map_err(|_| ()) //~ NOTE this can't be annotated with `?` because it has type `Result<(), ()>`
+        .map(|s| ())
+        .map_err(|_| ()) //~ NOTE this can't be annotated with `?` because it has type `Result<_, ()>`
         .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 `?`
     //~| NOTE in this expansion of desugaring of operator `?`
     //~| NOTE in this expansion of desugaring of operator `?`
-    //~| NOTE this can't be annotated with `?` because it has type `Result<&str, ()>`
     //~| 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, String>` to implement `FromResidual<Result<Infallible, ()>>`
@@ -22,15 +21,15 @@ fn foo() -> Result<String, String> { //~ NOTE expected `String` because of this
 }
 
 fn bar() -> Result<(), String> { //~ NOTE expected `String` because of this
-    let x = foo(); //~ NOTE this can be annotated with `?` because it has type `Result<String, String>`
+    let x = foo(); //~ NOTE this has type `Result<_, String>`
     let one = x
-        .map(|s| ()) //~ NOTE this can be annotated with `?` because it has type `Result<(), String>`
+        .map(|s| ())
         .map_err(|_| ())?; //~ ERROR `?` couldn't convert the error to `String`
     //~^ NOTE in this expansion of desugaring of operator `?`
     //~| NOTE in this expansion of desugaring of operator `?`
     //~| NOTE in this expansion of desugaring of operator `?`
     //~| NOTE in this expansion of desugaring of operator `?`
-    //~| NOTE this can't be annotated with `?` because it has type `Result<(), ()>`
+    //~| NOTE this can't be annotated with `?` because it has type `Result<_, ()>`
     //~| 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<Result<Infallible, ()>>`
@@ -42,7 +41,7 @@ fn baz() -> Result<String, String> { //~ NOTE expected `String` because of this
     let one = test
         .split_whitespace()
         .next()
-        .ok_or_else(|| { //~ NOTE this can't be annotated with `?` because it has type `Result<&str, ()>`
+        .ok_or_else(|| { //~ NOTE this can't be annotated with `?` because it has type `Result<_, ()>`
             "Couldn't split the test string";
         })?;
     //~^ ERROR `?` couldn't convert the error 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 f6acbc6dd0785..fc3b2e6b46b38 100644
--- a/tests/ui/traits/question-mark-result-err-mismatch.stderr
+++ b/tests/ui/traits/question-mark-result-err-mismatch.stderr
@@ -8,16 +8,12 @@ LL |           .ok_or_else(|| {
    |  __________-
 LL | |             "Couldn't split the test string"
 LL | |         });
-   | |__________- this can be annotated with `?` because it has type `Result<&str, &str>`
-LL |       let one = x
-LL |           .map(|s| ())
-   |            ----------- this can be annotated with `?` because it has type `Result<(), &str>`
+   | |__________- this has type `Result<_, &str>`
+...
 LL |           .map_err(|_| ())
-   |            --------------- this can't be annotated with `?` because it has type `Result<(), ()>`
+   |            --------------- this can't be annotated with `?` because it has type `Result<_, ()>`
 LL |           .map(|()| "")?;
-   |            ------------^ the trait `From<()>` is not implemented for `String`
-   |            |
-   |            this can't be annotated with `?` because it has type `Result<&str, ()>`
+   |                        ^ 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
    = help: the following other types implement trait `From<T>`:
@@ -30,19 +26,17 @@ LL |           .map(|()| "")?;
    = note: required for `Result<String, String>` to implement `FromResidual<Result<Infallible, ()>>`
 
 error[E0277]: `?` couldn't convert the error to `String`
-  --> $DIR/question-mark-result-err-mismatch.rs:28:25
+  --> $DIR/question-mark-result-err-mismatch.rs:27:25
    |
 LL | fn bar() -> Result<(), String> {
    |             ------------------ expected `String` because of this
 LL |     let x = foo();
-   |             ----- this can be annotated with `?` because it has type `Result<String, String>`
-LL |     let one = x
-LL |         .map(|s| ())
-   |          ----------- this can be annotated with `?` because it has type `Result<(), String>`
+   |             ----- this has type `Result<_, String>`
+...
 LL |         .map_err(|_| ())?;
    |          ---------------^ the trait `From<()>` is not implemented for `String`
    |          |
-   |          this can't be annotated with `?` because it has type `Result<(), ()>`
+   |          this can't be annotated with `?` because it has type `Result<_, ()>`
    |
    = note: the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait
    = help: the following other types implement trait `From<T>`:
@@ -55,7 +49,7 @@ LL |         .map_err(|_| ())?;
    = note: required for `Result<(), String>` to implement `FromResidual<Result<Infallible, ()>>`
 
 error[E0277]: `?` couldn't convert the error to `String`
-  --> $DIR/question-mark-result-err-mismatch.rs:47:11
+  --> $DIR/question-mark-result-err-mismatch.rs:46:11
    |
 LL |   fn baz() -> Result<String, String> {
    |               ---------------------- expected `String` because of this
@@ -66,7 +60,7 @@ LL | |             "Couldn't split the test string";
 LL | |         })?;
    | |          -^ the trait `From<()>` is not implemented for `String`
    | |__________|
-   |            this can't be annotated with `?` because it has type `Result<&str, ()>`
+   |            this can't be annotated with `?` because it has type `Result<_, ()>`
    |
    = note: the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait
    = help: the following other types implement trait `From<T>`:

From 98e5317173679763bd6f25e3d925d63dd644baee Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Esteban=20K=C3=BCber?= <esteban@kuber.com.ar>
Date: Sat, 7 Oct 2023 03:47:02 +0000
Subject: [PATCH 3/4] Detect incorrect `;` in `Option::ok_or_else` and
 `Result::map_err`

Fix #72124.
---
 compiler/rustc_span/src/symbol.rs             |  2 +
 .../error_reporting/type_err_ctxt_ext.rs      | 70 ++++++++++++++++++-
 .../question-mark-result-err-mismatch.rs      |  9 ++-
 .../question-mark-result-err-mismatch.stderr  | 15 ++--
 4 files changed, 86 insertions(+), 10 deletions(-)

diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs
index d7e822382ef92..07fe3a2314768 100644
--- a/compiler/rustc_span/src/symbol.rs
+++ b/compiler/rustc_span/src/symbol.rs
@@ -973,6 +973,7 @@ symbols! {
         managed_boxes,
         manually_drop,
         map,
+        map_err,
         marker,
         marker_trait_attr,
         masked,
@@ -1137,6 +1138,7 @@ symbols! {
         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 58ea7a0edd521..5f522fc29eb28 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::config::{DumpSolverProofTree, TraitSolver};
 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 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
             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 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
 
         // 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 e5ccca2e5f7c7..317029e004613 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<String, String> { //~ 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<String, 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, String>` to implement `FromResidual<Result<Infallible, ()>>`
+    //~| HELP the following other types implement trait `From<T>`:
     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<Result<Infallible, ()>>`
+    //~| HELP the following other types implement trait `From<T>`:
     Ok(one)
 }
 
@@ -42,7 +46,7 @@ fn baz() -> Result<String, String> { //~ 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<String, 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, String>` to implement `FromResidual<Result<Infallible, ()>>`
+    //~| HELP the following other types implement trait `From<T>`:
     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 fc3b2e6b46b38..1f9495a505ac3 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<String, String> {
    |               ---------------------- 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<String, String>` to implement `FromResidual<Result<Infallible, ()>>`
 
 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<Result<Infallible, ()>>`
 
 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<String, String> {
    |               ---------------------- expected `String` because of this
@@ -57,6 +61,7 @@ LL |   fn baz() -> Result<String, String> {
 LL |           .ok_or_else(|| {
    |  __________-
 LL | |             "Couldn't split the test string";
+   | |                                             - help: remove this semicolon
 LL | |         })?;
    | |          -^ the trait `From<()>` is not implemented for `String`
    | |__________|

From 70fe624b3d82aee2d32908f3fe2216002a207846 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Esteban=20K=C3=BCber?= <esteban@kuber.com.ar>
Date: Sat, 7 Oct 2023 03:58:45 +0000
Subject: [PATCH 4/4] Reduce verbosity of error

---
 .../error_reporting/type_err_ctxt_ext.rs      | 37 +++++++++++--------
 .../question-mark-result-err-mismatch.rs      |  6 +--
 .../question-mark-result-err-mismatch.stderr  | 24 +-----------
 3 files changed, 24 insertions(+), 43 deletions(-)

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 5f522fc29eb28..a779d69943ddf 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
@@ -112,7 +112,7 @@ pub trait TypeErrCtxtExt<'tcx> {
         obligation: &PredicateObligation<'tcx>,
         trait_ref: ty::TraitRef<'tcx>,
         err: &mut Diagnostic,
-    );
+    ) -> bool;
 
     fn report_const_param_not_wf(
         &self,
@@ -517,8 +517,9 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
 
                         let mut err = struct_span_err!(self.tcx.sess, span, E0277, "{}", err_msg);
 
+                        let mut suggested = false;
                         if is_try_conversion {
-                            self.try_conversion_context(&obligation, trait_ref.skip_binder(), &mut err);
+                            suggested = self.try_conversion_context(&obligation, trait_ref.skip_binder(), &mut err);
                         }
 
                         if is_try_conversion && let Some(ret_span) = self.return_type_span(&obligation) {
@@ -621,8 +622,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
 
                         self.suggest_floating_point_literal(&obligation, &mut err, &trait_ref);
                         self.suggest_dereferencing_index(&obligation, &mut err, trait_predicate);
-                        let mut suggested =
-                            self.suggest_dereferences(&obligation, &mut err, trait_predicate);
+                        suggested |= self.suggest_dereferences(&obligation, &mut err, trait_predicate);
                         suggested |= self.suggest_fn_call(&obligation, &mut err, trait_predicate);
                         let impl_candidates = self.find_similar_impl_candidates(trait_predicate);
                         suggested = if let &[cand] = &impl_candidates[..] {
@@ -1002,7 +1002,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
         obligation: &PredicateObligation<'tcx>,
         trait_ref: ty::TraitRef<'tcx>,
         err: &mut Diagnostic,
-    ) {
+    ) -> bool {
         let span = obligation.cause.span;
         struct V<'v> {
             search_span: Span,
@@ -1027,22 +1027,22 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
             Some(hir::Node::Item(hir::Item { kind: hir::ItemKind::Fn(_, _, body_id), .. })) => {
                 body_id
             }
-            _ => return,
+            _ => return false,
         };
         let mut v = V { search_span: span, found: None };
         v.visit_body(self.tcx.hir().body(*body_id));
         let Some(expr) = v.found else {
-            return;
+            return false;
         };
         let Some(typeck) = &self.typeck_results else {
-            return;
+            return false;
         };
         let Some((ObligationCauseCode::QuestionMark, Some(y))) = obligation.cause.code().parent()
         else {
-            return;
+            return false;
         };
         if !self.tcx.is_diagnostic_item(sym::FromResidual, y.def_id()) {
-            return;
+            return false;
         }
         let self_ty = trait_ref.self_ty();
         let found_ty = trait_ref.args.get(1).and_then(|a| a.as_type());
@@ -1067,6 +1067,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
             Some(arg.as_type()?)
         };
 
+        let mut suggested = false;
         let mut chain = vec![];
 
         // The following logic is simlar to `point_at_chain`, but that's focused on associated types
@@ -1135,6 +1136,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
                     )
                     .must_apply_modulo_regions()
             {
+                suggested = true;
                 err.span_suggestion_short(
                     stmt.span.with_lo(expr.span.hi()),
                     "remove this semicolon",
@@ -1193,17 +1195,20 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
                 )
                 .must_apply_modulo_regions()
             {
-                err.span_label(span, format!("this has type `Result<_, {err_ty}>`"));
+                if !suggested {
+                    err.span_label(span, format!("this has type `Result<_, {err_ty}>`"));
+                }
             } else {
                 err.span_label(
-                span,
-                format!(
-                    "this can't be annotated with `?` because it has type `Result<_, {err_ty}>`",
-                ),
-            );
+                    span,
+                    format!(
+                        "this can't be annotated with `?` because it has type `Result<_, {err_ty}>`",
+                    ),
+                );
             }
             prev = Some(err_ty);
         }
+        suggested
     }
 
     fn report_const_param_not_wf(
diff --git a/tests/ui/traits/question-mark-result-err-mismatch.rs b/tests/ui/traits/question-mark-result-err-mismatch.rs
index 317029e004613..0ca18b5b0ddce 100644
--- a/tests/ui/traits/question-mark-result-err-mismatch.rs
+++ b/tests/ui/traits/question-mark-result-err-mismatch.rs
@@ -3,7 +3,7 @@ fn foo() -> Result<String, String> { //~ NOTE expected `String` because of this
     let x = test
         .split_whitespace()
         .next()
-        .ok_or_else(|| { //~ NOTE this has type `Result<_, &str>`
+        .ok_or_else(|| {
             "Couldn't split the test string"
         });
     let one = x
@@ -15,11 +15,9 @@ fn foo() -> Result<String, String> { //~ NOTE expected `String` because of this
     //~^ NOTE in this expansion of desugaring of operator `?`
     //~| NOTE in this expansion of desugaring of operator `?`
     //~| NOTE in this expansion of desugaring of operator `?`
-    //~| NOTE in this expansion of desugaring of operator `?`
     //~| 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, String>` to implement `FromResidual<Result<Infallible, ()>>`
-    //~| HELP the following other types implement trait `From<T>`:
     Ok(one.to_string())
 }
 
@@ -52,11 +50,9 @@ fn baz() -> Result<String, String> { //~ NOTE expected `String` because of this
     //~| NOTE in this expansion of desugaring of operator `?`
     //~| NOTE in this expansion of desugaring of operator `?`
     //~| NOTE in this expansion of desugaring of operator `?`
-    //~| NOTE in this expansion of desugaring of operator `?`
     //~| 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, String>` to implement `FromResidual<Result<Infallible, ()>>`
-    //~| HELP the following other types implement trait `From<T>`:
     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 1f9495a505ac3..3059e0beca3e4 100644
--- a/tests/ui/traits/question-mark-result-err-mismatch.stderr
+++ b/tests/ui/traits/question-mark-result-err-mismatch.stderr
@@ -4,12 +4,6 @@ error[E0277]: `?` couldn't convert the error to `String`
 LL |   fn foo() -> Result<String, String> {
    |               ---------------------- expected `String` because of this
 ...
-LL |           .ok_or_else(|| {
-   |  __________-
-LL | |             "Couldn't split the test string"
-LL | |         });
-   | |__________- this has type `Result<_, &str>`
-...
 LL |           .map_err(|e| {
    |  __________-
 LL | |             e;
@@ -20,17 +14,10 @@ LL |           .map(|()| "")?;
    |                        ^ 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
-   = help: the following other types implement trait `From<T>`:
-             <String as From<char>>
-             <String as From<Box<str>>>
-             <String as From<Cow<'a, str>>>
-             <String as From<&str>>
-             <String as From<&mut str>>
-             <String as From<&String>>
    = note: required for `Result<String, String>` to implement `FromResidual<Result<Infallible, ()>>`
 
 error[E0277]: `?` couldn't convert the error to `String`
-  --> $DIR/question-mark-result-err-mismatch.rs:30:25
+  --> $DIR/question-mark-result-err-mismatch.rs:28:25
    |
 LL | fn bar() -> Result<(), String> {
    |             ------------------ expected `String` because of this
@@ -53,7 +40,7 @@ LL |         .map_err(|_| ())?;
    = note: required for `Result<(), String>` to implement `FromResidual<Result<Infallible, ()>>`
 
 error[E0277]: `?` couldn't convert the error to `String`
-  --> $DIR/question-mark-result-err-mismatch.rs:50:11
+  --> $DIR/question-mark-result-err-mismatch.rs:48:11
    |
 LL |   fn baz() -> Result<String, String> {
    |               ---------------------- expected `String` because of this
@@ -68,13 +55,6 @@ LL | |         })?;
    |            this can't be annotated with `?` because it has type `Result<_, ()>`
    |
    = note: the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait
-   = help: the following other types implement trait `From<T>`:
-             <String as From<char>>
-             <String as From<Box<str>>>
-             <String as From<Cow<'a, str>>>
-             <String as From<&str>>
-             <String as From<&mut str>>
-             <String as From<&String>>
    = note: required for `Result<String, String>` to implement `FromResidual<Result<Infallible, ()>>`
 
 error: aborting due to 3 previous errors