diff --git a/clippy_lints/src/methods/search_is_some.rs b/clippy_lints/src/methods/search_is_some.rs index ac5cc2f01e53f..f5f1e94bbf455 100644 --- a/clippy_lints/src/methods/search_is_some.rs +++ b/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 +} diff --git a/tests/ui/search_is_some_fixable_none.fixed b/tests/ui/search_is_some_fixable_none.fixed index 51636392f2bc0..86a937b4dae1c 100644 --- a/tests/ui/search_is_some_fixable_none.fixed +++ b/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); + } +} diff --git a/tests/ui/search_is_some_fixable_none.rs b/tests/ui/search_is_some_fixable_none.rs index c7d773e18a321..c0103a015097d 100644 --- a/tests/ui/search_is_some_fixable_none.rs +++ b/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); + } +} diff --git a/tests/ui/search_is_some_fixable_none.stderr b/tests/ui/search_is_some_fixable_none.stderr index 4ad1e2508c480..2c858b9fb10ef 100644 --- a/tests/ui/search_is_some_fixable_none.stderr +++ b/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