Skip to content

Commit 766d373

Browse files
committed
Rework redundant_closure
* Better track when a early-bound region appears when a late-bound region is required * Don't lint when the closure gives explicit types.
1 parent 71cc39e commit 766d373

File tree

8 files changed

+368
-154
lines changed

8 files changed

+368
-154
lines changed

clippy_lints/src/eta_reduction.rs

Lines changed: 213 additions & 129 deletions
Large diffs are not rendered by default.

clippy_lints/src/incorrect_impls.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,6 @@ impl LateLintPass<'_> for IncorrectImpls {
189189
.diagnostic_items(trait_impl.def_id.krate)
190190
.name_to_id
191191
.get(&sym::Ord)
192-
&& trait_impl.self_ty() == trait_impl.args.type_at(1)
193192
&& implements_trait(cx, hir_ty_to_ty(cx.tcx, imp.self_ty), *ord_def_id, &[])
194193
{
195194
// If the `cmp` call likely needs to be fully qualified in the suggestion

clippy_utils/src/ty.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -419,6 +419,11 @@ pub fn is_type_lang_item(cx: &LateContext<'_>, ty: Ty<'_>, lang_item: hir::LangI
419419
}
420420
}
421421

422+
/// Gets the diagnostic name of the type, if it has one
423+
pub fn type_diagnostic_name(cx: &LateContext<'_>, ty: Ty<'_>) -> Option<Symbol> {
424+
ty.ty_adt_def().and_then(|adt| cx.tcx.get_diagnostic_name(adt.did()))
425+
}
426+
422427
/// Return `true` if the passed `typ` is `isize` or `usize`.
423428
pub fn is_isize_or_usize(typ: Ty<'_>) -> bool {
424429
matches!(typ.kind(), ty::Int(IntTy::Isize) | ty::Uint(UintTy::Usize))

clippy_utils/src/usage.rs

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
1-
use crate::visitors::{for_each_expr, for_each_expr_with_closures, Descend};
1+
use crate::{self as utils, get_enclosing_loop_or_multi_call_closure};
2+
use crate::visitors::{for_each_expr, for_each_expr_with_closures, Descend, Visitable};
23
use core::ops::ControlFlow;
34
use hir::def::Res;
45
use rustc_hir::intravisit::{self, Visitor};
5-
use rustc_hir::{Expr, ExprKind, HirId, HirIdSet, Node};
6+
use rustc_hir::{self as hir, Expr, ExprKind, HirId, HirIdSet};
67
use rustc_hir_typeck::expr_use_visitor::{Delegate, ExprUseVisitor, PlaceBase, PlaceWithHirId};
78
use rustc_infer::infer::TyCtxtInferExt;
89
use rustc_lint::LateContext;
910
use rustc_middle::hir::nested_filter;
1011
use rustc_middle::mir::FakeReadCause;
1112
use rustc_middle::ty;
12-
use {crate as utils, rustc_hir as hir};
1313

1414
/// Returns a set of mutated local variable IDs, or `None` if mutations could not be determined.
1515
pub fn mutated_variables<'tcx>(expr: &'tcx Expr<'_>, cx: &LateContext<'tcx>) -> Option<HirIdSet> {
@@ -154,6 +154,17 @@ pub fn contains_return_break_continue_macro(expression: &Expr<'_>) -> bool {
154154
.is_some()
155155
}
156156

157+
pub fn local_used_in<'tcx>(cx: &LateContext<'tcx>, local_id: HirId, v: impl Visitable<'tcx>) -> bool {
158+
for_each_expr_with_closures(cx, v, |e| {
159+
if utils::path_to_local_id(e, local_id) {
160+
ControlFlow::Break(())
161+
} else {
162+
ControlFlow::Continue(())
163+
}
164+
})
165+
.is_some()
166+
}
167+
157168
pub fn local_used_after_expr(cx: &LateContext<'_>, local_id: HirId, after: &Expr<'_>) -> bool {
158169
let Some(block) = utils::get_enclosing_block(cx, local_id) else {
159170
return false;
@@ -166,32 +177,21 @@ pub fn local_used_after_expr(cx: &LateContext<'_>, local_id: HirId, after: &Expr
166177
// let closure = || local;
167178
// closure();
168179
// closure();
169-
let in_loop_or_closure = cx
170-
.tcx
171-
.hir()
172-
.parent_iter(after.hir_id)
173-
.take_while(|&(id, _)| id != block.hir_id)
174-
.any(|(_, node)| {
175-
matches!(
176-
node,
177-
Node::Expr(Expr {
178-
kind: ExprKind::Loop(..) | ExprKind::Closure { .. },
179-
..
180-
})
181-
)
182-
});
183-
if in_loop_or_closure {
184-
return true;
185-
}
180+
let loop_start = get_enclosing_loop_or_multi_call_closure(cx, after).map(|e| e.hir_id);
186181

187182
let mut past_expr = false;
188183
for_each_expr_with_closures(cx, block, |e| {
189-
if e.hir_id == after.hir_id {
184+
if past_expr {
185+
if utils::path_to_local_id(e, local_id) {
186+
ControlFlow::Break(())
187+
} else {
188+
ControlFlow::Continue(Descend::Yes)
189+
}
190+
} else if e.hir_id == after.hir_id {
190191
past_expr = true;
191192
ControlFlow::Continue(Descend::No)
192-
} else if past_expr && utils::path_to_local_id(e, local_id) {
193-
ControlFlow::Break(())
194193
} else {
194+
past_expr = Some(e.hir_id) == loop_start;
195195
ControlFlow::Continue(Descend::Yes)
196196
}
197197
})

clippy_utils/src/visitors.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,16 @@ pub trait Visitable<'tcx> {
5252
/// Calls the corresponding `visit_*` function on the visitor.
5353
fn visit<V: Visitor<'tcx>>(self, visitor: &mut V);
5454
}
55+
impl<'tcx, T> Visitable<'tcx> for &'tcx [T]
56+
where
57+
&'tcx T: Visitable<'tcx>,
58+
{
59+
fn visit<V: Visitor<'tcx>>(self, visitor: &mut V) {
60+
for x in self {
61+
x.visit(visitor);
62+
}
63+
}
64+
}
5565
macro_rules! visitable_ref {
5666
($t:ident, $f:ident) => {
5767
impl<'tcx> Visitable<'tcx> for &'tcx $t<'tcx> {

tests/ui/eta.fixed

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -345,3 +345,58 @@ fn angle_brackets_and_args() {
345345
let dyn_opt: Option<&dyn TestTrait> = Some(&test_struct);
346346
dyn_opt.map(<dyn TestTrait>::method_on_dyn);
347347
}
348+
349+
fn _late_bound_to_early_bound_regions() {
350+
struct Foo<'a>(&'a u32);
351+
impl<'a> Foo<'a> {
352+
fn f(x: &'a u32) -> Self {
353+
Foo(x)
354+
}
355+
}
356+
fn f(f: impl for<'a> Fn(&'a u32) -> Foo<'a>) -> Foo<'static> {
357+
f(&0)
358+
}
359+
360+
let _ = f(|x| Foo::f(x));
361+
362+
struct Bar;
363+
impl<'a> From<&'a u32> for Bar {
364+
fn from(x: &'a u32) -> Bar {
365+
Bar
366+
}
367+
}
368+
fn f2(f: impl for<'a> Fn(&'a u32) -> Bar) -> Bar {
369+
f(&0)
370+
}
371+
372+
let _ = f2(|x| <Bar>::from(x));
373+
374+
struct Baz<'a>(&'a u32);
375+
fn f3(f: impl Fn(&u32) -> Baz<'_>) -> Baz<'static> {
376+
f(&0)
377+
}
378+
379+
let _ = f3(|x| Baz(x));
380+
}
381+
382+
fn _mixed_late_bound_and_early_bound_regions() {
383+
fn f<T>(t: T, f: impl Fn(T, &u32) -> u32) -> u32 {
384+
f(t, &0)
385+
}
386+
fn f2<'a, T: 'a>(_: &'a T, y: &u32) -> u32 {
387+
*y
388+
}
389+
let _ = f(&0, f2);
390+
}
391+
392+
fn _closure_with_types() {
393+
fn f<T>(x: T) -> T {
394+
x
395+
}
396+
fn f2<T: Default>(f: impl Fn(T) -> T) -> T {
397+
f(T::default())
398+
}
399+
400+
let _ = f2(|x: u32| f(x));
401+
let _ = f2(|x| -> u32 { f(x) });
402+
}

tests/ui/eta.rs

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -345,3 +345,58 @@ fn angle_brackets_and_args() {
345345
let dyn_opt: Option<&dyn TestTrait> = Some(&test_struct);
346346
dyn_opt.map(|d| d.method_on_dyn());
347347
}
348+
349+
fn _late_bound_to_early_bound_regions() {
350+
struct Foo<'a>(&'a u32);
351+
impl<'a> Foo<'a> {
352+
fn f(x: &'a u32) -> Self {
353+
Foo(x)
354+
}
355+
}
356+
fn f(f: impl for<'a> Fn(&'a u32) -> Foo<'a>) -> Foo<'static> {
357+
f(&0)
358+
}
359+
360+
let _ = f(|x| Foo::f(x));
361+
362+
struct Bar;
363+
impl<'a> From<&'a u32> for Bar {
364+
fn from(x: &'a u32) -> Bar {
365+
Bar
366+
}
367+
}
368+
fn f2(f: impl for<'a> Fn(&'a u32) -> Bar) -> Bar {
369+
f(&0)
370+
}
371+
372+
let _ = f2(|x| <Bar>::from(x));
373+
374+
struct Baz<'a>(&'a u32);
375+
fn f3(f: impl Fn(&u32) -> Baz<'_>) -> Baz<'static> {
376+
f(&0)
377+
}
378+
379+
let _ = f3(|x| Baz(x));
380+
}
381+
382+
fn _mixed_late_bound_and_early_bound_regions() {
383+
fn f<T>(t: T, f: impl Fn(T, &u32) -> u32) -> u32 {
384+
f(t, &0)
385+
}
386+
fn f2<'a, T: 'a>(_: &'a T, y: &u32) -> u32 {
387+
*y
388+
}
389+
let _ = f(&0, |x, y| f2(x, y));
390+
}
391+
392+
fn _closure_with_types() {
393+
fn f<T>(x: T) -> T {
394+
x
395+
}
396+
fn f2<T: Default>(f: impl Fn(T) -> T) -> T {
397+
f(T::default())
398+
}
399+
400+
let _ = f2(|x: u32| f(x));
401+
let _ = f2(|x| -> u32 { f(x) });
402+
}

tests/ui/eta.stderr

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,5 +158,11 @@ error: redundant closure
158158
LL | dyn_opt.map(|d| d.method_on_dyn());
159159
| ^^^^^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `<dyn TestTrait>::method_on_dyn`
160160

161-
error: aborting due to 26 previous errors
161+
error: redundant closure
162+
--> $DIR/eta.rs:389:19
163+
|
164+
LL | let _ = f(&0, |x, y| f2(x, y));
165+
| ^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `f2`
166+
167+
error: aborting due to 27 previous errors
162168

0 commit comments

Comments
 (0)