Auto merge of rust-lang#7462 - xFrednet:7369-branches-sharing-code-el…

…se-expr-fp, r=camsteffen

FP fix and documentation for `branches_sharing_code` lint

Closes rust-lang/rust-clippy#7369

Related rust-lang/rust-clippy#7452 I'm still thinking about the best way to fix this. I could simply add another visitor to ensure that the moved expressions don't modify values being used in the condition, but I'm not totally happy with this due to the complexity. I therefore only documented it for now

changelog: [`branches_sharing_code`] fixed false positive where block expressions would sometimes be ignored.

clippy_lints/src/copies.rs

@@ -120,7 +120,10 @@ declare_clippy_lint! {
     ///
     /// **Why is this bad?** Duplicate code is less maintainable.
     ///
-    /// **Known problems:** Hopefully none.
+    /// **Known problems:**
+    /// * The lint doesn't check if the moved expressions modify values that are beeing used in
+    ///   the if condition. The suggestion can in that case modify the behavior of the program.
+    ///   See [rust-clippy#7452](https://github.com/rust-lang/rust-clippy/issues/7452)
     ///
     /// **Example:**
     /// ```ignore
@@ -358,8 +361,7 @@ fn scan_block_for_eq(cx: &LateContext<'tcx>, blocks: &[&Block<'tcx>]) -> Option<
         expr_eq &= block_expr_eq;
     }
 
-    let has_expr = blocks[0].expr.is_some();
-    if has_expr && !expr_eq {
+    if !expr_eq {
         end_eq = 0;
     }
 

tests/ui/branches_sharing_code/false_positives.rs

@@ -0,0 +1,28 @@
+#![allow(dead_code)]
+#![deny(clippy::if_same_then_else, clippy::branches_sharing_code)]
+
+// ##################################
+// # Issue clippy#7369
+// ##################################
+#[derive(Debug)]
+pub struct FooBar {
+    foo: Vec<u32>,
+}
+
+impl FooBar {
+    pub fn bar(&mut self) {
+        if true {
+            self.foo.pop();
+        } else {
+            self.baz();
+
+            self.foo.pop();
+
+            self.baz()
+        }
+    }
+
+    fn baz(&mut self) {}
+}
+
+fn main() {}