From 512f302fd29fbc4bc4b920b4b22b1d1c49cc3917 Mon Sep 17 00:00:00 2001
From: Martin Nordholts <martin.nordholts@codetale.se>
Date: Fri, 1 Dec 2023 08:15:55 +0100
Subject: [PATCH] needless_borrows_for_generic_args: Handle when field operand
 impl Drop

Before this fix, the lint had a false positive, namely when a reference
was taken to a field when the field operand implements a custom Drop.
The compiler will refuse to partially move a type that implements Drop,
because that would put the operand in a weird state. See added
regression test.
---
 .../src/needless_borrows_for_generic_args.rs      | 12 ++++++++++--
 tests/ui/needless_borrows_for_generic_args.fixed  | 15 +++++++++++++++
 tests/ui/needless_borrows_for_generic_args.rs     | 15 +++++++++++++++
 3 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/clippy_lints/src/needless_borrows_for_generic_args.rs b/clippy_lints/src/needless_borrows_for_generic_args.rs
index 85166b0dd..a32bca3d0 100644
--- a/clippy_lints/src/needless_borrows_for_generic_args.rs
+++ b/clippy_lints/src/needless_borrows_for_generic_args.rs
@@ -2,7 +2,7 @@ use clippy_config::msrvs::{self, Msrv};
 use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::mir::{enclosing_mir, expr_local, local_assignments, used_exactly_once, PossibleBorrowerMap};
 use clippy_utils::source::snippet_with_context;
-use clippy_utils::ty::is_copy;
+use clippy_utils::ty::{implements_trait, is_copy};
 use clippy_utils::{expr_use_ctxt, peel_n_hir_expr_refs, DefinedTy, ExprUseNode};
 use rustc_errors::Applicability;
 use rustc_hir::def::{DefKind, Res};
@@ -169,6 +169,7 @@ fn needless_borrow_count<'tcx>(
 ) -> usize {
     let destruct_trait_def_id = cx.tcx.lang_items().destruct_trait();
     let sized_trait_def_id = cx.tcx.lang_items().sized_trait();
+    let drop_trait_def_id = cx.tcx.lang_items().drop_trait();
 
     let fn_sig = cx.tcx.fn_sig(fn_id).instantiate_identity().skip_binder();
     let predicates = cx.tcx.param_env(fn_id).caller_bounds();
@@ -223,7 +224,14 @@ fn needless_borrow_count<'tcx>(
     // elements are modified each time `check_referent` is called.
     let mut args_with_referent_ty = callee_args.to_vec();
 
-    let mut check_reference_and_referent = |reference, referent| {
+    let mut check_reference_and_referent = |reference: &Expr<'tcx>, referent: &Expr<'tcx>| {
+        if let ExprKind::Field(base, _) = &referent.kind {
+            let base_ty = cx.typeck_results().expr_ty(base);
+            if drop_trait_def_id.map_or(false, |id| implements_trait(cx, base_ty, id, &[])) {
+                return false;
+            }
+        }
+
         let referent_ty = cx.typeck_results().expr_ty(referent);
 
         if !is_copy(cx, referent_ty)
diff --git a/tests/ui/needless_borrows_for_generic_args.fixed b/tests/ui/needless_borrows_for_generic_args.fixed
index 2a335516f..bd7a9a0b9 100644
--- a/tests/ui/needless_borrows_for_generic_args.fixed
+++ b/tests/ui/needless_borrows_for_generic_args.fixed
@@ -284,4 +284,19 @@ fn main() {
         {
         }
     }
+    // address of field when operand impl Drop
+    {
+        struct CustomDrop(String);
+
+        impl Drop for CustomDrop {
+            fn drop(&mut self) {}
+        }
+
+        fn check_str<P: AsRef<str>>(_to: P) {}
+
+        fn test() {
+            let owner = CustomDrop(String::default());
+            check_str(&owner.0); // Don't lint. `owner` can't be partially moved because it impl Drop
+        }
+    }
 }
diff --git a/tests/ui/needless_borrows_for_generic_args.rs b/tests/ui/needless_borrows_for_generic_args.rs
index f0567f486..5cfd4ce30 100644
--- a/tests/ui/needless_borrows_for_generic_args.rs
+++ b/tests/ui/needless_borrows_for_generic_args.rs
@@ -284,4 +284,19 @@ fn main() {
         {
         }
     }
+    // address of field when operand impl Drop
+    {
+        struct CustomDrop(String);
+
+        impl Drop for CustomDrop {
+            fn drop(&mut self) {}
+        }
+
+        fn check_str<P: AsRef<str>>(_to: P) {}
+
+        fn test() {
+            let owner = CustomDrop(String::default());
+            check_str(&owner.0); // Don't lint. `owner` can't be partially moved because it impl Drop
+        }
+    }
 }