Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 54 additions & 37 deletions clippy_lints/src/methods/filter_next.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,51 +10,68 @@ use rustc_span::sym;

use super::FILTER_NEXT;

/// lint use of `filter().next()` for `Iterators`
#[derive(Copy, Clone)]
pub(super) enum Direction {
Forward,
Backward,
}

/// lint use of `filter().next()` for `Iterator` and `filter().next_back()` for
/// `DoubleEndedIterator`
pub(super) fn check<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx hir::Expr<'_>,
recv: &'tcx hir::Expr<'_>,
filter_arg: &'tcx hir::Expr<'_>,
direction: Direction,
) {
// lint if caller of `.filter().next()` is an Iterator
let recv_impls_iterator = cx
// lint if caller of `.filter().next()` is an Iterator or `.filter().next_back()` is a
// DoubleEndedIterator
let (required_trait, next_method, find_method) = match direction {
Direction::Forward => (sym::Iterator, "next", "find"),
Direction::Backward => (sym::DoubleEndedIterator, "next_back", "rfind"),
};
if !cx
.tcx
.get_diagnostic_item(sym::Iterator)
.is_some_and(|id| implements_trait(cx, cx.typeck_results().expr_ty(recv), id, &[]));
if recv_impls_iterator {
let msg = "called `filter(..).next()` on an `Iterator`. This is more succinctly expressed by calling \
`.find(..)` instead";
let filter_snippet = snippet(cx, filter_arg.span, "..");
if filter_snippet.lines().count() <= 1 {
let iter_snippet = snippet(cx, recv.span, "..");
// add note if not multi-line
span_lint_and_then(cx, FILTER_NEXT, expr.span, msg, |diag| {
let (applicability, pat) = if let Some(id) = path_to_local_with_projections(recv)
&& let hir::Node::Pat(pat) = cx.tcx.hir_node(id)
&& let hir::PatKind::Binding(BindingMode(_, Mutability::Not), _, ident, _) = pat.kind
{
(Applicability::Unspecified, Some((pat.span, ident)))
} else {
(Applicability::MachineApplicable, None)
};
.get_diagnostic_item(required_trait)
.is_some_and(|id| implements_trait(cx, cx.typeck_results().expr_ty(recv), id, &[]))
{
return;
}
let msg = format!(
"called `filter(..).{next_method}()` on an `{}`. This is more succinctly expressed by calling \
`.{find_method}(..)` instead",
required_trait.as_str()
);
let filter_snippet = snippet(cx, filter_arg.span, "..");
if filter_snippet.lines().count() <= 1 {
let iter_snippet = snippet(cx, recv.span, "..");
// add note if not multi-line
span_lint_and_then(cx, FILTER_NEXT, expr.span, msg, |diag| {
let (applicability, pat) = if let Some(id) = path_to_local_with_projections(recv)
&& let hir::Node::Pat(pat) = cx.tcx.hir_node(id)
&& let hir::PatKind::Binding(BindingMode(_, Mutability::Not), _, ident, _) = pat.kind
{
(Applicability::Unspecified, Some((pat.span, ident)))
} else {
(Applicability::MachineApplicable, None)
};

diag.span_suggestion(
expr.span,
"try",
format!("{iter_snippet}.find({filter_snippet})"),
applicability,
);
diag.span_suggestion(
expr.span,
"try",
format!("{iter_snippet}.{find_method}({filter_snippet})"),
applicability,
);

if let Some((pat_span, ident)) = pat {
diag.span_help(
pat_span,
format!("you will also need to make `{ident}` mutable, because `find` takes `&mut self`"),
);
}
});
} else {
span_lint(cx, FILTER_NEXT, expr.span, msg);
}
if let Some((pat_span, ident)) = pat {
diag.span_help(
pat_span,
format!("you will also need to make `{ident}` mutable, because `{find_method}` takes `&mut self`"),
);
}
});
} else {
span_lint(cx, FILTER_NEXT, expr.span, msg);
}
}
12 changes: 11 additions & 1 deletion clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5369,7 +5369,9 @@ impl Methods {
iter_overeager_cloned::Op::LaterCloned,
false,
),
(sym::filter, [arg]) => filter_next::check(cx, expr, recv2, arg),
(sym::filter, [arg]) => {
filter_next::check(cx, expr, recv2, arg, filter_next::Direction::Forward);
},
(sym::filter_map, [arg]) => filter_map_next::check(cx, expr, recv2, arg, self.msrv),
(sym::iter, []) => iter_next_slice::check(cx, expr, recv2),
(sym::skip, [arg]) => iter_skip_next::check(cx, expr, recv2, arg),
Expand All @@ -5379,6 +5381,14 @@ impl Methods {
}
}
},
(sym::next_back, []) => {
if let Some((name2, recv2, args2, _, _)) = method_call(recv)
&& let (sym::filter, [arg]) = (name2, args2)
&& self.msrv.meets(cx, msrvs::DOUBLE_ENDED_ITERATOR_RFIND)
{
filter_next::check(cx, expr, recv2, arg, filter_next::Direction::Backward);
}
},
(sym::nth, [n_arg]) => match method_call(recv) {
Some((sym::bytes, recv2, [], _, _)) => bytes_nth::check(cx, expr, recv2, n_arg),
Some((sym::cloned, recv2, [], _, _)) => iter_overeager_cloned::check(
Expand Down
2 changes: 1 addition & 1 deletion clippy_utils/src/msrvs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ msrv_aliases! {
1,30,0 { ITERATOR_FIND_MAP, TOOL_ATTRIBUTES }
1,29,0 { ITER_FLATTEN }
1,28,0 { FROM_BOOL, REPEAT_WITH, SLICE_FROM_REF }
1,27,0 { ITERATOR_TRY_FOLD }
1,27,0 { ITERATOR_TRY_FOLD, DOUBLE_ENDED_ITERATOR_RFIND }
1,26,0 { RANGE_INCLUSIVE, STRING_RETAIN, POINTER_ADD_SUB_METHODS }
1,24,0 { IS_ASCII_DIGIT, PTR_NULL }
1,18,0 { HASH_MAP_RETAIN, HASH_SET_RETAIN }
Expand Down
4 changes: 4 additions & 0 deletions tests/ui/auxiliary/option_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ impl IteratorFalsePositives {
self
}

pub fn next_back(self) -> IteratorFalsePositives {
self
}

pub fn find(self) -> Option<u32> {
Some(self.foo)
}
Expand Down
20 changes: 20 additions & 0 deletions tests/ui/methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,26 @@ fn filter_next() {
let _ = foo.filter(42).next();
}

#[rustfmt::skip]
fn filter_next_back() {
let v = vec![3, 2, 1, 0, -1, -2, -3];

// Multi-line case.
let _ = v.iter().filter(|&x| {
//~^ filter_next
*x < 0
}
).next_back();

// Check that we don't lint if the caller is not an `Iterator`.
let foo = IteratorFalsePositives { foo: 0 };
let _ = foo.filter().next_back();

let foo = IteratorMethodFalsePositives {};
let _ = foo.filter(42).next_back();
}

fn main() {
filter_next();
filter_next_back();
}
13 changes: 12 additions & 1 deletion tests/ui/methods.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,16 @@ LL | | ).next();
= note: `-D clippy::filter-next` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::filter_next)]`

error: aborting due to 2 previous errors
error: called `filter(..).next_back()` on an `DoubleEndedIterator`. This is more succinctly expressed by calling `.rfind(..)` instead
--> tests/ui/methods.rs:143:13
|
LL | let _ = v.iter().filter(|&x| {
| _____________^
LL | |
LL | | *x < 0
LL | | }
LL | | ).next_back();
| |________________________________^

error: aborting due to 3 previous errors

14 changes: 14 additions & 0 deletions tests/ui/methods_fixable.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,18 @@ fn main() {
// Single-line case.
let _ = v.iter().find(|&x| *x < 0);
//~^ filter_next

let _ = v.iter().rfind(|&x| *x < 0);
//~^ filter_next
}

#[clippy::msrv = "1.27"]
fn msrv_1_27() {
let _ = vec![1].into_iter().rfind(|&x| x < 0);
//~^ filter_next
}

#[clippy::msrv = "1.26"]
fn msrv_1_26() {
let _ = vec![1].into_iter().filter(|&x| x < 0).next_back();
}
14 changes: 14 additions & 0 deletions tests/ui/methods_fixable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,18 @@ fn main() {
// Single-line case.
let _ = v.iter().filter(|&x| *x < 0).next();
//~^ filter_next

let _ = v.iter().filter(|&x| *x < 0).next_back();
//~^ filter_next
}

#[clippy::msrv = "1.27"]
fn msrv_1_27() {
let _ = vec![1].into_iter().filter(|&x| x < 0).next_back();
//~^ filter_next
}

#[clippy::msrv = "1.26"]
fn msrv_1_26() {
let _ = vec![1].into_iter().filter(|&x| x < 0).next_back();
}
14 changes: 13 additions & 1 deletion tests/ui/methods_fixable.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,17 @@ LL | let _ = v.iter().filter(|&x| *x < 0).next();
= note: `-D clippy::filter-next` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::filter_next)]`

error: aborting due to 1 previous error
error: called `filter(..).next_back()` on an `DoubleEndedIterator`. This is more succinctly expressed by calling `.rfind(..)` instead
--> tests/ui/methods_fixable.rs:12:13
|
LL | let _ = v.iter().filter(|&x| *x < 0).next_back();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `v.iter().rfind(|&x| *x < 0)`

error: called `filter(..).next_back()` on an `DoubleEndedIterator`. This is more succinctly expressed by calling `.rfind(..)` instead
--> tests/ui/methods_fixable.rs:18:13
|
LL | let _ = vec![1].into_iter().filter(|&x| x < 0).next_back();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec![1].into_iter().rfind(|&x| x < 0)`

error: aborting due to 3 previous errors