Better detect overwrites for owned values

clippy_lints/src/borrow_pats/owned.rs

@@ -195,6 +195,16 @@ pub enum OwnedPat {
     ConditionalOverwride,
     ConditionalMove,
     ConditionalDrop,
+    /// This value is being dropped (by rustc) early to be replaced.
+    ///
+    /// ```
+    /// let data = String::new();
+    ///
+    /// // Rustc will first drop the old value of `data`
+    /// // This is a drop to replacement
+    /// data = String::from("Example");
+    /// ```
+    DropForReplacement,
 }
 
 impl<'a, 'tcx> MyVisitor<'tcx> for OwnedAnalysis<'a, 'tcx> {
@@ -234,7 +244,11 @@ impl<'a, 'tcx> Visitor<'tcx> for OwnedAnalysis<'a, 'tcx> {
         }
 
         if target.local == self.local {
-            self.visit_assign_to_self(target, rvalue, loc.block);
+            if target.is_part() {
+                self.visit_assign_to_self_part(target, rvalue, loc.block);
+            } else {
+                self.visit_assign_to_self(target, rvalue, loc.block);
+            }
         }
 
         self.visit_assign_for_self_in_args(target, rvalue, loc.block);
@@ -330,19 +344,26 @@ impl<'a, 'tcx> OwnedAnalysis<'a, 'tcx> {
 
         let is_override = match self.states[bb].state() {
             // No-op the most normal and simple state
-            State::Empty => false,
+            State::Moved | State::Empty => false,
+
+            State::Dropped => {
+                // A manual drop has `Moved` as the previous state
+                if matches!(self.states[bb].prev_state(), Some(State::Filled | State::MaybeFilled)) {
+                    self.pats.insert(OwnedPat::DropForReplacement);
+                    true
+                } else {
+                    false
+                }
+            },
 
             // Filled should only ever be the case for !Drop types
-            State::Filled | State::MaybeFilled => {
-                // TODO: assert!(is_copy)
-                true
-            },
+            State::Filled | State::MaybeFilled => true,
 
-            State::Moved | State::Dropped => true,
             State::None => unreachable!(),
         };
         if is_override {
             let pat = if self.info.find_loop(bb).is_some() {
+                // TODO: This should be detected on state join instead of here
                 OwnedPat::OverwriteInLoop
             } else {
                 OwnedPat::Overwrite
@@ -353,6 +374,9 @@ impl<'a, 'tcx> OwnedAnalysis<'a, 'tcx> {
         // Regardless of the original state, we clear everything else
         self.states[bb].clear(State::Filled);
     }
+    fn visit_assign_to_self_part(&mut self, target: &Place<'tcx>, _rval: &Rvalue<'tcx>, _bb: BasicBlock) {
+        assert!(target.is_part());
+    }
     fn visit_assign_for_anon(&mut self, target: &Place<'tcx>, rval: &Rvalue<'tcx>, bb: BasicBlock) {
         if let Rvalue::Use(op) = &rval
             && let Operand::Move(place) = op
@@ -408,8 +432,19 @@ impl<'a, 'tcx> OwnedAnalysis<'a, 'tcx> {
 
     fn visit_terminator_for_args(&mut self, term: &Terminator<'tcx>, bb: BasicBlock) {
         match &term.kind {
-            TerminatorKind::Drop { place, .. } => {
+            TerminatorKind::Drop { place, replace, .. } => {
                 if place.local == self.local {
+                    assert!(place.just_local());
+                    // Fun facts: I tried to use this to detect `DropForReplacement`
+                    // but who would have guessed. Of course it's not always set.
+                    assert!(
+                        !(*replace),
+                        "Is this even ever set? {bb:?} Validity {:?}",
+                        self.states[bb].validity()
+                    );
+                    // if *replace {
+                    //     self.pats.insert(OwnedPat::DropForReplacement);
+                    // }
                     match self.states[bb].validity() {
                         Validity::Valid => {
                             self.states[bb].clear(State::Dropped);

clippy_lints/src/borrow_pats/owned/state.rs

@@ -85,6 +85,14 @@ pub enum BroKind {
 }
 
 impl<'tcx> StateInfo<'tcx> {
+    pub fn prev_state(&self) -> Option<State> {
+        if self.state.len() >= 2 {
+            Some(self.state[self.state.len() - 2].0)
+        } else {
+            None
+        }
+    }
+
     pub fn state(&self) -> State {
         self.state.last().unwrap().0
     }

tests/ui/thesis/owned_field.rs

@@ -0,0 +1,57 @@
+//@rustc-env: CLIPPY_PETS_PRINT=1
+
+#[derive(Default)]
+struct Example {
+    owned_1: String,
+    owned_2: String,
+    copy_1: u32,
+    copy_2: u32,
+}
+
+#[warn(clippy::borrow_pats)]
+fn replace_self() {
+    let mut drop = Example::default();
+    drop = Example::default();
+
+    let mut copy = 15;
+    copy = 17;
+}
+
+#[warn(clippy::borrow_pats)]
+fn conditional_replace_self() {
+    let mut drop = Example::default();
+    if true {
+        drop = Example::default();
+    }
+
+    let mut copy = 15;
+    if true {
+        copy = 17;
+    }
+}
+
+// #[forbid(clippy::borrow_pats)]
+fn assign_copy_field() {
+    let mut ex = Example::default();
+    ex.copy_1 = 10;
+}
+
+// #[forbid(clippy::borrow_pats)]
+fn assign_drop_field() {
+    let mut ex = Example::default();
+    ex.owned_1 = String::new();
+}
+
+// #[forbid(clippy::borrow_pats)]
+fn move_drop_field() {
+    let ex = Example::default();
+    let _hey = ex.owned_1;
+}
+
+// #[forbid(clippy::borrow_pats)]
+fn copy_field() {
+    let ex = Example::default();
+    let _hey = ex.copy_1;
+}
+
+fn main() {}

tests/ui/thesis/owned_field.stdout

@@ -0,0 +1,10 @@
+# "replace_self"
+- drop           : (Mutable  , Owned    , Local   , NonCopy, Drop   , UserDef  ) {Overwrite, DropForReplacement}
+- copy           : (Mutable  , Owned    , Local   , Copy   , NonDrop, Primitive) {Overwrite}
+- Body: Output(Unit)     , NotConst , Safe  , Sync , Free       {NoArguments}
+
+# "conditional_replace_self"
+- drop           : (Mutable  , Owned    , Local   , NonCopy, Drop   , UserDef  ) {Overwrite, ConditionalOverwride, ConditionalDrop, DropForReplacement}
+- copy           : (Mutable  , Owned    , Local   , Copy   , NonDrop, Primitive) {Overwrite, ConditionalOverwride}
+- Body: Output(Unit)     , NotConst , Safe  , Sync , Free       {NoArguments}
+

tests/ui/thesis/pass/owned.stdout

@@ -35,7 +35,7 @@
 - Body: Output(Unit)     , NotConst , Safe  , Sync , Free       {}
 
 # "conditional_overwrite"
-- animal         : (Mutable  , Owned    , Argument, NonCopy, Drop   , UserDef  ) {Overwrite, ConditionalOverwride, ConditionalDrop}
+- animal         : (Mutable  , Owned    , Argument, NonCopy, Drop   , UserDef  ) {Overwrite, ConditionalOverwride, ConditionalDrop, DropForReplacement}
 - cond           : (Immutable, Owned    , Argument, Copy   , NonDrop, Primitive) {}
 - Body: Output(Unit)     , NotConst , Safe  , Sync , Free       {}