mirror of
https://github.com/rust-lang/rust.git
synced 2024-11-27 09:14:20 +00:00
Auto merge of #12094 - yuxqiu:search_is_some, r=xFrednet,ARandomDev99
fix: incorrect suggestions when `.then` and `.then_some` is used fixes #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
This commit is contained in:
bors
commit
7063e3435c
@ -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
|
||||
}
|
||||
|
||||
@ -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);
|
||||
}
|
||||
}
|
||||
|
||||
@ -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);
|
||||
}
|
||||
}
|
||||
|
||||
@ -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
|
||||
|
||||
|
||||
Loading…
Reference in New Issue
Block a user