feat(search_is_some): Fix when the closure spans multiple lines

Previously the program only fixed the code when the closure supplied to
the method contained only 1 line.  This patch removes the restriction.

The code already works.  This patch only removes the extra check that
causes the restriction.  The test cases that can now be fixed are moved
into the files containing tests cases that can be fixed.

The unnecessary check has survived in the code this way.

- In Dec 2015, patch a6bd2d0, pull request 524.  The lint was first
  added.  The program did not support fixing code automatically yet.  So
  the suggested fix was printed as a part of the diagnostic message.
  When the original code contained multiple lines, the suggested fix was
  omitted in order to keep the diagnostic message concise.

- In May 2019, patch bd0b75f, pull request 4049.  Logic was added
  to strip the reference in the closure when the suggested replacement
  method required it.  Because the fix was still only printed when the
  code contained a single line, the new transformation was only done
  when the code contained a single line.

- In Aug 2019, patch 945d4cf, pull request 4454.  The lint was
  updated to fix code automatically.  Because the fixed code had only
  been printed in the diagnostic message for a single line, the fix was
  only added for a single line.

- In Nov 2021, patch 092fe20, pull request 7463.  The logic for
  transforming the closure was moved into another file.  A comment was
  added saying that it was only good for a single line because it had
  only been used for a single line.

changelog: [`search_is_some`] now fixes code spanning multiple lines

Author: ceptontech

clippy_lints/src/methods/search_is_some.rs

@@ -1,4 +1,4 @@
-use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg};
+use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::res::{MaybeDef, MaybeTypeckRes};
 use clippy_utils::source::{snippet, snippet_with_applicability};
 use clippy_utils::sugg::deref_closure_args;
