about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2021-07-14 20:29:56 +0000
committerbors <bors@rust-lang.org>2021-07-14 20:29:56 +0000
commit2b193e247f108c303d2ef9818ddd4aadceaf5760 (patch)
tree1c9a95cb99116b5a8a8186eec9ae077cad242494
parent4acbff9eb05a15008d33fd7558670fc3b2338461 (diff)
parent61e280863fb68c26bb1f976acfa4a45a26d70545 (diff)
downloadrust-2b193e247f108c303d2ef9818ddd4aadceaf5760.tar.gz
rust-2b193e247f108c303d2ef9818ddd4aadceaf5760.zip
Auto merge of #7462 - xFrednet:7369-branches-sharing-code-else-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.
-rw-r--r--clippy_lints/src/copies.rs8
-rw-r--r--tests/ui/branches_sharing_code/false_positives.rs28
2 files changed, 33 insertions, 3 deletions
diff --git a/clippy_lints/src/copies.rs b/clippy_lints/src/copies.rs
index aea1accccc6..9cbcde59768 100644
--- a/clippy_lints/src/copies.rs
+++ b/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;
     }
 
diff --git a/tests/ui/branches_sharing_code/false_positives.rs b/tests/ui/branches_sharing_code/false_positives.rs
new file mode 100644
index 00000000000..7f42df46341
--- /dev/null
+++ b/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() {}