Skip to content

Commit

Permalink
Handle multiple reference levels into binding type and add more tests
Browse files Browse the repository at this point in the history
  • Loading branch information
ThibsG committed Nov 20, 2021
1 parent 788c9cc commit ac45a83
Show file tree
Hide file tree
Showing 7 changed files with 94 additions and 7 deletions.
29 changes: 24 additions & 5 deletions clippy_lints/src/methods/search_is_some.rs
Expand Up @@ -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())
}
Expand Down Expand Up @@ -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();

Expand All @@ -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;
Expand Down
11 changes: 11 additions & 0 deletions tests/ui/search_is_some_fixable_none.fixed
Expand Up @@ -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);
}
}
11 changes: 11 additions & 0 deletions tests/ui/search_is_some_fixable_none.rs
Expand Up @@ -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();
}
}
14 changes: 13 additions & 1 deletion tests/ui/search_is_some_fixable_none.stderr
Expand Up @@ -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

11 changes: 11 additions & 0 deletions tests/ui/search_is_some_fixable_some.fixed
Expand Up @@ -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);
}
}
11 changes: 11 additions & 0 deletions tests/ui/search_is_some_fixable_some.rs
Expand Up @@ -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();
}
}
14 changes: 13 additions & 1 deletion tests/ui/search_is_some_fixable_some.stderr
Expand Up @@ -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

0 comments on commit ac45a83

Please sign in to comment.