Suggest calling when operator types mismatch

8 files changed, 199 insertions, 164 deletions

diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs
index 9b9c334d4df..b21e9c2c96c 100644
--- a/compiler/rustc_errors/src/lib.rs
+++ b/compiler/rustc_errors/src/lib.rs
@@ -1248,9 +1248,13 @@ impl HandlerInner {
     }
 
     fn treat_err_as_bug(&self) -> bool {
-        self.flags
-            .treat_err_as_bug
-            .map_or(false, |c| self.err_count() + self.lint_err_count >= c.get())
+        self.flags.treat_err_as_bug.map_or(false, |c| {
+            self.err_count()
+                + self.lint_err_count
+                + self.delayed_span_bugs.len()
+                + self.delayed_good_path_bugs.len()
+                >= c.get()
+        })
     }
 
     fn print_error_count(&mut self, registry: &Registry) {
@@ -1406,7 +1410,14 @@ impl HandlerInner {
         // This is technically `self.treat_err_as_bug()` but `delay_span_bug` is called before
         // incrementing `err_count` by one, so we need to +1 the comparing.
         // FIXME: Would be nice to increment err_count in a more coherent way.
-        if self.flags.treat_err_as_bug.map_or(false, |c| self.err_count() + 1 >= c.get()) {
+        if self.flags.treat_err_as_bug.map_or(false, |c| {
+            self.err_count()
+                + self.lint_err_count
+                + self.delayed_span_bugs.len()
+                + self.delayed_good_path_bugs.len()
+                + 1
+                >= c.get()
+        }) {
             // FIXME: don't abort here if report_delayed_bugs is off
             self.span_bug(sp, msg);
         }
diff --git a/compiler/rustc_lint_defs/src/lib.rs b/compiler/rustc_lint_defs/src/lib.rs
index 9e7cbba9511..9c6530c8a08 100644
--- a/compiler/rustc_lint_defs/src/lib.rs
+++ b/compiler/rustc_lint_defs/src/lib.rs
@@ -41,7 +41,8 @@ macro_rules! pluralize {
 /// All suggestions are marked with an `Applicability`. Tools use the applicability of a suggestion
 /// to determine whether it should be automatically applied or if the user should be consulted
 /// before applying the suggestion.
-#[derive(Copy, Clone, Debug, PartialEq, Hash, Encodable, Decodable, Serialize, Deserialize)]
+#[derive(Copy, Clone, Debug, Hash, Encodable, Decodable, Serialize, Deserialize)]
+#[derive(PartialEq, Eq, PartialOrd, Ord)]
 pub enum Applicability {
     /// The suggestion is definitely what the user intended, or maintains the exact meaning of the code.
     /// This suggestion should be automatically applied.
diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs b/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs
index 4eb0f045d77..96901b53303 100644
--- a/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs
+++ b/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs
@@ -76,10 +76,68 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         found: Ty<'tcx>,
         can_satisfy: impl FnOnce(Ty<'tcx>) -> bool,
     ) -> bool {
-        enum DefIdOrName {
-            DefId(DefId),
-            Name(&'static str),
+        let Some((def_id_or_name, output, num_inputs)) = self.extract_callable_info(expr, found)
+            else { return false; };
+        if can_satisfy(output) {
+            let (sugg_call, mut applicability) = match num_inputs {
+                0 => ("".to_string(), Applicability::MachineApplicable),
+                1..=4 => (
+                    (0..num_inputs).map(|_| "_").collect::<Vec<_>>().join(", "),
+                    Applicability::MachineApplicable,
+                ),
+                _ => ("...".to_string(), Applicability::HasPlaceholders),
+            };
+
+            let msg = match def_id_or_name {
+                DefIdOrName::DefId(def_id) => match self.tcx.def_kind(def_id) {
+                    DefKind::Ctor(CtorOf::Struct, _) => "instantiate this tuple struct".to_string(),
+                    DefKind::Ctor(CtorOf::Variant, _) => {
+                        "instantiate this tuple variant".to_string()
+                    }
+                    kind => format!("call this {}", kind.descr(def_id)),
+                },
+                DefIdOrName::Name(name) => format!("call this {name}"),
+            };
+
+            let sugg = match expr.kind {
+                hir::ExprKind::Call(..)
+                | hir::ExprKind::Path(..)
+                | hir::ExprKind::Index(..)
+                | hir::ExprKind::Lit(..) => {
+                    vec![(expr.span.shrink_to_hi(), format!("({sugg_call})"))]
+                }
+                hir::ExprKind::Closure { .. } => {
+                    // Might be `{ expr } || { bool }`
+                    applicability = Applicability::MaybeIncorrect;
+                    vec![
+                        (expr.span.shrink_to_lo(), "(".to_string()),
+                        (expr.span.shrink_to_hi(), format!(")({sugg_call})")),
+                    ]
+                }
+                _ => {
+                    vec![
+                        (expr.span.shrink_to_lo(), "(".to_string()),
+                        (expr.span.shrink_to_hi(), format!(")({sugg_call})")),
+                    ]
+                }
+            };
+
+            err.multipart_suggestion_verbose(
+                format!("use parentheses to {msg}"),
+                sugg,
+                applicability,
+            );
+
+            return true;
         }
+        false
+    }
+
+    fn extract_callable_info(
+        &self,
+        expr: &Expr<'_>,
+        found: Ty<'tcx>,
+    ) -> Option<(DefIdOrName, Ty<'tcx>, usize)> {
         // Autoderef is useful here because sometimes we box callables, etc.
         let Some((def_id_or_name, output, inputs)) = self.autoderef(expr.span, found).silence_errors().find_map(|(found, _)| {
             match *found.kind() {
@@ -148,67 +206,83 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 }
                 _ => None,
             }
-        }) else { return false; };
+        }) else { return None; };
 
         let output = self.replace_bound_vars_with_fresh_vars(expr.span, infer::FnCall, output);
+
         // We don't want to register any extra obligations, which should be
         // implied by wf, but also because that would possibly result in
         // erroneous errors later on.
         let infer::InferOk { value: output, obligations: _ } =
             self.normalize_associated_types_in_as_infer_ok(expr.span, output);
-        if !output.is_ty_var() && can_satisfy(output) {
-            let (sugg_call, mut applicability) = match inputs {
-                0 => ("".to_string(), Applicability::MachineApplicable),
-                1..=4 => (
-                    (0..inputs).map(|_| "_").collect::<Vec<_>>().join(", "),
-                    Applicability::MachineApplicable,
-                ),
-                _ => ("...".to_string(), Applicability::HasPlaceholders),
-            };
 
-            let msg = match def_id_or_name {
-                DefIdOrName::DefId(def_id) => match self.tcx.def_kind(def_id) {
-                    DefKind::Ctor(CtorOf::Struct, _) => "instantiate this tuple struct".to_string(),
-                    DefKind::Ctor(CtorOf::Variant, _) => {
-                        "instantiate this tuple variant".to_string()
-                    }
-                    kind => format!("call this {}", kind.descr(def_id)),
-                },
-                DefIdOrName::Name(name) => format!("call this {name}"),
-            };
+        if output.is_ty_var() { None } else { Some((def_id_or_name, output, inputs)) }
+    }
 
-            let sugg = match expr.kind {
-                hir::ExprKind::Call(..)
-                | hir::ExprKind::Path(..)
-                | hir::ExprKind::Index(..)
-                | hir::ExprKind::Lit(..) => {
-                    vec![(expr.span.shrink_to_hi(), format!("({sugg_call})"))]
-                }
-                hir::ExprKind::Closure { .. } => {
-                    // Might be `{ expr } || { bool }`
-                    applicability = Applicability::MaybeIncorrect;
-                    vec![
-                        (expr.span.shrink_to_lo(), "(".to_string()),
-                        (expr.span.shrink_to_hi(), format!(")({sugg_call})")),
-                    ]
-                }
-                _ => {
-                    vec![
-                        (expr.span.shrink_to_lo(), "(".to_string()),
-                        (expr.span.shrink_to_hi(), format!(")({sugg_call})")),
-                    ]
+    pub fn suggest_two_fn_call(
+        &self,
+        err: &mut Diagnostic,
+        lhs_expr: &'tcx hir::Expr<'tcx>,
+        lhs_ty: Ty<'tcx>,
+        rhs_expr: &'tcx hir::Expr<'tcx>,
+        rhs_ty: Ty<'tcx>,
+        can_satisfy: impl FnOnce(Ty<'tcx>, Ty<'tcx>) -> bool,
+    ) -> bool {
+        let Some((_, lhs_output_ty, num_lhs_inputs)) = self.extract_callable_info(lhs_expr, lhs_ty)
+            else { return false; };
+        let Some((_, rhs_output_ty, num_rhs_inputs)) = self.extract_callable_info(rhs_expr, rhs_ty)
+            else { return false; };
+
+        if can_satisfy(lhs_output_ty, rhs_output_ty) {
+            let mut sugg = vec![];
+            let mut applicability = Applicability::MachineApplicable;
+
+            for (expr, num_inputs) in [(lhs_expr, num_lhs_inputs), (rhs_expr, num_rhs_inputs)] {
+                let (sugg_call, this_applicability) = match num_inputs {
+                    0 => ("".to_string(), Applicability::MachineApplicable),
+                    1..=4 => (
+                        (0..num_inputs).map(|_| "_").collect::<Vec<_>>().join(", "),
+                        Applicability::MachineApplicable,
+                    ),
+                    _ => ("...".to_string(), Applicability::HasPlaceholders),
+                };
+
+                applicability = applicability.max(this_applicability);
+
+                match expr.kind {
+                    hir::ExprKind::Call(..)
+                    | hir::ExprKind::Path(..)
+                    | hir::ExprKind::Index(..)
+                    | hir::ExprKind::Lit(..) => {
+                        sugg.extend([(expr.span.shrink_to_hi(), format!("({sugg_call})"))]);
+                    }
+                    hir::ExprKind::Closure { .. } => {
+                        // Might be `{ expr } || { bool }`
+                        applicability = Applicability::MaybeIncorrect;
+                        sugg.extend([
+                            (expr.span.shrink_to_lo(), "(".to_string()),
+                            (expr.span.shrink_to_hi(), format!(")({sugg_call})")),
+                        ]);
+                    }
+                    _ => {
+                        sugg.extend([
+                            (expr.span.shrink_to_lo(), "(".to_string()),
+                            (expr.span.shrink_to_hi(), format!(")({sugg_call})")),
+                        ]);
+                    }
                 }
-            };
+            }
 
             err.multipart_suggestion_verbose(
-                format!("use parentheses to {msg}"),
+                format!("use parentheses to call these"),
                 sugg,
                 applicability,
             );
 
-            return true;
+            true
+        } else {
+            false
         }
-        false
     }
 
     pub fn suggest_deref_ref_or_into(
@@ -959,3 +1033,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         }
     }
 }
+
+enum DefIdOrName {
+    DefId(DefId),
+    Name(&'static str),
+}
diff --git a/compiler/rustc_typeck/src/check/op.rs b/compiler/rustc_typeck/src/check/op.rs
index eb0c51bb2f9..952086e898f 100644
--- a/compiler/rustc_typeck/src/check/op.rs
+++ b/compiler/rustc_typeck/src/check/op.rs
@@ -410,26 +410,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                         };
                         let mut err = struct_span_err!(self.tcx.sess, op.span, E0369, "{message}");
                         if !lhs_expr.span.eq(&rhs_expr.span) {
-                            self.add_type_neq_err_label(
-                                &mut err,
-                                lhs_expr.span,
-                                lhs_ty,
-                                rhs_ty,
-                                rhs_expr,
-                                op,
-                                is_assign,
-                                expected,
-                            );
-                            self.add_type_neq_err_label(
-                                &mut err,
-                                rhs_expr.span,
-                                rhs_ty,
-                                lhs_ty,
-                                lhs_expr,
-                                op,
-                                is_assign,
-                                expected,
-                            );
+                            err.span_label(lhs_expr.span, lhs_ty.to_string());
+                            err.span_label(rhs_expr.span, rhs_ty.to_string());
                         }
                         self.note_unmet_impls_on_type(&mut err, errors);
                         (err, missing_trait, use_output)
@@ -468,17 +450,50 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                     }
                 };
 
+                let is_compatible = |lhs_ty, rhs_ty| {
+                    self.lookup_op_method(
+                        lhs_ty,
+                        Some(rhs_ty),
+                        Some(rhs_expr),
+                        Op::Binary(op, is_assign),
+                        expected,
+                    )
+                    .is_ok()
+                };
+
                 // We should suggest `a + b` => `*a + b` if `a` is copy, and suggest
                 // `a += b` => `*a += b` if a is a mut ref.
-                if is_assign == IsAssign::Yes
-                    && let Some(lhs_deref_ty) = self.deref_once_mutably_for_diagnostic(lhs_ty) {
-                        suggest_deref_binop(lhs_deref_ty);
+                if !op.span.can_be_used_for_suggestions() {
+                    // Suppress suggestions when lhs and rhs are not in the same span as the error
+                } else if is_assign == IsAssign::Yes
+                    && let Some(lhs_deref_ty) = self.deref_once_mutably_for_diagnostic(lhs_ty)
+                {
+                    suggest_deref_binop(lhs_deref_ty);
                 } else if is_assign == IsAssign::No
-                    && let Ref(_, lhs_deref_ty, _) = lhs_ty.kind() {
-                    if self.type_is_copy_modulo_regions(self.param_env, *lhs_deref_ty, lhs_expr.span) {
+                    && let Ref(_, lhs_deref_ty, _) = lhs_ty.kind()
+                {
+                    if self.type_is_copy_modulo_regions(
+                        self.param_env,
+                        *lhs_deref_ty,
+                        lhs_expr.span,
+                    ) {
                         suggest_deref_binop(*lhs_deref_ty);
                     }
+                } else if self.suggest_fn_call(&mut err, lhs_expr, lhs_ty, |lhs_ty| {
+                    is_compatible(lhs_ty, rhs_ty)
+                }) || self.suggest_fn_call(&mut err, rhs_expr, rhs_ty, |rhs_ty| {
+                    is_compatible(lhs_ty, rhs_ty)
+                }) || self.suggest_two_fn_call(
+                    &mut err,
+                    rhs_expr,
+                    rhs_ty,
+                    lhs_expr,
+                    lhs_ty,
+                    |lhs_ty, rhs_ty| is_compatible(lhs_ty, rhs_ty),
+                ) {
+                    // Cool
                 }
+
                 if let Some(missing_trait) = missing_trait {
                     let mut visitor = TypeParamVisitor(vec![]);
                     visitor.visit_ty(lhs_ty);
@@ -548,69 +563,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         (lhs_ty, rhs_ty, return_ty)
     }
 
-    /// If one of the types is an uncalled function and calling it would yield the other type,
-    /// suggest calling the function. Returns `true` if suggestion would apply (even if not given).
-    fn add_type_neq_err_label(
-        &self,
-        err: &mut Diagnostic,
-        span: Span,
-        ty: Ty<'tcx>,
-        other_ty: Ty<'tcx>,
-        other_expr: &'tcx hir::Expr<'tcx>,
-        op: hir::BinOp,
-        is_assign: IsAssign,
-        expected: Expectation<'tcx>,
-    ) -> bool /* did we suggest to call a function because of missing parentheses? */ {
-        err.span_label(span, ty.to_string());
-        if let FnDef(def_id, _) = *ty.kind() {
-            if !self.tcx.has_typeck_results(def_id) {
-                return false;
-            }
-            // FIXME: Instead of exiting early when encountering bound vars in
-            // the function signature, consider keeping the binder here and
-            // propagating it downwards.
-            let Some(fn_sig) = self.tcx.fn_sig(def_id).no_bound_vars() else {
-                return false;
-            };
-
-            let other_ty = if let FnDef(def_id, _) = *other_ty.kind() {
-                if !self.tcx.has_typeck_results(def_id) {
-                    return false;
-                }
-                // We're emitting a suggestion, so we can just ignore regions
-                self.tcx.fn_sig(def_id).skip_binder().output()
-            } else {
-                other_ty
-            };
-
-            if self
-                .lookup_op_method(
-                    fn_sig.output(),
-                    Some(other_ty),
-                    Some(other_expr),
-                    Op::Binary(op, is_assign),
-                    expected,
-                )
-                .is_ok()
-            {
-                let (variable_snippet, applicability) = if !fn_sig.inputs().is_empty() {
-                    ("( /* arguments */ )", Applicability::HasPlaceholders)
-                } else {
-                    ("()", Applicability::MaybeIncorrect)
-                };
-
-                err.span_suggestion_verbose(
-                    span.shrink_to_hi(),
-                    "you might have forgotten to call this function",
-                    variable_snippet,
-                    applicability,
-                );
-                return true;
-            }
-        }
-        false
-    }
-
     /// Provide actionable suggestions when trying to add two strings with incorrect types,
     /// like `&str + &str`, `String + String` and `&str + &String`.
     ///
diff --git a/src/test/ui/binop/issue-77910-2.stderr b/src/test/ui/binop/issue-77910-2.stderr
index 5477a5762a8..74860a93f37 100644
--- a/src/test/ui/binop/issue-77910-2.stderr
+++ b/src/test/ui/binop/issue-77910-2.stderr
@@ -5,6 +5,11 @@ LL |     if foo == y {}
    |        --- ^^ - _
    |        |
    |        for<'r> fn(&'r i32) -> &'r i32 {foo}
+   |
+help: use parentheses to call this function
+   |
+LL |     if foo(_) == y {}
+   |           +++
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/fn/fn-compare-mismatch.stderr b/src/test/ui/fn/fn-compare-mismatch.stderr
index 096440225b9..df838cb1181 100644
--- a/src/test/ui/fn/fn-compare-mismatch.stderr
+++ b/src/test/ui/fn/fn-compare-mismatch.stderr
@@ -6,14 +6,10 @@ LL |     let x = f == g;
    |             |
    |             fn() {f}
    |
-help: you might have forgotten to call this function
+help: use parentheses to call these
    |
-LL |     let x = f() == g;
-   |              ++
-help: you might have forgotten to call this function
-   |
-LL |     let x = f == g();
-   |                   ++
+LL |     let x = f() == g();
+   |              ++     ++
 
 error[E0308]: mismatched types
   --> $DIR/fn-compare-mismatch.rs:4:18
diff --git a/src/test/ui/issues/issue-59488.stderr b/src/test/ui/issues/issue-59488.stderr
index bb6843a1958..df681eb2489 100644
--- a/src/test/ui/issues/issue-59488.stderr
+++ b/src/test/ui/issues/issue-59488.stderr
@@ -6,7 +6,7 @@ LL |     foo > 12;
    |     |
    |     fn() -> i32 {foo}
    |
-help: you might have forgotten to call this function
+help: use parentheses to call this function
    |
 LL |     foo() > 12;
    |        ++
@@ -28,10 +28,10 @@ LL |     bar > 13;
    |     |
    |     fn(i64) -> i64 {bar}
    |
-help: you might have forgotten to call this function
+help: use parentheses to call this function
    |
-LL |     bar( /* arguments */ ) > 13;
-   |        +++++++++++++++++++
+LL |     bar(_) > 13;
+   |        +++
 
 error[E0308]: mismatched types
   --> $DIR/issue-59488.rs:18:11
@@ -50,14 +50,10 @@ LL |     foo > foo;
    |     |
    |     fn() -> i32 {foo}
    |
-help: you might have forgotten to call this function
+help: use parentheses to call these
    |
-LL |     foo() > foo;
-   |        ++
-help: you might have forgotten to call this function
-   |
-LL |     foo > foo();
-   |              ++
+LL |     foo() > foo();
+   |        ++      ++
 
 error[E0369]: binary operation `>` cannot be applied to type `fn() -> i32 {foo}`
   --> $DIR/issue-59488.rs:25:9
diff --git a/src/test/ui/issues/issue-70724-add_type_neq_err_label-unwrap.stderr b/src/test/ui/issues/issue-70724-add_type_neq_err_label-unwrap.stderr
index c6e6ea1e096..9239385e643 100644
--- a/src/test/ui/issues/issue-70724-add_type_neq_err_label-unwrap.stderr
+++ b/src/test/ui/issues/issue-70724-add_type_neq_err_label-unwrap.stderr
@@ -8,11 +8,6 @@ LL |     assert_eq!(a, 0);
    |     {integer}
    |
    = note: this error originates in the macro `assert_eq` (in Nightly builds, run with -Z macro-backtrace for more info)
-help: you might have forgotten to call this function
-  --> $SRC_DIR/core/src/macros/mod.rs:LL:COL
-   |
-LL |                 if !(*left_val() == *right_val) {
-   |                               ++
 
 error[E0308]: mismatched types
   --> $DIR/issue-70724-add_type_neq_err_label-unwrap.rs:6:5
@@ -21,7 +16,7 @@ LL |     assert_eq!(a, 0);
    |     ^^^^^^^^^^^^^^^^ expected fn item, found integer
    |
    = note: expected fn item `fn() -> i32 {a}`
-                 found type `i32`
+                 found type `{integer}`
    = note: this error originates in the macro `assert_eq` (in Nightly builds, run with -Z macro-backtrace for more info)
 
 error[E0277]: `fn() -> i32 {a}` doesn't implement `Debug`