diff --git a/clippy_lints/src/methods/search_is_some.rs b/clippy_lints/src/methods/search_is_some.rs
index 98ad7f3866f..25a2c48e269 100644
--- a/clippy_lints/src/methods/search_is_some.rs
+++ b/clippy_lints/src/methods/search_is_some.rs
@@ -1,3 +1,5 @@
+use std::iter;
+
use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg};
use clippy_utils::source::{snippet, snippet_with_applicability};
use clippy_utils::ty::is_type_diagnostic_item;
@@ -223,6 +225,26 @@ impl DerefDelegate<'_, 'tcx> {
let end_snip = snippet_with_applicability(self.cx, end_span, "..", &mut self.applicability);
format!("{}{}", self.suggestion_start, end_snip)
}
+
+ fn func_takes_arg_by_ref(&self, parent_expr: &'tcx hir::Expr<'_>, cmt_hir_id: HirId) -> bool {
+ if_chain! {
+ if let ExprKind::Call(func, call_args) = parent_expr.kind;
+ let typ = self.cx.typeck_results().expr_ty(func);
+ if let ty::FnDef(..) = typ.kind();
+
+ then {
+ let mut takes_by_ref = false;
+ for (arg, ty) in iter::zip(call_args, typ.fn_sig(self.cx.tcx).skip_binder().inputs()) {
+ if arg.hir_id == cmt_hir_id {
+ takes_by_ref = matches!(ty.kind(), ty::Ref(_, inner, _) if inner.is_ref());
+ }
+ }
+ takes_by_ref
+ } else {
+ false
+ }
+ }
+ }
}
impl<'tcx> Delegate<'tcx> for DerefDelegate<'_, 'tcx> {
@@ -252,18 +274,23 @@ impl<'tcx> Delegate<'tcx> for DerefDelegate<'_, 'tcx> {
let start_span = Span::new(self.next_pos, span.lo(), span.ctxt());
let start_snip =
snippet_with_applicability(self.cx, start_span, "..", &mut self.applicability);
+
+ // suggest ampersand if call function is taking args by ref
+ let takes_arg_by_ref = self.func_takes_arg_by_ref(parent_expr, cmt.hir_id);
+
// do not suggest ampersand if the ident is the method caller
- let ident_sugg = if !call_args.is_empty() && call_args[0].hir_id == cmt.hir_id {
- format!("{}{}", start_snip, ident_str)
- } else {
- format!("{}&{}", start_snip, ident_str)
- };
+ let ident_sugg =
+ if !call_args.is_empty() && call_args[0].hir_id == cmt.hir_id && !takes_arg_by_ref {
+ format!("{}{}", start_snip, ident_str)
+ } else {
+ format!("{}&{}", start_snip, ident_str)
+ };
self.suggestion_start.push_str(&ident_sugg);
self.next_pos = span.hi();
return;
- } else {
- self.applicability = Applicability::Unspecified;
}
+
+ self.applicability = Applicability::Unspecified;
}
}
@@ -271,23 +298,8 @@ impl<'tcx> Delegate<'tcx> for DerefDelegate<'_, 'tcx> {
// i.e.: suggest `*x` instead of `**x`
let mut replacement_str = ident_str;
- // handle index projection first
- let index_handled = cmt.place.projections.iter().any(|proj| match proj.kind {
- // Index projection like `|x| foo[x]`
- // the index is dropped so we can't get it to build the suggestion,
- // so the span is set-up again to get more code, using `span.hi()` (i.e.: `foo[x]`)
- // instead of `span.lo()` (i.e.: `foo`)
- ProjectionKind::Index => {
- let start_span = Span::new(self.next_pos, span.hi(), span.ctxt());
- start_snip = snippet_with_applicability(self.cx, start_span, "..", &mut self.applicability);
- replacement_str.clear();
- true
- },
- _ => false,
- });
-
- // looking for projections other that need to be handled differently
- let other_projections_handled = cmt.place.projections.iter().enumerate().any(|(i, proj)| {
+ let mut projections_handled = false;
+ cmt.place.projections.iter().enumerate().for_each(|(i, proj)| {
match proj.kind {
// Field projection like `|v| v.foo`
ProjectionKind::Field(idx, variant) => match cmt.place.ty_before_projection(i).kind() {
@@ -297,34 +309,41 @@ impl<'tcx> Delegate<'tcx> for DerefDelegate<'_, 'tcx> {
replacement_str,
def.variants[variant].fields[idx as usize].ident.name.as_str()
);
- true
+ projections_handled = true;
},
ty::Tuple(_) => {
replacement_str = format!("{}.{}", replacement_str, idx);
- true
+ projections_handled = true;
},
- _ => false,
+ _ => (),
},
- // handled previously
- ProjectionKind::Index |
- // note: unable to trigger `Subslice` kind in tests
- ProjectionKind::Subslice => false,
+ // Index projection like `|x| foo[x]`
+ // the index is dropped so we can't get it to build the suggestion,
+ // so the span is set-up again to get more code, using `span.hi()` (i.e.: `foo[x]`)
+ // instead of `span.lo()` (i.e.: `foo`)
+ ProjectionKind::Index => {
+ let start_span = Span::new(self.next_pos, span.hi(), span.ctxt());
+ start_snip = snippet_with_applicability(self.cx, start_span, "..", &mut self.applicability);
+ replacement_str.clear();
+ projections_handled = true;
+ },
+ // note: unable to trigger `Subslice` kind in tests
+ ProjectionKind::Subslice => (),
ProjectionKind::Deref => {
// explicit deref for arrays should be avoided in the suggestion
// i.e.: `|sub| *sub[1..4].len() == 3` is not expected
- match cmt.place.ty_before_projection(i).kind() {
+ if let ty::Ref(_, inner, _) = cmt.place.ty_before_projection(i).kind() {
// dereferencing an array (i.e.: `|sub| sub[1..4].len() == 3`)
- ty::Ref(_, inner, _) => {
- matches!(inner.kind(), ty::Ref(_, innermost, _) if innermost.is_array())
- },
- _ => false,
+ if matches!(inner.kind(), ty::Ref(_, innermost, _) if innermost.is_array()) {
+ projections_handled = true;
+ }
}
},
}
});
// handle `ProjectionKind::Deref` if no special case detected
- if !index_handled && !other_projections_handled {
+ if !projections_handled {
let last_deref = cmt
.place
.projections
diff --git a/tests/ui/search_is_some_fixable_none.fixed b/tests/ui/search_is_some_fixable_none.fixed
index c7c0b766015..6b8a614bbda 100644
--- a/tests/ui/search_is_some_fixable_none.fixed
+++ b/tests/ui/search_is_some_fixable_none.fixed
@@ -102,10 +102,14 @@ mod issue7392 {
*x == 9
}
- fn simple_fn(x: u32) -> bool {
+ fn deref_enough(x: u32) -> bool {
x == 78
}
+ fn arg_no_deref(x: &&u32) -> bool {
+ **x == 78
+ }
+
fn more_projections() {
let x = 19;
let ppx: &u32 = &x;
@@ -113,6 +117,48 @@ mod issue7392 {
let _ = ![String::from("Hey hey")].iter().any(|s| s.len() == 2);
let v = vec![3, 2, 1, 0];
- let _ = !v.iter().any(|x| simple_fn(*x));
+ let _ = !v.iter().any(|x| deref_enough(*x));
+
+ #[allow(clippy::redundant_closure)]
+ let _ = !v.iter().any(|x| arg_no_deref(&x));
+ }
+
+ fn field_index_projection() {
+ struct FooDouble {
+ bar: Vec<Vec<i32>>,
+ }
+ struct Foo {
+ bar: Vec<i32>,
+ }
+ struct FooOuter {
+ inner: Foo,
+ inner_double: FooDouble,
+ }
+ let vfoo = vec![FooOuter {
+ inner: Foo { bar: vec![0, 1, 2, 3] },
+ inner_double: FooDouble {
+ bar: vec![vec![0, 1, 2, 3]],
+ },
+ }];
+ let _ = !vfoo
+ .iter().any(|v| v.inner_double.bar[0][0] == 2 && v.inner.bar[0] == 2);
+ }
+
+ fn index_field_projection() {
+ struct Foo {
+ bar: i32,
+ }
+ struct FooOuter {
+ inner: Vec<Foo>,
+ }
+ let vfoo = vec![FooOuter {
+ inner: vec![Foo { bar: 0 }],
+ }];
+ let _ = !vfoo.iter().any(|v| v.inner[0].bar == 2);
+ }
+
+ fn double_deref_index_projection() {
+ let vfoo = vec![&&[0, 1, 2, 3]];
+ let _ = !vfoo.iter().any(|x| (**x)[0] == 9);
}
}
diff --git a/tests/ui/search_is_some_fixable_none.rs b/tests/ui/search_is_some_fixable_none.rs
index 6b41da4f2f8..acaae650921 100644
--- a/tests/ui/search_is_some_fixable_none.rs
+++ b/tests/ui/search_is_some_fixable_none.rs
@@ -106,10 +106,14 @@ mod issue7392 {
*x == 9
}
- fn simple_fn(x: u32) -> bool {
+ fn deref_enough(x: u32) -> bool {
x == 78
}
+ fn arg_no_deref(x: &&u32) -> bool {
+ **x == 78
+ }
+
fn more_projections() {
let x = 19;
let ppx: &u32 = &x;
@@ -117,6 +121,50 @@ mod issue7392 {
let _ = [String::from("Hey hey")].iter().find(|s| s.len() == 2).is_none();
let v = vec![3, 2, 1, 0];
- let _ = v.iter().find(|x| simple_fn(**x)).is_none();
+ let _ = v.iter().find(|x| deref_enough(**x)).is_none();
+
+ #[allow(clippy::redundant_closure)]
+ let _ = v.iter().find(|x| arg_no_deref(x)).is_none();
+ }
+
+ fn field_index_projection() {
+ struct FooDouble {
+ bar: Vec<Vec<i32>>,
+ }
+ struct Foo {
+ bar: Vec<i32>,
+ }
+ struct FooOuter {
+ inner: Foo,
+ inner_double: FooDouble,
+ }
+ let vfoo = vec![FooOuter {
+ inner: Foo { bar: vec![0, 1, 2, 3] },
+ inner_double: FooDouble {
+ bar: vec![vec![0, 1, 2, 3]],
+ },
+ }];
+ let _ = vfoo
+ .iter()
+ .find(|v| v.inner_double.bar[0][0] == 2 && v.inner.bar[0] == 2)
+ .is_none();
+ }
+
+ fn index_field_projection() {
+ struct Foo {
+ bar: i32,
+ }
+ struct FooOuter {
+ inner: Vec<Foo>,
+ }
+ let vfoo = vec![FooOuter {
+ inner: vec![Foo { bar: 0 }],
+ }];
+ let _ = vfoo.iter().find(|v| v.inner[0].bar == 2).is_none();
+ }
+
+ fn double_deref_index_projection() {
+ let vfoo = vec![&&[0, 1, 2, 3]];
+ let _ = vfoo.iter().find(|x| (**x)[0] == 9).is_none();
}
}
diff --git a/tests/ui/search_is_some_fixable_none.stderr b/tests/ui/search_is_some_fixable_none.stderr
index 4313a2d526c..64e81c6b94a 100644
--- a/tests/ui/search_is_some_fixable_none.stderr
+++ b/tests/ui/search_is_some_fixable_none.stderr
@@ -170,22 +170,56 @@ LL | let _ = vfoo.iter().find(|sub| sub[1..4].len() == 3).is_none();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `!_.any()` instead: `!vfoo.iter().any(|sub| sub[1..4].len() == 3)`
error: called `is_none()` after searching an `Iterator` with `find`
- --> $DIR/search_is_some_fixable_none.rs:116:17
+ --> $DIR/search_is_some_fixable_none.rs:120:17
|
LL | let _ = [ppx].iter().find(|ppp_x: &&&u32| please(**ppp_x)).is_none();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `!_.any()` instead: `![ppx].iter().any(|ppp_x: &&u32| please(ppp_x))`
error: called `is_none()` after searching an `Iterator` with `find`
- --> $DIR/search_is_some_fixable_none.rs:117:17
+ --> $DIR/search_is_some_fixable_none.rs:121:17
|
LL | let _ = [String::from("Hey hey")].iter().find(|s| s.len() == 2).is_none();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `!_.any()` instead: `![String::from("Hey hey")].iter().any(|s| s.len() == 2)`
error: called `is_none()` after searching an `Iterator` with `find`
- --> $DIR/search_is_some_fixable_none.rs:120:17
+ --> $DIR/search_is_some_fixable_none.rs:124:17
|
-LL | let _ = v.iter().find(|x| simple_fn(**x)).is_none();
- | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `!_.any()` instead: `!v.iter().any(|x| simple_fn(*x))`
+LL | let _ = v.iter().find(|x| deref_enough(**x)).is_none();
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `!_.any()` instead: `!v.iter().any(|x| deref_enough(*x))`
-error: aborting due to 29 previous errors
+error: called `is_none()` after searching an `Iterator` with `find`
+ --> $DIR/search_is_some_fixable_none.rs:127:17
+ |
+LL | let _ = v.iter().find(|x| arg_no_deref(x)).is_none();
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `!_.any()` instead: `!v.iter().any(|x| arg_no_deref(&x))`
+
+error: called `is_none()` after searching an `Iterator` with `find`
+ --> $DIR/search_is_some_fixable_none.rs:147:17
+ |
+LL | let _ = vfoo
+ | _________________^
+LL | | .iter()
+LL | | .find(|v| v.inner_double.bar[0][0] == 2 && v.inner.bar[0] == 2)
+LL | | .is_none();
+ | |______________________^
+ |
+help: use `!_.any()` instead
+ |
+LL ~ let _ = !vfoo
+LL ~ .iter().any(|v| v.inner_double.bar[0][0] == 2 && v.inner.bar[0] == 2);
+ |
+
+error: called `is_none()` after searching an `Iterator` with `find`
+ --> $DIR/search_is_some_fixable_none.rs:163:17
+ |
+LL | let _ = vfoo.iter().find(|v| v.inner[0].bar == 2).is_none();
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `!_.any()` instead: `!vfoo.iter().any(|v| v.inner[0].bar == 2)`
+
+error: called `is_none()` after searching an `Iterator` with `find`
+ --> $DIR/search_is_some_fixable_none.rs:168:17
+ |
+LL | let _ = vfoo.iter().find(|x| (**x)[0] == 9).is_none();
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `!_.any()` instead: `!vfoo.iter().any(|x| (**x)[0] == 9)`
+
+error: aborting due to 33 previous errors
diff --git a/tests/ui/search_is_some_fixable_some.fixed b/tests/ui/search_is_some_fixable_some.fixed
index 833168cf242..7eb07d639a5 100644
--- a/tests/ui/search_is_some_fixable_some.fixed
+++ b/tests/ui/search_is_some_fixable_some.fixed
@@ -103,10 +103,14 @@ mod issue7392 {
*x == 9
}
- fn simple_fn(x: u32) -> bool {
+ fn deref_enough(x: u32) -> bool {
x == 78
}
+ fn arg_no_deref(x: &&u32) -> bool {
+ **x == 78
+ }
+
fn more_projections() {
let x = 19;
let ppx: &u32 = &x;
@@ -114,6 +118,49 @@ mod issue7392 {
let _ = [String::from("Hey hey")].iter().any(|s| s.len() == 2);
let v = vec![3, 2, 1, 0];
- let _ = v.iter().any(|x| simple_fn(*x));
+ let _ = v.iter().any(|x| deref_enough(*x));
+
+ #[allow(clippy::redundant_closure)]
+ let _ = v.iter().any(|x| arg_no_deref(&x));
+ }
+
+ fn field_index_projection() {
+ struct FooDouble {
+ bar: Vec<Vec<i32>>,
+ }
+ struct Foo {
+ bar: Vec<i32>,
+ }
+ struct FooOuter {
+ inner: Foo,
+ inner_double: FooDouble,
+ }
+ let vfoo = vec![FooOuter {
+ inner: Foo { bar: vec![0, 1, 2, 3] },
+ inner_double: FooDouble {
+ bar: vec![vec![0, 1, 2, 3]],
+ },
+ }];
+ let _ = vfoo
+ .iter()
+ .any(|v| v.inner_double.bar[0][0] == 2 && v.inner.bar[0] == 2);
+ }
+
+ fn index_field_projection() {
+ struct Foo {
+ bar: i32,
+ }
+ struct FooOuter {
+ inner: Vec<Foo>,
+ }
+ let vfoo = vec![FooOuter {
+ inner: vec![Foo { bar: 0 }],
+ }];
+ let _ = vfoo.iter().any(|v| v.inner[0].bar == 2);
+ }
+
+ fn double_deref_index_projection() {
+ let vfoo = vec![&&[0, 1, 2, 3]];
+ let _ = vfoo.iter().any(|x| (**x)[0] == 9);
}
}
diff --git a/tests/ui/search_is_some_fixable_some.rs b/tests/ui/search_is_some_fixable_some.rs
index f9fb241de3f..b9ebd399a35 100644
--- a/tests/ui/search_is_some_fixable_some.rs
+++ b/tests/ui/search_is_some_fixable_some.rs
@@ -105,10 +105,14 @@ mod issue7392 {
*x == 9
}
- fn simple_fn(x: u32) -> bool {
+ fn deref_enough(x: u32) -> bool {
x == 78
}
+ fn arg_no_deref(x: &&u32) -> bool {
+ **x == 78
+ }
+
fn more_projections() {
let x = 19;
let ppx: &u32 = &x;
@@ -116,6 +120,50 @@ mod issue7392 {
let _ = [String::from("Hey hey")].iter().find(|s| s.len() == 2).is_some();
let v = vec![3, 2, 1, 0];
- let _ = v.iter().find(|x| simple_fn(**x)).is_some();
+ let _ = v.iter().find(|x| deref_enough(**x)).is_some();
+
+ #[allow(clippy::redundant_closure)]
+ let _ = v.iter().find(|x| arg_no_deref(x)).is_some();
+ }
+
+ fn field_index_projection() {
+ struct FooDouble {
+ bar: Vec<Vec<i32>>,
+ }
+ struct Foo {
+ bar: Vec<i32>,
+ }
+ struct FooOuter {
+ inner: Foo,
+ inner_double: FooDouble,
+ }
+ let vfoo = vec![FooOuter {
+ inner: Foo { bar: vec![0, 1, 2, 3] },
+ inner_double: FooDouble {
+ bar: vec![vec![0, 1, 2, 3]],
+ },
+ }];
+ let _ = vfoo
+ .iter()
+ .find(|v| v.inner_double.bar[0][0] == 2 && v.inner.bar[0] == 2)
+ .is_some();
+ }
+
+ fn index_field_projection() {
+ struct Foo {
+ bar: i32,
+ }
+ struct FooOuter {
+ inner: Vec<Foo>,
+ }
+ let vfoo = vec![FooOuter {
+ inner: vec![Foo { bar: 0 }],
+ }];
+ let _ = vfoo.iter().find(|v| v.inner[0].bar == 2).is_some();
+ }
+
+ fn double_deref_index_projection() {
+ let vfoo = vec![&&[0, 1, 2, 3]];
+ let _ = vfoo.iter().find(|x| (**x)[0] == 9).is_some();
}
}
diff --git a/tests/ui/search_is_some_fixable_some.stderr b/tests/ui/search_is_some_fixable_some.stderr
index 01b13cea2cd..b62cf9c4702 100644
--- a/tests/ui/search_is_some_fixable_some.stderr
+++ b/tests/ui/search_is_some_fixable_some.stderr
@@ -161,22 +161,48 @@ LL | let _ = vfoo.iter().find(|sub| sub[1..4].len() == 3).is_some();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `any()` instead: `any(|sub| sub[1..4].len() == 3)`
error: called `is_some()` after searching an `Iterator` with `find`
- --> $DIR/search_is_some_fixable_some.rs:115:30
+ --> $DIR/search_is_some_fixable_some.rs:119:30
|
LL | let _ = [ppx].iter().find(|ppp_x: &&&u32| please(**ppp_x)).is_some();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `any()` instead: `any(|ppp_x: &&u32| please(ppp_x))`
error: called `is_some()` after searching an `Iterator` with `find`
- --> $DIR/search_is_some_fixable_some.rs:116:50
+ --> $DIR/search_is_some_fixable_some.rs:120:50
|
LL | let _ = [String::from("Hey hey")].iter().find(|s| s.len() == 2).is_some();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `any()` instead: `any(|s| s.len() == 2)`
error: called `is_some()` after searching an `Iterator` with `find`
- --> $DIR/search_is_some_fixable_some.rs:119:26
+ --> $DIR/search_is_some_fixable_some.rs:123:26
|
-LL | let _ = v.iter().find(|x| simple_fn(**x)).is_some();
- | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `any()` instead: `any(|x| simple_fn(*x))`
+LL | let _ = v.iter().find(|x| deref_enough(**x)).is_some();
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `any()` instead: `any(|x| deref_enough(*x))`
-error: aborting due to 29 previous errors
+error: called `is_some()` after searching an `Iterator` with `find`
+ --> $DIR/search_is_some_fixable_some.rs:126:26
+ |
+LL | let _ = v.iter().find(|x| arg_no_deref(x)).is_some();
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `any()` instead: `any(|x| arg_no_deref(&x))`
+
+error: called `is_some()` after searching an `Iterator` with `find`
+ --> $DIR/search_is_some_fixable_some.rs:148:14
+ |
+LL | .find(|v| v.inner_double.bar[0][0] == 2 && v.inner.bar[0] == 2)
+ | ______________^
+LL | | .is_some();
+ | |______________________^ help: use `any()` instead: `any(|v| v.inner_double.bar[0][0] == 2 && v.inner.bar[0] == 2)`
+
+error: called `is_some()` after searching an `Iterator` with `find`
+ --> $DIR/search_is_some_fixable_some.rs:162:29
+ |
+LL | let _ = vfoo.iter().find(|v| v.inner[0].bar == 2).is_some();
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `any()` instead: `any(|v| v.inner[0].bar == 2)`
+
+error: called `is_some()` after searching an `Iterator` with `find`
+ --> $DIR/search_is_some_fixable_some.rs:167:29
+ |
+LL | let _ = vfoo.iter().find(|x| (**x)[0] == 9).is_some();
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `any()` instead: `any(|x| (**x)[0] == 9)`
+
+error: aborting due to 33 previous errors