filter_next: big clean-up

clippy_lints/src/methods/filter_next.rs

@@ -1,31 +1,29 @@
-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,
 }
 
 /// 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);
-    }
+        },
+    );
 }

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();
+}

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();
+}

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
+

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
+    }
+}

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
+