Skip to content

Commit 6032abe

Browse files
JarchoJarcho
authored
filter_next: big clean-up (#17233)
changelog: none
2 parents 8df7348 + bef6f41 commit 6032abe

13 files changed

Lines changed: 295 additions & 205 deletions

‎clippy_lints/src/methods/filter_next.rs‎

Lines changed: 30 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,29 @@
1-
use clippy_utils::diagnostics::{span_lint, span_lint_and_then};
2-
use clippy_utils::source::snippet;
1+
use clippy_utils::diagnostics::span_lint_and_then;
2+
use clippy_utils::source::snippet_with_applicability;
33
use clippy_utils::ty::implements_trait;
44
use clippy_utils::{path_to_local_with_projections, sym};
55
use rustc_ast::{BindingMode, Mutability};
66
use rustc_errors::Applicability;
7-
use rustc_hir as hir;
7+
use rustc_hir::{Expr, Node, PatKind};
88
use rustc_lint::LateContext;
99

1010
use super::FILTER_NEXT;
1111

12-
#[derive(Copy, Clone)]
12+
#[derive(Clone, Copy)]
1313
pub(super) enum Direction {
1414
Forward,
1515
Backward,
1616
}
1717

1818
/// lint use of `filter().next()` for `Iterator` and `filter().next_back()` for
1919
/// `DoubleEndedIterator`
20-
pub(super) fn check<'tcx>(
21-
cx: &LateContext<'tcx>,
22-
expr: &'tcx hir::Expr<'_>,
23-
recv: &'tcx hir::Expr<'_>,
24-
filter_arg: &'tcx hir::Expr<'_>,
20+
pub(super) fn check(
21+
cx: &LateContext<'_>,
22+
expr: &Expr<'_>,
23+
recv: &Expr<'_>,
24+
filter_arg: &Expr<'_>,
2525
direction: Direction,
2626
) {
27-
// lint if caller of `.filter().next()` is an Iterator or `.filter().next_back()` is a
28-
// DoubleEndedIterator
2927
let (required_trait, next_method, find_method) = match direction {
3028
Direction::Forward => (sym::Iterator, "next", "find"),
3129
Direction::Backward => (sym::DoubleEndedIterator, "next_back", "rfind"),
@@ -37,30 +35,31 @@ pub(super) fn check<'tcx>(
3735
{
3836
return;
3937
}
40-
let msg = format!(
41-
"called `filter(..).{next_method}()` on an `{}`. This is more succinctly expressed by calling \
42-
`.{find_method}(..)` instead",
43-
required_trait.as_str()
44-
);
45-
let filter_snippet = snippet(cx, filter_arg.span, "..");
46-
if filter_snippet.lines().count() <= 1 {
47-
let iter_snippet = snippet(cx, recv.span, "..");
48-
// add note if not multi-line
49-
span_lint_and_then(cx, FILTER_NEXT, expr.span, msg, |diag| {
50-
let (applicability, pat) = if let Some(id) = path_to_local_with_projections(recv)
51-
&& let hir::Node::Pat(pat) = cx.tcx.hir_node(id)
52-
&& let hir::PatKind::Binding(BindingMode(_, Mutability::Not), _, ident, _) = pat.kind
38+
span_lint_and_then(
39+
cx,
40+
FILTER_NEXT,
41+
expr.span,
42+
format!("called `filter(..).{next_method}()` on an `{required_trait}`"),
43+
|diag| {
44+
let mut app = Applicability::MachineApplicable;
45+
let filter_snippet = snippet_with_applicability(cx, filter_arg.span, "..", &mut app);
46+
let iter_snippet = snippet_with_applicability(cx, recv.span, "..", &mut app);
47+
48+
let pat = if let Some(id) = path_to_local_with_projections(recv)
49+
&& let Node::Pat(pat) = cx.tcx.hir_node(id)
50+
&& let PatKind::Binding(BindingMode(_, Mutability::Not), _, ident, _) = pat.kind
5351
{
54-
(Applicability::Unspecified, Some((pat.span, ident)))
52+
app = Applicability::Unspecified;
53+
Some((pat.span, ident))
5554
} else {
56-
(Applicability::MachineApplicable, None)
55+
None
5756
};
5857

59-
diag.span_suggestion(
58+
diag.span_suggestion_verbose(
6059
expr.span,
61-
"try",
60+
format!("use `.{find_method}(..)` instead"),
6261
format!("{iter_snippet}.{find_method}({filter_snippet})"),
63-
applicability,
62+
app,
6463
);
6564

6665
if let Some((pat_span, ident)) = pat {
@@ -69,8 +68,6 @@ pub(super) fn check<'tcx>(
6968
format!("you will also need to make `{ident}` mutable, because `{find_method}` takes `&mut self`"),
7069
);
7170
}
72-
});
73-
} else {
74-
span_lint(cx, FILTER_NEXT, expr.span, msg);
75-
}
71+
},
72+
);
7673
}

‎tests/ui/filter_next.fixed‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
//@aux-build:option_helpers.rs
2+
#![warn(clippy::filter_next)]
3+
#![expect(clippy::disallowed_names)]
4+
#![allow(clippy::useless_vec)]
5+
6+
extern crate option_helpers;
7+
8+
use option_helpers::{IteratorFalsePositives, IteratorMethodFalsePositives};
9+
10+
fn main() {}
11+
12+
fn filter_next() {
13+
let v = [3, 2, 1, 0, -1, -2, -3];
14+
15+
// Single-line case.
16+
let _ = v.iter().find(|&x| *x < 0);
17+
//~^ filter_next
18+
19+
let _ = v.iter().rfind(|&x| *x < 0);
20+
//~^ filter_next
21+
22+
// Multi-line case.
23+
#[rustfmt::skip]
24+
let _ = v.iter().find(|&x| {
25+
//~^ filter_next
26+
*x < 0
27+
});
28+
29+
#[rustfmt::skip]
30+
let _ = v.iter().rfind(|&x| {
31+
//~^ filter_next
32+
*x < 0
33+
});
34+
35+
// Check that we don't lint if the caller is not an `Iterator`.
36+
let foo = IteratorFalsePositives { foo: 0 };
37+
let _ = foo.filter().next();
38+
39+
let foo = IteratorMethodFalsePositives {};
40+
let _ = foo.filter(42).next();
41+
}
42+
43+
fn filter_next_back() {
44+
let v = [3, 2, 1, 0, -1, -2, -3];
45+
46+
// Check that we don't lint if the caller is not an `Iterator`.
47+
let foo = IteratorFalsePositives { foo: 0 };
48+
let _ = foo.filter().next_back();
49+
50+
let foo = IteratorMethodFalsePositives {};
51+
let _ = foo.filter(42).next_back();
52+
}
53+
54+
#[clippy::msrv = "1.27"]
55+
fn msrv_1_27() {
56+
let _ = vec![1].into_iter().rfind(|&x| x < 0);
57+
//~^ filter_next
58+
}
59+
60+
#[clippy::msrv = "1.26"]
61+
fn msrv_1_26() {
62+
let _ = vec![1].into_iter().filter(|&x| x < 0).next_back();
63+
}

‎tests/ui/filter_next.rs‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
//@aux-build:option_helpers.rs
2+
#![warn(clippy::filter_next)]
3+
#![expect(clippy::disallowed_names)]
4+
#![allow(clippy::useless_vec)]
5+
6+
extern crate option_helpers;
7+
8+
use option_helpers::{IteratorFalsePositives, IteratorMethodFalsePositives};
9+
10+
fn main() {}
11+
12+
fn filter_next() {
13+
let v = [3, 2, 1, 0, -1, -2, -3];
14+
15+
// Single-line case.
16+
let _ = v.iter().filter(|&x| *x < 0).next();
17+
//~^ filter_next
18+
19+
let _ = v.iter().filter(|&x| *x < 0).next_back();
20+
//~^ filter_next
21+
22+
// Multi-line case.
23+
#[rustfmt::skip]
24+
let _ = v.iter().filter(|&x| {
25+
//~^ filter_next
26+
*x < 0
27+
}
28+
).next();
29+
30+
#[rustfmt::skip]
31+
let _ = v.iter().filter(|&x| {
32+
//~^ filter_next
33+
*x < 0
34+
}
35+
).next_back();
36+
37+
// Check that we don't lint if the caller is not an `Iterator`.
38+
let foo = IteratorFalsePositives { foo: 0 };
39+
let _ = foo.filter().next();
40+
41+
let foo = IteratorMethodFalsePositives {};
42+
let _ = foo.filter(42).next();
43+
}
44+
45+
fn filter_next_back() {
46+
let v = [3, 2, 1, 0, -1, -2, -3];
47+
48+
// Check that we don't lint if the caller is not an `Iterator`.
49+
let foo = IteratorFalsePositives { foo: 0 };
50+
let _ = foo.filter().next_back();
51+
52+
let foo = IteratorMethodFalsePositives {};
53+
let _ = foo.filter(42).next_back();
54+
}
55+
56+
#[clippy::msrv = "1.27"]
57+
fn msrv_1_27() {
58+
let _ = vec![1].into_iter().filter(|&x| x < 0).next_back();
59+
//~^ filter_next
60+
}
61+
62+
#[clippy::msrv = "1.26"]
63+
fn msrv_1_26() {
64+
let _ = vec![1].into_iter().filter(|&x| x < 0).next_back();
65+
}

‎tests/ui/filter_next.stderr‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
error: called `filter(..).next()` on an `Iterator`
2+
--> tests/ui/filter_next.rs:16:13
3+
|
4+
LL | let _ = v.iter().filter(|&x| *x < 0).next();
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
6+
|
7+
= note: `-D clippy::filter-next` implied by `-D warnings`
8+
= help: to override `-D warnings` add `#[allow(clippy::filter_next)]`
9+
help: use `.find(..)` instead
10+
|
11+
LL - let _ = v.iter().filter(|&x| *x < 0).next();
12+
LL + let _ = v.iter().find(|&x| *x < 0);
13+
|
14+
15+
error: called `filter(..).next_back()` on an `DoubleEndedIterator`
16+
--> tests/ui/filter_next.rs:19:13
17+
|
18+
LL | let _ = v.iter().filter(|&x| *x < 0).next_back();
19+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
20+
|
21+
help: use `.rfind(..)` instead
22+
|
23+
LL - let _ = v.iter().filter(|&x| *x < 0).next_back();
24+
LL + let _ = v.iter().rfind(|&x| *x < 0);
25+
|
26+
27+
error: called `filter(..).next()` on an `Iterator`
28+
--> tests/ui/filter_next.rs:24:13
29+
|
30+
LL | let _ = v.iter().filter(|&x| {
31+
| _____________^
32+
LL | |
33+
LL | | *x < 0
34+
LL | | }
35+
LL | | ).next();
36+
| |___________________________^
37+
|
38+
help: use `.find(..)` instead
39+
|
40+
LL ~ let _ = v.iter().find(|&x| {
41+
LL +
42+
LL + *x < 0
43+
LL ~ });
44+
|
45+
46+
error: called `filter(..).next_back()` on an `DoubleEndedIterator`
47+
--> tests/ui/filter_next.rs:31:13
48+
|
49+
LL | let _ = v.iter().filter(|&x| {
50+
| _____________^
51+
LL | |
52+
LL | | *x < 0
53+
LL | | }
54+
LL | | ).next_back();
55+
| |________________________________^
56+
|
57+
help: use `.rfind(..)` instead
58+
|
59+
LL ~ let _ = v.iter().rfind(|&x| {
60+
LL +
61+
LL + *x < 0
62+
LL ~ });
63+
|
64+
65+
error: called `filter(..).next_back()` on an `DoubleEndedIterator`
66+
--> tests/ui/filter_next.rs:58:13
67+
|
68+
LL | let _ = vec![1].into_iter().filter(|&x| x < 0).next_back();
69+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
70+
|
71+
help: use `.rfind(..)` instead
72+
|
73+
LL - let _ = vec![1].into_iter().filter(|&x| x < 0).next_back();
74+
LL + let _ = vec![1].into_iter().rfind(|&x| x < 0);
75+
|
76+
77+
error: aborting due to 5 previous errors
78+

‎tests/ui/filter_next_unfixable.rs‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
//@no-rustfix
2+
#![warn(clippy::filter_next)]
3+
4+
fn main() {}
5+
6+
// The fixed version doesn't compile, as `iter` isn't `mut`.
7+
// We do emit a note suggesting adding it, but not an autofix
8+
pub fn issue10029() {
9+
{
10+
let iter = (0..10);
11+
let _ = iter.filter(|_| true).next();
12+
//~^ filter_next
13+
}
14+
{
15+
let iter = (0..10);
16+
let _ = iter.filter(|_| true).next_back();
17+
//~^ filter_next
18+
}
19+
}

‎tests/ui/filter_next_unfixable.stderr‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
error: called `filter(..).next()` on an `Iterator`
2+
--> tests/ui/filter_next_unfixable.rs:11:17
3+
|
4+
LL | let _ = iter.filter(|_| true).next();
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
6+
|
7+
help: you will also need to make `iter` mutable, because `find` takes `&mut self`
8+
--> tests/ui/filter_next_unfixable.rs:10:13
9+
|
10+
LL | let iter = (0..10);
11+
| ^^^^
12+
= note: `-D clippy::filter-next` implied by `-D warnings`
13+
= help: to override `-D warnings` add `#[allow(clippy::filter_next)]`
14+
help: use `.find(..)` instead
15+
|
16+
LL - let _ = iter.filter(|_| true).next();
17+
LL + let _ = iter.find(|_| true);
18+
|
19+
20+
error: called `filter(..).next_back()` on an `DoubleEndedIterator`
21+
--> tests/ui/filter_next_unfixable.rs:16:17
22+
|
23+
LL | let _ = iter.filter(|_| true).next_back();
24+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
25+
|
26+
help: you will also need to make `iter` mutable, because `rfind` takes `&mut self`
27+
--> tests/ui/filter_next_unfixable.rs:15:13
28+
|
29+
LL | let iter = (0..10);
30+
| ^^^^
31+
help: use `.rfind(..)` instead
32+
|
33+
LL - let _ = iter.filter(|_| true).next_back();
34+
LL + let _ = iter.rfind(|_| true);
35+
|
36+
37+
error: aborting due to 2 previous errors
38+

0 commit comments

Comments
 (0)