Auto merge of rust-lang#6942 - mgacek8:issue_6815_search_is_none, r=llogiq

search_is_some: add checking for `is_none()`

fixes: rust-lang#6815
changelog: search_is_some: add checking for `is_none()`.

To be honest I don't know what is the process of renaming the lints. Appreciate any feedback if that needs to be handled differently. Thanks!

Author: bors

clippy_lints/src/methods/mod.rs

@@ -588,26 +588,31 @@ declare_clippy_lint! {
 
 declare_clippy_lint! {
     /// **What it does:** Checks for an iterator or string search (such as `find()`,
-    /// `position()`, or `rposition()`) followed by a call to `is_some()`.
+    /// `position()`, or `rposition()`) followed by a call to `is_some()` or `is_none()`.
     ///
-    /// **Why is this bad?** Readability, this can be written more concisely as
-    /// `_.any(_)` or `_.contains(_)`.
+    /// **Why is this bad?** Readability, this can be written more concisely as:
+    /// * `_.any(_)`, or `_.contains(_)` for `is_some()`,
+    /// * `!_.any(_)`, or `!_.contains(_)` for `is_none()`.
     ///
     /// **Known problems:** None.
     ///
     /// **Example:**
     /// ```rust
-    /// # let vec = vec![1];
+    /// let vec = vec![1];
     /// vec.iter().find(|x| **x == 0).is_some();
+    ///
+    /// let _ = "hello world".find("world").is_none();
     /// ```
     /// Could be written as
     /// ```rust
-    /// # let vec = vec![1];
+    /// let vec = vec![1];
     /// vec.iter().any(|x| *x == 0);
+    ///
+    /// let _ = !"hello world".contains("world");
     /// ```
     pub SEARCH_IS_SOME,
     complexity,
-    "using an iterator or string search followed by `is_some()`, which is more succinctly expressed as a call to `any()` or `contains()`"
+    "using an iterator or string search followed by `is_some()` or `is_none()`, which is more succinctly expressed as a call to `any()` or `contains()` (with negation in case of `is_none()`)"
 }
 
 declare_clippy_lint! {
@@ -1720,12 +1725,42 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
             ["flat_map", "filter_map"] => filter_map_flat_map::check(cx, expr, arg_lists[1], arg_lists[0]),
             ["flat_map", ..] => flat_map_identity::check(cx, expr, arg_lists[0], method_spans[0]),
             ["flatten", "map"] => map_flatten::check(cx, expr, arg_lists[1]),
-            ["is_some", "find"] => search_is_some::check(cx, expr, "find", arg_lists[1], arg_lists[0], method_spans[1]),
-            ["is_some", "position"] => {
-                search_is_some::check(cx, expr, "position", arg_lists[1], arg_lists[0], method_spans[1])
+            [option_check_method, "find"] if "is_some" == *option_check_method || "is_none" == *option_check_method => {
+                search_is_some::check(
+                    cx,
+                    expr,
+                    "find",
+                    option_check_method,
+                    arg_lists[1],
+                    arg_lists[0],
+                    method_spans[1],
+                )
             },
-            ["is_some", "rposition"] => {
-                search_is_some::check(cx, expr, "rposition", arg_lists[1], arg_lists[0], method_spans[1])
+            [option_check_method, "position"]
+                if "is_some" == *option_check_method || "is_none" == *option_check_method =>
+            {
+                search_is_some::check(
+                    cx,
+                    expr,
+                    "position",
+                    option_check_method,
+                    arg_lists[1],
+                    arg_lists[0],
+                    method_spans[1],
+                )
+            },
+            [option_check_method, "rposition"]
+                if "is_some" == *option_check_method || "is_none" == *option_check_method =>
+            {
+                search_is_some::check(
+                    cx,
+                    expr,
+                    "rposition",
+                    option_check_method,
+                    arg_lists[1],
+                    arg_lists[0],
+                    method_spans[1],
+                )
             },
             ["extend", ..] => string_extend_chars::check(cx, expr, arg_lists[0]),
             ["count", "into_iter"] => iter_count::check(cx, expr, &arg_lists[1], "into_iter"),

clippy_lints/src/methods/search_is_some.rs

@@ -14,22 +14,23 @@ use rustc_span::symbol::sym;
 use super::SEARCH_IS_SOME;
 
 /// lint searching an Iterator followed by `is_some()`
-/// or calling `find()` on a string followed by `is_some()`
+/// or calling `find()` on a string followed by `is_some()` or `is_none()`
+#[allow(clippy::too_many_lines)]
 pub(super) fn check<'tcx>(
     cx: &LateContext<'tcx>,
     expr: &'tcx hir::Expr<'_>,
     search_method: &str,
+    option_check_method: &str,
     search_args: &'tcx [hir::Expr<'_>],
     is_some_args: &'tcx [hir::Expr<'_>],
     method_span: Span,
 ) {
     // lint if caller of search is an Iterator
     if is_trait_method(cx, &is_some_args[0], sym::Iterator) {
         let msg = format!(
-            "called `is_some()` after searching an `Iterator` with `{}`",
-            search_method
+            "called `{}()` after searching an `Iterator` with `{}`",
+            option_check_method, search_method
         );
-        let hint = "this is more succinctly expressed by calling `any()`";
         let search_snippet = snippet(cx, search_args[1].span, "..");
         if search_snippet.lines().count() <= 1 {
             // suggest `any(|x| ..)` instead of `any(|&x| ..)` for `find(|&x| ..).is_some()`
@@ -53,20 +54,49 @@ pub(super) fn check<'tcx>(
                 }
             };
             // add note if not multi-line
-            span_lint_and_sugg(
-                cx,
-                SEARCH_IS_SOME,
-                method_span.with_hi(expr.span.hi()),
-                &msg,
-                "use `any()` instead",
-                format!(
-                    "any({})",
-                    any_search_snippet.as_ref().map_or(&*search_snippet, String::as_str)
-                ),
-                Applicability::MachineApplicable,
-            );
+            match option_check_method {
+                "is_some" => {
+                    span_lint_and_sugg(
+                        cx,
+                        SEARCH_IS_SOME,
+                        method_span.with_hi(expr.span.hi()),
+                        &msg,
+                        "use `any()` instead",
+                        format!(
+                            "any({})",
+                            any_search_snippet.as_ref().map_or(&*search_snippet, String::as_str)
+                        ),
+                        Applicability::MachineApplicable,
+                    );
+                },
+                "is_none" => {
+                    let iter = snippet(cx, search_args[0].span, "..");
+                    span_lint_and_sugg(
+                        cx,
+                        SEARCH_IS_SOME,
+                        expr.span,
+                        &msg,
+                        "use `!_.any()` instead",
+                        format!(
+                            "!{}.any({})",
+                            iter,
+                            any_search_snippet.as_ref().map_or(&*search_snippet, String::as_str)
+                        ),
+                        Applicability::MachineApplicable,
+                    );
+                },
+                _ => (),
+            }
         } else {
-            span_lint_and_help(cx, SEARCH_IS_SOME, expr.span, &msg, None, hint);
+            let hint = format!(
+                "this is more succinctly expressed by calling `any()`{}",
+                if option_check_method == "is_none" {
+                    " with negation"
+                } else {
+                    ""
+                }
+            );
+            span_lint_and_help(cx, SEARCH_IS_SOME, expr.span, &msg, None, &hint);
         }
     }
     // lint if `find()` is called by `String` or `&str`
