Auto merge of #102813 - Akida31:issue-64915/simpler_diagnostic_when_passing_arg_to_closure_and_missing_borrow, r=estebank

Simpler diagnostic when passing arg to closure and missing borrow

fixes #64915

I followed roughly the instructions and the older PR #76362.
The number of references for the expected and the found types will be compared and depending on which has more the diagnostic will be emitted.

I'm not quite sure if my approach with the many `span_bug!`s is good, it could lead to some ICEs. Would it be better if  those errors are ignored?

As far as I know the following code works similarly but in a different context. Is this probably reusable since it looks like it would emit better diagnostics?
https://github.com/rust-lang/rust/blob/a688a0305fad9219505a8f2576446510601bafe8/compiler/rustc_hir_analysis/src/check/demand.rs#L713-L1061

When running the tests locally, a codegen test failed. Is there something I can/ should do about that?

If you have some improvements/ corrections please say so and I will happily include them.

r? `@estebank` (as you added the mentoring instructions to the issue)

6 files changed, 137 insertions, 14 deletions

diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs
index 30ff07ee6c3..da6244acb31 100644
--- a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs
+++ b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs
@@ -1234,6 +1234,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
                     _ => None,
                 };
 
+                let found_node = found_did.and_then(|did| self.tcx.hir().get_if_local(did));
                 let found_span = found_did.and_then(|did| self.tcx.hir().span_if_local(did));
 
                 if self.reported_closure_mismatch.borrow().contains(&(span, found_span)) {
@@ -1287,6 +1288,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
                         found_trait_ref,
                         expected_trait_ref,
                         obligation.cause.code(),
+                        found_node,
                     )
                 } else {
                     let (closure_span, closure_arg_span, found) = found_did
diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs
index 40c81025471..6f79ca078d7 100644
--- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs
+++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs
@@ -1,4 +1,5 @@
 // ignore-tidy-filelength
+
 use super::{DefIdOrName, Obligation, ObligationCause, ObligationCauseCode, PredicateObligation};
 
 use crate::autoderef::Autoderef;
@@ -258,6 +259,7 @@ pub trait TypeErrCtxtExt<'tcx> {
         found: ty::PolyTraitRef<'tcx>,
         expected: ty::PolyTraitRef<'tcx>,
         cause: &ObligationCauseCode<'tcx>,
+        found_node: Option<Node<'_>>,
     ) -> DiagnosticBuilder<'tcx, ErrorGuaranteed>;
 
     fn note_conflicting_closure_bounds(
@@ -1695,6 +1697,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
         found: ty::PolyTraitRef<'tcx>,
         expected: ty::PolyTraitRef<'tcx>,
         cause: &ObligationCauseCode<'tcx>,
+        found_node: Option<Node<'_>>,
     ) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> {
         pub(crate) fn build_fn_sig_ty<'tcx>(
             infcx: &InferCtxt<'tcx>,
@@ -1756,6 +1759,10 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
 
         self.note_conflicting_closure_bounds(cause, &mut err);
 
+        if let Some(found_node) = found_node {
+            hint_missing_borrow(span, found_span, found, expected, found_node, &mut err);
+        }
+
         err
     }
 
@@ -3384,6 +3391,78 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
     }
 }
 
+/// Add a hint to add a missing borrow or remove an unnecessary one.
+fn hint_missing_borrow<'tcx>(
+    span: Span,
+    found_span: Span,
+    found: Ty<'tcx>,
+    expected: Ty<'tcx>,
+    found_node: Node<'_>,
+    err: &mut Diagnostic,
+) {
+    let found_args = match found.kind() {
+        ty::FnPtr(f) => f.inputs().skip_binder().iter(),
+        kind => {
+            span_bug!(span, "found was converted to a FnPtr above but is now {:?}", kind)
+        }
+    };
+    let expected_args = match expected.kind() {
+        ty::FnPtr(f) => f.inputs().skip_binder().iter(),
+        kind => {
+            span_bug!(span, "expected was converted to a FnPtr above but is now {:?}", kind)
+        }
+    };
+
+    let fn_decl = found_node
+        .fn_decl()
+        .unwrap_or_else(|| span_bug!(found_span, "found node must be a function"));
+
+    let arg_spans = fn_decl.inputs.iter().map(|ty| ty.span);
+
+    fn get_deref_type_and_refs<'tcx>(mut ty: Ty<'tcx>) -> (Ty<'tcx>, usize) {
+        let mut refs = 0;
+
+        while let ty::Ref(_, new_ty, _) = ty.kind() {
+            ty = *new_ty;
+            refs += 1;
+        }
+
+        (ty, refs)
+    }
+
+    let mut to_borrow = Vec::new();
+    let mut remove_borrow = Vec::new();
+
+    for ((found_arg, expected_arg), arg_span) in found_args.zip(expected_args).zip(arg_spans) {
+        let (found_ty, found_refs) = get_deref_type_and_refs(*found_arg);
+        let (expected_ty, expected_refs) = get_deref_type_and_refs(*expected_arg);
+
+        if found_ty == expected_ty {
+            if found_refs < expected_refs {
+                to_borrow.push((arg_span, expected_arg.to_string()));
+            } else if found_refs > expected_refs {
+                remove_borrow.push((arg_span, expected_arg.to_string()));
+            }
+        }
+    }
+
+    if !to_borrow.is_empty() {
+        err.multipart_suggestion(
+            "consider borrowing the argument",
+            to_borrow,
+            Applicability::MaybeIncorrect,
+        );
+    }
+
+    if !remove_borrow.is_empty() {
+        err.multipart_suggestion(
+            "do not borrow the argument",
+            remove_borrow,
+            Applicability::MaybeIncorrect,
+        );
+    }
+}
+
 /// Collect all the returned expressions within the input expression.
 /// Used to point at the return spans when we want to suggest some change to them.
 #[derive(Default)]
