Auto merge of rust-lang#12094 - yuxqiu:search_is_some, r=xFrednet,ARa…

…ndomDev99

fix: incorrect suggestions when `.then` and `.then_some` is used

fixes rust-lang#11910

In the current implementation of `search_is_some`, if a `.is_none` call is followed by a `.then` or `.then_some` call, the generated `!` will incorrectly negate the values returned by the `then` and `.then_some` calls. To fix this, we need to add parentheses to the generated suggestions when appropriate.

changelog: [`search_is_some`]: add parenthesis to suggestions when appropriate

clippy_lints/src/methods/search_is_some.rs

@@ -2,7 +2,8 @@ use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg};
 use clippy_utils::source::{snippet, snippet_with_applicability};
 use clippy_utils::sugg::deref_closure_args;
 use clippy_utils::ty::is_type_lang_item;
-use clippy_utils::{is_trait_method, strip_pat_refs};
+use clippy_utils::{get_parent_expr, is_trait_method, strip_pat_refs};
+use hir::ExprKind;
 use rustc_errors::Applicability;
 use rustc_hir as hir;
 use rustc_hir::PatKind;
@@ -35,7 +36,7 @@ pub(super) fn check<'tcx>(
             // suggest `any(|..| *..)` instead of `any(|..| **..)` for `find(|..| **..).is_some()`
             let mut applicability = Applicability::MachineApplicable;
             let any_search_snippet = if search_method == "find"
-                && let hir::ExprKind::Closure(&hir::Closure { body, .. }) = search_arg.kind
+                && 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()
             {
@@ -72,16 +73,24 @@ pub(super) fn check<'tcx>(
                 );
             } 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",
-                    format!(
-                        "!{iter}.any({})",
-                        any_search_snippet.as_ref().map_or(&*search_snippet, String::as_str)
-                    ),
+                    sugg,
                     applicability,
                 );
             }
@@ -127,13 +136,18 @@ pub(super) fn check<'tcx>(
                     let string = snippet(cx, search_recv.span, "..");
                     let mut applicability = Applicability::MachineApplicable;
                     let find_arg = snippet_with_applicability(cx, search_arg.span, "..", &mut applicability);
+                    let sugg = if is_receiver_of_method_call(cx, expr) {
+                        format!("(!{string}.contains({find_arg}))")
+                    } else {
+                        format!("!{string}.contains({find_arg})")
+                    };
                     span_lint_and_sugg(
                         cx,
                         SEARCH_IS_SOME,
                         expr.span,
                         msg,
                         "consider using",
-                        format!("!{string}.contains({find_arg})"),
+                        sugg,
                         applicability,
                     );
                 },
@@ -142,3 +156,13 @@ pub(super) fn check<'tcx>(
         }
     }
 }
+
+fn is_receiver_of_method_call(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> bool {
+    if let Some(parent_expr) = get_parent_expr(cx, expr)
+        && let ExprKind::MethodCall(_, receiver, ..) = parent_expr.kind
+        && receiver.hir_id == expr.hir_id
+    {
+        return true;
+    }
+    false
+}

tests/ui/search_is_some_fixable_none.fixed

@@ -213,3 +213,53 @@ mod issue7392 {
         let _ = !v.iter().any(|fp| test_u32_2(*fp.field));
     }
 }
+
+mod issue_11910 {
+    fn computations() -> u32 {
+        0
+    }
+
+    struct Foo;
+    impl Foo {
+        fn bar(&self, _: bool) {}
+    }
+
+    fn test_normal_for_iter() {
+        let v = vec![3, 2, 1, 0, -1, -2, -3];
+        let _ = !v.iter().any(|x| *x == 42);
+        Foo.bar(!v.iter().any(|x| *x == 42));
+    }
+
+    fn test_then_for_iter() {
+        let v = vec![3, 2, 1, 0, -1, -2, -3];
+        (!v.iter().any(|x| *x == 42)).then(computations);
+    }
+
+    fn test_then_some_for_iter() {
+        let v = vec![3, 2, 1, 0, -1, -2, -3];
+        (!v.iter().any(|x| *x == 42)).then_some(0);
+    }
+
+    fn test_normal_for_str() {
+        let s = "hello";
+        let _ = !s.contains("world");
+        Foo.bar(!s.contains("world"));
+        let s = String::from("hello");
+        let _ = !s.contains("world");
+        Foo.bar(!s.contains("world"));
+    }
+
+    fn test_then_for_str() {
+        let s = "hello";
+        let _ = (!s.contains("world")).then(computations);
+        let s = String::from("hello");
+        let _ = (!s.contains("world")).then(computations);
+    }
+
+    fn test_then_some_for_str() {
+        let s = "hello";
+        let _ = (!s.contains("world")).then_some(0);
+        let s = String::from("hello");
+        let _ = (!s.contains("world")).then_some(0);
+    }
+}

tests/ui/search_is_some_fixable_none.rs

@@ -219,3 +219,53 @@ mod issue7392 {
         let _ = v.iter().find(|fp| test_u32_2(*fp.field)).is_none();
     }
 }
