Rollup merge of rust-lang#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 rust-lang#64915

I followed roughly the instructions and the older PR rust-lang#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)

Author: Manishearth

compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs

@@ -1207,6 +1207,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)) {
@@ -1260,6 +1261,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
                         found_trait_ref,
                         expected_trait_ref,
                         obligation.cause.code(),
+                        found_node,
                     )
                 } else {
                     let (closure_span, found) = found_did

compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs

@@ -257,6 +257,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(
@@ -1680,6 +1681,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>,
@@ -1743,6 +1745,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
     }
 
@@ -3122,6 +3128,67 @@ 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)
+    }
+
+    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 {
+            let hint = if found_refs < expected_refs {
+                "consider borrowing the argument"
+            } else if found_refs == expected_refs {
+                continue;
+            } else {
+                "do not borrow the argument"
+            };
+            err.span_suggestion_verbose(
+                arg_span,
+                hint,
+                expected_arg.to_string(),
+                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)]

src/test/ui/anonymous-higher-ranked-lifetime.stderr

@@ -13,6 +13,14 @@ 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(|_: &(), _: ()| {});
+   |            ~~~
+help: consider borrowing the argument
+   |
+LL |     f1(|_: (), _: &()| {});
+   |                   ~~~
 
 error[E0631]: type mismatch in closure arguments
   --> $DIR/anonymous-higher-ranked-lifetime.rs:3:5
@@ -29,6 +37,14 @@ 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 (), _: ()| {});
+   |            ~~~~~~
+help: consider borrowing the argument
+   |
+LL |     f2(|_: (), _: &()| {});
+   |                   ~~~
 
 error[E0631]: type mismatch in closure arguments
   --> $DIR/anonymous-higher-ranked-lifetime.rs:4:5
@@ -45,6 +61,14 @@ 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(|_: &(), _: ()| {});
+   |            ~~~
+help: consider borrowing the argument
+   |
+LL |     f3(|_: (), _: &()| {});
+   |                   ~~~
 
 error[E0631]: type mismatch in closure arguments
   --> $DIR/anonymous-higher-ranked-lifetime.rs:5:5
@@ -61,6 +85,14 @@ 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(|_: &(), _: ()| {});
+   |            ~~~
+help: consider borrowing the argument
+   |
+LL |     f4(|_: (), _: &'r ()| {});
+   |                   ~~~~~~
 
 error[E0631]: type mismatch in closure arguments
   --> $DIR/anonymous-higher-ranked-lifetime.rs:6:5
@@ -77,6 +109,14 @@ 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 (), _: ()| {});
+   |            ~~~~~~
+help: consider borrowing the argument
+   |
+LL |     f5(|_: (), _: &'r ()| {});
+   |                   ~~~~~~
 
 error[E0631]: type mismatch in closure arguments
   --> $DIR/anonymous-higher-ranked-lifetime.rs:7:5
@@ -93,6 +133,10 @@ note: required by a bound in `g1`
    |
 LL | fn g1<F>(_: F) where F: Fn(&(), Box<dyn Fn(&())>) {}
    |                         ^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `g1`
+help: consider borrowing the argument
+   |
+LL |     g1(|_: &(), _: ()| {});
+   |            ~~~
 
 error[E0631]: type mismatch in closure arguments
   --> $DIR/anonymous-higher-ranked-lifetime.rs:8:5
@@ -109,6 +153,10 @@ note: required by a bound in `g2`
    |
 LL | fn g2<F>(_: F) where F: Fn(&(), fn(&())) {}
    |                         ^^^^^^^^^^^^^^^^ required by this bound in `g2`
+help: consider borrowing the argument
+   |
+LL |     g2(|_: &(), _: ()| {});
+   |            ~~~
 
 error[E0631]: type mismatch in closure arguments
   --> $DIR/anonymous-higher-ranked-lifetime.rs:9:5
@@ -125,6 +173,10 @@ note: required by a bound in `g3`
    |
 LL | fn g3<F>(_: F) where F: for<'s> Fn(&'s (), Box<dyn Fn(&())>) {}
    |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `g3`
+help: consider borrowing the argument
+   |
+LL |     g3(|_: &'s (), _: ()| {});
+   |            ~~~~~~
 
 error[E0631]: type mismatch in closure arguments
   --> $DIR/anonymous-higher-ranked-lifetime.rs:10:5
@@ -141,6 +193,10 @@ note: required by a bound in `g4`
    |
 LL | fn g4<F>(_: F) where F: Fn(&(), for<'r> fn(&'r ())) {}
    |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `g4`
+help: consider borrowing the argument
+   |
+LL |     g4(|_: &(), _: ()| {});
+   |            ~~~
 
 error[E0631]: type mismatch in closure arguments
   --> $DIR/anonymous-higher-ranked-lifetime.rs:11:5
@@ -157,6 +213,14 @@ 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(|_: &(), _: (), _: (), _: ()| {});
+   |            ~~~
+help: consider borrowing the argument
+   |
+LL |     h1(|_: (), _: (), _: &(), _: ()| {});
+   |                          ~~~
 
 error[E0631]: type mismatch in closure arguments
   --> $DIR/anonymous-higher-ranked-lifetime.rs:12:5
@@ -173,6 +237,14 @@ 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(|_: &(), _: (), _: (), _: ()| {});
+   |            ~~~
+help: consider borrowing the argument
+   |
+LL |     h2(|_: (), _: (), _: &'t0 (), _: ()| {});
+   |                          ~~~~~~~
 
 error: aborting due to 11 previous errors
 

src/test/ui/closures/multiple-fn-bounds.stderr

@@ -18,6 +18,10 @@ note: required by a bound in `foo`
    |
 LL | fn foo<F: Fn(&char) -> bool + Fn(char) -> bool>(f: F) {
    |                               ^^^^^^^^^^^^^^^^ required by this bound in `foo`
+help: do not borrow the argument
+   |
+LL |     foo(move |char| v);
+   |               ~~~~
 
 error: aborting due to previous error
 

src/test/ui/mismatched_types/closure-arg-type-mismatch.stderr

@@ -13,6 +13,10 @@ note: required by a bound in `map`
    |
 LL |         F: FnMut(Self::Item) -> B,
    |            ^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `map`
+help: consider borrowing the argument
+   |
+LL |     a.iter().map(|_: &(u32, u32)| 45);
+   |                      ~~~~~~~~~~~
 
 error[E0631]: type mismatch in closure arguments
   --> $DIR/closure-arg-type-mismatch.rs:4:14

src/test/ui/mismatched_types/issue-36053-2.stderr

@@ -13,6 +13,10 @@ note: required by a bound in `filter`
    |
 LL |         P: FnMut(&Self::Item) -> bool,
    |            ^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `filter`
+help: consider borrowing the argument
+   |
+LL |     once::<&str>("str").fuse().filter(|a: &&str| true).count();
+   |                                           ~~~~~
 
 error[E0599]: the method `count` exists for struct `Filter<Fuse<std::iter::Once<&str>>, [closure@$DIR/issue-36053-2.rs:7:39: 7:48]>`, but its trait bounds were not satisfied
   --> $DIR/issue-36053-2.rs:7:55