From ac45a83ad5df284a4680f0c3c90d86f7a177e41f Mon Sep 17 00:00:00 2001 From: ThibsG Date: Tue, 14 Sep 2021 14:53:28 +0200 Subject: [PATCH] Handle multiple reference levels into binding type and add more tests --- clippy_lints/src/methods/search_is_some.rs | 29 +++++++++++++++++---- tests/ui/search_is_some_fixable_none.fixed | 11 ++++++++ tests/ui/search_is_some_fixable_none.rs | 11 ++++++++ tests/ui/search_is_some_fixable_none.stderr | 14 +++++++++- tests/ui/search_is_some_fixable_some.fixed | 11 ++++++++ tests/ui/search_is_some_fixable_some.rs | 11 ++++++++ tests/ui/search_is_some_fixable_some.stderr | 14 +++++++++- 7 files changed, 94 insertions(+), 7 deletions(-) diff --git a/clippy_lints/src/methods/search_is_some.rs b/clippy_lints/src/methods/search_is_some.rs index a5e6a8df0..dca4d84f6 100644 --- a/clippy_lints/src/methods/search_is_some.rs +++ b/clippy_lints/src/methods/search_is_some.rs @@ -50,12 +50,26 @@ pub(super) fn check<'tcx>( then { if let hir::PatKind::Ref(..) = closure_arg.pat.kind { Some(search_snippet.replacen('&', "", 1)) - } else if let PatKind::Binding(..) = strip_pat_refs(closure_arg.pat).kind { + } else if let PatKind::Binding(_, binding_id, _, _) = strip_pat_refs(closure_arg.pat).kind { + // this binding is composed of at least two levels of references, so we need to remove one + let binding_type = cx.typeck_results().node_type(binding_id); + let innermost_is_ref = if let ty::Ref(_, inner,_) = binding_type.kind() { + matches!(inner.kind(), ty::Ref(_, innermost, _) if innermost.is_ref()) + } else { + false + }; + // `find()` provides a reference to the item, but `any` does not, // so we should fix item usages for suggestion if let Some(closure_sugg) = get_closure_suggestion(cx, search_arg, closure_body) { applicability = closure_sugg.applicability; - Some(closure_sugg.suggestion) + if innermost_is_ref { + Some(closure_sugg.suggestion.replacen('&', "", 1)) + } else { + Some(closure_sugg.suggestion) + } + } else if innermost_is_ref { + Some(search_snippet.replacen('&', "", 1)) } else { Some(search_snippet.to_string()) } @@ -230,7 +244,7 @@ impl<'tcx> Delegate<'tcx> for DerefDelegate<'_, 'tcx> { // cases where a parent call is using the item // i.e.: suggest `.contains(&x)` for `.find(|x| [1, 2, 3].contains(x)).is_none()` if let Some(parent_expr) = get_parent_expr_for_hir(self.cx, cmt.hir_id) { - if let ExprKind::Call(..) | ExprKind::MethodCall(..) = parent_expr.kind { + if let ExprKind::Call(_, call_args) | ExprKind::MethodCall(_, _, call_args, _) = parent_expr.kind { let expr = self.cx.tcx.hir().expect_expr(cmt.hir_id); let arg_ty_kind = self.cx.typeck_results().expr_ty(expr).kind(); @@ -239,8 +253,13 @@ 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); - - self.suggestion_start.push_str(&format!("{}&{}", start_snip, ident_str)); + // 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) + }; + self.suggestion_start.push_str(&ident_sugg); self.next_pos = span.hi(); } else { self.applicability = Applicability::Unspecified; diff --git a/tests/ui/search_is_some_fixable_none.fixed b/tests/ui/search_is_some_fixable_none.fixed index 222731fd2..811bb1e31 100644 --- a/tests/ui/search_is_some_fixable_none.fixed +++ b/tests/ui/search_is_some_fixable_none.fixed @@ -97,4 +97,15 @@ mod issue7392 { let vfoo = vec![[0, 1, 2, 3, 0, 1, 2, 3]]; let _ = !vfoo.iter().any(|sub| sub[1..4].len() == 3); } + + fn please(x: &u32) -> bool { + *x == 9 + } + + fn more_projections() { + let x = 19; + let ppx: &u32 = &x; + let _ = ![ppx].iter().any(|ppp_x: &&u32| please(ppp_x)); + let _ = ![String::from("Hey hey")].iter().any(|s| s.len() == 2); + } } diff --git a/tests/ui/search_is_some_fixable_none.rs b/tests/ui/search_is_some_fixable_none.rs index 43ac1a076..c6fbb5e2d 100644 --- a/tests/ui/search_is_some_fixable_none.rs +++ b/tests/ui/search_is_some_fixable_none.rs @@ -101,4 +101,15 @@ mod issue7392 { let vfoo = vec![[0, 1, 2, 3, 0, 1, 2, 3]]; let _ = vfoo.iter().find(|sub| sub[1..4].len() == 3).is_none(); } + + fn please(x: &u32) -> bool { + *x == 9 + } + + fn more_projections() { + let x = 19; + let ppx: &u32 = &x; + let _ = [ppx].iter().find(|ppp_x: &&&u32| please(**ppp_x)).is_none(); + let _ = [String::from("Hey hey")].iter().find(|s| s.len() == 2).is_none(); + } } diff --git a/tests/ui/search_is_some_fixable_none.stderr b/tests/ui/search_is_some_fixable_none.stderr index fa344046a..b89ddabee 100644 --- a/tests/ui/search_is_some_fixable_none.stderr +++ b/tests/ui/search_is_some_fixable_none.stderr @@ -169,5 +169,17 @@ error: called `is_none()` after searching an `Iterator` with `find` 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: aborting due to 26 previous errors +error: called `is_none()` after searching an `Iterator` with `find` + --> $DIR/search_is_some_fixable_none.rs:112: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:113: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: aborting due to 28 previous errors diff --git a/tests/ui/search_is_some_fixable_some.fixed b/tests/ui/search_is_some_fixable_some.fixed index ae9478a42..76b7f3d05 100644 --- a/tests/ui/search_is_some_fixable_some.fixed +++ b/tests/ui/search_is_some_fixable_some.fixed @@ -98,4 +98,15 @@ mod issue7392 { let vfoo = vec![[0, 1, 2, 3, 0, 1, 2, 3]]; let _ = vfoo.iter().any(|sub| sub[1..4].len() == 3); } + + fn please(x: &u32) -> bool { + *x == 9 + } + + fn more_projections() { + let x = 19; + let ppx: &u32 = &x; + let _ = [ppx].iter().any(|ppp_x: &&u32| please(ppp_x)); + let _ = [String::from("Hey hey")].iter().any(|s| s.len() == 2); + } } diff --git a/tests/ui/search_is_some_fixable_some.rs b/tests/ui/search_is_some_fixable_some.rs index 98c3e7c1a..364250e6e 100644 --- a/tests/ui/search_is_some_fixable_some.rs +++ b/tests/ui/search_is_some_fixable_some.rs @@ -100,4 +100,15 @@ mod issue7392 { let vfoo = vec![[0, 1, 2, 3, 0, 1, 2, 3]]; let _ = vfoo.iter().find(|sub| sub[1..4].len() == 3).is_some(); } + + fn please(x: &u32) -> bool { + *x == 9 + } + + fn more_projections() { + let x = 19; + let ppx: &u32 = &x; + let _ = [ppx].iter().find(|ppp_x: &&&u32| please(**ppp_x)).is_some(); + let _ = [String::from("Hey hey")].iter().find(|s| s.len() == 2).is_some(); + } } diff --git a/tests/ui/search_is_some_fixable_some.stderr b/tests/ui/search_is_some_fixable_some.stderr index 4161b5eec..183d99497 100644 --- a/tests/ui/search_is_some_fixable_some.stderr +++ b/tests/ui/search_is_some_fixable_some.stderr @@ -160,5 +160,17 @@ error: called `is_some()` after searching an `Iterator` with `find` 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: aborting due to 26 previous errors +error: called `is_some()` after searching an `Iterator` with `find` + --> $DIR/search_is_some_fixable_some.rs:111: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:112: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: aborting due to 28 previous errors