+
+mod issue_11910 {
+    fn computations() -> u32 {
+        0
+    }
+
+    struct Foo;
+    impl Foo {
+        fn bar(&self, _: bool) {}
+    }
+
+    fn test_normal_for_iter() {
+        let v = vec![3, 2, 1, 0, -1, -2, -3];
+        let _ = v.iter().find(|x| **x == 42).is_none();
+        Foo.bar(v.iter().find(|x| **x == 42).is_none());
+    }
+
+    fn test_then_for_iter() {
+        let v = vec![3, 2, 1, 0, -1, -2, -3];
+        v.iter().find(|x| **x == 42).is_none().then(computations);
+    }
+
+    fn test_then_some_for_iter() {
+        let v = vec![3, 2, 1, 0, -1, -2, -3];
+        v.iter().find(|x| **x == 42).is_none().then_some(0);
+    }
+
+    fn test_normal_for_str() {
+        let s = "hello";
+        let _ = s.find("world").is_none();
+        Foo.bar(s.find("world").is_none());
+        let s = String::from("hello");
+        let _ = s.find("world").is_none();
+        Foo.bar(s.find("world").is_none());
+    }
+
+    fn test_then_for_str() {
+        let s = "hello";
+        let _ = s.find("world").is_none().then(computations);
+        let s = String::from("hello");
+        let _ = s.find("world").is_none().then(computations);
+    }
+
+    fn test_then_some_for_str() {
+        let s = "hello";
+        let _ = s.find("world").is_none().then_some(0);
+        let s = String::from("hello");
+        let _ = s.find("world").is_none().then_some(0);
+    }
+}

tests/ui/search_is_some_fixable_none.stderr

@@ -282,5 +282,77 @@ error: called `is_none()` after searching an `Iterator` with `find`
 LL |         let _ = v.iter().find(|fp| test_u32_2(*fp.field)).is_none();
    |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `!v.iter().any(|fp| test_u32_2(*fp.field))`
 
-error: aborting due to 43 previous errors
+error: called `is_none()` after searching an `Iterator` with `find`
+  --> tests/ui/search_is_some_fixable_none.rs:235:17
+   |
+LL |         let _ = v.iter().find(|x| **x == 42).is_none();
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `!v.iter().any(|x| *x == 42)`
+
+error: called `is_none()` after searching an `Iterator` with `find`
+  --> tests/ui/search_is_some_fixable_none.rs:236:17
+   |
+LL |         Foo.bar(v.iter().find(|x| **x == 42).is_none());
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `!v.iter().any(|x| *x == 42)`
+
+error: called `is_none()` after searching an `Iterator` with `find`
+  --> tests/ui/search_is_some_fixable_none.rs:241:9
+   |
+LL |         v.iter().find(|x| **x == 42).is_none().then(computations);
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `(!v.iter().any(|x| *x == 42))`
+
+error: called `is_none()` after searching an `Iterator` with `find`
+  --> tests/ui/search_is_some_fixable_none.rs:246:9
+   |
+LL |         v.iter().find(|x| **x == 42).is_none().then_some(0);
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `(!v.iter().any(|x| *x == 42))`
+
+error: called `is_none()` after calling `find()` on a string
+  --> tests/ui/search_is_some_fixable_none.rs:251:17
+   |
+LL |         let _ = s.find("world").is_none();
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `!s.contains("world")`
+
+error: called `is_none()` after calling `find()` on a string
+  --> tests/ui/search_is_some_fixable_none.rs:252:17
+   |
+LL |         Foo.bar(s.find("world").is_none());
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `!s.contains("world")`
+
+error: called `is_none()` after calling `find()` on a string
+  --> tests/ui/search_is_some_fixable_none.rs:254:17
+   |
+LL |         let _ = s.find("world").is_none();
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `!s.contains("world")`
+
+error: called `is_none()` after calling `find()` on a string
+  --> tests/ui/search_is_some_fixable_none.rs:255:17
+   |
+LL |         Foo.bar(s.find("world").is_none());
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `!s.contains("world")`
+
+error: called `is_none()` after calling `find()` on a string
+  --> tests/ui/search_is_some_fixable_none.rs:260:17
+   |
+LL |         let _ = s.find("world").is_none().then(computations);
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `(!s.contains("world"))`
+
+error: called `is_none()` after calling `find()` on a string
+  --> tests/ui/search_is_some_fixable_none.rs:262:17
+   |
+LL |         let _ = s.find("world").is_none().then(computations);
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `(!s.contains("world"))`
+
+error: called `is_none()` after calling `find()` on a string
+  --> tests/ui/search_is_some_fixable_none.rs:267:17
+   |
+LL |         let _ = s.find("world").is_none().then_some(0);
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `(!s.contains("world"))`
+
+error: called `is_none()` after calling `find()` on a string
+  --> tests/ui/search_is_some_fixable_none.rs:269:17
+   |
+LL |         let _ = s.find("world").is_none().then_some(0);
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `(!s.contains("world"))`
+
+error: aborting due to 55 previous errors