diff --git a/src/test/ui/anonymous-higher-ranked-lifetime.stderr b/src/test/ui/anonymous-higher-ranked-lifetime.stderr
index bf5f642ca82..afb7f8fea92 100644
--- a/src/test/ui/anonymous-higher-ranked-lifetime.stderr
+++ b/src/test/ui/anonymous-higher-ranked-lifetime.stderr
@@ -13,6 +13,10 @@ note: required by a bound in `f1`
    |
 LL | fn f1<F>(_: F) where F: Fn(&(), &()) {}
    |                         ^^^^^^^^^^^^ required by this bound in `f1`
+help: consider borrowing the argument
+   |
+LL |     f1(|_: &(), _: &()| {});
+   |            ~~~     ~~~
 
 error[E0631]: type mismatch in closure arguments
   --> $DIR/anonymous-higher-ranked-lifetime.rs:3:5
@@ -29,6 +33,10 @@ note: required by a bound in `f2`
    |
 LL | fn f2<F>(_: F) where F: for<'a> Fn(&'a (), &()) {}
    |                         ^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `f2`
+help: consider borrowing the argument
+   |
+LL |     f2(|_: &'a (), _: &()| {});
+   |            ~~~~~~     ~~~
 
 error[E0631]: type mismatch in closure arguments
   --> $DIR/anonymous-higher-ranked-lifetime.rs:4:5
@@ -45,6 +53,10 @@ note: required by a bound in `f3`
    |
 LL | fn f3<'a, F>(_: F) where F: Fn(&'a (), &()) {}
    |                             ^^^^^^^^^^^^^^^ required by this bound in `f3`
+help: consider borrowing the argument
+   |
+LL |     f3(|_: &(), _: &()| {});
+   |            ~~~     ~~~
 
 error[E0631]: type mismatch in closure arguments
   --> $DIR/anonymous-higher-ranked-lifetime.rs:5:5
@@ -61,6 +73,10 @@ note: required by a bound in `f4`
    |
 LL | fn f4<F>(_: F) where F: for<'r> Fn(&(), &'r ()) {}
    |                         ^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `f4`