@@ -34,79 +34,67 @@ pub(super) fn check<'tcx>(
     {
         let msg = format!("called `{option_check_method}()` after searching an `Iterator` with `{search_method}`");
         let search_snippet = snippet(cx, search_arg.span, "..");
-        if search_snippet.lines().count() <= 1 {
-            // suggest `any(|x| ..)` instead of `any(|&x| ..)` for `find(|&x| ..).is_some()`
-            // suggest `any(|..| *..)` instead of `any(|..| **..)` for `find(|..| **..).is_some()`
-            let mut applicability = Applicability::MachineApplicable;
-            let any_search_snippet = if search_method == sym::find
-                && let ExprKind::Closure(&hir::Closure { body, .. }) = search_arg.kind
-                && let closure_body = cx.tcx.hir_body(body)
-                && let Some(closure_arg) = closure_body.params.first()
-            {
-                if let PatKind::Ref(..) = closure_arg.pat.kind {
-                    Some(search_snippet.replacen('&', "", 1))
-                } else if let PatKind::Binding(..) = strip_pat_refs(closure_arg.pat).kind {
-                    // `find()` provides a reference to the item, but `any` does not,
-                    // so we should fix item usages for suggestion
-                    if let Some(closure_sugg) = deref_closure_args(cx, search_arg) {
-                        applicability = closure_sugg.applicability;
-                        Some(closure_sugg.suggestion)
-                    } else {
-                        Some(search_snippet.to_string())
-                    }
+        // suggest `any(|x| ..)` instead of `any(|&x| ..)` for `find(|&x| ..).is_some()`
+        // suggest `any(|..| *..)` instead of `any(|..| **..)` for `find(|..| **..).is_some()`
+        let mut applicability = Applicability::MachineApplicable;
+        let any_search_snippet = if search_method == sym::find
+            && let ExprKind::Closure(&hir::Closure { body, .. }) = search_arg.kind
+            && let closure_body = cx.tcx.hir_body(body)
+            && let Some(closure_arg) = closure_body.params.first()
+        {
+            if let PatKind::Ref(..) = closure_arg.pat.kind {
+                Some(search_snippet.replacen('&', "", 1))
+            } else if let PatKind::Binding(..) = strip_pat_refs(closure_arg.pat).kind {
+                // `find()` provides a reference to the item, but `any` does not,
+                // so we should fix item usages for suggestion
+                if let Some(closure_sugg) = deref_closure_args(cx, search_arg) {
+                    applicability = closure_sugg.applicability;
+                    Some(closure_sugg.suggestion)
                 } else {
-                    None
+                    Some(search_snippet.to_string())
                 }
             } else {
                 None
-            };
-            // add note if not multi-line
-            if is_some {
-                span_lint_and_sugg(
-                    cx,
-                    SEARCH_IS_SOME,
-                    method_span.with_hi(expr.span.hi()),
-                    msg,
-                    "consider using",
-                    format!(
-                        "any({})",
-                        any_search_snippet.as_ref().map_or(&*search_snippet, String::as_str)
-                    ),
-                    applicability,
-                );
-            } else {
-                let iter = snippet(cx, search_recv.span, "..");
-                let sugg = if is_receiver_of_method_call(cx, expr) {
-                    format!(
-                        "(!{iter}.any({}))",
-                        any_search_snippet.as_ref().map_or(&*search_snippet, String::as_str)
-                    )
-                } else {
-                    format!(
-                        "!{iter}.any({})",
-                        any_search_snippet.as_ref().map_or(&*search_snippet, String::as_str)
-                    )
-                };
-                span_lint_and_sugg(
-                    cx,
-                    SEARCH_IS_SOME,
-                    expr.span,
-                    msg,
-                    "consider using",
-                    sugg,
-                    applicability,
-                );
             }
         } else {
-            let hint = format!(
-                "this is more succinctly expressed by calling `any()`{}",
-                if option_check_method == "is_none" {
-                    " with negation"
-                } else {
-                    ""
-                }
+            None
+        };
+        // add note if not multi-line
+        if is_some {
+            span_lint_and_sugg(
+                cx,
+                SEARCH_IS_SOME,
+                method_span.with_hi(expr.span.hi()),
+                msg,
+                "consider using",
+                format!(
+                    "any({})",
+                    any_search_snippet.as_ref().map_or(&*search_snippet, String::as_str)
+                ),
+                applicability,
+            );
+        } else {
+            let iter = snippet(cx, search_recv.span, "..");
+            let sugg = if is_receiver_of_method_call(cx, expr) {
+                format!(
+                    "(!{iter}.any({}))",
+                    any_search_snippet.as_ref().map_or(&*search_snippet, String::as_str)
+                )
+            } else {
+                format!(
+                    "!{iter}.any({})",
+                    any_search_snippet.as_ref().map_or(&*search_snippet, String::as_str)
+                )
+            };
+            span_lint_and_sugg(
+                cx,
+                SEARCH_IS_SOME,
+                expr.span,
+                msg,
+                "consider using",
+                sugg,
+                applicability,
             );
-            span_lint_and_help(cx, SEARCH_IS_SOME, expr.span, msg, None, hint);
         }
     }
     // lint if `find()` is called by `String` or `&str`

clippy_utils/src/sugg.rs

@@ -765,7 +765,7 @@ pub struct DerefClosure {
 /// such as explicit deref and borrowing cases.
 /// Returns `None` if no such use cases have been triggered in closure body
 ///
-/// note: this only works on single line immutable closures with exactly one input parameter.
+/// note: This only works on immutable closures with exactly one input parameter.
 pub fn deref_closure_args(cx: &LateContext<'_>, closure: &hir::Expr<'_>) -> Option<DerefClosure> {
     if let ExprKind::Closure(&Closure {
         fn_decl, def_id, body, ..

tests/ui/search_is_some.rs

@@ -8,31 +8,6 @@ use option_helpers::IteratorFalsePositives;
 //@no-rustfix
 #[rustfmt::skip]
 fn main() {
-    let v = vec![3, 2, 1, 0, -1, -2, -3];
-    let y = &&42;
-
-
-    // Check `find().is_some()`, multi-line case.
-    let _ = v.iter().find(|&x| {
-    //~^ search_is_some
-                              *x < 0
-                          }
-                   ).is_some();
-
-    // Check `position().is_some()`, multi-line case.
-    let _ = v.iter().position(|&x| {
-    //~^ search_is_some
-                                  x < 0
-                              }
-                   ).is_some();
-
-    // Check `rposition().is_some()`, multi-line case.
-    let _ = v.iter().rposition(|&x| {
-    //~^ search_is_some
-                                   x < 0
-                               }
-                   ).is_some();
-
     // Check that we don't lint if the caller is not an `Iterator` or string
     let falsepos = IteratorFalsePositives { foo: 0 };
     let _ = falsepos.find().is_some();
@@ -49,31 +24,6 @@ fn main() {
 
 #[rustfmt::skip]
 fn is_none() {
-    let v = vec![3, 2, 1, 0, -1, -2, -3];
-    let y = &&42;
-
-
-    // Check `find().is_none()`, multi-line case.
-    let _ = v.iter().find(|&x| {
-    //~^ search_is_some
-                              *x < 0
-                          }
-                   ).is_none();
-
-    // Check `position().is_none()`, multi-line case.
-    let _ = v.iter().position(|&x| {
-    //~^ search_is_some
-                                  x < 0
-                              }
-                   ).is_none();
-
-    // Check `rposition().is_none()`, multi-line case.
-    let _ = v.iter().rposition(|&x| {
-    //~^ search_is_some
-                                   x < 0
-                               }
-                   ).is_none();
-
     // Check that we don't lint if the caller is not an `Iterator` or string
     let falsepos = IteratorFalsePositives { foo: 0 };
     let _ = falsepos.find().is_none();
@@ -87,18 +37,3 @@ fn is_none() {
     let _ = (0..1).find(some_closure).is_none();
     //~^ search_is_some
 }
-
-#[allow(clippy::match_like_matches_macro)]
-fn issue15102() {
-    let values = [None, Some(3)];
-    let has_even = values
-        //~^ search_is_some
-        .iter()
-        .find(|v| match v {
-            Some(x) if x % 2 == 0 => true,
-            _ => false,
-        })
-        .is_some();
-
-    println!("{has_even}");
-}

tests/ui/search_is_some.stderr

@@ -1,109 +1,17 @@
 error: called `is_some()` after searching an `Iterator` with `find`
-  --> tests/ui/search_is_some.rs:16:13
-   |
-LL |       let _ = v.iter().find(|&x| {
-   |  _____________^
-LL | |
-LL | |                               *x < 0
-LL | |                           }
-LL | |                    ).is_some();
-   | |______________________________^
-   |
-   = help: this is more succinctly expressed by calling `any()`
-   = note: `-D clippy::search-is-some` implied by `-D warnings`
-   = help: to override `-D warnings` add `#[allow(clippy::search_is_some)]`
-
-error: called `is_some()` after searching an `Iterator` with `position`
-  --> tests/ui/search_is_some.rs:23:13
-   |
-LL |       let _ = v.iter().position(|&x| {
-   |  _____________^
-LL | |
-LL | |                                   x < 0
-LL | |                               }
-LL | |                    ).is_some();
-   | |______________________________^
-   |
-   = help: this is more succinctly expressed by calling `any()`
-
-error: called `is_some()` after searching an `Iterator` with `rposition`
-  --> tests/ui/search_is_some.rs:30:13
-   |
-LL |       let _ = v.iter().rposition(|&x| {
-   |  _____________^
-LL | |
-LL | |                                    x < 0
-LL | |                                }
-LL | |                    ).is_some();
-   | |______________________________^
-   |
-   = help: this is more succinctly expressed by calling `any()`
-
-error: called `is_some()` after searching an `Iterator` with `find`
-  --> tests/ui/search_is_some.rs:46:20
+  --> tests/ui/search_is_some.rs:21:20
    |
 LL |     let _ = (0..1).find(some_closure).is_some();
    |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `any(some_closure)`
-
-error: called `is_none()` after searching an `Iterator` with `find`
-  --> tests/ui/search_is_some.rs:57:13
    |
-LL |       let _ = v.iter().find(|&x| {
-   |  _____________^
-LL | |
-LL | |                               *x < 0
-LL | |                           }
-LL | |                    ).is_none();
-   | |______________________________^
-   |
-   = help: this is more succinctly expressed by calling `any()` with negation
-
-error: called `is_none()` after searching an `Iterator` with `position`
-  --> tests/ui/search_is_some.rs:64:13
-   |
-LL |       let _ = v.iter().position(|&x| {
-   |  _____________^
-LL | |
-LL | |                                   x < 0
-LL | |                               }
-LL | |                    ).is_none();
-   | |______________________________^
-   |
-   = help: this is more succinctly expressed by calling `any()` with negation
-
-error: called `is_none()` after searching an `Iterator` with `rposition`
-  --> tests/ui/search_is_some.rs:71:13
-   |
-LL |       let _ = v.iter().rposition(|&x| {
-   |  _____________^
-LL | |
-LL | |                                    x < 0
-LL | |                                }
-LL | |                    ).is_none();
-   | |______________________________^
-   |
-   = help: this is more succinctly expressed by calling `any()` with negation
+   = note: `-D clippy::search-is-some` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::search_is_some)]`
 
 error: called `is_none()` after searching an `Iterator` with `find`
-  --> tests/ui/search_is_some.rs:87:13
+  --> tests/ui/search_is_some.rs:37:13
    |
 LL |     let _ = (0..1).find(some_closure).is_none();
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `!(0..1).any(some_closure)`
 
-error: called `is_some()` after searching an `Iterator` with `find`
-  --> tests/ui/search_is_some.rs:94:20
-   |
-LL |       let has_even = values
-   |  ____________________^
-LL | |
-LL | |         .iter()
-LL | |         .find(|v| match v {
-...  |
-LL | |         })
-LL | |         .is_some();
-   | |__________________^
-   |
-   = help: this is more succinctly expressed by calling `any()`
-
-error: aborting due to 9 previous errors
+error: aborting due to 2 previous errors
 

tests/ui/search_is_some_fixable_none.fixed

@@ -24,14 +24,32 @@ fn main() {
     let _ = !(1..3).any(|x| [1, 2, 3].contains(&x) || x == 0);
     //~^ search_is_some
     let _ = !(1..3).any(|x| [1, 2, 3].contains(&x) || x == 0 || [4, 5, 6].contains(&x) || x == -1);
+    // Check `find().is_none()`, multi-line case.
+    let _ = !v
+        //~^ search_is_some
+        .iter().any(|x| {
+            *x < 0 //
+        });
 
     // Check `position().is_none()`, single-line case.
     let _ = !v.iter().any(|&x| x < 0);
     //~^ search_is_some
+    // Check `position().is_none()`, multi-line case.
+    let _ = !v
+        //~^ search_is_some
+        .iter().any(|&x| {
+            x < 0 //
+        });
 
     // Check `rposition().is_none()`, single-line case.
     let _ = !v.iter().any(|&x| x < 0);
     //~^ search_is_some
+    // Check `rposition().is_none()`, multi-line case.
+    let _ = !v
+        //~^ search_is_some
+        .iter().any(|&x| {
+            x < 0 //
+        });
 
     let s1 = String::from("hello world");
     let s2 = String::from("world");