diff --git a/clippy_lints/src/methods/filter_next.rs b/clippy_lints/src/methods/filter_next.rs index 9c6b16b88bfd..3584833bc61c 100644 --- a/clippy_lints/src/methods/filter_next.rs +++ b/clippy_lints/src/methods/filter_next.rs @@ -1,15 +1,15 @@ -use clippy_utils::diagnostics::{span_lint, span_lint_and_then}; -use clippy_utils::source::snippet; +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::source::snippet_with_applicability; use clippy_utils::ty::implements_trait; use clippy_utils::{path_to_local_with_projections, sym}; use rustc_ast::{BindingMode, Mutability}; use rustc_errors::Applicability; -use rustc_hir as hir; +use rustc_hir::{Expr, Node, PatKind}; use rustc_lint::LateContext; use super::FILTER_NEXT; -#[derive(Copy, Clone)] +#[derive(Clone, Copy)] pub(super) enum Direction { Forward, Backward, @@ -17,15 +17,13 @@ pub(super) enum Direction { /// 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<'_>, +pub(super) fn check( + cx: &LateContext<'_>, + expr: &Expr<'_>, + recv: &Expr<'_>, + filter_arg: &Expr<'_>, direction: Direction, ) { - // 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"), @@ -37,30 +35,31 @@ pub(super) fn check<'tcx>( { 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 + span_lint_and_then( + cx, + FILTER_NEXT, + expr.span, + format!("called `filter(..).{next_method}()` on an `{required_trait}`"), + |diag| { + let mut app = Applicability::MachineApplicable; + let filter_snippet = snippet_with_applicability(cx, filter_arg.span, "..", &mut app); + let iter_snippet = snippet_with_applicability(cx, recv.span, "..", &mut app); + + let pat = if let Some(id) = path_to_local_with_projections(recv) + && let Node::Pat(pat) = cx.tcx.hir_node(id) + && let PatKind::Binding(BindingMode(_, Mutability::Not), _, ident, _) = pat.kind { - (Applicability::Unspecified, Some((pat.span, ident))) + app = Applicability::Unspecified; + Some((pat.span, ident)) } else { - (Applicability::MachineApplicable, None) + None }; - diag.span_suggestion( + diag.span_suggestion_verbose( expr.span, - "try", + format!("use `.{find_method}(..)` instead"), format!("{iter_snippet}.{find_method}({filter_snippet})"), - applicability, + app, ); if let Some((pat_span, ident)) = pat { @@ -69,8 +68,6 @@ pub(super) fn check<'tcx>( format!("you will also need to make `{ident}` mutable, because `{find_method}` takes `&mut self`"), ); } - }); - } else { - span_lint(cx, FILTER_NEXT, expr.span, msg); - } + }, + ); } diff --git a/tests/ui/filter_next.fixed b/tests/ui/filter_next.fixed new file mode 100644 index 000000000000..5502fa1fb273 --- /dev/null +++ b/tests/ui/filter_next.fixed @@ -0,0 +1,63 @@ +//@aux-build:option_helpers.rs +#![warn(clippy::filter_next)] +#![expect(clippy::disallowed_names)] +#![allow(clippy::useless_vec)] + +extern crate option_helpers; + +use option_helpers::{IteratorFalsePositives, IteratorMethodFalsePositives}; + +fn main() {} + +fn filter_next() { + let v = [3, 2, 1, 0, -1, -2, -3]; + + // Single-line case. + let _ = v.iter().find(|&x| *x < 0); + //~^ filter_next + + let _ = v.iter().rfind(|&x| *x < 0); + //~^ filter_next + + // Multi-line case. + #[rustfmt::skip] + let _ = v.iter().find(|&x| { + //~^ filter_next + *x < 0 + }); + + #[rustfmt::skip] + let _ = v.iter().rfind(|&x| { + //~^ filter_next + *x < 0 + }); + + // Check that we don't lint if the caller is not an `Iterator`. + let foo = IteratorFalsePositives { foo: 0 }; + let _ = foo.filter().next(); + + let foo = IteratorMethodFalsePositives {}; + let _ = foo.filter(42).next(); +} + +fn filter_next_back() { + let v = [3, 2, 1, 0, -1, -2, -3]; + + // 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(); +} + +#[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(); +} diff --git a/tests/ui/filter_next.rs b/tests/ui/filter_next.rs new file mode 100644 index 000000000000..c8e9f060cd60 --- /dev/null +++ b/tests/ui/filter_next.rs @@ -0,0 +1,65 @@ +//@aux-build:option_helpers.rs +#![warn(clippy::filter_next)] +#![expect(clippy::disallowed_names)] +#![allow(clippy::useless_vec)] + +extern crate option_helpers; + +use option_helpers::{IteratorFalsePositives, IteratorMethodFalsePositives}; + +fn main() {} + +fn filter_next() { + let v = [3, 2, 1, 0, -1, -2, -3]; + + // Single-line case. + let _ = v.iter().filter(|&x| *x < 0).next(); + //~^ filter_next + + let _ = v.iter().filter(|&x| *x < 0).next_back(); + //~^ filter_next + + // Multi-line case. + #[rustfmt::skip] + let _ = v.iter().filter(|&x| { + //~^ filter_next + *x < 0 + } + ).next(); + + #[rustfmt::skip] + 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(); + + let foo = IteratorMethodFalsePositives {}; + let _ = foo.filter(42).next(); +} + +fn filter_next_back() { + let v = [3, 2, 1, 0, -1, -2, -3]; + + // 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(); +} + +#[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(); +} diff --git a/tests/ui/filter_next.stderr b/tests/ui/filter_next.stderr new file mode 100644 index 000000000000..19c2428c3d17 --- /dev/null +++ b/tests/ui/filter_next.stderr @@ -0,0 +1,78 @@ +error: called `filter(..).next()` on an `Iterator` + --> tests/ui/filter_next.rs:16:13 + | +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)]` +help: use `.find(..)` instead + | +LL - let _ = v.iter().filter(|&x| *x < 0).next(); +LL + let _ = v.iter().find(|&x| *x < 0); + | + +error: called `filter(..).next_back()` on an `DoubleEndedIterator` + --> tests/ui/filter_next.rs:19:13 + | +LL | let _ = v.iter().filter(|&x| *x < 0).next_back(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: use `.rfind(..)` instead + | +LL - let _ = v.iter().filter(|&x| *x < 0).next_back(); +LL + let _ = v.iter().rfind(|&x| *x < 0); + | + +error: called `filter(..).next()` on an `Iterator` + --> tests/ui/filter_next.rs:24:13 + | +LL | let _ = v.iter().filter(|&x| { + | _____________^ +LL | | +LL | | *x < 0 +LL | | } +LL | | ).next(); + | |___________________________^ + | +help: use `.find(..)` instead + | +LL ~ let _ = v.iter().find(|&x| { +LL + +LL + *x < 0 +LL ~ }); + | + +error: called `filter(..).next_back()` on an `DoubleEndedIterator` + --> tests/ui/filter_next.rs:31:13 + | +LL | let _ = v.iter().filter(|&x| { + | _____________^ +LL | | +LL | | *x < 0 +LL | | } +LL | | ).next_back(); + | |________________________________^ + | +help: use `.rfind(..)` instead + | +LL ~ let _ = v.iter().rfind(|&x| { +LL + +LL + *x < 0 +LL ~ }); + | + +error: called `filter(..).next_back()` on an `DoubleEndedIterator` + --> tests/ui/filter_next.rs:58:13 + | +LL | let _ = vec![1].into_iter().filter(|&x| x < 0).next_back(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: use `.rfind(..)` instead + | +LL - let _ = vec![1].into_iter().filter(|&x| x < 0).next_back(); +LL + let _ = vec![1].into_iter().rfind(|&x| x < 0); + | + +error: aborting due to 5 previous errors + diff --git a/tests/ui/filter_next_unfixable.rs b/tests/ui/filter_next_unfixable.rs new file mode 100644 index 000000000000..07a0db135350 --- /dev/null +++ b/tests/ui/filter_next_unfixable.rs @@ -0,0 +1,19 @@ +//@no-rustfix +#![warn(clippy::filter_next)] + +fn main() {} + +// The fixed version doesn't compile, as `iter` isn't `mut`. +// We do emit a note suggesting adding it, but not an autofix +pub fn issue10029() { + { + let iter = (0..10); + let _ = iter.filter(|_| true).next(); + //~^ filter_next + } + { + let iter = (0..10); + let _ = iter.filter(|_| true).next_back(); + //~^ filter_next + } +} diff --git a/tests/ui/filter_next_unfixable.stderr b/tests/ui/filter_next_unfixable.stderr new file mode 100644 index 000000000000..2c35badec485 --- /dev/null +++ b/tests/ui/filter_next_unfixable.stderr @@ -0,0 +1,38 @@ +error: called `filter(..).next()` on an `Iterator` + --> tests/ui/filter_next_unfixable.rs:11:17 + | +LL | let _ = iter.filter(|_| true).next(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: you will also need to make `iter` mutable, because `find` takes `&mut self` + --> tests/ui/filter_next_unfixable.rs:10:13 + | +LL | let iter = (0..10); + | ^^^^ + = note: `-D clippy::filter-next` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::filter_next)]` +help: use `.find(..)` instead + | +LL - let _ = iter.filter(|_| true).next(); +LL + let _ = iter.find(|_| true); + | + +error: called `filter(..).next_back()` on an `DoubleEndedIterator` + --> tests/ui/filter_next_unfixable.rs:16:17 + | +LL | let _ = iter.filter(|_| true).next_back(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: you will also need to make `iter` mutable, because `rfind` takes `&mut self` + --> tests/ui/filter_next_unfixable.rs:15:13 + | +LL | let iter = (0..10); + | ^^^^ +help: use `.rfind(..)` instead + | +LL - let _ = iter.filter(|_| true).next_back(); +LL + let _ = iter.rfind(|_| true); + | + +error: aborting due to 2 previous errors + diff --git a/tests/ui/methods.rs b/tests/ui/methods.rs index b45b7dcd56a4..10f0934bede7 100644 --- a/tests/ui/methods.rs +++ b/tests/ui/methods.rs @@ -1,18 +1,10 @@ -//@aux-build:option_helpers.rs - #![warn(clippy::filter_next, clippy::new_ret_no_self)] -#![expect(clippy::disallowed_names, clippy::useless_vec)] - -#[macro_use] -extern crate option_helpers; use std::collections::{BTreeMap, HashMap, HashSet, VecDeque}; use std::ops::Mul; use std::rc::{self, Rc}; use std::sync::{self, Arc}; -use option_helpers::{IteratorFalsePositives, IteratorMethodFalsePositives}; - struct Lt<'a> { foo: &'a u32, } @@ -97,43 +89,4 @@ impl Mul for T { } } -/// Checks implementation of `FILTER_NEXT` lint. -#[rustfmt::skip] -fn filter_next() { - let v = vec![3, 2, 1, 0, -1, -2, -3]; - - // Multi-line case. - let _ = v.iter().filter(|&x| { - //~^ filter_next - *x < 0 - } - ).next(); - - // Check that we don't lint if the caller is not an `Iterator`. - let foo = IteratorFalsePositives { foo: 0 }; - let _ = foo.filter().next(); - - let foo = IteratorMethodFalsePositives {}; - 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() {} diff --git a/tests/ui/methods.stderr b/tests/ui/methods.stderr index a637818cee48..1136b33ea910 100644 --- a/tests/ui/methods.stderr +++ b/tests/ui/methods.stderr @@ -1,5 +1,5 @@ error: methods called `new` usually return `Self` - --> tests/ui/methods.rs:84:5 + --> tests/ui/methods.rs:76:5 | LL | / fn new() -> i32 { LL | | @@ -10,30 +10,5 @@ LL | | } = note: `-D clippy::new-ret-no-self` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::new_ret_no_self)]` -error: called `filter(..).next()` on an `Iterator`. This is more succinctly expressed by calling `.find(..)` instead - --> tests/ui/methods.rs:106:13 - | -LL | let _ = v.iter().filter(|&x| { - | _____________^ -LL | | -LL | | *x < 0 -LL | | } -LL | | ).next(); - | |___________________________^ - | - = note: `-D clippy::filter-next` implied by `-D warnings` - = help: to override `-D warnings` add `#[allow(clippy::filter_next)]` - -error: called `filter(..).next_back()` on an `DoubleEndedIterator`. This is more succinctly expressed by calling `.rfind(..)` instead - --> tests/ui/methods.rs:125:13 - | -LL | let _ = v.iter().filter(|&x| { - | _____________^ -LL | | -LL | | *x < 0 -LL | | } -LL | | ).next_back(); - | |________________________________^ - -error: aborting due to 3 previous errors +error: aborting due to 1 previous error diff --git a/tests/ui/methods_fixable.fixed b/tests/ui/methods_fixable.fixed deleted file mode 100644 index 17a751eb7945..000000000000 --- a/tests/ui/methods_fixable.fixed +++ /dev/null @@ -1,25 +0,0 @@ -#![warn(clippy::filter_next)] -#![expect(clippy::useless_vec)] - -/// Checks implementation of `FILTER_NEXT` lint. -fn main() { - let v = vec![3, 2, 1, 0, -1, -2, -3]; - - // 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(); -} diff --git a/tests/ui/methods_fixable.rs b/tests/ui/methods_fixable.rs deleted file mode 100644 index b2520acc3638..000000000000 --- a/tests/ui/methods_fixable.rs +++ /dev/null @@ -1,25 +0,0 @@ -#![warn(clippy::filter_next)] -#![expect(clippy::useless_vec)] - -/// Checks implementation of `FILTER_NEXT` lint. -fn main() { - let v = vec![3, 2, 1, 0, -1, -2, -3]; - - // 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(); -} diff --git a/tests/ui/methods_fixable.stderr b/tests/ui/methods_fixable.stderr deleted file mode 100644 index d26b5e9ac271..000000000000 --- a/tests/ui/methods_fixable.stderr +++ /dev/null @@ -1,23 +0,0 @@ -error: called `filter(..).next()` on an `Iterator`. This is more succinctly expressed by calling `.find(..)` instead - --> tests/ui/methods_fixable.rs:9:13 - | -LL | let _ = v.iter().filter(|&x| *x < 0).next(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `v.iter().find(|&x| *x < 0)` - | - = note: `-D clippy::filter-next` implied by `-D warnings` - = help: to override `-D warnings` add `#[allow(clippy::filter_next)]` - -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 - diff --git a/tests/ui/methods_unfixable.rs b/tests/ui/methods_unfixable.rs deleted file mode 100644 index 46a5d95eb1b7..000000000000 --- a/tests/ui/methods_unfixable.rs +++ /dev/null @@ -1,9 +0,0 @@ -#![warn(clippy::filter_next)] -//@no-rustfix -fn main() {} - -pub fn issue10029() { - let iter = (0..10); - let _ = iter.filter(|_| true).next(); - //~^ filter_next -} diff --git a/tests/ui/methods_unfixable.stderr b/tests/ui/methods_unfixable.stderr deleted file mode 100644 index 137112ae4179..000000000000 --- a/tests/ui/methods_unfixable.stderr +++ /dev/null @@ -1,16 +0,0 @@ -error: called `filter(..).next()` on an `Iterator`. This is more succinctly expressed by calling `.find(..)` instead - --> tests/ui/methods_unfixable.rs:7:13 - | -LL | let _ = iter.filter(|_| true).next(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `iter.find(|_| true)` - | -help: you will also need to make `iter` mutable, because `find` takes `&mut self` - --> tests/ui/methods_unfixable.rs:6:9 - | -LL | let iter = (0..10); - | ^^^^ - = 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 -