Auto merge of #12865 - J-ZhengLi:issue12853, r=y21

fix [`redundant_closure`] suggesting incorrect code with `F: Fn()`

fixes: #12853

---

changelog: fix [`redundant_closure`] suggesting incorrect code with `F: Fn()`
This commit is contained in:
bors 2024-05-31 15:23:39 +00:00
commit 0b598b636b
4 changed files with 54 additions and 9 deletions

View file

@ -123,7 +123,8 @@ impl<'tcx> LateLintPass<'tcx> for EtaReduction {
ExprKind::Path(QPath::Resolved(..) | QPath::TypeRelative(..))
) =>
{
let callee_ty = typeck.expr_ty(callee).peel_refs();
let callee_ty_raw = typeck.expr_ty(callee);
let callee_ty = callee_ty_raw.peel_refs();
if matches!(type_diagnostic_name(cx, callee_ty), Some(sym::Arc | sym::Rc))
|| !check_inputs(typeck, body.params, None, args)
{
@ -170,15 +171,25 @@ impl<'tcx> LateLintPass<'tcx> for EtaReduction {
{
span_lint_and_then(cx, REDUNDANT_CLOSURE, expr.span, "redundant closure", |diag| {
if let Some(mut snippet) = snippet_opt(cx, callee.span) {
if let Ok((ClosureKind::FnMut, _)) = cx.tcx.infer_ctxt().build().type_implements_fn_trait(
cx.param_env,
Binder::bind_with_vars(callee_ty_adjusted, List::empty()),
ty::PredicatePolarity::Positive,
) && path_to_local(callee).map_or(false, |l| {
if path_to_local(callee).map_or(false, |l| {
// FIXME: Do we really need this `local_used_in` check?
// Isn't it checking something like... `callee(callee)`?
// If somehow this check is needed, add some test for it,
// 'cuz currently nothing changes after deleting this check.
local_used_in(cx, l, args) || local_used_after_expr(cx, l, expr)
}) {
// Mutable closure is used after current expr; we cannot consume it.
snippet = format!("&mut {snippet}");
match cx.tcx.infer_ctxt().build().type_implements_fn_trait(
cx.param_env,
Binder::bind_with_vars(callee_ty_adjusted, List::empty()),
ty::PredicatePolarity::Positive,
) {
// Mutable closure is used after current expr; we cannot consume it.
Ok((ClosureKind::FnMut, _)) => snippet = format!("&mut {snippet}"),
Ok((ClosureKind::Fn, _)) if !callee_ty_raw.is_ref() => {
snippet = format!("&{snippet}");
},
_ => (),
}
}
diag.span_suggestion(
expr.span,

View file

@ -471,3 +471,14 @@ mod issue_10854 {
}
}
}
mod issue_12853 {
fn f_by_value<F: Fn(u32)>(f: F) {
let x = Box::new(|| None.map(&f));
x();
}
fn f_by_ref<F: Fn(u32)>(f: &F) {
let x = Box::new(|| None.map(f));
x();
}
}

View file

@ -471,3 +471,14 @@ mod issue_10854 {
}
}
}
mod issue_12853 {
fn f_by_value<F: Fn(u32)>(f: F) {
let x = Box::new(|| None.map(|x| f(x)));
x();
}
fn f_by_ref<F: Fn(u32)>(f: &F) {
let x = Box::new(|| None.map(|x| f(x)));
x();
}
}

View file

@ -190,5 +190,17 @@ error: redundant closure
LL | test.map(|t| t.method())
| ^^^^^^^^^^^^^^ help: replace the closure with the method itself: `crate::issue_10854::d::Test::method`
error: aborting due to 31 previous errors
error: redundant closure
--> tests/ui/eta.rs:477:38
|
LL | let x = Box::new(|| None.map(|x| f(x)));
| ^^^^^^^^ help: replace the closure with the function itself: `&f`
error: redundant closure
--> tests/ui/eta.rs:481:38
|
LL | let x = Box::new(|| None.map(|x| f(x)));
| ^^^^^^^^ help: replace the closure with the function itself: `f`
error: aborting due to 33 previous errors