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

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: Jarcho

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");