Skip to content

Commit dfe32b6

Browse files
estebankestebank
committed
On Fn arg mismatch for a fn path, suggest a closure
When encountering a fn call that has a path to another fn being passed in, where an `Fn` impl is expected, and the arguments differ, suggest wrapping the argument with a closure with the appropriate arguments.
1 parent abe34e9 commit dfe32b6

15 files changed

+247
-54
lines changed

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

Lines changed: 167 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,15 @@ pub trait TypeErrCtxtExt<'tcx> {
222222
param_env: ty::ParamEnv<'tcx>,
223223
) -> DiagnosticBuilder<'tcx, ErrorGuaranteed>;
224224

225+
fn note_conflicting_fn_args(
226+
&self,
227+
err: &mut Diagnostic,
228+
cause: &ObligationCauseCode<'tcx>,
229+
expected: Ty<'tcx>,
230+
found: Ty<'tcx>,
231+
param_env: ty::ParamEnv<'tcx>,
232+
);
233+
225234
fn note_conflicting_closure_bounds(
226235
&self,
227236
cause: &ObligationCauseCode<'tcx>,
@@ -2005,6 +2014,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
20052014
let signature_kind = format!("{argument_kind} signature");
20062015
err.note_expected_found(&signature_kind, expected_str, &signature_kind, found_str);
20072016

2017+
self.note_conflicting_fn_args(&mut err, cause, expected, found, param_env);
20082018
self.note_conflicting_closure_bounds(cause, &mut err);
20092019

20102020
if let Some(found_node) = found_node {
@@ -2014,6 +2024,152 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
20142024
err
20152025
}
20162026

2027+
fn note_conflicting_fn_args(
2028+
&self,
2029+
err: &mut Diagnostic,
2030+
cause: &ObligationCauseCode<'tcx>,
2031+
expected: Ty<'tcx>,
2032+
found: Ty<'tcx>,
2033+
param_env: ty::ParamEnv<'tcx>,
2034+
) {
2035+
let ObligationCauseCode::FunctionArgumentObligation { arg_hir_id, .. } = cause else {
2036+
return;
2037+
};
2038+
let ty::FnPtr(expected) = expected.kind() else {
2039+
return;
2040+
};
2041+
let ty::FnPtr(found) = found.kind() else {
2042+
return;
2043+
};
2044+
let Some(Node::Expr(arg)) = self.tcx.hir().find(*arg_hir_id) else {
2045+
return;
2046+
};
2047+
let hir::ExprKind::Path(path) = arg.kind else {
2048+
return;
2049+
};
2050+
let expected_inputs = self.tcx.erase_late_bound_regions(*expected).inputs();
2051+
let found_inputs = self.tcx.erase_late_bound_regions(*found).inputs();
2052+
let both_tys = expected_inputs.iter().cloned().zip(found_inputs.iter().cloned());
2053+
2054+
let arg_expr = |infcx: &InferCtxt<'tcx>, name, expected: Ty<'tcx>, found: Ty<'tcx>| {
2055+
let (expected_ty, expected_refs) = get_deref_type_and_refs(expected);
2056+
let (found_ty, found_refs) = get_deref_type_and_refs(found);
2057+
2058+
if infcx.can_eq(param_env, found_ty, expected_ty) {
2059+
if found_refs.len() == expected_refs.len()
2060+
&& found_refs.iter().zip(expected_refs.iter()).all(|(e, f)| e == f)
2061+
{
2062+
name
2063+
} else if found_refs.len() > expected_refs.len() {
2064+
if found_refs[..found_refs.len() - expected_refs.len()]
2065+
.iter()
2066+
.zip(expected_refs.iter())
2067+
.any(|(e, f)| e != f)
2068+
{
2069+
// The refs have different mutability.
2070+
format!(
2071+
"{}*{name}",
2072+
found_refs[..found_refs.len() - expected_refs.len()]
2073+
.iter()
2074+
.map(|mutbl| format!("&{}", mutbl.prefix_str()))
2075+
.collect::<Vec<_>>()
2076+
.join(""),
2077+
)
2078+
} else {
2079+
format!(
2080+
"{}{name}",
2081+
found_refs[..found_refs.len() - expected_refs.len()]
2082+
.iter()
2083+
.map(|mutbl| format!("&{}", mutbl.prefix_str()))
2084+
.collect::<Vec<_>>()
2085+
.join(""),
2086+
)
2087+
}
2088+
} else if expected_refs.len() > found_refs.len() {
2089+
format!(
2090+
"{}{name}",
2091+
(0..(expected_refs.len() - found_refs.len()))
2092+
.map(|_| "*")
2093+
.collect::<Vec<_>>()
2094+
.join(""),
2095+
)
2096+
} else {
2097+
format!(
2098+
"{}{name}",
2099+
found_refs
2100+
.iter()
2101+
.map(|mutbl| format!("&{}", mutbl.prefix_str()))
2102+
.chain(found_refs.iter().map(|_| "*".to_string()))
2103+
.collect::<Vec<_>>()
2104+
.join(""),
2105+
)
2106+
}
2107+
} else {
2108+
format!("/* {found} */")
2109+
}
2110+
};
2111+
let (closure_names, call_names): (Vec<_>, Vec<_>) =
2112+
if both_tys.clone().all(|(expected, found)| {
2113+
let (expected_ty, _) = get_deref_type_and_refs(expected);
2114+
let (found_ty, _) = get_deref_type_and_refs(found);
2115+
self.can_eq(param_env, found_ty, expected_ty)
2116+
}) && !expected_inputs.is_empty()
2117+
&& expected_inputs.len() == found_inputs.len()
2118+
&& let hir::QPath::Resolved(_, path) = path
2119+
&& let hir::def::Res::Def(_, fn_def_id) = path.res
2120+
&& let Some(node) = self.tcx.hir().get_if_local(fn_def_id)
2121+
&& let Some(body_id) = node.body_id()
2122+
{
2123+
let closure = self
2124+
.tcx
2125+
.hir()
2126+
.body_param_names(body_id)
2127+
.map(|name| format!("{name}"))
2128+
.collect();
2129+
let args = self
2130+
.tcx
2131+
.hir()
2132+
.body_param_names(body_id)
2133+
.zip(both_tys)
2134+
.map(|(name, (expected, found))| {
2135+
arg_expr(self.infcx, format!("{name}"), expected, found)
2136+
})
2137+
.collect();
2138+
(closure, args)
2139+
} else {
2140+
let closure_args = expected_inputs
2141+
.iter()
2142+
.enumerate()
2143+
.map(|(i, _)| format!("arg{i}"))
2144+
.collect::<Vec<_>>();
2145+
let call_args = both_tys
2146+
.enumerate()
2147+
.map(|(i, (expected, found))| {
2148+
arg_expr(self.infcx, format!("arg{i}"), expected, found)
2149+
})
2150+
.collect::<Vec<_>>();
2151+
(closure_args, call_args)
2152+
};
2153+
let closure_names: Vec<_> = closure_names
2154+
.into_iter()
2155+
.zip(expected_inputs.iter())
2156+
.map(|(name, ty)| {
2157+
format!(
2158+
"{name}{}",
2159+
if ty.has_infer_types() { String::new() } else { format!(": {ty}") }
2160+
)
2161+
})
2162+
.collect();
2163+
err.multipart_suggestion(
2164+
format!("consider wrapping the function in a closure"),
2165+
vec![
2166+
(arg.span.shrink_to_lo(), format!("|{}| ", closure_names.join(", "))),
2167+
(arg.span.shrink_to_hi(), format!("({})", call_names.join(", "))),
2168+
],
2169+
Applicability::MaybeIncorrect,
2170+
);
2171+
}
2172+
20172173
// Add a note if there are two `Fn`-family bounds that have conflicting argument
20182174
// requirements, which will always cause a closure to have a type error.
20192175
fn note_conflicting_closure_bounds(
@@ -4349,17 +4505,6 @@ fn hint_missing_borrow<'tcx>(
43494505

43504506
let args = fn_decl.inputs.iter();
43514507

4352-
fn get_deref_type_and_refs(mut ty: Ty<'_>) -> (Ty<'_>, Vec<hir::Mutability>) {
4353-
let mut refs = vec![];
4354-
4355-
while let ty::Ref(_, new_ty, mutbl) = ty.kind() {
4356-
ty = *new_ty;
4357-
refs.push(*mutbl);
4358-
}
4359-
4360-
(ty, refs)
4361-
}
4362-
43634508
let mut to_borrow = Vec::new();
43644509
let mut remove_borrow = Vec::new();
43654510

@@ -4652,3 +4797,14 @@ pub fn suggest_desugaring_async_fn_to_impl_future_in_trait<'tcx>(
46524797

46534798
Some(sugg)
46544799
}
4800+
4801+
fn get_deref_type_and_refs(mut ty: Ty<'_>) -> (Ty<'_>, Vec<hir::Mutability>) {
4802+
let mut refs = vec![];
4803+
4804+
while let ty::Ref(_, new_ty, mutbl) = ty.kind() {
4805+
ty = *new_ty;
4806+
refs.push(*mutbl);
4807+
}
4808+
4809+
(ty, refs)
4810+
}

