Skip to content

Commit abdc61f

Browse files
authored
filter_next: check for filter().next_back() (#15748)
Closes: #15715 changelog: [`filter_next`]: suggest replacing `filter().next_back()` with `rfind()` for `DoubleEndedIterator`
2 parents 24b0282 + 5619465 commit abdc61f

File tree

9 files changed

+143
-41
lines changed

9 files changed

+143
-41
lines changed

clippy_lints/src/methods/filter_next.rs

Lines changed: 54 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -10,51 +10,68 @@ use rustc_span::sym;
1010

1111
use super::FILTER_NEXT;
1212

13-
/// lint use of `filter().next()` for `Iterators`
13+
#[derive(Copy, Clone)]
14+
pub(super) enum Direction {
15+
Forward,
16+
Backward,
17+
}
18+
19+
/// lint use of `filter().next()` for `Iterator` and `filter().next_back()` for
20+
/// `DoubleEndedIterator`
1421
pub(super) fn check<'tcx>(
1522
cx: &LateContext<'tcx>,
1623
expr: &'tcx hir::Expr<'_>,
1724
recv: &'tcx hir::Expr<'_>,
1825
filter_arg: &'tcx hir::Expr<'_>,
26+
direction: Direction,
1927
) {
20-
// lint if caller of `.filter().next()` is an Iterator
21-
let recv_impls_iterator = cx
28+
// lint if caller of `.filter().next()` is an Iterator or `.filter().next_back()` is a
29+
// DoubleEndedIterator
30+
let (required_trait, next_method, find_method) = match direction {
31+
Direction::Forward => (sym::Iterator, "next", "find"),
32+
Direction::Backward => (sym::DoubleEndedIterator, "next_back", "rfind"),
33+
};
34+
if !cx
2235
.tcx
23-
.get_diagnostic_item(sym::Iterator)
24-
.is_some_and(|id| implements_trait(cx, cx.typeck_results().expr_ty(recv), id, &[]));
25-
if recv_impls_iterator {
26-
let msg = "called `filter(..).next()` on an `Iterator`. This is more succinctly expressed by calling \
27-
`.find(..)` instead";
28-
let filter_snippet = snippet(cx, filter_arg.span, "..");
29-
if filter_snippet.lines().count() <= 1 {
30-
let iter_snippet = snippet(cx, recv.span, "..");
31-
// add note if not multi-line
32-
span_lint_and_then(cx, FILTER_NEXT, expr.span, msg, |diag| {
33-
let (applicability, pat) = if let Some(id) = path_to_local_with_projections(recv)
34-
&& let hir::Node::Pat(pat) = cx.tcx.hir_node(id)
35-
&& let hir::PatKind::Binding(BindingMode(_, Mutability::Not), _, ident, _) = pat.kind
36-
{
37-
(Applicability::Unspecified, Some((pat.span, ident)))
38-
} else {
39-
(Applicability::MachineApplicable, None)
40-
};
36+
.get_diagnostic_item(required_trait)
37+
.is_some_and(|id| implements_trait(cx, cx.typeck_results().expr_ty(recv), id, &[]))
38+
{
39+
return;
40+
}
41+
let msg = format!(
42+
"called `filter(..).{next_method}()` on an `{}`. This is more succinctly expressed by calling \
43+
`.{find_method}(..)` instead",
44+
required_trait.as_str()
45+
);
46+
let filter_snippet = snippet(cx, filter_arg.span, "..");
47+
if filter_snippet.lines().count() <= 1 {
48+
let iter_snippet = snippet(cx, recv.span, "..");
49+
// add note if not multi-line
50+
span_lint_and_then(cx, FILTER_NEXT, expr.span, msg, |diag| {
51+
let (applicability, pat) = if let Some(id) = path_to_local_with_projections(recv)
52+
&& let hir::Node::Pat(pat) = cx.tcx.hir_node(id)
53+
&& let hir::PatKind::Binding(BindingMode(_, Mutability::Not), _, ident, _) = pat.kind
54+
{
55+
(Applicability::Unspecified, Some((pat.span, ident)))
56+
} else {
57+
(Applicability::MachineApplicable, None)
58+
};
4159

42-
diag.span_suggestion(
43-
expr.span,
44-
"try",
45-
format!("{iter_snippet}.find({filter_snippet})"),
46-
applicability,
47-
);
60+
diag.span_suggestion(
61+
expr.span,
62+
"try",
63+
format!("{iter_snippet}.{find_method}({filter_snippet})"),
64+
applicability,
65+
);
4866

49-
if let Some((pat_span, ident)) = pat {
50-
diag.span_help(
51-
pat_span,
52-
format!("you will also need to make `{ident}` mutable, because `find` takes `&mut self`"),
53-
);
54-
}
55-
});
56-
} else {
57-
span_lint(cx, FILTER_NEXT, expr.span, msg);
58-
}
67+
if let Some((pat_span, ident)) = pat {
68+
diag.span_help(
69+
pat_span,
70+
format!("you will also need to make `{ident}` mutable, because `{find_method}` takes `&mut self`"),
71+
);
72+
}
73+
});
74+
} else {
75+
span_lint(cx, FILTER_NEXT, expr.span, msg);
5976
}
6077
}

clippy_lints/src/methods/mod.rs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5367,7 +5367,9 @@ impl Methods {
53675367
iter_overeager_cloned::Op::LaterCloned,
53685368
false,
53695369
),
5370-
(sym::filter, [arg]) => filter_next::check(cx, expr, recv2, arg),
5370+
(sym::filter, [arg]) => {
5371+
filter_next::check(cx, expr, recv2, arg, filter_next::Direction::Forward);
5372+
},
53715373
(sym::filter_map, [arg]) => filter_map_next::check(cx, expr, recv2, arg, self.msrv),
53725374
(sym::iter, []) => iter_next_slice::check(cx, expr, recv2),
53735375
(sym::skip, [arg]) => iter_skip_next::check(cx, expr, recv2, arg),
@@ -5377,6 +5379,14 @@ impl Methods {
53775379
}
53785380
}
53795381
},
5382+
(sym::next_back, []) => {
5383+
if let Some((name2, recv2, args2, _, _)) = method_call(recv)
5384+
&& let (sym::filter, [arg]) = (name2, args2)
5385+
&& self.msrv.meets(cx, msrvs::DOUBLE_ENDED_ITERATOR_RFIND)
5386+
{
5387+
filter_next::check(cx, expr, recv2, arg, filter_next::Direction::Backward);
5388+
}
5389+
},
53805390
(sym::nth, [n_arg]) => match method_call(recv) {
53815391
Some((sym::bytes, recv2, [], _, _)) => bytes_nth::check(cx, expr, recv2, n_arg),
53825392
Some((sym::cloned, recv2, [], _, _)) => iter_overeager_cloned::check(

clippy_utils/src/msrvs.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ msrv_aliases! {
7272
1,30,0 { ITERATOR_FIND_MAP, TOOL_ATTRIBUTES }
7373
1,29,0 { ITER_FLATTEN }
7474
1,28,0 { FROM_BOOL, REPEAT_WITH, SLICE_FROM_REF }
75-
1,27,0 { ITERATOR_TRY_FOLD }
75+
1,27,0 { ITERATOR_TRY_FOLD, DOUBLE_ENDED_ITERATOR_RFIND }
7676
1,26,0 { RANGE_INCLUSIVE, STRING_RETAIN, POINTER_ADD_SUB_METHODS }
7777
1,24,0 { IS_ASCII_DIGIT, PTR_NULL }
7878
1,18,0 { HASH_MAP_RETAIN, HASH_SET_RETAIN }

tests/ui/auxiliary/option_helpers.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@ impl IteratorFalsePositives {
2525
self
2626
}
2727

28+
pub fn next_back(self) -> IteratorFalsePositives {
29+
self
30+
}
31+
2832
pub fn find(self) -> Option<u32> {
2933
Some(self.foo)
3034
}

tests/ui/methods.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,26 @@ fn filter_next() {
135135
let _ = foo.filter(42).next();
136136
}
137137

138+
#[rustfmt::skip]
139+
fn filter_next_back() {
140+
let v = vec![3, 2, 1, 0, -1, -2, -3];
141+
142+
// Multi-line case.
143+
let _ = v.iter().filter(|&x| {
144+
//~^ filter_next
145+
*x < 0
146+
}
147+
).next_back();
148+
149+
// Check that we don't lint if the caller is not an `Iterator`.
150+
let foo = IteratorFalsePositives { foo: 0 };
151+
let _ = foo.filter().next_back();
152+
153+
let foo = IteratorMethodFalsePositives {};
154+
let _ = foo.filter(42).next_back();
155+
}
156+
138157
fn main() {
139158
filter_next();
159+
filter_next_back();
140160
}

tests/ui/methods.stderr

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,5 +24,16 @@ LL | | ).next();
2424
= note: `-D clippy::filter-next` implied by `-D warnings`
2525
= help: to override `-D warnings` add `#[allow(clippy::filter_next)]`
2626

27-
error: aborting due to 2 previous errors
27+
error: called `filter(..).next_back()` on an `DoubleEndedIterator`. This is more succinctly expressed by calling `.rfind(..)` instead
28+
--> tests/ui/methods.rs:143:13
29+
|
30+
LL | let _ = v.iter().filter(|&x| {
31+
| _____________^
32+
LL | |
33+
LL | | *x < 0
34+
LL | | }
35+
LL | | ).next_back();
36+
| |________________________________^
37+
38+
error: aborting due to 3 previous errors
2839

tests/ui/methods_fixable.fixed

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,18 @@ fn main() {
88
// Single-line case.
99
let _ = v.iter().find(|&x| *x < 0);
1010
//~^ filter_next
11+
12+
let _ = v.iter().rfind(|&x| *x < 0);
13+
//~^ filter_next
14+
}
15+
16+
#[clippy::msrv = "1.27"]
17+
fn msrv_1_27() {
18+
let _ = vec![1].into_iter().rfind(|&x| x < 0);
19+
//~^ filter_next
20+
}
21+
22+
#[clippy::msrv = "1.26"]
23+
fn msrv_1_26() {
24+
let _ = vec![1].into_iter().filter(|&x| x < 0).next_back();
1125
}

tests/ui/methods_fixable.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,18 @@ fn main() {
88
// Single-line case.
99
let _ = v.iter().filter(|&x| *x < 0).next();
1010
//~^ filter_next
11+
12+
let _ = v.iter().filter(|&x| *x < 0).next_back();
13+
//~^ filter_next
14+
}
15+
16+
#[clippy::msrv = "1.27"]
17+
fn msrv_1_27() {
18+
let _ = vec![1].into_iter().filter(|&x| x < 0).next_back();
19+
//~^ filter_next
20+
}
21+
22+
#[clippy::msrv = "1.26"]
23+
fn msrv_1_26() {
24+
let _ = vec![1].into_iter().filter(|&x| x < 0).next_back();
1125
}

tests/ui/methods_fixable.stderr

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,5 +7,17 @@ LL | let _ = v.iter().filter(|&x| *x < 0).next();
77
= note: `-D clippy::filter-next` implied by `-D warnings`
88
= help: to override `-D warnings` add `#[allow(clippy::filter_next)]`
99

10-
error: aborting due to 1 previous error
10+
error: called `filter(..).next_back()` on an `DoubleEndedIterator`. This is more succinctly expressed by calling `.rfind(..)` instead
11+
--> tests/ui/methods_fixable.rs:12:13
12+
|
13+
LL | let _ = v.iter().filter(|&x| *x < 0).next_back();
14+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `v.iter().rfind(|&x| *x < 0)`
15+
16+
error: called `filter(..).next_back()` on an `DoubleEndedIterator`. This is more succinctly expressed by calling `.rfind(..)` instead
17+
--> tests/ui/methods_fixable.rs:18:13
18+
|
19+
LL | let _ = vec![1].into_iter().filter(|&x| x < 0).next_back();
20+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec![1].into_iter().rfind(|&x| x < 0)`
21+
22+
error: aborting due to 3 previous errors
1123

0 commit comments

Comments
 (0)