Skip to content

Commit b0e27cc

Browse files
committed
New lint: copy_then_borrow_mut
1 parent e1c1ac1 commit b0e27cc

17 files changed

+333
-67
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5539,6 +5539,7 @@ Released 2018-09-13
55395539
[`const_is_empty`]: https://rust-lang.github.io/rust-clippy/master/index.html#const_is_empty
55405540
[`const_static_lifetime`]: https://rust-lang.github.io/rust-clippy/master/index.html#const_static_lifetime
55415541
[`copy_iterator`]: https://rust-lang.github.io/rust-clippy/master/index.html#copy_iterator
5542+
[`copy_then_borrow_mut`]: https://rust-lang.github.io/rust-clippy/master/index.html#copy_then_borrow_mut
55425543
[`crate_in_macro_def`]: https://rust-lang.github.io/rust-clippy/master/index.html#crate_in_macro_def
55435544
[`create_dir`]: https://rust-lang.github.io/rust-clippy/master/index.html#create_dir
55445545
[`crosspointer_transmute`]: https://rust-lang.github.io/rust-clippy/master/index.html#crosspointer_transmute
@@ -6343,6 +6344,7 @@ Released 2018-09-13
63436344
[`avoid-breaking-exported-api`]: https://doc.rust-lang.org/clippy/lint_configuration.html#avoid-breaking-exported-api
63446345
[`await-holding-invalid-types`]: https://doc.rust-lang.org/clippy/lint_configuration.html#await-holding-invalid-types
63456346
[`cargo-ignore-publish`]: https://doc.rust-lang.org/clippy/lint_configuration.html#cargo-ignore-publish
6347+
[`check-copy-then-borrow-mut-in-test`]: https://doc.rust-lang.org/clippy/lint_configuration.html#check-copy-then-borrow-mut-in-test
63466348
[`check-incompatible-msrv-in-tests`]: https://doc.rust-lang.org/clippy/lint_configuration.html#check-incompatible-msrv-in-tests
63476349
[`check-private-items`]: https://doc.rust-lang.org/clippy/lint_configuration.html#check-private-items
63486350
[`cognitive-complexity-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#cognitive-complexity-threshold

book/src/lint_configuration.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -415,6 +415,16 @@ For internal testing only, ignores the current `publish` settings in the Cargo m
415415
* [`cargo_common_metadata`](https://rust-lang.github.io/rust-clippy/master/index.html#cargo_common_metadata)
416416

417417

418+
## `check-copy-then-borrow-mut-in-test`
419+
Whether to search for mutable borrows of freshly copied data in tests.
420+
421+
**Default Value:** `true`
422+
423+
---
424+
**Affected lints:**
425+
* [`copy_then_borrow_mut`](https://rust-lang.github.io/rust-clippy/master/index.html#copy_then_borrow_mut)
426+
427+
418428
## `check-incompatible-msrv-in-tests`
419429
Whether to check MSRV compatibility in `#[test]` and `#[cfg(test)]` code.
420430

clippy_config/src/conf.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -464,6 +464,9 @@ define_Conf! {
464464
/// For internal testing only, ignores the current `publish` settings in the Cargo manifest.
465465
#[lints(cargo_common_metadata)]
466466
cargo_ignore_publish: bool = false,
467+
/// Whether to search for mutable borrows of freshly copied data in tests.
468+
#[lints(copy_then_borrow_mut)]
469+
check_copy_then_borrow_mut_in_test: bool = true,
467470
/// Whether to check MSRV compatibility in `#[test]` and `#[cfg(test)]` code.
468471
#[lints(incompatible_msrv)]
469472
check_incompatible_msrv_in_tests: bool = false,
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
use clippy_config::Conf;
2+
use clippy_utils::diagnostics::span_lint_and_note;
3+
use clippy_utils::is_in_test;
4+
use clippy_utils::ty::is_copy;
5+
use rustc_hir::{BorrowKind, Expr, ExprKind, Mutability};
6+
use rustc_lint::{LateContext, LateLintPass};
7+
use rustc_session::impl_lint_pass;
8+
9+
declare_clippy_lint! {
10+
/// ### What it does
11+
/// Checks for mutable reference on a freshly copied data due to
12+
/// the use of a block to return an value implementing `Copy`.
13+
///
14+
/// ### Why is this bad?
15+
/// Using a block will make a copy of the block result if its type
16+
/// implements `Copy`. This might be an indication of a failed attempt
17+
/// to borrow the original data instead.
18+
///
19+
/// ### Example
20+
/// ```no_run
21+
/// # fn f(_: &mut [i32]) {}
22+
/// let arr = &mut [10, 20, 30];
23+
/// f(&mut { *arr });
24+
/// ```
25+
/// If you intend to modify `arr` in `f`, use instead:
26+
/// ```no_run
27+
/// # fn f(_: &mut [i32]) {}
28+
/// let arr = &mut [10, 20, 30];
29+
/// f(arr);
30+
/// ```
31+
#[clippy::version = "1.86.0"]
32+
pub COPY_THEN_BORROW_MUT,
33+
suspicious,
34+
"mutable borrow of a data which was just copied"
35+
}
36+
37+
pub struct CopyThenBorrowMut {
38+
check_in_test: bool,
39+
}
40+
41+
impl CopyThenBorrowMut {
42+
pub const fn new(conf: &Conf) -> Self {
43+
Self {
44+
check_in_test: conf.check_copy_then_borrow_mut_in_test,
45+
}
46+
}
47+
}
48+
49+
impl_lint_pass!(CopyThenBorrowMut => [COPY_THEN_BORROW_MUT]);
50+
51+
impl LateLintPass<'_> for CopyThenBorrowMut {
52+
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
53+
if !expr.span.from_expansion()
54+
&& let ExprKind::AddrOf(BorrowKind::Ref, Mutability::Mut, sub_expr) = expr.kind
55+
&& let ExprKind::Block(block, _) = sub_expr.kind
56+
&& block.span.eq_ctxt(expr.span)
57+
&& let Some(block_expr) = block.expr
58+
&& let block_ty = cx.typeck_results().expr_ty_adjusted(block_expr)
59+
&& is_copy(cx, block_ty)
60+
&& (self.check_in_test || !is_in_test(cx.tcx, expr.hir_id))
61+
{
62+
span_lint_and_note(
63+
cx,
64+
COPY_THEN_BORROW_MUT,
65+
expr.span,
66+
"mutable borrow of a value which was just copied",
67+
(!block.targeted_by_break).then_some(block_expr.span),
68+
"the return value of the block implements `Copy` and will be copied",
69+
);
70+
}
71+
}
72+
}

