New lint: copy_then_borrow_mut

Author: samueltardieu

CHANGELOG.md

@@ -5497,6 +5497,7 @@ Released 2018-09-13
 [`const_is_empty`]: https://rust-lang.github.io/rust-clippy/master/index.html#const_is_empty
 [`const_static_lifetime`]: https://rust-lang.github.io/rust-clippy/master/index.html#const_static_lifetime
 [`copy_iterator`]: https://rust-lang.github.io/rust-clippy/master/index.html#copy_iterator
+[`copy_then_borrow_mut`]: https://rust-lang.github.io/rust-clippy/master/index.html#copy_then_borrow_mut
 [`crate_in_macro_def`]: https://rust-lang.github.io/rust-clippy/master/index.html#crate_in_macro_def
 [`create_dir`]: https://rust-lang.github.io/rust-clippy/master/index.html#create_dir
 [`crosspointer_transmute`]: https://rust-lang.github.io/rust-clippy/master/index.html#crosspointer_transmute
@@ -6296,6 +6297,7 @@ Released 2018-09-13
 [`avoid-breaking-exported-api`]: https://doc.rust-lang.org/clippy/lint_configuration.html#avoid-breaking-exported-api
 [`await-holding-invalid-types`]: https://doc.rust-lang.org/clippy/lint_configuration.html#await-holding-invalid-types
 [`cargo-ignore-publish`]: https://doc.rust-lang.org/clippy/lint_configuration.html#cargo-ignore-publish
+[`check-copy-then-borrow-mut-in-test`]: https://doc.rust-lang.org/clippy/lint_configuration.html#check-copy-then-borrow-mut-in-test
 [`check-private-items`]: https://doc.rust-lang.org/clippy/lint_configuration.html#check-private-items
 [`cognitive-complexity-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#cognitive-complexity-threshold
 [`disallowed-macros`]: https://doc.rust-lang.org/clippy/lint_configuration.html#disallowed-macros

book/src/lint_configuration.md

@@ -414,6 +414,16 @@ For internal testing only, ignores the current `publish` settings in the Cargo m
 * [`cargo_common_metadata`](https://rust-lang.github.io/rust-clippy/master/index.html#cargo_common_metadata)
 
 
+## `check-copy-then-borrow-mut-in-test`
+Whether to search for mutable borrows of freshly copied data in tests.
+
+**Default Value:** `true`
+
+---
+**Affected lints:**
+* [`copy_then_borrow_mut`](https://rust-lang.github.io/rust-clippy/master/index.html#copy_then_borrow_mut)
+
+
 ## `check-private-items`
 Whether to also run the listed lints on private items.
 

clippy_config/src/conf.rs

@@ -463,6 +463,9 @@ define_Conf! {
     /// For internal testing only, ignores the current `publish` settings in the Cargo manifest.
     #[lints(cargo_common_metadata)]
     cargo_ignore_publish: bool = false,
+    /// Whether to search for mutable borrows of freshly copied data in tests.
+    #[lints(copy_then_borrow_mut)]
+    check_copy_then_borrow_mut_in_test: bool = true,
     /// Whether to also run the listed lints on private items.
     #[lints(missing_errors_doc, missing_panics_doc, missing_safety_doc, unnecessary_safety_doc)]
     check_private_items: bool = false,

clippy_lints/src/copy_then_borrow_mut.rs

@@ -0,0 +1,72 @@
+use clippy_config::Conf;
+use clippy_utils::diagnostics::span_lint_and_note;
+use clippy_utils::is_in_test;
+use clippy_utils::ty::is_copy;
+use rustc_hir::{BorrowKind, Expr, ExprKind, Mutability};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::impl_lint_pass;
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for mutable reference on a freshly copied data due to
+    /// the use of a block to return an value implementing `Copy`.
+    ///
+    /// ### Why is this bad?
+    /// Using a block will make a copy of the block result if its type
+    /// implements `Copy`. This might be an indication of a failed attempt
+    /// to borrow the original data instead.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// # fn f(_: &mut [i32]) {}
+    /// let arr = &mut [10, 20, 30];
+    /// f(&mut { *arr });
+    /// ```
+    /// If you intend to modify `arr` in `f`, use instead:
+    /// ```no_run
+    /// # fn f(_: &mut [i32]) {}
+    /// let arr = &mut [10, 20, 30];
+    /// f(arr);
+    /// ```
+    #[clippy::version = "1.86.0"]
+    pub COPY_THEN_BORROW_MUT,
+    suspicious,
+    "mutable borrow of a data which was just copied"
+}
+
+pub struct CopyThenBorrowMut {
+    check_in_test: bool,
+}
+
+impl CopyThenBorrowMut {
+    pub const fn new(conf: &Conf) -> Self {
+        Self {
+            check_in_test: conf.check_copy_then_borrow_mut_in_test,
+        }
+    }
+}
+
+impl_lint_pass!(CopyThenBorrowMut => [COPY_THEN_BORROW_MUT]);
+
+impl LateLintPass<'_> for CopyThenBorrowMut {
+    fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
+        if !expr.span.from_expansion()
+            && let ExprKind::AddrOf(BorrowKind::Ref, Mutability::Mut, sub_expr) = expr.kind
+            && let ExprKind::Block(block, _) = sub_expr.kind
+            && block.span.eq_ctxt(expr.span)
+            && let Some(block_expr) = block.expr
+            && let block_ty = cx.typeck_results().expr_ty_adjusted(block_expr)
+            && is_copy(cx, block_ty)
+            && (self.check_in_test || !is_in_test(cx.tcx, expr.hir_id))
+        {
+            span_lint_and_note(
+                cx,
+                COPY_THEN_BORROW_MUT,
+                expr.span,
+                "mutable borrow of a value which was just copied",
+                (!block.targeted_by_break).then_some(block_expr.span),
+                "the return value of the block implements `Copy` and will be copied",
+            );
+        }
+    }
+}