+help: consider borrowing the argument
+   |
+LL |     f4(|_: &(), _: &'r ()| {});
+   |            ~~~     ~~~~~~
 
 error[E0631]: type mismatch in closure arguments
   --> $DIR/anonymous-higher-ranked-lifetime.rs:6:5
@@ -77,13 +93,19 @@ note: required by a bound in `f5`
    |
 LL | fn f5<F>(_: F) where F: for<'r> Fn(&'r (), &'r ()) {}
    |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `f5`
+help: consider borrowing the argument
+   |
+LL |     f5(|_: &'r (), _: &'r ()| {});
+   |            ~~~~~~     ~~~~~~
 
 error[E0631]: type mismatch in closure arguments
   --> $DIR/anonymous-higher-ranked-lifetime.rs:7:5
    |
 LL |     g1(|_: (), _: ()| {});
-   |     ^^ -------------- found signature defined here
-   |     |
+   |     ^^ --------------
+   |     |  |   |
+   |     |  |   help: consider borrowing the argument: `&()`
+   |     |  found signature defined here
    |     expected due to this
    |
    = note: expected closure signature `for<'a> fn(&'a (), Box<(dyn for<'a> Fn(&'a ()) + 'static)>) -> _`
@@ -98,8 +120,10 @@ error[E0631]: type mismatch in closure arguments
   --> $DIR/anonymous-higher-ranked-lifetime.rs:8:5
    |
 LL |     g2(|_: (), _: ()| {});
-   |     ^^ -------------- found signature defined here
-   |     |
+   |     ^^ --------------
+   |     |  |   |
+   |     |  |   help: consider borrowing the argument: `&()`
+   |     |  found signature defined here
    |     expected due to this
    |
    = note: expected closure signature `for<'a> fn(&'a (), for<'a> fn(&'a ())) -> _`
@@ -114,8 +138,10 @@ error[E0631]: type mismatch in closure arguments
   --> $DIR/anonymous-higher-ranked-lifetime.rs:9:5
    |
 LL |     g3(|_: (), _: ()| {});
-   |     ^^ -------------- found signature defined here
-   |     |
+   |     ^^ --------------
+   |     |  |   |
+   |     |  |   help: consider borrowing the argument: `&'s ()`
+   |     |  found signature defined here
    |     expected due to this
    |
    = note: expected closure signature `for<'s> fn(&'s (), Box<(dyn for<'a> Fn(&'a ()) + 'static)>) -> _`
@@ -130,8 +156,10 @@ error[E0631]: type mismatch in closure arguments
   --> $DIR/anonymous-higher-ranked-lifetime.rs:10:5
    |
 LL |     g4(|_: (), _: ()| {});
-   |     ^^ -------------- found signature defined here
-   |     |
+   |     ^^ --------------
+   |     |  |   |
+   |     |  |   help: consider borrowing the argument: `&()`
+   |     |  found signature defined here
    |     expected due to this
    |
    = note: expected closure signature `for<'a> fn(&'a (), for<'r> fn(&'r ())) -> _`
@@ -157,6 +185,10 @@ note: required by a bound in `h1`
    |
 LL | fn h1<F>(_: F) where F: Fn(&(), Box<dyn Fn(&())>, &(), fn(&(), &())) {}
    |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `h1`
+help: consider borrowing the argument
+   |
+LL |     h1(|_: &(), _: (), _: &(), _: ()| {});
+   |            ~~~            ~~~
 
 error[E0631]: type mismatch in closure arguments
   --> $DIR/anonymous-higher-ranked-lifetime.rs:12:5
@@ -173,6 +205,10 @@ note: required by a bound in `h2`
    |
 LL | fn h2<F>(_: F) where F: for<'t0> Fn(&(), Box<dyn Fn(&())>, &'t0 (), fn(&(), &())) {}
    |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `h2`
+help: consider borrowing the argument
+   |
+LL |     h2(|_: &(), _: (), _: &'t0 (), _: ()| {});
+   |            ~~~            ~~~~~~~
 
 error: aborting due to 11 previous errors
 
diff --git a/src/test/ui/closures/multiple-fn-bounds.stderr b/src/test/ui/closures/multiple-fn-bounds.stderr
index eefc123fed7..da26302c9d8 100644
--- a/src/test/ui/closures/multiple-fn-bounds.stderr
+++ b/src/test/ui/closures/multiple-fn-bounds.stderr
@@ -2,8 +2,10 @@ error[E0631]: type mismatch in closure arguments
   --> $DIR/multiple-fn-bounds.rs:10:5
    |
 LL |     foo(move |x| v);
-   |     ^^^ -------- found signature defined here
-   |     |
+   |     ^^^ --------
+   |     |   |     |
+   |     |   |     help: do not borrow the argument: `char`
+   |     |   found signature defined here
    |     expected due to this
    |
    = note: expected closure signature `fn(char) -> _`
diff --git a/src/test/ui/mismatched_types/closure-arg-type-mismatch.stderr b/src/test/ui/mismatched_types/closure-arg-type-mismatch.stderr
index f2e2a4c7fd5..961ef32cbd3 100644
--- a/src/test/ui/mismatched_types/closure-arg-type-mismatch.stderr
+++ b/src/test/ui/mismatched_types/closure-arg-type-mismatch.stderr
@@ -2,8 +2,10 @@ error[E0631]: type mismatch in closure arguments
   --> $DIR/closure-arg-type-mismatch.rs:3:14
    |
 LL |     a.iter().map(|_: (u32, u32)| 45);
-   |              ^^^ --------------- found signature defined here
-   |              |
+   |              ^^^ ---------------
+   |              |   |   |
+   |              |   |   help: consider borrowing the argument: `&(u32, u32)`
+   |              |   found signature defined here
    |              expected due to this
    |
    = note: expected closure signature `fn(&(u32, u32)) -> _`
diff --git a/src/test/ui/mismatched_types/issue-36053-2.stderr b/src/test/ui/mismatched_types/issue-36053-2.stderr
index b3509abbf84..1a696752e87 100644
--- a/src/test/ui/mismatched_types/issue-36053-2.stderr
+++ b/src/test/ui/mismatched_types/issue-36053-2.stderr
@@ -2,8 +2,10 @@ error[E0631]: type mismatch in closure arguments
   --> $DIR/issue-36053-2.rs:7:32
    |
 LL |     once::<&str>("str").fuse().filter(|a: &str| true).count();
-   |                                ^^^^^^ --------- found signature defined here
-   |                                |
+   |                                ^^^^^^ ---------
+   |                                |      |   |
+   |                                |      |   help: consider borrowing the argument: `&&str`
+   |                                |      found signature defined here
    |                                expected due to this
    |
    = note: expected closure signature `for<'a> fn(&'a &str) -> _`