@@ -83,18 +113,37 @@ pub(super) fn check<'tcx>(
             if is_string_or_str_slice(&search_args[0]);
             if is_string_or_str_slice(&search_args[1]);
             then {
-                let msg = "called `is_some()` after calling `find()` on a string";
-                let mut applicability = Applicability::MachineApplicable;
-                let find_arg = snippet_with_applicability(cx, search_args[1].span, "..", &mut applicability);
-                span_lint_and_sugg(
-                    cx,
-                    SEARCH_IS_SOME,
-                    method_span.with_hi(expr.span.hi()),
-                    msg,
-                    "use `contains()` instead",
-                    format!("contains({})", find_arg),
-                    applicability,
-                );
+                let msg = format!("called `{}()` after calling `find()` on a string", option_check_method);
+                match option_check_method {
+                    "is_some" => {
+                        let mut applicability = Applicability::MachineApplicable;
+                        let find_arg = snippet_with_applicability(cx, search_args[1].span, "..", &mut applicability);
+                        span_lint_and_sugg(
+                            cx,
+                            SEARCH_IS_SOME,
+                            method_span.with_hi(expr.span.hi()),
+                            &msg,
+                            "use `contains()` instead",
+                            format!("contains({})", find_arg),
+                            applicability,
+                        );
+                    },
+                    "is_none" => {
+                        let string = snippet(cx, search_args[0].span, "..");
+                        let mut applicability = Applicability::MachineApplicable;
+                        let find_arg = snippet_with_applicability(cx, search_args[1].span, "..", &mut applicability);
+                        span_lint_and_sugg(
+                            cx,
+                            SEARCH_IS_SOME,
+                            expr.span,
+                            &msg,
+                            "use `!_.contains()` instead",
+                            format!("!{}.contains({})", string, find_arg),
+                            applicability,
+                        );
+                    },
+                    _ => (),
+                }
             }
         }
     }

tests/ui/search_is_some.rs

@@ -1,8 +1,9 @@
 // aux-build:option_helpers.rs
+#![warn(clippy::search_is_some)]
+#![allow(dead_code)]
 extern crate option_helpers;
 use option_helpers::IteratorFalsePositives;
 
