Skip to content

Commit c0dd2d5

Browse files
committed
Emit lint about redundant closure on the closure node itself
1 parent bfc6ad0 commit c0dd2d5

File tree

3 files changed

+89
-39
lines changed

3 files changed

+89
-39
lines changed

clippy_lints/src/eta_reduction.rs

Lines changed: 53 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
1+
use clippy_utils::diagnostics::span_lint_hir_and_then;
22
use clippy_utils::higher::VecArgs;
33
use clippy_utils::source::{snippet_opt, snippet_with_applicability};
44
use clippy_utils::ty::get_type_diagnostic_name;
@@ -108,14 +108,20 @@ fn check_closure<'tcx>(cx: &LateContext<'tcx>, outer_receiver: Option<&Expr<'tcx
108108
{
109109
let vec_crate = if is_no_std_crate(cx) { "alloc" } else { "std" };
110110
// replace `|| vec![]` with `Vec::new`
111-
span_lint_and_sugg(
111+
span_lint_hir_and_then(
112112
cx,
113113
REDUNDANT_CLOSURE,
114+
expr.hir_id,
114115
expr.span,
115116
"redundant closure",
116-
"replace the closure with `Vec::new`",
117-
format!("{vec_crate}::vec::Vec::new"),
118-
Applicability::MachineApplicable,
117+
|diag| {
118+
diag.span_suggestion(
119+
expr.span,
120+
"replace the closure with `Vec::new`",
121+
format!("{vec_crate}::vec::Vec::new"),
122+
Applicability::MachineApplicable,
123+
);
124+
},
119125
);
120126
}
121127
// skip `foo(|| macro!())`
@@ -197,41 +203,48 @@ fn check_closure<'tcx>(cx: &LateContext<'tcx>, outer_receiver: Option<&Expr<'tcx
197203
// For now ignore all callee types which reference a type parameter.
198204
&& !generic_args.types().any(|t| matches!(t.kind(), ty::Param(_)))
199205
{
200-
span_lint_and_then(cx, REDUNDANT_CLOSURE, expr.span, "redundant closure", |diag| {
201-
if let Some(mut snippet) = snippet_opt(cx, callee.span) {
202-
if path_to_local(callee).is_some_and(|l| {
203-
// FIXME: Do we really need this `local_used_in` check?
204-
// Isn't it checking something like... `callee(callee)`?
205-
// If somehow this check is needed, add some test for it,
206-
// 'cuz currently nothing changes after deleting this check.
207-
local_used_in(cx, l, args) || local_used_after_expr(cx, l, expr)
208-
}) {
209-
match cx
210-
.tcx
211-
.infer_ctxt()
212-
.build(cx.typing_mode())
213-
.err_ctxt()
214-
.type_implements_fn_trait(
215-
cx.param_env,
216-
Binder::bind_with_vars(callee_ty_adjusted, List::empty()),
217-
ty::PredicatePolarity::Positive,
218-
) {
219-
// Mutable closure is used after current expr; we cannot consume it.
220-
Ok((ClosureKind::FnMut, _)) => snippet = format!("&mut {snippet}"),
221-
Ok((ClosureKind::Fn, _)) if !callee_ty_raw.is_ref() => {
222-
snippet = format!("&{snippet}");
223-
},
224-
_ => (),
206+
span_lint_hir_and_then(
207+
cx,
208+
REDUNDANT_CLOSURE,
209+
expr.hir_id,
210+
expr.span,
211+
"redundant closure",
212+
|diag| {
213+
if let Some(mut snippet) = snippet_opt(cx, callee.span) {
214+
if path_to_local(callee).is_some_and(|l| {
215+
// FIXME: Do we really need this `local_used_in` check?
216+
// Isn't it checking something like... `callee(callee)`?
217+
// If somehow this check is needed, add some test for it,
218+
// 'cuz currently nothing changes after deleting this check.
219+
local_used_in(cx, l, args) || local_used_after_expr(cx, l, expr)
220+
}) {
221+
match cx
222+
.tcx
223+
.infer_ctxt()
224+
.build(cx.typing_mode())
225+
.err_ctxt()
226+
.type_implements_fn_trait(
227+
cx.param_env,
228+
Binder::bind_with_vars(callee_ty_adjusted, List::empty()),
229+
ty::PredicatePolarity::Positive,
230+
) {
231+
// Mutable closure is used after current expr; we cannot consume it.
232+
Ok((ClosureKind::FnMut, _)) => snippet = format!("&mut {snippet}"),
233+
Ok((ClosureKind::Fn, _)) if !callee_ty_raw.is_ref() => {
234+
snippet = format!("&{snippet}");
235+
},
236+
_ => (),
237+
}
225238
}
239+
diag.span_suggestion(
240+
expr.span,
241+
"replace the closure with the function itself",
242+
snippet,
243+
Applicability::MachineApplicable,
244+
);
226245
}
227-
diag.span_suggestion(
228-
expr.span,
229-
"replace the closure with the function itself",
230-
snippet,
231-
Applicability::MachineApplicable,
232-
);
233-
}
234-
});
246+
},
247+
);
235248
}
236249
},
237250
ExprKind::MethodCall(path, self_, args, _) if check_inputs(typeck, body.params, Some(self_), args) => {
@@ -244,9 +257,10 @@ fn check_closure<'tcx>(cx: &LateContext<'tcx>, outer_receiver: Option<&Expr<'tcx
244257
Some(span) => format!("::{}", snippet_with_applicability(cx, span, "<..>", &mut app)),
245258
None => String::new(),
246259
};
247-
span_lint_and_then(
260+
span_lint_hir_and_then(
248261
cx,
249262
REDUNDANT_CLOSURE_FOR_METHOD_CALLS,
263+
expr.hir_id,
250264
expr.span,
251265
"redundant closure",
252266
|diag| {

tests/ui/eta.fixed

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -543,3 +543,21 @@ mod issue_13073 {
543543
//~^ redundant_closure
544544
}
545545
}
546+
547+
fn issue_14789() {
548+
_ = Some(1u8).map(
549+
#[expect(clippy::redundant_closure)]
550+
|a| foo(a),
551+
);
552+
553+
_ = Some("foo").map(
554+
#[expect(clippy::redundant_closure_for_method_calls)]
555+
|s| s.to_owned(),
556+
);
557+
558+
let _: Vec<u8> = None.map_or_else(
559+
#[expect(clippy::redundant_closure)]
560+
|| vec![],
561+
std::convert::identity,
562+
);
563+
}

tests/ui/eta.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -543,3 +543,21 @@ mod issue_13073 {
543543
//~^ redundant_closure
544544
}
545545
}
546+
547+
fn issue_14789() {
548+
_ = Some(1u8).map(
549+
#[expect(clippy::redundant_closure)]
550+
|a| foo(a),
551+
);
552+
553+
_ = Some("foo").map(
554+
#[expect(clippy::redundant_closure_for_method_calls)]
555+
|s| s.to_owned(),
556+
);
557+
558+
let _: Vec<u8> = None.map_or_else(
559+
#[expect(clippy::redundant_closure)]
560+
|| vec![],
561+
std::convert::identity,
562+
);
563+
}

0 commit comments

Comments
 (0)