Skip to content

Commit e53182a

Browse files
tesujitesuji
committed
Factor out check_closure function
Also get the receiver T in `T.method(|| f())`.
1 parent 32dc02a commit e53182a

File tree

1 file changed

+160
-150
lines changed

1 file changed

+160
-150
lines changed

‎clippy_lints/src/eta_reduction.rs

Lines changed: 160 additions & 150 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ use rustc_hir::{BindingMode, Expr, ExprKind, FnRetTy, Param, PatKind, QPath, Saf
99
use rustc_infer::infer::TyCtxtInferExt;
1010
use rustc_lint::{LateContext, LateLintPass};
1111
use rustc_middle::ty::{
12-
self, Binder, ClosureArgs, ClosureKind, FnSig, GenericArg, GenericArgKind, List, Region, RegionKind, Ty, TyCtxt,
13-
TypeVisitableExt, TypeckResults,
12+
self, Binder, ClosureKind, FnSig, GenericArg, GenericArgKind, List, Region, RegionKind, Ty, TypeVisitableExt,
13+
TypeckResults,
1414
};
1515
use rustc_session::declare_lint_pass;
1616
use rustc_span::symbol::sym;
@@ -74,159 +74,173 @@ declare_clippy_lint! {
7474
declare_lint_pass!(EtaReduction => [REDUNDANT_CLOSURE, REDUNDANT_CLOSURE_FOR_METHOD_CALLS]);
7575

7676
impl<'tcx> LateLintPass<'tcx> for EtaReduction {
77-
#[allow(clippy::too_many_lines)]
78-
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
79-
let body = if let ExprKind::Closure(c) = expr.kind
80-
&& c.fn_decl.inputs.iter().all(|ty| matches!(ty.kind, TyKind::Infer))
81-
&& matches!(c.fn_decl.output, FnRetTy::DefaultReturn(_))
82-
&& !expr.span.from_expansion()
83-
{
84-
cx.tcx.hir().body(c.body)
85-
} else {
86-
return;
87-
};
88-
89-
if body.value.span.from_expansion() {
90-
if body.params.is_empty() {
91-
if let Some(VecArgs::Vec(&[])) = VecArgs::hir(cx, body.value) {
92-
// replace `|| vec![]` with `Vec::new`
93-
span_lint_and_sugg(
94-
cx,
95-
REDUNDANT_CLOSURE,
96-
expr.span,
97-
"redundant closure",
98-
"replace the closure with `Vec::new`",
99-
"std::vec::Vec::new".into(),
100-
Applicability::MachineApplicable,
101-
);
102-
}
77+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) {
78+
if let ExprKind::MethodCall(_method, receiver, args, _) = expr.kind {
79+
for arg in args {
80+
check_clousure(cx, Some(receiver), arg);
81+
}
82+
}
83+
if let ExprKind::Call(func, args) = expr.kind {
84+
check_clousure(cx, None, func);
85+
for arg in args {
86+
check_clousure(cx, None, arg);
10387
}
104-
// skip `foo(|| macro!())`
105-
return;
10688
}
89+
}
90+
}
10791

108-
let typeck = cx.typeck_results();
109-
let closure = if let ty::Closure(_, closure_subs) = typeck.expr_ty(expr).kind() {
110-
closure_subs.as_closure()
111-
} else {
112-
return;
113-
};
92+
#[allow(clippy::too_many_lines)]
93+
fn check_clousure<'tcx>(cx: &LateContext<'tcx>, outer_receiver: Option<&Expr<'tcx>>, expr: &Expr<'tcx>) {
94+
let body = if let ExprKind::Closure(c) = expr.kind
95+
&& c.fn_decl.inputs.iter().all(|ty| matches!(ty.kind, TyKind::Infer))
96+
&& matches!(c.fn_decl.output, FnRetTy::DefaultReturn(_))
97+
&& !expr.span.from_expansion()
98+
{
99+
cx.tcx.hir().body(c.body)
100+
} else {
101+
return;
102+
};
114103

115-
if is_adjusted(cx, body.value) {
116-
return;
104+
if body.value.span.from_expansion() {
105+
if body.params.is_empty() {
106+
if let Some(VecArgs::Vec(&[])) = VecArgs::hir(cx, body.value) {
107+
// replace `|| vec![]` with `Vec::new`
108+
span_lint_and_sugg(
109+
cx,
110+
REDUNDANT_CLOSURE,
111+
expr.span,
112+
"redundant closure",
113+
"replace the closure with `Vec::new`",
114+
"std::vec::Vec::new".into(),
115+
Applicability::MachineApplicable,
116+
);
117+
}
117118
}
119+
// skip `foo(|| macro!())`
120+
return;
121+
}
118122

119-
match body.value.kind {
120-
ExprKind::Call(callee, args)
121-
if matches!(
122-
callee.kind,
123-
ExprKind::Path(QPath::Resolved(..) | QPath::TypeRelative(..))
124-
) =>
123+
if is_adjusted(cx, body.value) {
124+
return;
125+
}
126+
127+
let typeck = cx.typeck_results();
128+
let closure = if let ty::Closure(_, closure_subs) = typeck.expr_ty(expr).kind() {
129+
closure_subs.as_closure()
130+
} else {
131+
return;
132+
};
133+
let closure_sig = cx.tcx.signature_unclosure(closure.sig(), Safety::Safe).skip_binder();
134+
match body.value.kind {
135+
ExprKind::Call(callee, args)
136+
if matches!(
137+
callee.kind,
138+
ExprKind::Path(QPath::Resolved(..) | QPath::TypeRelative(..))
139+
) =>
140+
{
141+
let callee_ty_raw = typeck.expr_ty(callee);
142+
let callee_ty = callee_ty_raw.peel_refs();
143+
if matches!(type_diagnostic_name(cx, callee_ty), Some(sym::Arc | sym::Rc))
144+
|| !check_inputs(typeck, body.params, None, args)
125145
{
126-
let callee_ty_raw = typeck.expr_ty(callee);
127-
let callee_ty = callee_ty_raw.peel_refs();
128-
if matches!(type_diagnostic_name(cx, callee_ty), Some(sym::Arc | sym::Rc))
129-
|| !check_inputs(typeck, body.params, None, args)
130-
{
131-
return;
132-
}
133-
let callee_ty_adjusted = typeck
134-
.expr_adjustments(callee)
135-
.last()
136-
.map_or(callee_ty, |a| a.target.peel_refs());
146+
return;
147+
}
148+
let callee_ty_adjusted = typeck
149+
.expr_adjustments(callee)
150+
.last()
151+
.map_or(callee_ty, |a| a.target.peel_refs());
137152

138-
let sig = match callee_ty_adjusted.kind() {
139-
ty::FnDef(def, _) => {
140-
// Rewriting `x(|| f())` to `x(f)` where f is marked `#[track_caller]` moves the `Location`
141-
if cx.tcx.has_attr(*def, sym::track_caller) {
142-
return;
143-
}
153+
let sig = match callee_ty_adjusted.kind() {
154+
ty::FnDef(def, _) => {
155+
// Rewriting `x(|| f())` to `x(f)` where f is marked `#[track_caller]` moves the `Location`
156+
if cx.tcx.has_attr(*def, sym::track_caller) {
157+
return;
158+
}
144159

145-
cx.tcx.fn_sig(def).skip_binder().skip_binder()
146-
},
147-
ty::FnPtr(sig) => sig.skip_binder(),
148-
ty::Closure(_, subs) => cx
149-
.tcx
150-
.signature_unclosure(subs.as_closure().sig(), Safety::Safe)
151-
.skip_binder(),
152-
_ => {
153-
if typeck.type_dependent_def_id(body.value.hir_id).is_some()
154-
&& let subs = typeck.node_args(body.value.hir_id)
155-
&& let output = typeck.expr_ty(body.value)
156-
&& let ty::Tuple(tys) = *subs.type_at(1).kind()
157-
{
158-
cx.tcx.mk_fn_sig(tys, output, false, Safety::Safe, Abi::Rust)
159-
} else {
160-
return;
161-
}
162-
},
163-
};
164-
if check_sig(cx, closure, sig)
165-
&& let generic_args = typeck.node_args(callee.hir_id)
166-
// Given some trait fn `fn f() -> ()` and some type `T: Trait`, `T::f` is not
167-
// `'static` unless `T: 'static`. The cast `T::f as fn()` will, however, result
168-
// in a type which is `'static`.
169-
// For now ignore all callee types which reference a type parameter.
170-
&& !generic_args.types().any(|t| matches!(t.kind(), ty::Param(_)))
171-
{
172-
span_lint_and_then(cx, REDUNDANT_CLOSURE, expr.span, "redundant closure", |diag| {
173-
if let Some(mut snippet) = snippet_opt(cx, callee.span) {
174-
if path_to_local(callee).map_or(false, |l| {
175-
// FIXME: Do we really need this `local_used_in` check?
176-
// Isn't it checking something like... `callee(callee)`?
177-
// If somehow this check is needed, add some test for it,
178-
// 'cuz currently nothing changes after deleting this check.
179-
local_used_in(cx, l, args) || local_used_after_expr(cx, l, expr)
180-
}) {
181-
match cx.tcx.infer_ctxt().build().err_ctxt().type_implements_fn_trait(
182-
cx.param_env,
183-
Binder::bind_with_vars(callee_ty_adjusted, List::empty()),
184-
ty::PredicatePolarity::Positive,
185-
) {
186-
// Mutable closure is used after current expr; we cannot consume it.
187-
Ok((ClosureKind::FnMut, _)) => snippet = format!("&mut {snippet}"),
188-
Ok((ClosureKind::Fn, _)) if !callee_ty_raw.is_ref() => {
189-
snippet = format!("&{snippet}");
190-
},
191-
_ => (),
192-
}
160+
cx.tcx.fn_sig(def).skip_binder().skip_binder()
161+
},
162+
ty::FnPtr(sig) => sig.skip_binder(),
163+
ty::Closure(_, subs) => cx
164+
.tcx
165+
.signature_unclosure(subs.as_closure().sig(), Safety::Safe)
166+
.skip_binder(),
167+
_ => {
168+
if typeck.type_dependent_def_id(body.value.hir_id).is_some()
169+
&& let subs = typeck.node_args(body.value.hir_id)
170+
&& let output = typeck.expr_ty(body.value)
171+
&& let ty::Tuple(tys) = *subs.type_at(1).kind()
172+
{
173+
cx.tcx.mk_fn_sig(tys, output, false, Safety::Safe, Abi::Rust)
174+
} else {
175+
return;
176+
}
177+
},
178+
};
179+
if check_sig(closure_sig, sig)
180+
&& let generic_args = typeck.node_args(callee.hir_id)
181+
// Given some trait fn `fn f() -> ()` and some type `T: Trait`, `T::f` is not
182+
// `'static` unless `T: 'static`. The cast `T::f as fn()` will, however, result
183+
// in a type which is `'static`.
184+
// For now ignore all callee types which reference a type parameter.
185+
&& !generic_args.types().any(|t| matches!(t.kind(), ty::Param(_)))
186+
{
187+
span_lint_and_then(cx, REDUNDANT_CLOSURE, expr.span, "redundant closure", |diag| {
188+
if let Some(mut snippet) = snippet_opt(cx, callee.span) {
189+
if path_to_local(callee).map_or(false, |l| {
190+
// FIXME: Do we really need this `local_used_in` check?
191+
// Isn't it checking something like... `callee(callee)`?
192+
// If somehow this check is needed, add some test for it,
193+
// 'cuz currently nothing changes after deleting this check.
194+
local_used_in(cx, l, args) || local_used_after_expr(cx, l, expr)
195+
}) {
196+
match cx.tcx.infer_ctxt().build().err_ctxt().type_implements_fn_trait(
197+
cx.param_env,
198+
Binder::bind_with_vars(callee_ty_adjusted, List::empty()),
199+
ty::PredicatePolarity::Positive,
200+
) {
201+
// Mutable closure is used after current expr; we cannot consume it.
202+
Ok((ClosureKind::FnMut, _)) => snippet = format!("&mut {snippet}"),
203+
Ok((ClosureKind::Fn, _)) if !callee_ty_raw.is_ref() => {
204+
snippet = format!("&{snippet}");
205+
},
206+
_ => (),
193207
}
194-
diag.span_suggestion(
195-
expr.span,
196-
"replace the closure with the function itself",
197-
snippet,
198-
Applicability::MachineApplicable,
199-
);
200208
}
201-
});
202-
}
203-
},
204-
ExprKind::MethodCall(path, self_, args, _) if check_inputs(typeck, body.params, Some(self_), args) => {
205-
if let Some(method_def_id) = typeck.type_dependent_def_id(body.value.hir_id)
206-
&& !cx.tcx.has_attr(method_def_id, sym::track_caller)
207-
&& check_sig(cx, closure, cx.tcx.fn_sig(method_def_id).skip_binder().skip_binder())
208-
{
209-
span_lint_and_then(
210-
cx,
211-
REDUNDANT_CLOSURE_FOR_METHOD_CALLS,
212-
expr.span,
213-
"redundant closure",
214-
|diag| {
215-
let args = typeck.node_args(body.value.hir_id);
216-
let caller = self_.hir_id.owner.def_id;
217-
let type_name = get_path_from_caller_to_method_type(cx.tcx, caller, method_def_id, args);
218-
diag.span_suggestion(
219-
expr.span,
220-
"replace the closure with the method itself",
221-
format!("{}::{}", type_name, path.ident.name),
222-
Applicability::MachineApplicable,
223-
);
224-
},
225-
);
226-
}
227-
},
228-
_ => (),
229-
}
209+
diag.span_suggestion(
210+
expr.span,
211+
"replace the closure with the function itself",
212+
snippet,
213+
Applicability::MachineApplicable,
214+
);
215+
}
216+
});
217+
}
218+
},
219+
ExprKind::MethodCall(path, self_, args, _) if check_inputs(typeck, body.params, Some(self_), args) => {
220+
if let Some(method_def_id) = typeck.type_dependent_def_id(body.value.hir_id)
221+
&& !cx.tcx.has_attr(method_def_id, sym::track_caller)
222+
&& check_sig(closure_sig, cx.tcx.fn_sig(method_def_id).skip_binder().skip_binder())
223+
{
224+
span_lint_and_then(
225+
cx,
226+
REDUNDANT_CLOSURE_FOR_METHOD_CALLS,
227+
expr.span,
228+
"redundant closure",
229+
|diag| {
230+
let args = typeck.node_args(body.value.hir_id);
231+
let caller = self_.hir_id.owner.def_id;
232+
let type_name = get_path_from_caller_to_method_type(cx.tcx, caller, method_def_id, args);
233+
diag.span_suggestion(
234+
expr.span,
235+
"replace the closure with the method itself",
236+
format!("{}::{}", type_name, path.ident.name),
237+
Applicability::MachineApplicable,
238+
);
239+
},
240+
);
241+
}
242+
},
243+
_ => (),
230244
}
231245
}
232246

@@ -251,12 +265,8 @@ fn check_inputs(
251265
})
252266
}
253267

254-
fn check_sig<'tcx>(cx: &LateContext<'tcx>, closure: ClosureArgs<TyCtxt<'tcx>>, call_sig: FnSig<'_>) -> bool {
255-
call_sig.safety == Safety::Safe
256-
&& !has_late_bound_to_non_late_bound_regions(
257-
cx.tcx.signature_unclosure(closure.sig(), Safety::Safe).skip_binder(),
258-
call_sig,
259-
)
268+
fn check_sig<'tcx>(closure_sig: FnSig<'tcx>, call_sig: FnSig<'tcx>) -> bool {
269+
call_sig.safety == Safety::Safe && !has_late_bound_to_non_late_bound_regions(closure_sig, call_sig)
260270
}
261271

262272
/// This walks through both signatures and checks for any time a late-bound region is expected by an

0 commit comments

Comments
 (0)