-#[warn(clippy::search_is_some)]
 #[rustfmt::skip]
 fn main() {
     let v = vec![3, 2, 1, 0, -1, -2, -3];
@@ -36,3 +37,37 @@ fn main() {
     // `Pattern` that is not a string
     let _ = "hello world".find(|c: char| c == 'o' || c == 'l').is_some();
 }
+
+#[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| {
+                              *x < 0
+                          }
+                   ).is_none();
+
+    // Check `position().is_none()`, multi-line case.
+    let _ = v.iter().position(|&x| {
+                                  x < 0
+                              }
+                   ).is_none();
+
+    // Check `rposition().is_none()`, multi-line case.
+    let _ = v.iter().rposition(|&x| {
+                                   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();
+    let _ = falsepos.position().is_none();
+    let _ = falsepos.rposition().is_none();
+    // check that we don't lint if `find()` is called with
+    // `Pattern` that is not a string
+    let _ = "hello world".find(|c: char| c == 'o' || c == 'l').is_none();
+}

tests/ui/search_is_some.stderr

@@ -1,5 +1,5 @@
 error: called `is_some()` after searching an `Iterator` with `find`
-  --> $DIR/search_is_some.rs:13:13
+  --> $DIR/search_is_some.rs:14:13
    |
 LL |       let _ = v.iter().find(|&x| {
    |  _____________^
@@ -12,7 +12,7 @@ LL | |                    ).is_some();
    = help: this is more succinctly expressed by calling `any()`
 
 error: called `is_some()` after searching an `Iterator` with `position`
-  --> $DIR/search_is_some.rs:19:13
+  --> $DIR/search_is_some.rs:20:13
    |
 LL |       let _ = v.iter().position(|&x| {
    |  _____________^
@@ -24,7 +24,7 @@ LL | |                    ).is_some();
    = help: this is more succinctly expressed by calling `any()`
 
 error: called `is_some()` after searching an `Iterator` with `rposition`
-  --> $DIR/search_is_some.rs:25:13
+  --> $DIR/search_is_some.rs:26:13
    |
 LL |       let _ = v.iter().rposition(|&x| {
    |  _____________^
@@ -35,5 +35,41 @@ LL | |                    ).is_some();
    |
    = help: this is more succinctly expressed by calling `any()`
 
-error: aborting due to 3 previous errors
+error: called `is_none()` after searching an `Iterator` with `find`
+  --> $DIR/search_is_some.rs:48:13
+   |
+LL |       let _ = v.iter().find(|&x| {
+   |  _____________^
+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`
+  --> $DIR/search_is_some.rs:54:13
+   |
+LL |       let _ = v.iter().position(|&x| {
+   |  _____________^
+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`
+  --> $DIR/search_is_some.rs:60:13
+   |
+LL |       let _ = v.iter().rposition(|&x| {
+   |  _____________^
+LL | |                                    x < 0
+LL | |                                }
+LL | |                    ).is_none();
+   | |______________________________^
+   |
+   = help: this is more succinctly expressed by calling `any()` with negation
+
+error: aborting due to 6 previous errors
 

tests/ui/search_is_some_fixable.fixed

@@ -1,5 +1,5 @@
 // run-rustfix
-
+#![allow(dead_code)]
 #![warn(clippy::search_is_some)]
 
 fn main() {
@@ -33,3 +33,36 @@ fn main() {
     let _ = s1[2..].contains(&s2);
     let _ = s1[2..].contains(&s2[2..]);
 }
+
+fn is_none() {
+    let v = vec![3, 2, 1, 0, -1, -2, -3];
+    let y = &&42;
+
+    // Check `find().is_none()`, single-line case.
+    let _ = !v.iter().any(|x| *x < 0);
+    let _ = !(0..1).any(|x| **y == x); // one dereference less
+    let _ = !(0..1).any(|x| x == 0);
+    let _ = !v.iter().any(|x| *x == 0);
+
+    // Check `position().is_none()`, single-line case.
+    let _ = !v.iter().any(|&x| x < 0);
+
+    // Check `rposition().is_none()`, single-line case.
+    let _ = !v.iter().any(|&x| x < 0);
+
+    let s1 = String::from("hello world");
+    let s2 = String::from("world");
+
+    // caller of `find()` is a `&`static str`
+    let _ = !"hello world".contains("world");
+    let _ = !"hello world".contains(&s2);
+    let _ = !"hello world".contains(&s2[2..]);
+    // caller of `find()` is a `String`
+    let _ = !s1.contains("world");
+    let _ = !s1.contains(&s2);
+    let _ = !s1.contains(&s2[2..]);
+    // caller of `find()` is slice of `String`
+    let _ = !s1[2..].contains("world");
+    let _ = !s1[2..].contains(&s2);
+    let _ = !s1[2..].contains(&s2[2..]);
+}