Skip to content

Commit 90a72f5

Browse files
ThibsGThibsG
committed
Handle args taken by ref also for MethodCall 
1 parent 7a55407 commit 90a72f5

7 files changed

+85
-26
lines changed

‎clippy_lints/src/methods/search_is_some.rs

Lines changed: 19 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -227,23 +227,23 @@ impl DerefDelegate<'_, 'tcx> {
227227
}
228228

229229
fn func_takes_arg_by_ref(&self, parent_expr: &'tcx hir::Expr<'_>, cmt_hir_id: HirId) -> bool {
230-
if_chain! {
231-
if let ExprKind::Call(func, call_args) = parent_expr.kind;
232-
let typ = self.cx.typeck_results().expr_ty(func);
233-
if let ty::FnDef(..) = typ.kind();
234-
235-
then {
236-
let mut takes_by_ref = false;
237-
for (arg, ty) in iter::zip(call_args, typ.fn_sig(self.cx.tcx).skip_binder().inputs()) {
238-
if arg.hir_id == cmt_hir_id {
239-
takes_by_ref = matches!(ty.kind(), ty::Ref(_, inner, _) if inner.is_ref());
240-
}
230+
let (call_args, inputs) = match parent_expr.kind {
231+
ExprKind::MethodCall(_, _, call_args, _) => {
232+
if let Some(method_did) = self.cx.typeck_results().type_dependent_def_id(parent_expr.hir_id) {
233+
(call_args, self.cx.tcx.fn_sig(method_did).skip_binder().inputs())
234+
} else {
235+
return false;
241236
}
242-
takes_by_ref
243-
} else {
244-
false
245-
}
246-
}
237+
},
238+
ExprKind::Call(func, call_args) => {
239+
let typ = self.cx.typeck_results().expr_ty(func);
240+
(call_args, typ.fn_sig(self.cx.tcx).skip_binder().inputs())
241+
},
242+
_ => return false,
243+
};
244+
245+
iter::zip(call_args, inputs)
246+
.any(|(arg, ty)| arg.hir_id == cmt_hir_id && matches!(ty.kind(), ty::Ref(_, inner, _) if inner.is_ref()))
247247
}
248248
}
249249

