Implement a lint for RefCell<impl Copy>

CHANGELOG.md

@@ -5374,6 +5374,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_refcell`]: https://rust-lang.github.io/rust-clippy/master/index.html#copy_refcell
 [`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
@@ -6154,6 +6155,7 @@ Released 2018-09-13
 [`excessive-nesting-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#excessive-nesting-threshold
 [`future-size-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#future-size-threshold
 [`ignore-interior-mutability`]: https://doc.rust-lang.org/clippy/lint_configuration.html#ignore-interior-mutability
+[`large-cell-limit`]: https://doc.rust-lang.org/clippy/lint_configuration.html#large-cell-limit
 [`large-error-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#large-error-threshold
 [`literal-representation-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#literal-representation-threshold
 [`matches-for-let-else`]: https://doc.rust-lang.org/clippy/lint_configuration.html#matches-for-let-else

book/src/lint_configuration.md

@@ -561,6 +561,16 @@ A list of paths to types that should be treated as if they do not contain interi
 * [`mutable_key_type`](https://rust-lang.github.io/rust-clippy/master/index.html#mutable_key_type)
 
 
+## `large-cell-limit`
+The maximum size of a `T` in `RefCell<T>` to suggest to swap to `Cell` if applicable.
+
+**Default Value:** `128`
+
+---
+**Affected lints:**
+* [`copy_refcell`](https://rust-lang.github.io/rust-clippy/master/index.html#copy_refcell)
+
+
 ## `large-error-threshold`
 The maximum size of the `Err`-variant in a `Result` returned from a function
 

clippy_config/src/conf.rs

@@ -471,6 +471,9 @@ define_Conf! {
     /// A list of paths to types that should be treated as if they do not contain interior mutability
     #[lints(borrow_interior_mutable_const, declare_interior_mutable_const, ifs_same_cond, mutable_key_type)]
     ignore_interior_mutability: Vec<String> = Vec::from(["bytes::Bytes".into()]),
+    /// The maximum size of a `T` in `RefCell<T>` to suggest to swap to `Cell` if applicable.
+    #[lints(copy_refcell)]
+    large_cell_limit: u64 = 128,
     /// The maximum size of the `Err`-variant in a `Result` returned from a function
     #[lints(result_large_err)]
     large_error_threshold: u64 = 128,

clippy_lints/src/copy_refcell.rs

@@ -0,0 +1,118 @@
+use clippy_config::Conf;
+use rustc_hir::{FieldDef, LetStmt, LocalSource};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty;
+use rustc_middle::ty::layout::TyAndLayout;
+use rustc_session::impl_lint_pass;
+use rustc_span::symbol::sym;
+use rustc_span::Span;
+
+use clippy_utils::diagnostics::span_lint_and_help;
+use clippy_utils::ty::implements_trait;
+
+declare_clippy_lint! {
+    /// ### What it does
+    ///
+    /// Detects crate-local usage of `RefCell<T>` where `T` implements `Copy`
+    ///
+    /// ### Why is this bad?
+    ///
+    /// `RefCell` has to perform extra book-keeping in order to support references, whereas `Cell` does not.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// let interior_mutable = std::cell::RefCell::new(0_u8);
+    ///
+    /// *interior_mutable.borrow_mut() = 1;
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// let interior_mutable = std::cell::Cell::new(0_u8);
+    ///
+    /// interior_mutable.set(1);
+    /// ```
+    #[clippy::version = "1.83.0"]
+    pub COPY_REFCELL,
+    pedantic,
+    "usage of `RefCell<T>` where `T` implements `Copy`"
+}
+
+pub struct CopyRefCell {
+    maximum_size: u64,
+    avoid_breaking_exported_api: bool,
+}
+
+impl CopyRefCell {
+    pub fn new(conf: &'static Conf) -> Self {
+        Self {
+            maximum_size: conf.large_cell_limit,
+            avoid_breaking_exported_api: conf.avoid_breaking_exported_api,
+        }
+    }
+
+    fn check_copy_refcell<'tcx>(&self, cx: &LateContext<'tcx>, ty: ty::Ty<'tcx>, span: Span) {
+        let Some(copy_def_id) = cx.tcx.get_diagnostic_item(sym::Copy) else {
+            return;
+        };
+
+        let ty::Adt(adt, generics) = ty.kind() else {
+            return;
+        };
+
+        if !cx.tcx.is_diagnostic_item(sym::RefCell, adt.did()) {
+            return;
+        }
+
+        let inner_ty = generics.type_at(0);
+        let Ok(TyAndLayout { layout, .. }) = cx.tcx.layout_of(cx.param_env.and(inner_ty)) else {
+            return;
+        };
+
+        if layout.size().bytes() >= self.maximum_size {
+            return;
+        }
+
+        if implements_trait(cx, inner_ty, copy_def_id, &[]) {
+            span_lint_and_help(
+                cx,
+                COPY_REFCELL,
+                span,
+                "`RefCell` used with a type that implements `Copy`",
+                None,
+                "replace the usage of `RefCell` with `Cell`, which does not have to track borrowing at runtime",
+            );
+        }
+    }
+}
+
+impl_lint_pass!(CopyRefCell => [COPY_REFCELL]);
+
+impl<'tcx> LateLintPass<'tcx> for CopyRefCell {
+    fn check_field_def(&mut self, cx: &LateContext<'tcx>, field_def: &'tcx FieldDef<'tcx>) {
+        // Don't want to lint macro expansions.
+        if field_def.span.from_expansion() {
+            return;
+        }
+
+        if self.avoid_breaking_exported_api && cx.effective_visibilities.is_exported(field_def.def_id) {
+            return;
+        }
+
+        let field_ty = rustc_hir_analysis::lower_ty(cx.tcx, field_def.ty);
+        self.check_copy_refcell(cx, field_ty, field_def.ty.span);
+    }
+
+    fn check_local(&mut self, cx: &LateContext<'tcx>, local_def: &'tcx LetStmt<'tcx>) {
+        // Don't want to lint macro expansions or desugaring.
+        if local_def.span.from_expansion() || !matches!(local_def.source, LocalSource::Normal) {
+            return;
+        }
+
+        let Some(init_expr) = local_def.init else {
+            return;
+        };
+
+        let init_ty = cx.typeck_results().expr_ty(init_expr);
+        self.check_copy_refcell(cx, init_ty, init_expr.span);
+    }
+}

clippy_lints/src/declared_lints.rs

@@ -108,6 +108,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_refcell::COPY_REFCELL_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

@@ -99,6 +99,7 @@ mod collection_is_never_read;
 mod comparison_chain;
 mod copies;
 mod copy_iterator;
+mod copy_refcell;
 mod crate_in_macro_def;
 mod create_dir;
 mod dbg_macro;
@@ -942,5 +943,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
     store.register_late_pass(move |_| Box::new(manual_div_ceil::ManualDivCeil::new(conf)));
     store.register_late_pass(|_| Box::new(manual_is_power_of_two::ManualIsPowerOfTwo));
     store.register_late_pass(|_| Box::new(non_zero_suggestions::NonZeroSuggestions));
+    store.register_late_pass(move |_| Box::new(copy_refcell::CopyRefCell::new(conf)));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }

tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr

@@ -44,6 +44,7 @@ error: error reading Clippy's configuration file: unknown field `foobar`, expect
            excessive-nesting-threshold
            future-size-threshold
            ignore-interior-mutability
+           large-cell-limit
            large-error-threshold
            literal-representation-threshold
            matches-for-let-else
@@ -128,6 +129,7 @@ error: error reading Clippy's configuration file: unknown field `barfoo`, expect
            excessive-nesting-threshold
            future-size-threshold
            ignore-interior-mutability
+           large-cell-limit
            large-error-threshold
            literal-representation-threshold
            matches-for-let-else
@@ -212,6 +214,7 @@ error: error reading Clippy's configuration file: unknown field `allow_mixed_uni
            excessive-nesting-threshold
            future-size-threshold
            ignore-interior-mutability
+           large-cell-limit
            large-error-threshold
            literal-representation-threshold
            matches-for-let-else

tests/ui/await_holding_refcell_ref.rs

@@ -1,4 +1,5 @@
 #![warn(clippy::await_holding_refcell_ref)]
+#![allow(clippy::copy_refcell)]
 
 use std::cell::RefCell;
 

tests/ui/await_holding_refcell_ref.stderr

@@ -1,40 +1,40 @@
 error: this `RefCell` reference is held across an await point
-  --> tests/ui/await_holding_refcell_ref.rs:6:9
+  --> tests/ui/await_holding_refcell_ref.rs:7:9
    |
 LL |     let b = x.borrow();
    |         ^
    |
    = help: ensure the reference is dropped before calling `await`
 note: these are all the await points this reference is held through
-  --> tests/ui/await_holding_refcell_ref.rs:8:11
+  --> tests/ui/await_holding_refcell_ref.rs:9:11
    |
 LL |     baz().await
    |           ^^^^^
    = note: `-D clippy::await-holding-refcell-ref` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::await_holding_refcell_ref)]`
 
 error: this `RefCell` reference is held across an await point
-  --> tests/ui/await_holding_refcell_ref.rs:12:9
+  --> tests/ui/await_holding_refcell_ref.rs:13:9
    |
 LL |     let b = x.borrow_mut();
    |         ^
    |
    = help: ensure the reference is dropped before calling `await`
 note: these are all the await points this reference is held through
-  --> tests/ui/await_holding_refcell_ref.rs:14:11
+  --> tests/ui/await_holding_refcell_ref.rs:15:11
    |
 LL |     baz().await
    |           ^^^^^
 
 error: this `RefCell` reference is held across an await point
-  --> tests/ui/await_holding_refcell_ref.rs:34:9
+  --> tests/ui/await_holding_refcell_ref.rs:35:9
    |
 LL |     let b = x.borrow_mut();
    |         ^
    |
    = help: ensure the reference is dropped before calling `await`
 note: these are all the await points this reference is held through
-  --> tests/ui/await_holding_refcell_ref.rs:37:24
+  --> tests/ui/await_holding_refcell_ref.rs:38:24
    |
 LL |     let second = baz().await;
    |                        ^^^^^
@@ -43,40 +43,40 @@ LL |     let third = baz().await;
    |                       ^^^^^
 
 error: this `RefCell` reference is held across an await point
-  --> tests/ui/await_holding_refcell_ref.rs:47:9
+  --> tests/ui/await_holding_refcell_ref.rs:48:9
    |
 LL |     let b = x.borrow_mut();
    |         ^
    |
    = help: ensure the reference is dropped before calling `await`
 note: these are all the await points this reference is held through
-  --> tests/ui/await_holding_refcell_ref.rs:50:24
+  --> tests/ui/await_holding_refcell_ref.rs:51:24
    |
 LL |     let second = baz().await;
    |                        ^^^^^
 
 error: this `RefCell` reference is held across an await point
-  --> tests/ui/await_holding_refcell_ref.rs:63:13
+  --> tests/ui/await_holding_refcell_ref.rs:64:13
    |
 LL |         let b = x.borrow_mut();
    |             ^
    |
    = help: ensure the reference is dropped before calling `await`
 note: these are all the await points this reference is held through
-  --> tests/ui/await_holding_refcell_ref.rs:65:15
+  --> tests/ui/await_holding_refcell_ref.rs:66:15
    |
 LL |         baz().await
    |               ^^^^^
 
 error: this `RefCell` reference is held across an await point
-  --> tests/ui/await_holding_refcell_ref.rs:76:13
+  --> tests/ui/await_holding_refcell_ref.rs:77:13
    |
 LL |         let b = x.borrow_mut();
    |             ^
    |
    = help: ensure the reference is dropped before calling `await`
 note: these are all the await points this reference is held through
-  --> tests/ui/await_holding_refcell_ref.rs:78:15
+  --> tests/ui/await_holding_refcell_ref.rs:79:15
    |
 LL |         baz().await
    |               ^^^^^

tests/ui/clone_on_copy.fixed

@@ -6,7 +6,8 @@
     clippy::unnecessary_operation,
     clippy::vec_init_then_push,
     clippy::toplevel_ref_arg,
-    clippy::needless_borrow
+    clippy::needless_borrow,
+    clippy::copy_refcell
 )]
 
 use std::cell::RefCell;

tests/ui/clone_on_copy.rs

@@ -6,7 +6,8 @@
     clippy::unnecessary_operation,
     clippy::vec_init_then_push,
     clippy::toplevel_ref_arg,
-    clippy::needless_borrow
+    clippy::needless_borrow,
+    clippy::copy_refcell
 )]
 
 use std::cell::RefCell;

tests/ui/clone_on_copy.stderr

@@ -1,5 +1,5 @@
 error: using `clone` on type `i32` which implements the `Copy` trait
-  --> tests/ui/clone_on_copy.rs:23:5
+  --> tests/ui/clone_on_copy.rs:24:5
    |
 LL |     42.clone();
    |     ^^^^^^^^^^ help: try removing the `clone` call: `42`
@@ -8,49 +8,49 @@ LL |     42.clone();
    = help: to override `-D warnings` add `#[allow(clippy::clone_on_copy)]`
 
 error: using `clone` on type `i32` which implements the `Copy` trait
-  --> tests/ui/clone_on_copy.rs:27:5
+  --> tests/ui/clone_on_copy.rs:28:5
    |
 LL |     (&42).clone();
    |     ^^^^^^^^^^^^^ help: try dereferencing it: `*(&42)`
 
 error: using `clone` on type `i32` which implements the `Copy` trait
-  --> tests/ui/clone_on_copy.rs:30:5
+  --> tests/ui/clone_on_copy.rs:31:5
    |
 LL |     rc.borrow().clone();
    |     ^^^^^^^^^^^^^^^^^^^ help: try dereferencing it: `*rc.borrow()`
 
 error: using `clone` on type `u32` which implements the `Copy` trait
-  --> tests/ui/clone_on_copy.rs:33:5
+  --> tests/ui/clone_on_copy.rs:34:5
    |
 LL |     x.clone().rotate_left(1);
    |     ^^^^^^^^^ help: try removing the `clone` call: `x`
 
 error: using `clone` on type `i32` which implements the `Copy` trait
-  --> tests/ui/clone_on_copy.rs:47:5
+  --> tests/ui/clone_on_copy.rs:48:5
    |
 LL |     m!(42).clone();
    |     ^^^^^^^^^^^^^^ help: try removing the `clone` call: `m!(42)`
 
 error: using `clone` on type `[u32; 2]` which implements the `Copy` trait
-  --> tests/ui/clone_on_copy.rs:57:5
+  --> tests/ui/clone_on_copy.rs:58:5
    |
 LL |     x.clone()[0];
    |     ^^^^^^^^^ help: try dereferencing it: `(*x)`
 
 error: using `clone` on type `char` which implements the `Copy` trait
-  --> tests/ui/clone_on_copy.rs:67:14
+  --> tests/ui/clone_on_copy.rs:68:14
    |
 LL |     is_ascii('z'.clone());
    |              ^^^^^^^^^^^ help: try removing the `clone` call: `'z'`
 
 error: using `clone` on type `i32` which implements the `Copy` trait
-  --> tests/ui/clone_on_copy.rs:71:14
+  --> tests/ui/clone_on_copy.rs:72:14
    |
 LL |     vec.push(42.clone());
    |              ^^^^^^^^^^ help: try removing the `clone` call: `42`
 
 error: using `clone` on type `Option<i32>` which implements the `Copy` trait
-  --> tests/ui/clone_on_copy.rs:75:17
+  --> tests/ui/clone_on_copy.rs:76:17
    |
 LL |     let value = opt.clone()?; // operator precedence needed (*opt)?
    |                 ^^^^^^^^^^^ help: try dereferencing it: `(*opt)`