clippy_lints/src/declared_lints.rs

@@ -113,6 +113,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
     crate::copies::IF_SAME_THEN_ELSE_INFO,
     crate::copies::SAME_FUNCTIONS_IN_IF_CONDITION_INFO,
     crate::copy_iterator::COPY_ITERATOR_INFO,
+    crate::copy_then_borrow_mut::COPY_THEN_BORROW_MUT_INFO,
     crate::crate_in_macro_def::CRATE_IN_MACRO_DEF_INFO,
     crate::create_dir::CREATE_DIR_INFO,
     crate::dbg_macro::DBG_MACRO_INFO,

clippy_lints/src/lib.rs

@@ -103,6 +103,7 @@ mod collection_is_never_read;
 mod comparison_chain;
 mod copies;
 mod copy_iterator;
+mod copy_then_borrow_mut;
 mod crate_in_macro_def;
 mod create_dir;
 mod dbg_macro;
@@ -980,5 +981,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
     store.register_late_pass(move |_| Box::new(non_std_lazy_statics::NonStdLazyStatic::new(conf)));
     store.register_late_pass(|_| Box::new(manual_option_as_slice::ManualOptionAsSlice::new(conf)));
     store.register_late_pass(|_| Box::new(single_option_map::SingleOptionMap));
+    store.register_late_pass(|_| Box::new(copy_then_borrow_mut::CopyThenBorrowMut::new(conf)));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }

tests/ui-toml/copy_then_borrow_mut/clippy.toml

@@ -0,0 +1 @@
+check-copy-then-borrow-mut-in-test = false

tests/ui-toml/copy_then_borrow_mut/copy_then_borrow_mut.rs

@@ -0,0 +1,8 @@
+#[test]
+fn in_test() {
+    let _ = &mut { 42 }; // Do not lint
+}
+
+fn main() {
+    let _ = &mut { 42 }; //~ ERROR: mutable borrow
+}

tests/ui-toml/copy_then_borrow_mut/copy_then_borrow_mut.stderr

@@ -0,0 +1,16 @@
+error: mutable borrow of a value which was just copied
+  --> tests/ui-toml/copy_then_borrow_mut/copy_then_borrow_mut.rs:7:13
+   |
+LL |     let _ = &mut { 42 };
+   |             ^^^^^^^^^^^
+   |
+note: the return value of the block implements `Copy` and will be copied
+  --> tests/ui-toml/copy_then_borrow_mut/copy_then_borrow_mut.rs:7:20
+   |
+LL |     let _ = &mut { 42 };
+   |                    ^^
+   = note: `-D clippy::copy-then-borrow-mut` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::copy_then_borrow_mut)]`
+
+error: aborting due to 1 previous error
+

tests/ui-toml/excessive_nesting/excessive_nesting.rs

@@ -13,6 +13,7 @@
     clippy::collapsible_if,
     clippy::blocks_in_conditions,
     clippy::single_match,
+    clippy::copy_then_borrow_mut
 )]
 
 #[macro_use]