@@ -271,10 +271,6 @@ impl<'tcx> Delegate<'tcx> for DerefDelegate<'_, 'tcx> {
271271
let arg_ty_kind = self.cx.typeck_results().expr_ty(expr).kind();
272272

273273
if matches!(arg_ty_kind, ty::Ref(_, _, Mutability::Not)) {
274-
let start_span = Span::new(self.next_pos, span.lo(), span.ctxt());
275-
let start_snip =
276-
snippet_with_applicability(self.cx, start_span, "..", &mut self.applicability);
277-
278274
// suggest ampersand if call function is taking args by ref
279275
let takes_arg_by_ref = self.func_takes_arg_by_ref(parent_expr, cmt.hir_id);
280276

@@ -294,14 +290,12 @@ impl<'tcx> Delegate<'tcx> for DerefDelegate<'_, 'tcx> {
294290
}
295291
}
296292

297-
// handle item projections by removing one explicit deref
298-
// i.e.: suggest `*x` instead of `**x`
299293
let mut replacement_str = ident_str;
300-
301294
let mut projections_handled = false;
302295
cmt.place.projections.iter().enumerate().for_each(|(i, proj)| {
303296
match proj.kind {
304297
// Field projection like `|v| v.foo`
298+
// no adjustment needed here, as field projections are handled by the compiler
305299
ProjectionKind::Field(idx, variant) => match cmt.place.ty_before_projection(i).kind() {
306300
ty::Adt(def, ..) => {
307301
replacement_str = format!(
@@ -342,7 +336,8 @@ impl<'tcx> Delegate<'tcx> for DerefDelegate<'_, 'tcx> {
342336
}
343337
});
344338

345-
// handle `ProjectionKind::Deref` if no special case detected
339+
// handle `ProjectionKind::Deref` by removing one explicit deref
340+
// if no special case was detected (i.e.: suggest `*x` instead of `**x`)
346341
if !projections_handled {
347342
let last_deref = cmt
348343
.place

‎tests/ui/search_is_some_fixable_none.fixed

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,4 +161,17 @@ mod issue7392 {
161161
let vfoo = vec![&&[0, 1, 2, 3]];
162162
let _ = !vfoo.iter().any(|x| (**x)[0] == 9);
163163
}
164+
165+
fn method_call_by_ref() {
166+
struct Foo {
167+
bar: u32,
168+
}
169+
impl Foo {
170+
pub fn by_ref(&self, x: &u32) -> bool {
171+
*x == self.bar
172+
}
173+
}
174+
let vfoo = vec![Foo { bar: 1 }];
175+
let _ = !vfoo.iter().any(|v| v.by_ref(&v.bar));
176+
}
164177
}

‎tests/ui/search_is_some_fixable_none.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,4 +167,17 @@ mod issue7392 {
167167
let vfoo = vec![&&[0, 1, 2, 3]];
168168
let _ = vfoo.iter().find(|x| (**x)[0] == 9).is_none();
169169
}
170+
171+
fn method_call_by_ref() {
172+
struct Foo {
173+
bar: u32,
174+
}
175+
impl Foo {
176+
pub fn by_ref(&self, x: &u32) -> bool {
177+
*x == self.bar
178+
}
179+
}
180+
let vfoo = vec![Foo { bar: 1 }];
181+
let _ = vfoo.iter().find(|v| v.by_ref(&v.bar)).is_none();
182+
}
170183
}

‎tests/ui/search_is_some_fixable_none.stderr

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -221,5 +221,11 @@ error: called `is_none()` after searching an `Iterator` with `find`
221221
LL | let _ = vfoo.iter().find(|x| (**x)[0] == 9).is_none();
222222
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `!_.any()` instead: `!vfoo.iter().any(|x| (**x)[0] == 9)`
223223

224-
error: aborting due to 33 previous errors
224+
error: called `is_none()` after searching an `Iterator` with `find`
225+
--> $DIR/search_is_some_fixable_none.rs:181:17
226+
|
227+
LL | let _ = vfoo.iter().find(|v| v.by_ref(&v.bar)).is_none();
228+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `!_.any()` instead: `!vfoo.iter().any(|v| v.by_ref(&v.bar))`
229+
230+
error: aborting due to 34 previous errors
225231

‎tests/ui/search_is_some_fixable_some.fixed

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,4 +163,17 @@ mod issue7392 {
163163
let vfoo = vec![&&[0, 1, 2, 3]];
164164
let _ = vfoo.iter().any(|x| (**x)[0] == 9);
165165
}
166+
167+
fn method_call_by_ref() {
168+
struct Foo {
169+
bar: u32,
170+
}
171+
impl Foo {
172+
pub fn by_ref(&self, x: &u32) -> bool {
173+
*x == self.bar
174+
}
175+
}
176+
let vfoo = vec![Foo { bar: 1 }];
177+
let _ = vfoo.iter().any(|v| v.by_ref(&v.bar));
178+
}
166179
}

‎tests/ui/search_is_some_fixable_some.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,4 +166,17 @@ mod issue7392 {
166166
let vfoo = vec![&&[0, 1, 2, 3]];
167167
let _ = vfoo.iter().find(|x| (**x)[0] == 9).is_some();
168168
}
169+
170+
fn method_call_by_ref() {
171+
struct Foo {
172+
bar: u32,
173+
}
174+
impl Foo {
175+
pub fn by_ref(&self, x: &u32) -> bool {
176+
*x == self.bar
177+
}
178+
}
179+
let vfoo = vec![Foo { bar: 1 }];
180+
let _ = vfoo.iter().find(|v| v.by_ref(&v.bar)).is_some();
181+
}
169182
}

‎tests/ui/search_is_some_fixable_some.stderr

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,5 +204,11 @@ error: called `is_some()` after searching an `Iterator` with `find`
204204
LL | let _ = vfoo.iter().find(|x| (**x)[0] == 9).is_some();
205205
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `any()` instead: `any(|x| (**x)[0] == 9)`
206206

207-
error: aborting due to 33 previous errors
207+
error: called `is_some()` after searching an `Iterator` with `find`
208+
--> $DIR/search_is_some_fixable_some.rs:180:29
209+
|
210+
LL | let _ = vfoo.iter().find(|v| v.by_ref(&v.bar)).is_some();
211+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `any()` instead: `any(|v| v.by_ref(&v.bar))`
212+
213+
error: aborting due to 34 previous errors
208214

0 commit comments

Comments
 (0)