‎tests/ui/generic-associated-types/bugs/issue-88382.stderr‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@ note: required by a bound in `do_something`
1616
|
1717
LL | fn do_something<I: Iterable>(i: I, mut f: impl for<'a> Fn(&mut I::Iterator<'a>)) {
1818
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `do_something`
19+
help: consider wrapping the function in a closure
20+
|
21+
LL | do_something(SomeImplementation(), |arg0: &mut std::iter::Empty<usize>| test(/* &mut <_ as Iterable>::Iterator<'_> */));
22+
| ++++++++++++++++++++++++++++++++++++ ++++++++++++++++++++++++++++++++++++++++++
1923

2024
error: aborting due to 1 previous error
2125

‎tests/ui/intrinsics/const-eval-select-bad.stderr‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,10 @@ LL | const_eval_select((true,), foo, baz);
8686
found function signature `fn(i32) -> _`
8787
note: required by a bound in `const_eval_select`
8888
--> $SRC_DIR/core/src/intrinsics.rs:LL:COL
89+
help: consider wrapping the function in a closure
90+
|
91+
LL | const_eval_select((true,), |arg0: bool| foo(/* i32 */), baz);
92+
| ++++++++++++ +++++++++++
8993

9094
error: this argument must be a `const fn`
9195
--> $DIR/const-eval-select-bad.rs:42:29

‎tests/ui/mismatched_types/E0631.stderr‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,10 @@ note: required by a bound in `foo`
4848
|
4949
LL | fn foo<F: Fn(usize)>(_: F) {}
5050
| ^^^^^^^^^ required by this bound in `foo`
51+
help: consider wrapping the function in a closure
52+
|
53+
LL | foo(|arg0: usize| f(/* u64 */));
54+
| +++++++++++++ +++++++++++
5155

5256
error[E0631]: type mismatch in function arguments
5357
--> $DIR/E0631.rs:10:9
@@ -67,6 +71,10 @@ note: required by a bound in `bar`
6771
|
6872
LL | fn bar<F: Fn<(usize,)>>(_: F) {}
6973
| ^^^^^^^^^^^^ required by this bound in `bar`
74+
help: consider wrapping the function in a closure
75+
|
76+
LL | bar(|arg0: usize| f(/* u64 */));
77+
| +++++++++++++ +++++++++++
7078

7179
error: aborting due to 4 previous errors
7280

‎tests/ui/mismatched_types/closure-ref-114180.stderr‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@ LL | v.sort_by(compare);
1212
found closure signature `fn((_,), (_,)) -> _`
1313
note: required by a bound in `slice::<impl [T]>::sort_by`
1414
--> $SRC_DIR/alloc/src/slice.rs:LL:COL
15+
help: consider wrapping the function in a closure
16+
|
17+
LL | v.sort_by(|arg0, arg1| compare(*arg0, *arg1));
18+
| ++++++++++++ ++++++++++++++
1519
help: consider adjusting the signature so it borrows its arguments
1620
|
1721
LL | let compare = |&(a,), &(e,)| todo!();

‎tests/ui/mismatched_types/fn-variance-1.stderr‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@ note: required by a bound in `apply`
1616
|
1717
LL | fn apply<T, F>(t: T, f: F) where F: FnOnce(T) {
1818
| ^^^^^^^^^ required by this bound in `apply`
19+
help: consider wrapping the function in a closure
20+
|
21+
LL | apply(&3, |x| takes_mut(&mut *x));
22+
| +++ +++++++++
1923

2024
error[E0631]: type mismatch in function arguments
2125
--> $DIR/fn-variance-1.rs:15:19
@@ -35,6 +39,10 @@ note: required by a bound in `apply`
3539
|
3640
LL | fn apply<T, F>(t: T, f: F) where F: FnOnce(T) {
3741
| ^^^^^^^^^ required by this bound in `apply`
42+
help: consider wrapping the function in a closure
43+
|
44+
LL | apply(&mut 3, |x| takes_imm(&*x));
45+
| +++ +++++
3846

3947
error: aborting due to 2 previous errors
4048

‎tests/ui/mismatched_types/suggest-option-asderef-inference-var.stderr‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,10 @@ LL | let _has_inference_vars: Option<i32> = Some(0).map(deref_int);
1313
found function signature `for<'a> fn(&'a i32) -> _`
1414
note: required by a bound in `Option::<T>::map`
1515
--> $SRC_DIR/core/src/option.rs:LL:COL
16+
help: consider wrapping the function in a closure
17+
|
18+
LL | let _has_inference_vars: Option<i32> = Some(0).map(|a| deref_int(&a));
19+
| +++ ++++
1620
help: consider adjusting the signature so it does not borrow its argument
1721
|
1822
LL - fn deref_int(a: &i32) -> i32 {

‎tests/ui/mismatched_types/suggest-option-asderef-unfixable.stderr‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,10 @@ LL | let _ = produces_string().and_then(takes_str_but_too_many_refs);
1313
found function signature `for<'a, 'b> fn(&'a &'b str) -> _`
1414
note: required by a bound in `Option::<T>::and_then`
1515
--> $SRC_DIR/core/src/option.rs:LL:COL
16+
help: consider wrapping the function in a closure
17+
|
18+
LL | let _ = produces_string().and_then(|arg0: String| takes_str_but_too_many_refs(/* &&str */));
19+
| ++++++++++++++ +++++++++++++
1620

1721
error[E0277]: expected a `FnOnce(String)` closure, found `for<'a> extern "C" fn(&'a str) -> Option<()> {takes_str_but_wrong_abi}`
1822
--> $DIR/suggest-option-asderef-unfixable.rs:26:40
@@ -68,6 +72,10 @@ LL | let _ = Some(TypeWithoutDeref).and_then(takes_str_but_too_many_refs);
6872
found function signature `for<'a, 'b> fn(&'a &'b str) -> _`
6973
note: required by a bound in `Option::<T>::and_then`
7074
--> $SRC_DIR/core/src/option.rs:LL:COL
75+
help: consider wrapping the function in a closure
76+
|
77+
LL | let _ = Some(TypeWithoutDeref).and_then(|arg0: TypeWithoutDeref| takes_str_but_too_many_refs(/* &&str */));
78+
| ++++++++++++++++++++++++ +++++++++++++
7179

7280
error: aborting due to 5 previous errors
7381

‎tests/ui/mismatched_types/suggest-option-asderef.fixed‎

Lines changed: 0 additions & 39 deletions
This file was deleted.

‎tests/ui/mismatched_types/suggest-option-asderef.rs‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// run-rustfix
1+
// this isn't auto-fixable now because we produce two similar suggestions
22

33
fn produces_string() -> Option<String> {
44
Some("my cool string".to_owned())
@@ -25,15 +25,19 @@ fn main() {
2525
let _: Option<()> = produces_string().and_then(takes_str);
2626
//~^ ERROR type mismatch in function arguments
2727
//~| HELP call `Option::as_deref()` first
28+
//~| HELP consider wrapping the function in a closure
2829
let _: Option<Option<()>> = produces_string().map(takes_str);
2930
//~^ ERROR type mismatch in function arguments
3031
//~| HELP call `Option::as_deref()` first
32+
//~| HELP consider wrapping the function in a closure
3133
let _: Option<Option<()>> = produces_string().map(takes_str_mut);
3234
//~^ ERROR type mismatch in function arguments
3335
//~| HELP call `Option::as_deref_mut()` first
36+
//~| HELP consider wrapping the function in a closure
3437
let _ = produces_string().and_then(generic);
3538

3639
let _ = produces_string().and_then(generic_ref);
3740
//~^ ERROR type mismatch in function arguments
3841
//~| HELP call `Option::as_deref()` first
42+
//~| HELP consider wrapping the function in a closure
3943
}

0 commit comments

Comments
 (0)