From 83771c524225176616db45ebe67045a6f89b7117 Mon Sep 17 00:00:00 2001 From: Samuel Moelius Date: Wed, 26 Oct 2022 19:31:01 -0400 Subject: [PATCH] Fix `needless_borrow` false positive --- clippy_lints/src/dereference.rs | 47 ++++++++++++++++++++++++++++++--- tests/ui/needless_borrow.fixed | 23 ++++++++++++++++ tests/ui/needless_borrow.rs | 23 ++++++++++++++++ 3 files changed, 90 insertions(+), 3 deletions(-) diff --git a/clippy_lints/src/dereference.rs b/clippy_lints/src/dereference.rs index 7b5c10db4e1b..7e2e32a20d4f 100644 --- a/clippy_lints/src/dereference.rs +++ b/clippy_lints/src/dereference.rs @@ -1049,7 +1049,7 @@ fn ty_contains_infer(ty: &hir::Ty<'_>) -> bool { // If the conditions are met, returns `Some(Position::ImplArg(..))`; otherwise, returns `None`. // The "is copyable" condition is to avoid the case where removing the `&` means `e` would have to // be moved, but it cannot be. -#[expect(clippy::too_many_arguments)] +#[expect(clippy::too_many_arguments, clippy::too_many_lines)] fn needless_borrow_impl_arg_position<'tcx>( cx: &LateContext<'tcx>, possible_borrowers: &mut Vec<(LocalDefId, PossibleBorrowerMap<'tcx, 'tcx>)>, @@ -1092,7 +1092,7 @@ fn needless_borrow_impl_arg_position<'tcx>( .iter() .filter_map(|predicate| { if let PredicateKind::Trait(trait_predicate) = predicate.kind().skip_binder() - && trait_predicate.trait_ref.self_ty() == param_ty.to_ty(cx.tcx) + && trait_predicate.self_ty() == param_ty.to_ty(cx.tcx) { Some(trait_predicate.trait_ref.def_id) } else { @@ -1111,6 +1111,16 @@ fn needless_borrow_impl_arg_position<'tcx>( return Position::Other(precedence); } + // See: + // - https://github.com/rust-lang/rust-clippy/pull/9674#issuecomment-1289294201 + // - https://github.com/rust-lang/rust-clippy/pull/9674#issuecomment-1292225232 + if projection_predicates + .iter() + .any(|projection_predicate| is_mixed_projection_predicate(cx, callee_def_id, projection_predicate)) + { + return Position::Other(precedence); + } + // `substs_with_referent_ty` can be constructed outside of `check_referent` because the same // elements are modified each time `check_referent` is called. let mut substs_with_referent_ty = substs_with_expr_ty.to_vec(); @@ -1190,8 +1200,39 @@ fn has_ref_mut_self_method(cx: &LateContext<'_>, trait_def_id: DefId) -> bool { }) } -fn referent_used_exactly_once<'tcx>( +fn is_mixed_projection_predicate<'tcx>( cx: &LateContext<'tcx>, + callee_def_id: DefId, + projection_predicate: &ProjectionPredicate<'tcx>, +) -> bool { + let generics = cx.tcx.generics_of(callee_def_id); + // The predicate requires the projected type to equal a type parameter from the parent context. + if let Some(term_ty) = projection_predicate.term.ty() + && let ty::Param(term_param_ty) = term_ty.kind() + && (term_param_ty.index as usize) < generics.parent_count + { + // The inner-most self type is a type parameter from the current function. + let mut projection_ty = projection_predicate.projection_ty; + loop { + match projection_ty.self_ty().kind() { + ty::Projection(inner_projection_ty) => { + projection_ty = *inner_projection_ty; + } + ty::Param(param_ty) => { + return (param_ty.index as usize) >= generics.parent_count; + } + _ => { + return false; + } + } + } + } else { + false + } +} + +fn referent_used_exactly_once<'a, 'tcx>( + cx: &'a LateContext<'tcx>, possible_borrowers: &mut Vec<(LocalDefId, PossibleBorrowerMap<'tcx, 'tcx>)>, reference: &Expr<'tcx>, ) -> bool { diff --git a/tests/ui/needless_borrow.fixed b/tests/ui/needless_borrow.fixed index 340e89d2db1d..57a682d62c0c 100644 --- a/tests/ui/needless_borrow.fixed +++ b/tests/ui/needless_borrow.fixed @@ -385,3 +385,26 @@ mod used_more_than_once { fn use_x(_: impl AsRef) {} fn use_x_again(_: impl AsRef) {} } + +// https://github.com/rust-lang/rust-clippy/issues/9111#issuecomment-1277114280 +#[allow(dead_code)] +mod issue_9111 { + struct A; + + impl Extend for A { + fn extend>(&mut self, _: T) { + unimplemented!() + } + } + + impl<'a> Extend<&'a u8> for A { + fn extend>(&mut self, _: T) { + unimplemented!() + } + } + + fn main() { + let mut a = A; + a.extend(&[]); // vs a.extend([]); + } +} diff --git a/tests/ui/needless_borrow.rs b/tests/ui/needless_borrow.rs index c93711ac8e28..0d325b48ab81 100644 --- a/tests/ui/needless_borrow.rs +++ b/tests/ui/needless_borrow.rs @@ -385,3 +385,26 @@ mod used_more_than_once { fn use_x(_: impl AsRef) {} fn use_x_again(_: impl AsRef) {} } + +// https://github.com/rust-lang/rust-clippy/issues/9111#issuecomment-1277114280 +#[allow(dead_code)] +mod issue_9111 { + struct A; + + impl Extend for A { + fn extend>(&mut self, _: T) { + unimplemented!() + } + } + + impl<'a> Extend<&'a u8> for A { + fn extend>(&mut self, _: T) { + unimplemented!() + } + } + + fn main() { + let mut a = A; + a.extend(&[]); // vs a.extend([]); + } +}