clippy_lints/src/declared_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
113113
crate::copies::IF_SAME_THEN_ELSE_INFO,
114114
crate::copies::SAME_FUNCTIONS_IN_IF_CONDITION_INFO,
115115
crate::copy_iterator::COPY_ITERATOR_INFO,
116+
crate::copy_then_borrow_mut::COPY_THEN_BORROW_MUT_INFO,
116117
crate::crate_in_macro_def::CRATE_IN_MACRO_DEF_INFO,
117118
crate::create_dir::CREATE_DIR_INFO,
118119
crate::dbg_macro::DBG_MACRO_INFO,

clippy_lints/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ mod collection_is_never_read;
103103
mod comparison_chain;
104104
mod copies;
105105
mod copy_iterator;
106+
mod copy_then_borrow_mut;
106107
mod crate_in_macro_def;
107108
mod create_dir;
108109
mod dbg_macro;
@@ -984,5 +985,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
984985
store.register_late_pass(move |_| Box::new(non_std_lazy_statics::NonStdLazyStatic::new(conf)));
985986
store.register_late_pass(|_| Box::new(manual_option_as_slice::ManualOptionAsSlice::new(conf)));
986987
store.register_late_pass(|_| Box::new(single_option_map::SingleOptionMap));
988+
store.register_late_pass(|_| Box::new(copy_then_borrow_mut::CopyThenBorrowMut::new(conf)));
987989
// add lints here, do not remove this comment, it's used in `new_lint`
988990
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
check-copy-then-borrow-mut-in-test = false
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
#[test]
2+
fn in_test() {
3+
let _ = &mut { 42 }; // Do not lint
4+
}
5+
6+
fn main() {
7+
let _ = &mut { 42 }; //~ ERROR: mutable borrow
8+
}
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
error: mutable borrow of a value which was just copied
2+
--> tests/ui-toml/copy_then_borrow_mut/copy_then_borrow_mut.rs:7:13
3+
|
4+
LL | let _ = &mut { 42 };
5+
| ^^^^^^^^^^^
6+
|
7+
note: the return value of the block implements `Copy` and will be copied
8+
--> tests/ui-toml/copy_then_borrow_mut/copy_then_borrow_mut.rs:7:20
9+
|
10+
LL | let _ = &mut { 42 };
11+
| ^^
12+
= note: `-D clippy::copy-then-borrow-mut` implied by `-D warnings`
13+
= help: to override `-D warnings` add `#[allow(clippy::copy_then_borrow_mut)]`
14+
15+
error: aborting due to 1 previous error
16+

tests/ui-toml/excessive_nesting/excessive_nesting.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
clippy::collapsible_if,
1414
clippy::blocks_in_conditions,
1515
clippy::single_match,
16+
clippy::copy_then_borrow_mut
1617
)]
1718

1819
#[macro_use]

0 commit comments

Comments
 (0)