From 9f357db13bb537589fcbd94844053239f1585db0 Mon Sep 17 00:00:00 2001 From: qdot3 Date: Fri, 7 Aug 2026 16:15:43 +0900 Subject: [PATCH 01/16] add tests --- clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/lib.rs | 2 + clippy_lints/src/refcell_cell.rs | 25 ++++++++++ tests/ui/refcell_cell.rs | 74 ++++++++++++++++++++++++++++++ tests/ui/refcell_cell.stderr | 23 ++++++++++ 5 files changed, 125 insertions(+) create mode 100644 clippy_lints/src/refcell_cell.rs create mode 100644 tests/ui/refcell_cell.rs create mode 100644 tests/ui/refcell_cell.stderr diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 5fd28aae0bee..8ed84a1c84f3 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -682,6 +682,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[ crate::redundant_type_annotations::REDUNDANT_TYPE_ANNOTATIONS_INFO, crate::ref_option_ref::REF_OPTION_REF_INFO, crate::ref_patterns::REF_PATTERNS_INFO, + crate::refcell_cell::REFCELL_CELL_INFO, crate::reference::DEREF_ADDROF_INFO, crate::regex::INVALID_REGEX_INFO, crate::regex::REGEX_CREATION_IN_LOOPS_INFO, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 8ab462c05374..06f8e2013e34 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -322,6 +322,7 @@ mod redundant_test_prefix; mod redundant_type_annotations; mod ref_option_ref; mod ref_patterns; +mod refcell_cell; mod reference; mod regex; mod repeat_vec_with_capacity; @@ -869,6 +870,7 @@ rustc_lint::late_lint_methods!( BlockScrutinee: block_scrutinee::BlockScrutinee = block_scrutinee::BlockScrutinee, NonnullUncheckedOnBoxPtr: nonnull_unchecked_on_box_ptr::NonnullUncheckedOnBoxPtr = nonnull_unchecked_on_box_ptr::NonnullUncheckedOnBoxPtr::new(conf), NeedlessNonzeroGet: needless_nonzero_get::NeedlessNonzeroGet = needless_nonzero_get::NeedlessNonzeroGet::new(conf), + RefcellCell: refcell_cell::RefcellCell = refcell_cell::RefcellCell, // add late passes here, used by `cargo dev new_lint` ]] ); diff --git a/clippy_lints/src/refcell_cell.rs b/clippy_lints/src/refcell_cell.rs new file mode 100644 index 000000000000..b977adef2add --- /dev/null +++ b/clippy_lints/src/refcell_cell.rs @@ -0,0 +1,25 @@ +use rustc_hir::*; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::declare_lint_pass; + +declare_clippy_lint! { + /// ### What it does + /// + /// ### Why is this bad? + /// + /// ### Example + /// ```no_run + /// // example code where clippy issues a warning + /// ``` + /// Use instead: + /// ```no_run + /// // example code which does not raise clippy warning + /// ``` + #[clippy::version = "1.99.0"] + pub REFCELL_CELL, + perf, + "default lint description" +} +declare_lint_pass!(RefcellCell => [REFCELL_CELL]); + +impl LateLintPass<'_> for RefcellCell {} diff --git a/tests/ui/refcell_cell.rs b/tests/ui/refcell_cell.rs new file mode 100644 index 000000000000..2c2579ac48b3 --- /dev/null +++ b/tests/ui/refcell_cell.rs @@ -0,0 +1,74 @@ +#![warn(clippy::refcell_cell)] + +use std::cell::{Cell, RefCell}; +use std::hint::black_box; + +struct NameFieldStruct { + ng: RefCell, //~ refcell_cell + ok: RefCell, +} + +struct TupleStruct( + RefCell, //~ refcell_cell + RefCell, +); + +enum Enum { + NG(RefCell), //~ refcell_cell + OK(RefCell), + Named { + ng: RefCell, //~ refcell_cell + ok: RefCell, + }, +} + +trait Trait { + fn func( + ng: RefCell, //~ refcell_cell + ok: RefCell, + ) -> ( + RefCell, //~ refcell_cell + RefCell, + ); +} + +struct Dummy; + +impl Trait for Dummy { + fn func(ng: RefCell, ok: RefCell) -> (RefCell, RefCell) { + todo!() + } +} + +fn func( + ng: RefCell, //~ refcell_cell + ok: RefCell, +) -> ( + RefCell, //~ refcell_cell + RefCell, +) { + todo!() +} + +impl Dummy { + type NG = RefCell; //~ refcell_cell + type OK = RefCell; + + fn func( + ng: RefCell, //~ refcell_cell + ok: RefCell, + ) -> ( + RefCell, //~ refcell_cell + RefCell, + ) { + todo!() + } +} + +fn main() { + let _ = RefCell::new(1); //~ refcell_cell + let _ = RefCell::new("Rust".to_string()); + + // False negative: This lint does not check trait bounds on call site. + let _ = black_box(RefCell::new(1)); +} diff --git a/tests/ui/refcell_cell.stderr b/tests/ui/refcell_cell.stderr new file mode 100644 index 000000000000..cc18ce758425 --- /dev/null +++ b/tests/ui/refcell_cell.stderr @@ -0,0 +1,23 @@ +error[E0658]: inherent associated types are unstable + --> tests/ui/refcell_cell.rs:53:5 + | +LL | type NG = RefCell; + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: see issue #8995 for more information + = help: add `#![feature(inherent_associated_types)]` to the crate attributes to enable + = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date + +error[E0658]: inherent associated types are unstable + --> tests/ui/refcell_cell.rs:54:5 + | +LL | type OK = RefCell; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: see issue #8995 for more information + = help: add `#![feature(inherent_associated_types)]` to the crate attributes to enable + = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0658`. From 732e11c8e320d71931103d13606ca65ccdd8dadd Mon Sep 17 00:00:00 2001 From: qdot3 Date: Fri, 7 Aug 2026 23:07:48 +0900 Subject: [PATCH 02/16] impl logic --- clippy_lints/src/refcell_cell.rs | 165 ++++++++++++++++++++++++++++++- tests/ui/refcell_cell.rs | 22 ++++- tests/ui/refcell_cell.stderr | 122 ++++++++++++++++++++--- 3 files changed, 288 insertions(+), 21 deletions(-) diff --git a/clippy_lints/src/refcell_cell.rs b/clippy_lints/src/refcell_cell.rs index b977adef2add..4f9aa4a0433a 100644 --- a/clippy_lints/src/refcell_cell.rs +++ b/clippy_lints/src/refcell_cell.rs @@ -1,6 +1,20 @@ -use rustc_hir::*; +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::res::{MaybeDef, MaybeQPath, MaybeResPath}; +use clippy_utils::source::{snippet_with_applicability, snippet_with_context}; +use clippy_utils::ty::{is_copy, ty_from_hir_ty}; +use clippy_utils::{is_trait_impl_item, sym}; +use rustc_errors::Applicability; +use rustc_hir::def_id::LocalDefId; +use rustc_hir::intravisit::FnKind; +use rustc_hir::{ + self, Body, Expr, ExprKind, FieldDef, FnDecl, FnRetTy, GenericArg, ImplItem, ImplItemKind, Item, ItemKind, QPath, + Stmt, StmtKind, TraitItem, TraitItemKind, Ty, TyKind, +}; use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty; use rustc_session::declare_lint_pass; +use rustc_span::Span; +use rustc_span::sym::RefCell; declare_clippy_lint! { /// ### What it does @@ -20,6 +34,153 @@ declare_clippy_lint! { perf, "default lint description" } + declare_lint_pass!(RefcellCell => [REFCELL_CELL]); -impl LateLintPass<'_> for RefcellCell {} +impl<'tcx> LateLintPass<'tcx> for RefcellCell { + fn check_field_def(&mut self, cx: &LateContext<'tcx>, def: &'tcx FieldDef<'tcx>) { + if !def.span.from_expansion() { + emit_refcell_copy_def(cx, def.ty); + } + } + + fn check_fn( + &mut self, + cx: &LateContext<'tcx>, + _: FnKind<'tcx>, + decl: &'tcx FnDecl<'tcx>, + _: &'tcx Body<'tcx>, + span: Span, + def_id: LocalDefId, + ) { + if !span.from_expansion() && !is_trait_impl_item(cx, cx.tcx.local_def_id_to_hir_id(def_id)) { + for hir_ty in decl.inputs { + emit_refcell_copy_def(cx, hir_ty); + } + if let FnRetTy::Return(hir_ty) = decl.output { + emit_refcell_copy_def(cx, hir_ty); + } + } + } + + fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx ImplItem<'tcx>) { + if let ImplItemKind::Type(hir_ty) = item.kind + && !item.span.from_expansion() + { + emit_refcell_copy_def(cx, hir_ty); + } + } + + fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) { + if let ItemKind::TyAlias(_, _, hir_ty) = item.kind + && !item.span.from_expansion() + { + emit_refcell_copy_def(cx, hir_ty); + } + } + + fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx TraitItem<'tcx>) { + if let TraitItemKind::Fn(sig, _) = item.kind + && !item.span.from_expansion() + { + for hir_ty in sig.decl.inputs { + emit_refcell_copy_def(cx, hir_ty); + } + if let FnRetTy::Return(hir_ty) = sig.decl.output { + emit_refcell_copy_def(cx, hir_ty); + } + } + } + + fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'tcx>) { + if let StmtKind::Let(let_stmt) = stmt.kind + && !stmt.span.from_expansion() + && let Some(init) = let_stmt.init + // RefCell + && let Some(ty) = cx.typeck_results().expr_ty_opt(init) + && let ty::Adt(def,args) = ty.kind() + && def.is_diag_item(&cx.tcx, RefCell) + && is_copy(cx, args.type_at(0)) + // `init` is a constructor of `RefCell` + && let ExprKind::Call(maybe_qpath, args) = init.kind + && is_refcell_ctor(cx, maybe_qpath, args.len()) + { + // Because this may be required to be `RefCell` + let mut app = Applicability::MaybeIncorrect; + let sugg = { + let (init, _) = snippet_with_context(cx, let_stmt.span, stmt.span.ctxt(), "..", &mut app); + init.to_owned().replace("RefCell", "Cell") + }; + span_lint_and_sugg( + cx, + REFCELL_CELL, + let_stmt.span, + "using a `RefCell` for a `Copy` type", + "try", + sugg, + app, + ); + } + } +} + +fn emit_refcell_copy_def<'tcx>(cx: &LateContext<'tcx>, hir_ty: &'tcx Ty<'tcx>) { + match hir_ty.kind { + TyKind::Array(hir_ty, _) | TyKind::Slice(hir_ty) => { + emit_refcell_copy_def(cx, hir_ty); + }, + TyKind::Tup(hir_tys) => { + for hir_ty in hir_tys { + emit_refcell_copy_def(cx, hir_ty); + } + }, + TyKind::Path(qpath) + if let (None, Some(path)) = qpath.opt_res_path() + // RefCell + && let [segment] = path.segments + && segment.res.is_diag_item(&cx.tcx, RefCell) + && let Some(args) = segment.args + && let [GenericArg::Type(hir_ty)] = args.args + // T: Copy + && is_copy(cx, ty_from_hir_ty(cx, hir_ty.as_unambig_ty())) + // be conservative about macro + && !hir_ty.span.from_expansion() => + { + let mut app = Applicability::MaybeIncorrect; + let sugg = { + let hir_ty = snippet_with_applicability(cx, hir_ty.span, "_", &mut app); + format!("Cell<{hir_ty}>") + }; + span_lint_and_sugg( + cx, + REFCELL_CELL, + path.span, + "using a `RefCell` for a `Copy` type", + "try", + sugg, + app, + ); + }, + _ => return, + } +} + +fn is_refcell_ctor<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, num_args: usize) -> bool { + if let Some(def_id) = expr.res(cx).opt_def_id() + && let Some(name) = cx.tcx.get_diagnostic_name(def_id) + && (matches!(name, sym::from_fn if num_args == 1) || matches!(name, sym::default_fn if num_args == 0)) + { + return true; + } + + // RefCell::new() + if let ExprKind::Path(QPath::TypeRelative(hir_ty, segment)) = expr.kind + && segment.ident.name == sym::new + && hir_ty.res(cx).is_diag_item(&cx.tcx, RefCell) + && num_args == 1 + { + return true; + } + + false +} diff --git a/tests/ui/refcell_cell.rs b/tests/ui/refcell_cell.rs index 2c2579ac48b3..d33ee4ad04c6 100644 --- a/tests/ui/refcell_cell.rs +++ b/tests/ui/refcell_cell.rs @@ -1,3 +1,4 @@ +//@no-rustfix #![warn(clippy::refcell_cell)] use std::cell::{Cell, RefCell}; @@ -22,7 +23,15 @@ enum Enum { }, } +type AliasNG = RefCell; //~ refcell_cell + +type AliasOK = RefCell; + trait Trait { + // associated type defaults are unstable + type NG; + type OK; + fn func( ng: RefCell, //~ refcell_cell ok: RefCell, @@ -35,6 +44,9 @@ trait Trait { struct Dummy; impl Trait for Dummy { + type NG = RefCell; //~ refcell_cell + type OK = RefCell; + fn func(ng: RefCell, ok: RefCell) -> (RefCell, RefCell) { todo!() } @@ -51,9 +63,6 @@ fn func( } impl Dummy { - type NG = RefCell; //~ refcell_cell - type OK = RefCell; - fn func( ng: RefCell, //~ refcell_cell ok: RefCell, @@ -67,8 +76,15 @@ impl Dummy { fn main() { let _ = RefCell::new(1); //~ refcell_cell + let _ = RefCell::::new(1); //~ refcell_cell let _ = RefCell::new("Rust".to_string()); + let _ = RefCell::from(1); //~ refcell_cell + let _ = RefCell::::from(1); //~ refcell_cell + + let _: RefCell = Default::default(); //~ refcell_cell + let _ = as Default>::default(); //~ refcell_cell + // False negative: This lint does not check trait bounds on call site. let _ = black_box(RefCell::new(1)); } diff --git a/tests/ui/refcell_cell.stderr b/tests/ui/refcell_cell.stderr index cc18ce758425..1649e6e6df2b 100644 --- a/tests/ui/refcell_cell.stderr +++ b/tests/ui/refcell_cell.stderr @@ -1,23 +1,113 @@ -error[E0658]: inherent associated types are unstable - --> tests/ui/refcell_cell.rs:53:5 +error: using a `RefCell` for a `Copy` type + --> tests/ui/refcell_cell.rs:8:9 | -LL | type NG = RefCell; - | ^^^^^^^^^^^^^^^^^^^^^^^ +LL | ng: RefCell, + | ^^^^^^^^^^ help: try: `Cell` | - = note: see issue #8995 for more information - = help: add `#![feature(inherent_associated_types)]` to the crate attributes to enable - = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date + = note: `-D clippy::refcell-cell` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::refcell_cell)]` -error[E0658]: inherent associated types are unstable - --> tests/ui/refcell_cell.rs:54:5 +error: using a `RefCell` for a `Copy` type + --> tests/ui/refcell_cell.rs:13:5 | -LL | type OK = RefCell; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | RefCell, + | ^^^^^^^^^^ help: try: `Cell` + +error: using a `RefCell` for a `Copy` type + --> tests/ui/refcell_cell.rs:18:8 + | +LL | NG(RefCell), + | ^^^^^^^^^^ help: try: `Cell` + +error: using a `RefCell` for a `Copy` type + --> tests/ui/refcell_cell.rs:21:13 + | +LL | ng: RefCell, + | ^^^^^^^^^^ help: try: `Cell` + +error: using a `RefCell` for a `Copy` type + --> tests/ui/refcell_cell.rs:26:16 + | +LL | type AliasNG = RefCell; + | ^^^^^^^^^^^^ help: try: `Cell` + +error: using a `RefCell` for a `Copy` type + --> tests/ui/refcell_cell.rs:36:13 + | +LL | ng: RefCell, + | ^^^^^^^^^^ help: try: `Cell` + +error: using a `RefCell` for a `Copy` type + --> tests/ui/refcell_cell.rs:39:9 + | +LL | RefCell, + | ^^^^^^^^^^ help: try: `Cell` + +error: using a `RefCell` for a `Copy` type + --> tests/ui/refcell_cell.rs:47:15 + | +LL | type NG = RefCell; + | ^^^^^^^^^^ help: try: `Cell` + +error: using a `RefCell` for a `Copy` type + --> tests/ui/refcell_cell.rs:56:9 + | +LL | ng: RefCell, + | ^^^^^^^^^^ help: try: `Cell` + +error: using a `RefCell` for a `Copy` type + --> tests/ui/refcell_cell.rs:59:5 + | +LL | RefCell, + | ^^^^^^^^^^ help: try: `Cell` + +error: using a `RefCell` for a `Copy` type + --> tests/ui/refcell_cell.rs:67:13 + | +LL | ng: RefCell, + | ^^^^^^^^^^ help: try: `Cell` + +error: using a `RefCell` for a `Copy` type + --> tests/ui/refcell_cell.rs:70:9 + | +LL | RefCell, + | ^^^^^^^^^^ help: try: `Cell` + +error: using a `RefCell` for a `Copy` type + --> tests/ui/refcell_cell.rs:78:5 + | +LL | let _ = RefCell::new(1); + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `let _ = Cell::new(1);` + +error: using a `RefCell` for a `Copy` type + --> tests/ui/refcell_cell.rs:79:5 + | +LL | let _ = RefCell::::new(1); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `let _ = Cell::::new(1);` + +error: using a `RefCell` for a `Copy` type + --> tests/ui/refcell_cell.rs:82:5 + | +LL | let _ = RefCell::from(1); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `let _ = Cell::from(1);` + +error: using a `RefCell` for a `Copy` type + --> tests/ui/refcell_cell.rs:83:5 + | +LL | let _ = RefCell::::from(1); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `let _ = Cell::::from(1);` + +error: using a `RefCell` for a `Copy` type + --> tests/ui/refcell_cell.rs:85:5 + | +LL | let _: RefCell = Default::default(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `let _: Cell = Default::default();` + +error: using a `RefCell` for a `Copy` type + --> tests/ui/refcell_cell.rs:86:5 | - = note: see issue #8995 for more information - = help: add `#![feature(inherent_associated_types)]` to the crate attributes to enable - = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date +LL | let _ = as Default>::default(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `let _ = as Default>::default();` -error: aborting due to 2 previous errors +error: aborting due to 18 previous errors -For more information about this error, try `rustc --explain E0658`. From 4b1a6a75864def0f977ce907d047383f71f9ca5d Mon Sep 17 00:00:00 2001 From: qdot3 Date: Fri, 7 Aug 2026 23:18:10 +0900 Subject: [PATCH 03/16] improve let-stmt pattern --- clippy_lints/src/refcell_cell.rs | 22 ++++++++++++++++++---- tests/ui/refcell_cell.stderr | 24 ++++++++++++------------ 2 files changed, 30 insertions(+), 16 deletions(-) diff --git a/clippy_lints/src/refcell_cell.rs b/clippy_lints/src/refcell_cell.rs index 4f9aa4a0433a..7430b7dae405 100644 --- a/clippy_lints/src/refcell_cell.rs +++ b/clippy_lints/src/refcell_cell.rs @@ -1,6 +1,6 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::res::{MaybeDef, MaybeQPath, MaybeResPath}; -use clippy_utils::source::{snippet_with_applicability, snippet_with_context}; +use clippy_utils::source::{snippet, snippet_with_applicability, snippet_with_context}; use clippy_utils::ty::{is_copy, ty_from_hir_ty}; use clippy_utils::{is_trait_impl_item, sym}; use rustc_errors::Applicability; @@ -104,17 +104,31 @@ impl<'tcx> LateLintPass<'tcx> for RefcellCell { // `init` is a constructor of `RefCell` && let ExprKind::Call(maybe_qpath, args) = init.kind && is_refcell_ctor(cx, maybe_qpath, args.len()) + // Be conservative about macro + && args.iter().all(|e| !e.span.from_expansion()) { + let span = match ( + snippet(cx, init.span, "").contains("RefCell"), + let_stmt + .ty + .is_some_and(|ty| snippet(cx, ty.span, "").contains("RefCell")), + ) { + (true, true) => let_stmt.span, + (true, false) => init.span, + (false, true) => let_stmt.ty.unwrap().span, + (false, false) => return, + }; + // Because this may be required to be `RefCell` let mut app = Applicability::MaybeIncorrect; let sugg = { - let (init, _) = snippet_with_context(cx, let_stmt.span, stmt.span.ctxt(), "..", &mut app); + let (init, _) = snippet_with_context(cx, span, stmt.span.ctxt(), "..", &mut app); init.to_owned().replace("RefCell", "Cell") }; span_lint_and_sugg( cx, REFCELL_CELL, - let_stmt.span, + span, "using a `RefCell` for a `Copy` type", "try", sugg, @@ -143,7 +157,7 @@ fn emit_refcell_copy_def<'tcx>(cx: &LateContext<'tcx>, hir_ty: &'tcx Ty<'tcx>) { && let [GenericArg::Type(hir_ty)] = args.args // T: Copy && is_copy(cx, ty_from_hir_ty(cx, hir_ty.as_unambig_ty())) - // be conservative about macro + // Be conservative about macro && !hir_ty.span.from_expansion() => { let mut app = Applicability::MaybeIncorrect; diff --git a/tests/ui/refcell_cell.stderr b/tests/ui/refcell_cell.stderr index 1649e6e6df2b..153ff11bdd55 100644 --- a/tests/ui/refcell_cell.stderr +++ b/tests/ui/refcell_cell.stderr @@ -74,40 +74,40 @@ LL | RefCell, | ^^^^^^^^^^ help: try: `Cell` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:78:5 + --> tests/ui/refcell_cell.rs:78:13 | LL | let _ = RefCell::new(1); - | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `let _ = Cell::new(1);` + | ^^^^^^^^^^^^^^^ help: try: `Cell::new(1)` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:79:5 + --> tests/ui/refcell_cell.rs:79:13 | LL | let _ = RefCell::::new(1); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `let _ = Cell::::new(1);` + | ^^^^^^^^^^^^^^^^^^^^^^ help: try: `Cell::::new(1)` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:82:5 + --> tests/ui/refcell_cell.rs:82:13 | LL | let _ = RefCell::from(1); - | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `let _ = Cell::from(1);` + | ^^^^^^^^^^^^^^^^ help: try: `Cell::from(1)` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:83:5 + --> tests/ui/refcell_cell.rs:83:13 | LL | let _ = RefCell::::from(1); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `let _ = Cell::::from(1);` + | ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Cell::::from(1)` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:85:5 + --> tests/ui/refcell_cell.rs:85:12 | LL | let _: RefCell = Default::default(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `let _: Cell = Default::default();` + | ^^^^^^^^^^^^ help: try: `Cell` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:86:5 + --> tests/ui/refcell_cell.rs:86:13 | LL | let _ = as Default>::default(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `let _ = as Default>::default();` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: ` as Default>::default()` error: aborting due to 18 previous errors From 9803d778953c963f8cdb307255c33b8503808fb2 Mon Sep 17 00:00:00 2001 From: qdot3 Date: Sat, 8 Aug 2026 08:35:51 +0900 Subject: [PATCH 04/16] add tests --- tests/ui/refcell_cell.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/ui/refcell_cell.rs b/tests/ui/refcell_cell.rs index d33ee4ad04c6..93b97cf3e3ea 100644 --- a/tests/ui/refcell_cell.rs +++ b/tests/ui/refcell_cell.rs @@ -87,4 +87,21 @@ fn main() { // False negative: This lint does not check trait bounds on call site. let _ = black_box(RefCell::new(1)); + + // Macro expansion + macro_rules! black_box { + ( $e:expr ) => { + black_box($e) + }; + } + let _ = RefCell::new(black_box!(1)); + let _ = RefCell::from(black_box!(1)); + let _ = black_box!(RefCell::new(1)); + + macro_rules! ignore_me { + () => { + let _ = RefCell::new(1); + }; + } + ignore_me!(); } From 68e6d3e4003cce1b56c938b011dffbdb4b33b551 Mon Sep 17 00:00:00 2001 From: qdot3 Date: Sat, 8 Aug 2026 08:55:21 +0900 Subject: [PATCH 05/16] write doc --- clippy_lints/src/refcell_cell.rs | 19 ++++++++++++++++--- tests/ui/refcell_cell.rs | 2 +- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/clippy_lints/src/refcell_cell.rs b/clippy_lints/src/refcell_cell.rs index 7430b7dae405..ac32370208d9 100644 --- a/clippy_lints/src/refcell_cell.rs +++ b/clippy_lints/src/refcell_cell.rs @@ -18,21 +18,34 @@ use rustc_span::sym::RefCell; declare_clippy_lint! { /// ### What it does + /// This lint suggests using `Cell` for `Copy` types instead of `RefCell`. /// /// ### Why is this bad? + /// `RefCell` avoids cloning at the cost of additional memory usage and + /// instructions, which isn't worth it for `Copy` types. /// /// ### Example /// ```no_run - /// // example code where clippy issues a warning + /// use std::cell::RefCell; + /// + /// struct Counter(RefCell); + /// + /// let _ = RefCell::new(1); + /// let _ = RefCell::default(); /// ``` /// Use instead: /// ```no_run - /// // example code which does not raise clippy warning + /// use std::cell::Cell; + /// + /// struct Counter(Cell); + /// + /// let _ = Cell::new(1); + /// let _ = Cell::default(); /// ``` #[clippy::version = "1.99.0"] pub REFCELL_CELL, perf, - "default lint description" + "using a `RefCell` for a `Copy` type" } declare_lint_pass!(RefcellCell => [REFCELL_CELL]); diff --git a/tests/ui/refcell_cell.rs b/tests/ui/refcell_cell.rs index 93b97cf3e3ea..8ef540d64647 100644 --- a/tests/ui/refcell_cell.rs +++ b/tests/ui/refcell_cell.rs @@ -77,7 +77,7 @@ impl Dummy { fn main() { let _ = RefCell::new(1); //~ refcell_cell let _ = RefCell::::new(1); //~ refcell_cell - let _ = RefCell::new("Rust".to_string()); + let _ = RefCell::new(vec![1]); let _ = RefCell::from(1); //~ refcell_cell let _ = RefCell::::from(1); //~ refcell_cell From 71c45a26eb1aa0ce60819372899e93a7d951df13 Mon Sep 17 00:00:00 2001 From: qdot3 Date: Sat, 8 Aug 2026 10:23:45 +0900 Subject: [PATCH 06/16] configure lint --- clippy_lints/src/refcell_cell.rs | 6 +- tests/ui/await_holding_refcell_ref.rs | 1 + tests/ui/await_holding_refcell_ref.stderr | 24 +++---- tests/ui/clone_on_copy.fixed | 3 +- tests/ui/clone_on_copy.rs | 1 + tests/ui/clone_on_copy.stderr | 76 +++++++++++------------ 6 files changed, 57 insertions(+), 54 deletions(-) diff --git a/clippy_lints/src/refcell_cell.rs b/clippy_lints/src/refcell_cell.rs index ac32370208d9..2e4fe845aa2e 100644 --- a/clippy_lints/src/refcell_cell.rs +++ b/clippy_lints/src/refcell_cell.rs @@ -1,5 +1,5 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::res::{MaybeDef, MaybeQPath, MaybeResPath}; +use clippy_utils::res::{MaybeDef as _, MaybeQPath as _, MaybeResPath as _}; use clippy_utils::source::{snippet, snippet_with_applicability, snippet_with_context}; use clippy_utils::ty::{is_copy, ty_from_hir_ty}; use clippy_utils::{is_trait_impl_item, sym}; @@ -136,7 +136,7 @@ impl<'tcx> LateLintPass<'tcx> for RefcellCell { let mut app = Applicability::MaybeIncorrect; let sugg = { let (init, _) = snippet_with_context(cx, span, stmt.span.ctxt(), "..", &mut app); - init.to_owned().replace("RefCell", "Cell") + init.into_owned().replace("RefCell", "Cell") }; span_lint_and_sugg( cx, @@ -188,7 +188,7 @@ fn emit_refcell_copy_def<'tcx>(cx: &LateContext<'tcx>, hir_ty: &'tcx Ty<'tcx>) { app, ); }, - _ => return, + _ => (), } } diff --git a/tests/ui/await_holding_refcell_ref.rs b/tests/ui/await_holding_refcell_ref.rs index e1db2e387497..601b4e35a806 100644 --- a/tests/ui/await_holding_refcell_ref.rs +++ b/tests/ui/await_holding_refcell_ref.rs @@ -1,3 +1,4 @@ +#![allow(clippy::refcell_cell)] #![warn(clippy::await_holding_refcell_ref)] use std::cell::RefCell; diff --git a/tests/ui/await_holding_refcell_ref.stderr b/tests/ui/await_holding_refcell_ref.stderr index ed8aa747bd07..5daefdf257ed 100644 --- a/tests/ui/await_holding_refcell_ref.stderr +++ b/tests/ui/await_holding_refcell_ref.stderr @@ -1,12 +1,12 @@ 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:9:11 + --> tests/ui/await_holding_refcell_ref.rs:10:11 | LL | baz().await | ^^^^^ @@ -14,27 +14,27 @@ LL | baz().await = 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:13:9 + --> tests/ui/await_holding_refcell_ref.rs:14: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:16:11 + --> tests/ui/await_holding_refcell_ref.rs:17:11 | LL | baz().await | ^^^^^ error: this `RefCell` reference is held across an await point - --> tests/ui/await_holding_refcell_ref.rs:36:9 + --> tests/ui/await_holding_refcell_ref.rs:37: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:39:24 + --> tests/ui/await_holding_refcell_ref.rs:40: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:49:9 + --> tests/ui/await_holding_refcell_ref.rs:50: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:52:24 + --> tests/ui/await_holding_refcell_ref.rs:53:24 | LL | let second = baz().await; | ^^^^^ error: this `RefCell` reference is held across an await point - --> tests/ui/await_holding_refcell_ref.rs:65:13 + --> tests/ui/await_holding_refcell_ref.rs:66: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:68:15 + --> tests/ui/await_holding_refcell_ref.rs:69:15 | LL | baz().await | ^^^^^ error: this `RefCell` reference is held across an await point - --> tests/ui/await_holding_refcell_ref.rs:79:13 + --> tests/ui/await_holding_refcell_ref.rs:80: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:82:15 + --> tests/ui/await_holding_refcell_ref.rs:83:15 | LL | baz().await | ^^^^^ diff --git a/tests/ui/clone_on_copy.fixed b/tests/ui/clone_on_copy.fixed index ad0a4343146a..2d195638b342 100644 --- a/tests/ui/clone_on_copy.fixed +++ b/tests/ui/clone_on_copy.fixed @@ -3,7 +3,8 @@ clippy::deref_addrof, clippy::needless_borrow, clippy::no_effect, - clippy::unnecessary_operation + clippy::unnecessary_operation, + clippy::refcell_cell )] #![expect(clippy::toplevel_ref_arg)] diff --git a/tests/ui/clone_on_copy.rs b/tests/ui/clone_on_copy.rs index d5371eda0fcb..14be7fc7a83d 100644 --- a/tests/ui/clone_on_copy.rs +++ b/tests/ui/clone_on_copy.rs @@ -3,6 +3,7 @@ clippy::deref_addrof, clippy::needless_borrow, clippy::no_effect, + clippy::refcell_cell, clippy::unnecessary_operation )] #![expect(clippy::toplevel_ref_arg)] diff --git a/tests/ui/clone_on_copy.stderr b/tests/ui/clone_on_copy.stderr index c1bf81152902..ca6245e4e8d6 100644 --- a/tests/ui/clone_on_copy.stderr +++ b/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:13:5 + --> tests/ui/clone_on_copy.rs:14:5 | LL | 42.clone(); | ^^^^^^^^^^ help: try removing the `clone` call: `42` @@ -8,223 +8,223 @@ 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:18:5 + --> tests/ui/clone_on_copy.rs:19: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:22:5 + --> tests/ui/clone_on_copy.rs:23: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:26:5 + --> tests/ui/clone_on_copy.rs:27: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:41:5 + --> tests/ui/clone_on_copy.rs:42: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:52:5 + --> tests/ui/clone_on_copy.rs:53:5 | LL | x.clone()[0]; | ^^^^^^^^^ help: try dereferencing it: `(*x)` error: using `clone` on type `E` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:91:20 + --> tests/ui/clone_on_copy.rs:92:20 | LL | let _: E = a.clone(); | ^^^^^^^^^ help: try dereferencing it: `*****a` error: using `clone` on type `E` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:93:20 + --> tests/ui/clone_on_copy.rs:94:20 | LL | let _: E = E::clone(&a); | ^^^^^^^^^^^^ help: try dereferencing it: `*****a` error: using `clone` on type `E` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:95:20 + --> tests/ui/clone_on_copy.rs:96:20 | LL | let _: E = Clone::clone(&a); | ^^^^^^^^^^^^^^^^ help: try dereferencing it: `*****a` error: using `clone` on type `E` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:97:20 + --> tests/ui/clone_on_copy.rs:98:20 | LL | let _: E = ::clone(&a); | ^^^^^^^^^^^^^^^^^^^^^^^ help: try dereferencing it: `*****a` error: using `clone` on type `char` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:110:14 + --> tests/ui/clone_on_copy.rs:111: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:129:14 + --> tests/ui/clone_on_copy.rs:130:14 | LL | vec.push(42.clone()); | ^^^^^^^^^^ help: try removing the `clone` call: `42` error: using `clone` on type `Option` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:136:17 + --> tests/ui/clone_on_copy.rs:137:17 | LL | let value = opt.clone()?; | ^^^^^^^^^^^ help: try dereferencing it: `(*opt)` error: using `clone` on type `i32` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:185:17 + --> tests/ui/clone_on_copy.rs:186:17 | LL | let _ = i32::clone(&x); | ^^^^^^^^^^^^^^ help: try removing the `clone` call: `x` error: using `clone` on type `i32` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:187:17 + --> tests/ui/clone_on_copy.rs:188:17 | LL | let _ = Clone::clone(&x); | ^^^^^^^^^^^^^^^^ help: try removing the `clone` call: `x` error: using `clone` on type `i32` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:189:17 + --> tests/ui/clone_on_copy.rs:190:17 | LL | let _ = ::clone(&x); | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try removing the `clone` call: `x` error: using `clone` on type `&i32` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:195:17 + --> tests/ui/clone_on_copy.rs:196:17 | LL | let _ = Clone::clone(ref_ref); | ^^^^^^^^^^^^^^^^^^^^^ help: try dereferencing it: `*ref_ref` error: using `clone` on type `&i32` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:197:17 + --> tests/ui/clone_on_copy.rs:198:17 | LL | let _ = <&i32 as Clone>::clone(ref_ref); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try dereferencing it: `*ref_ref` error: using `clone` on type `i32` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:201:17 + --> tests/ui/clone_on_copy.rs:202:17 | LL | let _ = i32::clone(&*imm_ref); | ^^^^^^^^^^^^^^^^^^^^^ help: try removing the `clone` call: `*imm_ref` error: using `clone` on type `i32` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:203:17 + --> tests/ui/clone_on_copy.rs:204:17 | LL | let _ = Clone::clone(&*imm_ref); | ^^^^^^^^^^^^^^^^^^^^^^^ help: try removing the `clone` call: `*imm_ref` error: using `clone` on type `i32` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:205:17 + --> tests/ui/clone_on_copy.rs:206:17 | LL | let _ = ::clone(&*imm_ref); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try removing the `clone` call: `*imm_ref` error: using `clone` on type `i32` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:209:17 + --> tests/ui/clone_on_copy.rs:210:17 | LL | let _ = i32::clone(&*x_ref); | ^^^^^^^^^^^^^^^^^^^ help: try removing the `clone` call: `*x_ref` error: using `clone` on type `i32` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:211:17 + --> tests/ui/clone_on_copy.rs:212:17 | LL | let _ = Clone::clone(&*x_ref); | ^^^^^^^^^^^^^^^^^^^^^ help: try removing the `clone` call: `*x_ref` error: using `clone` on type `i32` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:213:17 + --> tests/ui/clone_on_copy.rs:214:17 | LL | let _ = ::clone(&*x_ref); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try removing the `clone` call: `*x_ref` error: using `clone` on type `i32` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:216:17 + --> tests/ui/clone_on_copy.rs:217:17 | LL | let _ = i32::clone(&x).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:218:17 + --> tests/ui/clone_on_copy.rs:219:17 | LL | let _ = Clone::clone(&x).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:220:17 + --> tests/ui/clone_on_copy.rs:221:17 | LL | let _ = ::clone(&x).rotate_left(1); | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try removing the `clone` call: `x` error: using `clone` on type `char` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:227:18 + --> tests/ui/clone_on_copy.rs:228:18 | LL | is_ascii(char::clone(&'z')); | ^^^^^^^^^^^^^^^^^ help: try removing the `clone` call: `'z'` error: using `clone` on type `char` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:229:18 + --> tests/ui/clone_on_copy.rs:230:18 | LL | is_ascii(Clone::clone(&'z')); | ^^^^^^^^^^^^^^^^^^ help: try removing the `clone` call: `'z'` error: using `clone` on type `char` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:231:18 + --> tests/ui/clone_on_copy.rs:232:18 | LL | is_ascii(::clone(&'z')); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try removing the `clone` call: `'z'` error: using `clone` on type `i32` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:236:17 + --> tests/ui/clone_on_copy.rs:237:17 | LL | let _ = Clone::clone(&m!(42)); | ^^^^^^^^^^^^^^^^^^^^^ help: try removing the `clone` call: `m!(42)` error: using `clone` on type `i32` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:240:17 + --> tests/ui/clone_on_copy.rs:241:17 | LL | let _ = Clone::clone(&*rc.borrow()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try removing the `clone` call: `*rc.borrow()` error: using `clone` on type `Bar` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:244:17 + --> tests/ui/clone_on_copy.rs:245:17 | LL | let _ = Clone::clone(&bar).field; | ^^^^^^^^^^^^^^^^^^ help: try removing the `clone` call: `bar` error: using `clone` on type `[i32; 2]` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:248:17 + --> tests/ui/clone_on_copy.rs:249:17 | LL | let _ = Clone::clone(&array)[0]; | ^^^^^^^^^^^^^^^^^^^^ help: try removing the `clone` call: `array` error: using `clone` on type `[u32; 2]` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:252:17 + --> tests/ui/clone_on_copy.rs:253:17 | LL | let _ = Clone::clone(&*wrapped)[0]; | ^^^^^^^^^^^^^^^^^^^^^^^ help: try dereferencing it: `(*wrapped)` error: using `clone` on type `Option` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:275:17 + --> tests/ui/clone_on_copy.rs:276:17 | LL | let _ = Option::clone(opt)?; | ^^^^^^^^^^^^^^^^^^ help: try dereferencing it: `(*opt)` error: using `clone` on type `Option` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:277:17 + --> tests/ui/clone_on_copy.rs:278:17 | LL | let _ = Clone::clone(opt)?; | ^^^^^^^^^^^^^^^^^ help: try dereferencing it: `(*opt)` error: using `clone` on type `Option` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:279:17 + --> tests/ui/clone_on_copy.rs:280:17 | LL | let _ = as Clone>::clone(opt)?; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try dereferencing it: `(*opt)` From a8cc34724841dd46b1e75cb0ca2cb0d8fd9c5c20 Mon Sep 17 00:00:00 2001 From: qdot3 Date: Sat, 8 Aug 2026 10:32:50 +0900 Subject: [PATCH 07/16] fix `*.fixed` file --- tests/ui/clone_on_copy.fixed | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/ui/clone_on_copy.fixed b/tests/ui/clone_on_copy.fixed index 2d195638b342..05c3649aebb3 100644 --- a/tests/ui/clone_on_copy.fixed +++ b/tests/ui/clone_on_copy.fixed @@ -3,8 +3,8 @@ clippy::deref_addrof, clippy::needless_borrow, clippy::no_effect, - clippy::unnecessary_operation, - clippy::refcell_cell + clippy::refcell_cell, + clippy::unnecessary_operation )] #![expect(clippy::toplevel_ref_arg)] From 2fd2ac582096fb3b9264964c8ec8d4af547513fa Mon Sep 17 00:00:00 2001 From: qdot3 Date: Sat, 8 Aug 2026 10:41:07 +0900 Subject: [PATCH 08/16] fix example --- clippy_lints/src/refcell_cell.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/refcell_cell.rs b/clippy_lints/src/refcell_cell.rs index 2e4fe845aa2e..118a79515cbf 100644 --- a/clippy_lints/src/refcell_cell.rs +++ b/clippy_lints/src/refcell_cell.rs @@ -31,7 +31,7 @@ declare_clippy_lint! { /// struct Counter(RefCell); /// /// let _ = RefCell::new(1); - /// let _ = RefCell::default(); + /// let _ = RefCell::::default(); /// ``` /// Use instead: /// ```no_run @@ -40,7 +40,7 @@ declare_clippy_lint! { /// struct Counter(Cell); /// /// let _ = Cell::new(1); - /// let _ = Cell::default(); + /// let _ = Cell::::default(); /// ``` #[clippy::version = "1.99.0"] pub REFCELL_CELL, From f943fcd3cd0dea4968386a483a03b094b26d484a Mon Sep 17 00:00:00 2001 From: qdot3 Date: Wed, 12 Aug 2026 08:38:07 +0900 Subject: [PATCH 09/16] fix simple ctor pattern --- clippy_lints/src/refcell_cell.rs | 128 ++++++++++++++++++++++--------- tests/ui/refcell_cell.rs | 1 + tests/ui/refcell_cell.stderr | 20 +++-- 3 files changed, 106 insertions(+), 43 deletions(-) diff --git a/clippy_lints/src/refcell_cell.rs b/clippy_lints/src/refcell_cell.rs index 118a79515cbf..75e849d463e9 100644 --- a/clippy_lints/src/refcell_cell.rs +++ b/clippy_lints/src/refcell_cell.rs @@ -1,8 +1,8 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::res::{MaybeDef as _, MaybeQPath as _, MaybeResPath as _}; -use clippy_utils::source::{snippet, snippet_with_applicability, snippet_with_context}; +use clippy_utils::source::{snippet_with_applicability, snippet_with_context}; use clippy_utils::ty::{is_copy, ty_from_hir_ty}; -use clippy_utils::{is_trait_impl_item, sym}; +use clippy_utils::{is_trait_impl_item, qpath_generic_tys, sym}; use rustc_errors::Applicability; use rustc_hir::def_id::LocalDefId; use rustc_hir::intravisit::FnKind; @@ -111,33 +111,79 @@ impl<'tcx> LateLintPass<'tcx> for RefcellCell { && let Some(init) = let_stmt.init // RefCell && let Some(ty) = cx.typeck_results().expr_ty_opt(init) - && let ty::Adt(def,args) = ty.kind() + && let ty::Adt(def, args) = ty.kind() && def.is_diag_item(&cx.tcx, RefCell) && is_copy(cx, args.type_at(0)) // `init` is a constructor of `RefCell` && let ExprKind::Call(maybe_qpath, args) = init.kind - && is_refcell_ctor(cx, maybe_qpath, args.len()) + && let Some(kind) = CtorKind::new(cx, maybe_qpath, args.len()) // Be conservative about macro && args.iter().all(|e| !e.span.from_expansion()) { - let span = match ( - snippet(cx, init.span, "").contains("RefCell"), - let_stmt - .ty - .is_some_and(|ty| snippet(cx, ty.span, "").contains("RefCell")), - ) { - (true, true) => let_stmt.span, - (true, false) => init.span, - (false, true) => let_stmt.ty.unwrap().span, - (false, false) => return, - }; - - // Because this may be required to be `RefCell` + // This can be a false positive, for example, when passed to a function + // that requires `RefCell`. let mut app = Applicability::MaybeIncorrect; - let sugg = { - let (init, _) = snippet_with_context(cx, span, stmt.span.ctxt(), "..", &mut app); - init.into_owned().replace("RefCell", "Cell") + + let (span, sugg) = if let Some(annotation) = let_stmt.ty + && let TyKind::Path(qpath) = annotation.kind + && let Some(inner_ty) = qpath_generic_tys(&qpath).next() + { + let sugg = { + let inner_ty = snippet_with_context(cx, inner_ty.span, stmt.span.ctxt(), "_", &mut app).0; + + let arg = args + .first() + .map_or_default(|arg| snippet_with_context(cx, arg.span, stmt.span.ctxt(), "..", &mut app).0); + let init = match kind { + CtorKind::New => format!("Cell::new({arg})"), + CtorKind::From => format!("Cell::from({arg})"), + CtorKind::Default => "Cell::default()".to_string(), + }; + + format!(/* let pat: */ "Cell<{inner_ty}> = {init};") + }; + + (annotation.span.to(init.span), sugg) + } else { + let sugg = { + let arg = args + .first() + .map_or_default(|arg| snippet_with_context(cx, arg.span, stmt.span.ctxt(), "..", &mut app).0); + let init = match kind { + CtorKind::New => format!("Cell::new({arg})"), + CtorKind::From => format!("Cell::from({arg})"), + CtorKind::Default => "Cell::default()".to_string(), + }; + + let extract_ty = |qpath| { + // ::ctor + let ty = match qpath { + QPath::Resolved(ty, _) => ty, + QPath::TypeRelative(ty, _) => Some(ty), + }; + + if let Some(TyKind::Path(qpath)) = ty.map(|ty| ty.kind) + && let Some(ty) = qpath_generic_tys(&qpath).next() + { + Some(ty) + } else { + None + } + }; + + if let ExprKind::Path(qpath) = maybe_qpath.kind + && let Some(ty) = extract_ty(qpath) + { + let ty = snippet_with_context(cx, ty.span, stmt.span.ctxt(), "_", &mut app).0; + init.replacen("::", &format!("::<{ty}>::"), 1) + } else { + init + } + }; + + (init.span, sugg) }; + span_lint_and_sugg( cx, REFCELL_CELL, @@ -192,22 +238,32 @@ fn emit_refcell_copy_def<'tcx>(cx: &LateContext<'tcx>, hir_ty: &'tcx Ty<'tcx>) { } } -fn is_refcell_ctor<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, num_args: usize) -> bool { - if let Some(def_id) = expr.res(cx).opt_def_id() - && let Some(name) = cx.tcx.get_diagnostic_name(def_id) - && (matches!(name, sym::from_fn if num_args == 1) || matches!(name, sym::default_fn if num_args == 0)) - { - return true; - } +enum CtorKind { + New, + From, + Default, +} - // RefCell::new() - if let ExprKind::Path(QPath::TypeRelative(hir_ty, segment)) = expr.kind - && segment.ident.name == sym::new - && hir_ty.res(cx).is_diag_item(&cx.tcx, RefCell) - && num_args == 1 - { - return true; +impl CtorKind { + fn new<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, num_args: usize) -> Option { + if let Some(def_id) = expr.res(cx).opt_def_id() + && def_id.is_diag_item(&cx.tcx, sym::from_fn) + && num_args == 1 + { + Some(Self::From) + } else if let Some(def_id) = expr.res(cx).opt_def_id() + && def_id.is_diag_item(&cx.tcx, sym::default_fn) + && num_args == 0 + { + Some(Self::Default) + } else if let ExprKind::Path(QPath::TypeRelative(hir_ty, segment)) = expr.kind + && segment.ident.name == sym::new + && hir_ty.res(cx).is_diag_item(&cx.tcx, RefCell) + && num_args == 1 + { + Some(Self::New) + } else { + None + } } - - false } diff --git a/tests/ui/refcell_cell.rs b/tests/ui/refcell_cell.rs index 8ef540d64647..9194ace9a285 100644 --- a/tests/ui/refcell_cell.rs +++ b/tests/ui/refcell_cell.rs @@ -77,6 +77,7 @@ impl Dummy { fn main() { let _ = RefCell::new(1); //~ refcell_cell let _ = RefCell::::new(1); //~ refcell_cell + let _ = RefCell::<_>::new(1); //~ refcell_cell let _ = RefCell::new(vec![1]); let _ = RefCell::from(1); //~ refcell_cell diff --git a/tests/ui/refcell_cell.stderr b/tests/ui/refcell_cell.stderr index 153ff11bdd55..c270aa467154 100644 --- a/tests/ui/refcell_cell.stderr +++ b/tests/ui/refcell_cell.stderr @@ -86,28 +86,34 @@ LL | let _ = RefCell::::new(1); | ^^^^^^^^^^^^^^^^^^^^^^ help: try: `Cell::::new(1)` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:82:13 + --> tests/ui/refcell_cell.rs:80:13 + | +LL | let _ = RefCell::<_>::new(1); + | ^^^^^^^^^^^^^^^^^^^^ help: try: `Cell::new(1)` + +error: using a `RefCell` for a `Copy` type + --> tests/ui/refcell_cell.rs:83:13 | LL | let _ = RefCell::from(1); | ^^^^^^^^^^^^^^^^ help: try: `Cell::from(1)` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:83:13 + --> tests/ui/refcell_cell.rs:84:13 | LL | let _ = RefCell::::from(1); | ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Cell::::from(1)` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:85:12 + --> tests/ui/refcell_cell.rs:86:12 | LL | let _: RefCell = Default::default(); - | ^^^^^^^^^^^^ help: try: `Cell` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Cell = Cell::default();` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:86:13 + --> tests/ui/refcell_cell.rs:87:13 | LL | let _ = as Default>::default(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: ` as Default>::default()` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Cell::::default()` -error: aborting due to 18 previous errors +error: aborting due to 19 previous errors From 453d1d429120761ac3751abf8546b7e5a5fc2057 Mon Sep 17 00:00:00 2001 From: qdot3 Date: Fri, 14 Aug 2026 14:55:57 +0900 Subject: [PATCH 10/16] suppress the lint for large `Copy` types --- CHANGELOG.md | 2 + book/src/lint_configuration.md | 10 ++ clippy_config/src/conf.rs | 3 + clippy_lints/src/lib.rs | 2 +- clippy_lints/src/refcell_cell.rs | 128 ++++++++++-------- .../toml_unknown_key/conf_unknown_key.stderr | 1 + tests/ui/refcell_cell.rs | 4 + tests/ui/refcell_cell.stderr | 36 ++--- 8 files changed, 114 insertions(+), 72 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 62c8a63baed2..09ba79656658 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7394,6 +7394,7 @@ Released 2018-09-13 [`ref_option`]: https://rust-lang.github.io/rust-clippy/main/index.html#ref_option [`ref_option_ref`]: https://rust-lang.github.io/rust-clippy/main/index.html#ref_option_ref [`ref_patterns`]: https://rust-lang.github.io/rust-clippy/main/index.html#ref_patterns +[`refcell_cell`]: https://rust-lang.github.io/rust-clippy/main/index.html#refcell_cell [`regex_creation_in_loops`]: https://rust-lang.github.io/rust-clippy/main/index.html#regex_creation_in_loops [`regex_macro`]: https://rust-lang.github.io/rust-clippy/main/index.html#regex_macro [`renamed_function_params`]: https://rust-lang.github.io/rust-clippy/main/index.html#renamed_function_params @@ -7722,6 +7723,7 @@ Released 2018-09-13 [`lint-commented-code`]: https://doc.rust-lang.org/clippy/lint_configuration.html#lint-commented-code [`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 +[`max_cheap-copy-size`]: https://doc.rust-lang.org/clippy/lint_configuration.html#max_cheap-copy-size [`max-fn-params-bools`]: https://doc.rust-lang.org/clippy/lint_configuration.html#max-fn-params-bools [`max-include-file-size`]: https://doc.rust-lang.org/clippy/lint_configuration.html#max-include-file-size [`max-struct-bools`]: https://doc.rust-lang.org/clippy/lint_configuration.html#max-struct-bools diff --git a/book/src/lint_configuration.md b/book/src/lint_configuration.md index 92a27c5fe598..6711cc85bc25 100644 --- a/book/src/lint_configuration.md +++ b/book/src/lint_configuration.md @@ -810,6 +810,16 @@ be filtering for common types. * [`manual_let_else`](https://rust-lang.github.io/rust-clippy/main/index.html#manual_let_else) +## `max_cheap-copy-size` +Maximum size in bytes for a `Copy` type to be considered cheap to copy. + +**Default Value:** `16` + +--- +**Affected lints:** +* [`refcell_cell`](https://rust-lang.github.io/rust-clippy/main/index.html#refcell_cell) + + ## `max-fn-params-bools` The maximum number of bool parameters a function can have. Use `0` to lint on any function with a bool parameter. diff --git a/clippy_config/src/conf.rs b/clippy_config/src/conf.rs index 13c73e944581..b226f675f299 100644 --- a/clippy_config/src/conf.rs +++ b/clippy_config/src/conf.rs @@ -597,6 +597,9 @@ define_Conf! { /// be filtering for common types. #[lints(manual_let_else)] matches_for_let_else("matches-for-let-else"): MatchLintBehaviour = MatchLintBehaviour::WellKnownTypes, + /// Maximum size in bytes for a `Copy` type to be considered cheap to copy. + #[lints(refcell_cell)] + max_cheap_copy_size("max_cheap-copy-size"): u64 = 16, /// The maximum number of bool parameters a function can have. /// Use `0` to lint on any function with a bool parameter. #[lints(fn_params_excessive_bools)] diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 06f8e2013e34..b03bf35890cf 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -870,7 +870,7 @@ rustc_lint::late_lint_methods!( BlockScrutinee: block_scrutinee::BlockScrutinee = block_scrutinee::BlockScrutinee, NonnullUncheckedOnBoxPtr: nonnull_unchecked_on_box_ptr::NonnullUncheckedOnBoxPtr = nonnull_unchecked_on_box_ptr::NonnullUncheckedOnBoxPtr::new(conf), NeedlessNonzeroGet: needless_nonzero_get::NeedlessNonzeroGet = needless_nonzero_get::NeedlessNonzeroGet::new(conf), - RefcellCell: refcell_cell::RefcellCell = refcell_cell::RefcellCell, + RefcellCell: refcell_cell::RefcellCell = refcell_cell::RefcellCell::new(conf), // add late passes here, used by `cargo dev new_lint` ]] ); diff --git a/clippy_lints/src/refcell_cell.rs b/clippy_lints/src/refcell_cell.rs index 75e849d463e9..6e307fc16c97 100644 --- a/clippy_lints/src/refcell_cell.rs +++ b/clippy_lints/src/refcell_cell.rs @@ -1,7 +1,8 @@ +use clippy_config::Conf; use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::res::{MaybeDef as _, MaybeQPath as _, MaybeResPath as _}; use clippy_utils::source::{snippet_with_applicability, snippet_with_context}; -use clippy_utils::ty::{is_copy, ty_from_hir_ty}; +use clippy_utils::ty::{approx_ty_size, is_copy, ty_from_hir_ty}; use clippy_utils::{is_trait_impl_item, qpath_generic_tys, sym}; use rustc_errors::Applicability; use rustc_hir::def_id::LocalDefId; @@ -12,7 +13,7 @@ use rustc_hir::{ }; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty; -use rustc_session::declare_lint_pass; +use rustc_session::impl_lint_pass; use rustc_span::Span; use rustc_span::sym::RefCell; @@ -22,7 +23,11 @@ declare_clippy_lint! { /// /// ### Why is this bad? /// `RefCell` avoids cloning at the cost of additional memory usage and - /// instructions, which isn't worth it for `Copy` types. + /// instructions, which isn't worth it for **small** `Copy` types. + /// + /// ### Known problems + /// `RefCell` might be useful for **large** `Copy` types. + /// See for details. /// /// ### Example /// ```no_run @@ -48,12 +53,68 @@ declare_clippy_lint! { "using a `RefCell` for a `Copy` type" } -declare_lint_pass!(RefcellCell => [REFCELL_CELL]); +impl_lint_pass!(RefcellCell => [REFCELL_CELL]); + +pub struct RefcellCell { + max_cheap_copy_size: u64, +} + +impl RefcellCell { + pub fn new(conf: &'static Conf) -> Self { + Self { + max_cheap_copy_size: conf.max_cheap_copy_size, + } + } + + fn emit_refcell_copy_def<'tcx>(&self, cx: &LateContext<'tcx>, hir_ty: &'tcx Ty<'tcx>) { + match hir_ty.kind { + TyKind::Array(hir_ty, _) | TyKind::Slice(hir_ty) => { + self.emit_refcell_copy_def(cx, hir_ty); + }, + TyKind::Tup(hir_tys) => { + for hir_ty in hir_tys { + self.emit_refcell_copy_def(cx, hir_ty); + } + }, + TyKind::Path(qpath) + if let (None, Some(path)) = qpath.opt_res_path() + // RefCell + && let [segment] = path.segments + && segment.res.is_diag_item(&cx.tcx, RefCell) + && let Some(args) = segment.args + && let [GenericArg::Type(hir_ty)] = args.args + // `T` is copyable and small + && { + let ty = ty_from_hir_ty(cx, hir_ty.as_unambig_ty()); + is_copy(cx, ty) && approx_ty_size(cx, ty) <= self.max_cheap_copy_size + } + // Be conservative about macro + && !hir_ty.span.from_expansion() => + { + let mut app = Applicability::MaybeIncorrect; + let sugg = { + let hir_ty = snippet_with_applicability(cx, hir_ty.span, "_", &mut app); + format!("Cell<{hir_ty}>") + }; + span_lint_and_sugg( + cx, + REFCELL_CELL, + path.span, + "using a `RefCell` for a `Copy` type", + "try", + sugg, + app, + ); + }, + _ => (), + } + } +} impl<'tcx> LateLintPass<'tcx> for RefcellCell { fn check_field_def(&mut self, cx: &LateContext<'tcx>, def: &'tcx FieldDef<'tcx>) { if !def.span.from_expansion() { - emit_refcell_copy_def(cx, def.ty); + self.emit_refcell_copy_def(cx, def.ty); } } @@ -68,10 +129,10 @@ impl<'tcx> LateLintPass<'tcx> for RefcellCell { ) { if !span.from_expansion() && !is_trait_impl_item(cx, cx.tcx.local_def_id_to_hir_id(def_id)) { for hir_ty in decl.inputs { - emit_refcell_copy_def(cx, hir_ty); + self.emit_refcell_copy_def(cx, hir_ty); } if let FnRetTy::Return(hir_ty) = decl.output { - emit_refcell_copy_def(cx, hir_ty); + self.emit_refcell_copy_def(cx, hir_ty); } } } @@ -80,7 +141,7 @@ impl<'tcx> LateLintPass<'tcx> for RefcellCell { if let ImplItemKind::Type(hir_ty) = item.kind && !item.span.from_expansion() { - emit_refcell_copy_def(cx, hir_ty); + self.emit_refcell_copy_def(cx, hir_ty); } } @@ -88,7 +149,7 @@ impl<'tcx> LateLintPass<'tcx> for RefcellCell { if let ItemKind::TyAlias(_, _, hir_ty) = item.kind && !item.span.from_expansion() { - emit_refcell_copy_def(cx, hir_ty); + self.emit_refcell_copy_def(cx, hir_ty); } } @@ -97,10 +158,10 @@ impl<'tcx> LateLintPass<'tcx> for RefcellCell { && !item.span.from_expansion() { for hir_ty in sig.decl.inputs { - emit_refcell_copy_def(cx, hir_ty); + self.emit_refcell_copy_def(cx, hir_ty); } if let FnRetTy::Return(hir_ty) = sig.decl.output { - emit_refcell_copy_def(cx, hir_ty); + self.emit_refcell_copy_def(cx, hir_ty); } } } @@ -109,11 +170,13 @@ impl<'tcx> LateLintPass<'tcx> for RefcellCell { if let StmtKind::Let(let_stmt) = stmt.kind && !stmt.span.from_expansion() && let Some(init) = let_stmt.init - // RefCell + // RefCell && let Some(ty) = cx.typeck_results().expr_ty_opt(init) && let ty::Adt(def, args) = ty.kind() && def.is_diag_item(&cx.tcx, RefCell) + // `T` is a small `Copy` type && is_copy(cx, args.type_at(0)) + && approx_ty_size(cx, args.type_at(0)) <= self.max_cheap_copy_size // `init` is a constructor of `RefCell` && let ExprKind::Call(maybe_qpath, args) = init.kind && let Some(kind) = CtorKind::new(cx, maybe_qpath, args.len()) @@ -197,47 +260,6 @@ impl<'tcx> LateLintPass<'tcx> for RefcellCell { } } -fn emit_refcell_copy_def<'tcx>(cx: &LateContext<'tcx>, hir_ty: &'tcx Ty<'tcx>) { - match hir_ty.kind { - TyKind::Array(hir_ty, _) | TyKind::Slice(hir_ty) => { - emit_refcell_copy_def(cx, hir_ty); - }, - TyKind::Tup(hir_tys) => { - for hir_ty in hir_tys { - emit_refcell_copy_def(cx, hir_ty); - } - }, - TyKind::Path(qpath) - if let (None, Some(path)) = qpath.opt_res_path() - // RefCell - && let [segment] = path.segments - && segment.res.is_diag_item(&cx.tcx, RefCell) - && let Some(args) = segment.args - && let [GenericArg::Type(hir_ty)] = args.args - // T: Copy - && is_copy(cx, ty_from_hir_ty(cx, hir_ty.as_unambig_ty())) - // Be conservative about macro - && !hir_ty.span.from_expansion() => - { - let mut app = Applicability::MaybeIncorrect; - let sugg = { - let hir_ty = snippet_with_applicability(cx, hir_ty.span, "_", &mut app); - format!("Cell<{hir_ty}>") - }; - span_lint_and_sugg( - cx, - REFCELL_CELL, - path.span, - "using a `RefCell` for a `Copy` type", - "try", - sugg, - app, - ); - }, - _ => (), - } -} - enum CtorKind { New, From, diff --git a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr index bee9e63ef742..91a4fe329673 100644 --- a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr +++ b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr @@ -65,6 +65,7 @@ LL | foobar = 42 lint-commented-code literal-representation-threshold matches-for-let-else + max_cheap-copy-size max-fn-params-bools max-include-file-size max-struct-bools diff --git a/tests/ui/refcell_cell.rs b/tests/ui/refcell_cell.rs index 9194ace9a285..8e35d5c8937c 100644 --- a/tests/ui/refcell_cell.rs +++ b/tests/ui/refcell_cell.rs @@ -7,6 +7,7 @@ use std::hint::black_box; struct NameFieldStruct { ng: RefCell, //~ refcell_cell ok: RefCell, + large_copy: RefCell<[i32; 16]>, } struct TupleStruct( @@ -78,7 +79,10 @@ fn main() { let _ = RefCell::new(1); //~ refcell_cell let _ = RefCell::::new(1); //~ refcell_cell let _ = RefCell::<_>::new(1); //~ refcell_cell + // non-`Copy` type let _ = RefCell::new(vec![1]); + // large `Copy` type + let _ = RefCell::new([1; 1024]); let _ = RefCell::from(1); //~ refcell_cell let _ = RefCell::::from(1); //~ refcell_cell diff --git a/tests/ui/refcell_cell.stderr b/tests/ui/refcell_cell.stderr index c270aa467154..899e4f37608f 100644 --- a/tests/ui/refcell_cell.stderr +++ b/tests/ui/refcell_cell.stderr @@ -8,109 +8,109 @@ LL | ng: RefCell, = help: to override `-D warnings` add `#[allow(clippy::refcell_cell)]` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:13:5 + --> tests/ui/refcell_cell.rs:14:5 | LL | RefCell, | ^^^^^^^^^^ help: try: `Cell` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:18:8 + --> tests/ui/refcell_cell.rs:19:8 | LL | NG(RefCell), | ^^^^^^^^^^ help: try: `Cell` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:21:13 + --> tests/ui/refcell_cell.rs:22:13 | LL | ng: RefCell, | ^^^^^^^^^^ help: try: `Cell` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:26:16 + --> tests/ui/refcell_cell.rs:27:16 | LL | type AliasNG = RefCell; | ^^^^^^^^^^^^ help: try: `Cell` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:36:13 + --> tests/ui/refcell_cell.rs:37:13 | LL | ng: RefCell, | ^^^^^^^^^^ help: try: `Cell` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:39:9 + --> tests/ui/refcell_cell.rs:40:9 | LL | RefCell, | ^^^^^^^^^^ help: try: `Cell` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:47:15 + --> tests/ui/refcell_cell.rs:48:15 | LL | type NG = RefCell; | ^^^^^^^^^^ help: try: `Cell` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:56:9 + --> tests/ui/refcell_cell.rs:57:9 | LL | ng: RefCell, | ^^^^^^^^^^ help: try: `Cell` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:59:5 + --> tests/ui/refcell_cell.rs:60:5 | LL | RefCell, | ^^^^^^^^^^ help: try: `Cell` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:67:13 + --> tests/ui/refcell_cell.rs:68:13 | LL | ng: RefCell, | ^^^^^^^^^^ help: try: `Cell` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:70:9 + --> tests/ui/refcell_cell.rs:71:9 | LL | RefCell, | ^^^^^^^^^^ help: try: `Cell` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:78:13 + --> tests/ui/refcell_cell.rs:79:13 | LL | let _ = RefCell::new(1); | ^^^^^^^^^^^^^^^ help: try: `Cell::new(1)` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:79:13 + --> tests/ui/refcell_cell.rs:80:13 | LL | let _ = RefCell::::new(1); | ^^^^^^^^^^^^^^^^^^^^^^ help: try: `Cell::::new(1)` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:80:13 + --> tests/ui/refcell_cell.rs:81:13 | LL | let _ = RefCell::<_>::new(1); | ^^^^^^^^^^^^^^^^^^^^ help: try: `Cell::new(1)` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:83:13 + --> tests/ui/refcell_cell.rs:87:13 | LL | let _ = RefCell::from(1); | ^^^^^^^^^^^^^^^^ help: try: `Cell::from(1)` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:84:13 + --> tests/ui/refcell_cell.rs:88:13 | LL | let _ = RefCell::::from(1); | ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Cell::::from(1)` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:86:12 + --> tests/ui/refcell_cell.rs:90:12 | LL | let _: RefCell = Default::default(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Cell = Cell::default();` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:87:13 + --> tests/ui/refcell_cell.rs:91:13 | LL | let _ = as Default>::default(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Cell::::default()` From 4af35b815cdf7570cb46119eeecc8eb581286579 Mon Sep 17 00:00:00 2001 From: qdot3 Date: Fri, 14 Aug 2026 20:29:26 +0900 Subject: [PATCH 11/16] Lint `&RefCell` usage except in return types --- clippy_lints/src/refcell_cell.rs | 23 ++++++++------- tests/ui/refcell_cell.rs | 10 +++++-- tests/ui/refcell_cell.stderr | 50 ++++++++++++++++++++------------ 3 files changed, 52 insertions(+), 31 deletions(-) diff --git a/clippy_lints/src/refcell_cell.rs b/clippy_lints/src/refcell_cell.rs index 6e307fc16c97..13f289f68096 100644 --- a/clippy_lints/src/refcell_cell.rs +++ b/clippy_lints/src/refcell_cell.rs @@ -66,16 +66,19 @@ impl RefcellCell { } } - fn emit_refcell_copy_def<'tcx>(&self, cx: &LateContext<'tcx>, hir_ty: &'tcx Ty<'tcx>) { + fn emit_refcell_copy_def<'tcx>(&self, cx: &LateContext<'tcx>, hir_ty: &'tcx Ty<'tcx>, no_ref: bool) { match hir_ty.kind { TyKind::Array(hir_ty, _) | TyKind::Slice(hir_ty) => { - self.emit_refcell_copy_def(cx, hir_ty); + self.emit_refcell_copy_def(cx, hir_ty, no_ref); }, TyKind::Tup(hir_tys) => { for hir_ty in hir_tys { - self.emit_refcell_copy_def(cx, hir_ty); + self.emit_refcell_copy_def(cx, hir_ty, no_ref); } }, + TyKind::Ref(_, mut_ty) if !no_ref => { + self.emit_refcell_copy_def(cx, mut_ty.ty, no_ref); + }, TyKind::Path(qpath) if let (None, Some(path)) = qpath.opt_res_path() // RefCell @@ -114,7 +117,7 @@ impl RefcellCell { impl<'tcx> LateLintPass<'tcx> for RefcellCell { fn check_field_def(&mut self, cx: &LateContext<'tcx>, def: &'tcx FieldDef<'tcx>) { if !def.span.from_expansion() { - self.emit_refcell_copy_def(cx, def.ty); + self.emit_refcell_copy_def(cx, def.ty, false); } } @@ -129,10 +132,10 @@ impl<'tcx> LateLintPass<'tcx> for RefcellCell { ) { if !span.from_expansion() && !is_trait_impl_item(cx, cx.tcx.local_def_id_to_hir_id(def_id)) { for hir_ty in decl.inputs { - self.emit_refcell_copy_def(cx, hir_ty); + self.emit_refcell_copy_def(cx, hir_ty, false); } if let FnRetTy::Return(hir_ty) = decl.output { - self.emit_refcell_copy_def(cx, hir_ty); + self.emit_refcell_copy_def(cx, hir_ty, true); } } } @@ -141,7 +144,7 @@ impl<'tcx> LateLintPass<'tcx> for RefcellCell { if let ImplItemKind::Type(hir_ty) = item.kind && !item.span.from_expansion() { - self.emit_refcell_copy_def(cx, hir_ty); + self.emit_refcell_copy_def(cx, hir_ty, false); } } @@ -149,7 +152,7 @@ impl<'tcx> LateLintPass<'tcx> for RefcellCell { if let ItemKind::TyAlias(_, _, hir_ty) = item.kind && !item.span.from_expansion() { - self.emit_refcell_copy_def(cx, hir_ty); + self.emit_refcell_copy_def(cx, hir_ty, false); } } @@ -158,10 +161,10 @@ impl<'tcx> LateLintPass<'tcx> for RefcellCell { && !item.span.from_expansion() { for hir_ty in sig.decl.inputs { - self.emit_refcell_copy_def(cx, hir_ty); + self.emit_refcell_copy_def(cx, hir_ty, false); } if let FnRetTy::Return(hir_ty) = sig.decl.output { - self.emit_refcell_copy_def(cx, hir_ty); + self.emit_refcell_copy_def(cx, hir_ty, true); } } } diff --git a/tests/ui/refcell_cell.rs b/tests/ui/refcell_cell.rs index 8e35d5c8937c..e04c9b6f6fd5 100644 --- a/tests/ui/refcell_cell.rs +++ b/tests/ui/refcell_cell.rs @@ -4,9 +4,11 @@ use std::cell::{Cell, RefCell}; use std::hint::black_box; -struct NameFieldStruct { +struct NameFieldStruct<'a, T: Copy, U> { ng: RefCell, //~ refcell_cell ok: RefCell, + ng_ref: &'a RefCell, //~ refcell_cell + ok_ref: &'a RefCell, large_copy: RefCell<[i32; 16]>, } @@ -53,12 +55,16 @@ impl Trait for Dummy { } } -fn func( +fn func<'a, T: Copy, U>( ng: RefCell, //~ refcell_cell ok: RefCell, + ng_ref: &'a RefCell, //~ refcell_cell + ok_ref: &'a RefCell, ) -> ( RefCell, //~ refcell_cell RefCell, + &'a RefCell, // should be linted on definition site + &'a RefCell, ) { todo!() } diff --git a/tests/ui/refcell_cell.stderr b/tests/ui/refcell_cell.stderr index 899e4f37608f..d2b4b07fa463 100644 --- a/tests/ui/refcell_cell.stderr +++ b/tests/ui/refcell_cell.stderr @@ -8,112 +8,124 @@ LL | ng: RefCell, = help: to override `-D warnings` add `#[allow(clippy::refcell_cell)]` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:14:5 + --> tests/ui/refcell_cell.rs:10:17 + | +LL | ng_ref: &'a RefCell, + | ^^^^^^^^^^ help: try: `Cell` + +error: using a `RefCell` for a `Copy` type + --> tests/ui/refcell_cell.rs:16:5 | LL | RefCell, | ^^^^^^^^^^ help: try: `Cell` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:19:8 + --> tests/ui/refcell_cell.rs:21:8 | LL | NG(RefCell), | ^^^^^^^^^^ help: try: `Cell` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:22:13 + --> tests/ui/refcell_cell.rs:24:13 | LL | ng: RefCell, | ^^^^^^^^^^ help: try: `Cell` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:27:16 + --> tests/ui/refcell_cell.rs:29:16 | LL | type AliasNG = RefCell; | ^^^^^^^^^^^^ help: try: `Cell` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:37:13 + --> tests/ui/refcell_cell.rs:39:13 | LL | ng: RefCell, | ^^^^^^^^^^ help: try: `Cell` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:40:9 + --> tests/ui/refcell_cell.rs:42:9 | LL | RefCell, | ^^^^^^^^^^ help: try: `Cell` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:48:15 + --> tests/ui/refcell_cell.rs:50:15 | LL | type NG = RefCell; | ^^^^^^^^^^ help: try: `Cell` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:57:9 + --> tests/ui/refcell_cell.rs:59:9 | LL | ng: RefCell, | ^^^^^^^^^^ help: try: `Cell` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:60:5 + --> tests/ui/refcell_cell.rs:61:17 + | +LL | ng_ref: &'a RefCell, + | ^^^^^^^^^^ help: try: `Cell` + +error: using a `RefCell` for a `Copy` type + --> tests/ui/refcell_cell.rs:64:5 | LL | RefCell, | ^^^^^^^^^^ help: try: `Cell` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:68:13 + --> tests/ui/refcell_cell.rs:74:13 | LL | ng: RefCell, | ^^^^^^^^^^ help: try: `Cell` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:71:9 + --> tests/ui/refcell_cell.rs:77:9 | LL | RefCell, | ^^^^^^^^^^ help: try: `Cell` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:79:13 + --> tests/ui/refcell_cell.rs:85:13 | LL | let _ = RefCell::new(1); | ^^^^^^^^^^^^^^^ help: try: `Cell::new(1)` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:80:13 + --> tests/ui/refcell_cell.rs:86:13 | LL | let _ = RefCell::::new(1); | ^^^^^^^^^^^^^^^^^^^^^^ help: try: `Cell::::new(1)` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:81:13 + --> tests/ui/refcell_cell.rs:87:13 | LL | let _ = RefCell::<_>::new(1); | ^^^^^^^^^^^^^^^^^^^^ help: try: `Cell::new(1)` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:87:13 + --> tests/ui/refcell_cell.rs:93:13 | LL | let _ = RefCell::from(1); | ^^^^^^^^^^^^^^^^ help: try: `Cell::from(1)` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:88:13 + --> tests/ui/refcell_cell.rs:94:13 | LL | let _ = RefCell::::from(1); | ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Cell::::from(1)` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:90:12 + --> tests/ui/refcell_cell.rs:96:12 | LL | let _: RefCell = Default::default(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Cell = Cell::default();` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:91:13 + --> tests/ui/refcell_cell.rs:97:13 | LL | let _ = as Default>::default(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Cell::::default()` -error: aborting due to 19 previous errors +error: aborting due to 21 previous errors From 734b20cf2d56e22424e27d7869b983e1d3065464 Mon Sep 17 00:00:00 2001 From: qdot3 Date: Mon, 17 Aug 2026 22:41:28 +0900 Subject: [PATCH 12/16] simple usage analysis: exclude local variables that is used as an argument of function and required to be `RefCell` --- clippy_lints/src/refcell_cell.rs | 70 ++++++++++--- tests/ui/refcell_cell.rs | 162 ++++++++++++++++++++++++------- tests/ui/refcell_cell.stderr | 96 +++++++++++------- 3 files changed, 247 insertions(+), 81 deletions(-) diff --git a/clippy_lints/src/refcell_cell.rs b/clippy_lints/src/refcell_cell.rs index 13f289f68096..6a2bee207fe5 100644 --- a/clippy_lints/src/refcell_cell.rs +++ b/clippy_lints/src/refcell_cell.rs @@ -1,15 +1,18 @@ +use std::ops::ControlFlow::{Break, Continue}; + use clippy_config::Conf; use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::res::{MaybeDef as _, MaybeQPath as _, MaybeResPath as _}; use clippy_utils::source::{snippet_with_applicability, snippet_with_context}; use clippy_utils::ty::{approx_ty_size, is_copy, ty_from_hir_ty}; -use clippy_utils::{is_trait_impl_item, qpath_generic_tys, sym}; +use clippy_utils::visitors::for_each_local_use_after_expr; +use clippy_utils::{ExprUseNode, get_expr_use_site, get_parent_expr, is_trait_impl_item, qpath_generic_tys, sym}; use rustc_errors::Applicability; use rustc_hir::def_id::LocalDefId; use rustc_hir::intravisit::FnKind; use rustc_hir::{ - self, Body, Expr, ExprKind, FieldDef, FnDecl, FnRetTy, GenericArg, ImplItem, ImplItemKind, Item, ItemKind, QPath, - Stmt, StmtKind, TraitItem, TraitItemKind, Ty, TyKind, + self, Body, Expr, ExprKind, FieldDef, FnDecl, FnRetTy, GenericArg, ImplItem, ImplItemKind, Item, ItemKind, Pat, + QPath, Stmt, StmtKind, TraitItem, TraitItemKind, Ty, TyKind, }; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty; @@ -173,6 +176,7 @@ impl<'tcx> LateLintPass<'tcx> for RefcellCell { if let StmtKind::Let(let_stmt) = stmt.kind && !stmt.span.from_expansion() && let Some(init) = let_stmt.init + && !callee_requires_refcell(cx, let_stmt.pat, init) // RefCell && let Some(ty) = cx.typeck_results().expr_ty_opt(init) && let ty::Adt(def, args) = ty.kind() @@ -180,14 +184,13 @@ impl<'tcx> LateLintPass<'tcx> for RefcellCell { // `T` is a small `Copy` type && is_copy(cx, args.type_at(0)) && approx_ty_size(cx, args.type_at(0)) <= self.max_cheap_copy_size - // `init` is a constructor of `RefCell` - && let ExprKind::Call(maybe_qpath, args) = init.kind - && let Some(kind) = CtorKind::new(cx, maybe_qpath, args.len()) + // `init` is a (simple) constructor of `RefCell` + && let ExprKind::Call(maybe_qpath, ctor_args) = init.kind + && let Some(kind) = CtorKind::new(cx, maybe_qpath, ctor_args.len()) // Be conservative about macro - && args.iter().all(|e| !e.span.from_expansion()) + && ctor_args.iter().all(|e| !e.span.from_expansion()) { - // This can be a false positive, for example, when passed to a function - // that requires `RefCell`. + // This can be a false positive because trait bounds are not considered. let mut app = Applicability::MaybeIncorrect; let (span, sugg) = if let Some(annotation) = let_stmt.ty @@ -197,7 +200,7 @@ impl<'tcx> LateLintPass<'tcx> for RefcellCell { let sugg = { let inner_ty = snippet_with_context(cx, inner_ty.span, stmt.span.ctxt(), "_", &mut app).0; - let arg = args + let arg = ctor_args .first() .map_or_default(|arg| snippet_with_context(cx, arg.span, stmt.span.ctxt(), "..", &mut app).0); let init = match kind { @@ -212,7 +215,7 @@ impl<'tcx> LateLintPass<'tcx> for RefcellCell { (annotation.span.to(init.span), sugg) } else { let sugg = { - let arg = args + let arg = ctor_args .first() .map_or_default(|arg| snippet_with_context(cx, arg.span, stmt.span.ctxt(), "..", &mut app).0); let init = match kind { @@ -292,3 +295,48 @@ impl CtorKind { } } } + +/// Trait bounds are not considered due to complexity (for now). +fn callee_requires_refcell<'tcx>(cx: &LateContext<'tcx>, pat: &'tcx Pat<'tcx>, init: &'tcx Expr<'tcx>) -> bool { + for_each_local_use_after_expr(cx, pat.hir_id, init.hir_id, |expr| { + // attach `&`s as much as possible + let expr = { + let mut expr = expr; + while let Some(p) = get_parent_expr(cx, expr) { + match p.kind { + ExprKind::AddrOf(..) => expr = p, + _ => break, + } + } + expr + }; + + let node = get_expr_use_site(cx.tcx, cx.typeck_results(), expr.span.ctxt(), expr); + let (def_id, i) = match node.use_node(cx) { + ExprUseNode::FnArg(path, i) if let Some(def_id) = path.res(cx).opt_def_id() => (def_id, i), + ExprUseNode::MethodArg(hir_id, _, i) + if let Some(def_id) = cx.typeck_results().type_dependent_def_id(hir_id) => + { + (def_id, i) + }, + _ => return Continue(()), + }; + + // Does callee require `RefCell`? + let input_ty = cx + .tcx + .fn_sig(def_id) + .instantiate_identity() + .skip_normalization() + .input(i) + // Need early bound params but not late ones + .skip_binder(); + + if input_ty.peel_refs().is_diag_item(&cx.tcx, RefCell) { + Break(()) + } else { + Continue(()) + } + }) + .is_break() +} diff --git a/tests/ui/refcell_cell.rs b/tests/ui/refcell_cell.rs index e04c9b6f6fd5..8fa1eae424f1 100644 --- a/tests/ui/refcell_cell.rs +++ b/tests/ui/refcell_cell.rs @@ -9,7 +9,7 @@ struct NameFieldStruct<'a, T: Copy, U> { ok: RefCell, ng_ref: &'a RefCell, //~ refcell_cell ok_ref: &'a RefCell, - large_copy: RefCell<[i32; 16]>, + large_copy: RefCell<[i32; 1024]>, } struct TupleStruct( @@ -35,13 +35,13 @@ trait Trait { type NG; type OK; - fn func( - ng: RefCell, //~ refcell_cell - ok: RefCell, - ) -> ( - RefCell, //~ refcell_cell - RefCell, - ); + fn ng(ng: RefCell); //~ refcell_cell + fn ok(ok: RefCell); + fn ng_ref(ng: &RefCell); //~ refcell_cell + fn ok_ref(ok: &RefCell); + fn ng_ret() -> RefCell; //~ refcell_cell + fn ok_ret() -> RefCell; + fn ok_ret_ref<'a>() -> &'a RefCell; // Should be linted at definition site } struct Dummy; @@ -50,7 +50,18 @@ impl Trait for Dummy { type NG = RefCell; //~ refcell_cell type OK = RefCell; - fn func(ng: RefCell, ok: RefCell) -> (RefCell, RefCell) { + // Should be linted at definition site + fn ng(ng: RefCell) {} + fn ok(ok: RefCell) {} + fn ng_ref(ng: &RefCell) {} + fn ok_ref(ok: &RefCell) {} + fn ng_ret() -> RefCell { + todo!() + } + fn ok_ret() -> RefCell { + todo!() + } + fn ok_ret_ref<'a>() -> &'a RefCell { todo!() } } @@ -63,7 +74,7 @@ fn func<'a, T: Copy, U>( ) -> ( RefCell, //~ refcell_cell RefCell, - &'a RefCell, // should be linted on definition site + &'a RefCell, // Should be linted at definition site &'a RefCell, ) { todo!() @@ -82,37 +93,114 @@ impl Dummy { } fn main() { - let _ = RefCell::new(1); //~ refcell_cell - let _ = RefCell::::new(1); //~ refcell_cell - let _ = RefCell::<_>::new(1); //~ refcell_cell - // non-`Copy` type - let _ = RefCell::new(vec![1]); - // large `Copy` type - let _ = RefCell::new([1; 1024]); + // Simple constructors + { + let _ = RefCell::new(1); //~ refcell_cell + let _ = RefCell::::new(1); //~ refcell_cell + let _ = RefCell::<_>::new(1); //~ refcell_cell + // non-`Copy` type + let _ = RefCell::new(vec![1]); + // large `Copy` type + let _ = RefCell::new([1; 1024]); + + let _ = RefCell::from(1); //~ refcell_cell + let _ = RefCell::::from(1); //~ refcell_cell + + let _: RefCell = Default::default(); //~ refcell_cell + let _ = as Default>::default(); //~ refcell_cell + + // False negative: This lint does not check trait bounds on call site. + let _ = black_box(RefCell::new(1)); + } - let _ = RefCell::from(1); //~ refcell_cell - let _ = RefCell::::from(1); //~ refcell_cell + // Macro expansion + { + macro_rules! black_box { + ( $e:expr ) => { + black_box($e) + }; + } + let _ = RefCell::new(black_box!(1)); + let _ = RefCell::from(black_box!(1)); + let _ = black_box!(RefCell::new(1)); + + macro_rules! ignore_me { + () => { + let _ = RefCell::new(1); + }; + } + ignore_me!(); + } +} - let _: RefCell = Default::default(); //~ refcell_cell - let _ = as Default>::default(); //~ refcell_cell +// Suppress false positives for the simple ctor pattern +mod usage_analysis { + use std::cell::RefCell; + use std::hint::black_box; - // False negative: This lint does not check trait bounds on call site. - let _ = black_box(RefCell::new(1)); + fn callee(x: RefCell) -> T { + todo!() + } - // Macro expansion - macro_rules! black_box { - ( $e:expr ) => { - black_box($e) - }; + fn callee_ref(x: &RefCell) -> T { + todo!() + } + + fn check_calls() { + { + let x = RefCell::new(1); //~ refcell_cell + let _ = black_box(x); + } + { + let x = RefCell::new(1); //~ refcell_cell + let _ = black_box(&x); + } + // Should be linted at definition of `callee` and `callee_ref` + { + let x = RefCell::new(1); + let _ = callee(x); + } + { + let x = RefCell::new(1); + let _ = callee_ref(&x); + } } - let _ = RefCell::new(black_box!(1)); - let _ = RefCell::from(black_box!(1)); - let _ = black_box!(RefCell::new(1)); - - macro_rules! ignore_me { - () => { - let _ = RefCell::new(1); - }; + + struct MethodCall {} + + impl MethodCall { + fn black_box(&self, x: T) -> T { + todo!() + } + + fn callee(&self, x: RefCell) -> T { + todo!() + } + + fn callee_ref(&self, x: &RefCell) -> T { + todo!() + } + } + + fn check_method_calls() { + let m = MethodCall {}; + + { + let x = RefCell::new(1); //~ refcell_cell + let _ = m.black_box(x); + } + { + let x = RefCell::new(1); //~ refcell_cell + let _ = m.black_box(&x); + } + // Should be linted at definition of `callee` and `callee_ref` + { + let x = RefCell::new(1); + let _ = m.callee(x); + } + { + let x = RefCell::new(1); + let _ = m.callee_ref(&x); + } } - ignore_me!(); } diff --git a/tests/ui/refcell_cell.stderr b/tests/ui/refcell_cell.stderr index d2b4b07fa463..389d78a3e702 100644 --- a/tests/ui/refcell_cell.stderr +++ b/tests/ui/refcell_cell.stderr @@ -38,16 +38,22 @@ LL | type AliasNG = RefCell; | ^^^^^^^^^^^^ help: try: `Cell` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:39:13 + --> tests/ui/refcell_cell.rs:38:15 | -LL | ng: RefCell, - | ^^^^^^^^^^ help: try: `Cell` +LL | fn ng(ng: RefCell); + | ^^^^^^^^^^ help: try: `Cell` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:42:9 + --> tests/ui/refcell_cell.rs:40:20 | -LL | RefCell, - | ^^^^^^^^^^ help: try: `Cell` +LL | fn ng_ref(ng: &RefCell); + | ^^^^^^^^^^ help: try: `Cell` + +error: using a `RefCell` for a `Copy` type + --> tests/ui/refcell_cell.rs:42:20 + | +LL | fn ng_ret() -> RefCell; + | ^^^^^^^^^^ help: try: `Cell` error: using a `RefCell` for a `Copy` type --> tests/ui/refcell_cell.rs:50:15 @@ -56,76 +62,100 @@ LL | type NG = RefCell; | ^^^^^^^^^^ help: try: `Cell` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:59:9 + --> tests/ui/refcell_cell.rs:70:9 | LL | ng: RefCell, | ^^^^^^^^^^ help: try: `Cell` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:61:17 + --> tests/ui/refcell_cell.rs:72:17 | LL | ng_ref: &'a RefCell, | ^^^^^^^^^^ help: try: `Cell` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:64:5 + --> tests/ui/refcell_cell.rs:75:5 | LL | RefCell, | ^^^^^^^^^^ help: try: `Cell` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:74:13 + --> tests/ui/refcell_cell.rs:85:13 | LL | ng: RefCell, | ^^^^^^^^^^ help: try: `Cell` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:77:9 + --> tests/ui/refcell_cell.rs:88:9 | LL | RefCell, | ^^^^^^^^^^ help: try: `Cell` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:85:13 + --> tests/ui/refcell_cell.rs:98:17 + | +LL | let _ = RefCell::new(1); + | ^^^^^^^^^^^^^^^ help: try: `Cell::new(1)` + +error: using a `RefCell` for a `Copy` type + --> tests/ui/refcell_cell.rs:99:17 + | +LL | let _ = RefCell::::new(1); + | ^^^^^^^^^^^^^^^^^^^^^^ help: try: `Cell::::new(1)` + +error: using a `RefCell` for a `Copy` type + --> tests/ui/refcell_cell.rs:100:17 + | +LL | let _ = RefCell::<_>::new(1); + | ^^^^^^^^^^^^^^^^^^^^ help: try: `Cell::new(1)` + +error: using a `RefCell` for a `Copy` type + --> tests/ui/refcell_cell.rs:106:17 + | +LL | let _ = RefCell::from(1); + | ^^^^^^^^^^^^^^^^ help: try: `Cell::from(1)` + +error: using a `RefCell` for a `Copy` type + --> tests/ui/refcell_cell.rs:107:17 | -LL | let _ = RefCell::new(1); - | ^^^^^^^^^^^^^^^ help: try: `Cell::new(1)` +LL | let _ = RefCell::::from(1); + | ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Cell::::from(1)` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:86:13 + --> tests/ui/refcell_cell.rs:109:16 | -LL | let _ = RefCell::::new(1); - | ^^^^^^^^^^^^^^^^^^^^^^ help: try: `Cell::::new(1)` +LL | let _: RefCell = Default::default(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Cell = Cell::default();` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:87:13 + --> tests/ui/refcell_cell.rs:110:17 | -LL | let _ = RefCell::<_>::new(1); - | ^^^^^^^^^^^^^^^^^^^^ help: try: `Cell::new(1)` +LL | let _ = as Default>::default(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Cell::::default()` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:93:13 + --> tests/ui/refcell_cell.rs:151:21 | -LL | let _ = RefCell::from(1); - | ^^^^^^^^^^^^^^^^ help: try: `Cell::from(1)` +LL | let x = RefCell::new(1); + | ^^^^^^^^^^^^^^^ help: try: `Cell::new(1)` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:94:13 + --> tests/ui/refcell_cell.rs:155:21 | -LL | let _ = RefCell::::from(1); - | ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Cell::::from(1)` +LL | let x = RefCell::new(1); + | ^^^^^^^^^^^^^^^ help: try: `Cell::new(1)` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:96:12 + --> tests/ui/refcell_cell.rs:189:21 | -LL | let _: RefCell = Default::default(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Cell = Cell::default();` +LL | let x = RefCell::new(1); + | ^^^^^^^^^^^^^^^ help: try: `Cell::new(1)` error: using a `RefCell` for a `Copy` type - --> tests/ui/refcell_cell.rs:97:13 + --> tests/ui/refcell_cell.rs:193:21 | -LL | let _ = as Default>::default(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Cell::::default()` +LL | let x = RefCell::new(1); + | ^^^^^^^^^^^^^^^ help: try: `Cell::new(1)` -error: aborting due to 21 previous errors +error: aborting due to 26 previous errors From 424b7f1d0381a56e93eea098e3b8558459b5dfc4 Mon Sep 17 00:00:00 2001 From: qdot3 Date: Mon, 17 Aug 2026 23:24:17 +0900 Subject: [PATCH 13/16] config lint --- clippy_lints/src/refcell_cell.rs | 76 +++++++++-------------- tests/ui/await_holding_refcell_ref.rs | 8 ++- tests/ui/await_holding_refcell_ref.stderr | 20 +++--- tests/ui/clone_on_copy.fixed | 1 - tests/ui/clone_on_copy.rs | 1 - tests/ui/clone_on_copy.stderr | 76 +++++++++++------------ 6 files changed, 83 insertions(+), 99 deletions(-) diff --git a/clippy_lints/src/refcell_cell.rs b/clippy_lints/src/refcell_cell.rs index 6a2bee207fe5..4d4fb809f235 100644 --- a/clippy_lints/src/refcell_cell.rs +++ b/clippy_lints/src/refcell_cell.rs @@ -190,9 +190,20 @@ impl<'tcx> LateLintPass<'tcx> for RefcellCell { // Be conservative about macro && ctor_args.iter().all(|e| !e.span.from_expansion()) { - // This can be a false positive because trait bounds are not considered. + // Suggestion can be a false positive because trait bounds are not considered. let mut app = Applicability::MaybeIncorrect; + let init_str = { + let arg = ctor_args + .first() + .map_or_default(|arg| snippet_with_context(cx, arg.span, stmt.span.ctxt(), "..", &mut app).0); + match kind { + CtorKind::New => format!("Cell::new({arg})"), + CtorKind::From => format!("Cell::from({arg})"), + CtorKind::Default => "Cell::default()".to_string(), + } + }; + let (span, sugg) = if let Some(annotation) = let_stmt.ty && let TyKind::Path(qpath) = annotation.kind && let Some(inner_ty) = qpath_generic_tys(&qpath).next() @@ -200,54 +211,25 @@ impl<'tcx> LateLintPass<'tcx> for RefcellCell { let sugg = { let inner_ty = snippet_with_context(cx, inner_ty.span, stmt.span.ctxt(), "_", &mut app).0; - let arg = ctor_args - .first() - .map_or_default(|arg| snippet_with_context(cx, arg.span, stmt.span.ctxt(), "..", &mut app).0); - let init = match kind { - CtorKind::New => format!("Cell::new({arg})"), - CtorKind::From => format!("Cell::from({arg})"), - CtorKind::Default => "Cell::default()".to_string(), - }; - - format!(/* let pat: */ "Cell<{inner_ty}> = {init};") + format!(/* let pat: */ "Cell<{inner_ty}> = {init_str};") }; (annotation.span.to(init.span), sugg) } else { - let sugg = { - let arg = ctor_args - .first() - .map_or_default(|arg| snippet_with_context(cx, arg.span, stmt.span.ctxt(), "..", &mut app).0); - let init = match kind { - CtorKind::New => format!("Cell::new({arg})"), - CtorKind::From => format!("Cell::from({arg})"), - CtorKind::Default => "Cell::default()".to_string(), - }; - - let extract_ty = |qpath| { - // ::ctor - let ty = match qpath { - QPath::Resolved(ty, _) => ty, - QPath::TypeRelative(ty, _) => Some(ty), - }; - - if let Some(TyKind::Path(qpath)) = ty.map(|ty| ty.kind) - && let Some(ty) = qpath_generic_tys(&qpath).next() - { - Some(ty) - } else { - None - } - }; - - if let ExprKind::Path(qpath) = maybe_qpath.kind - && let Some(ty) = extract_ty(qpath) - { - let ty = snippet_with_context(cx, ty.span, stmt.span.ctxt(), "_", &mut app).0; - init.replacen("::", &format!("::<{ty}>::"), 1) - } else { - init + let sugg = if let ExprKind::Path(qpath) = maybe_qpath.kind + // <..>::method -> <..> + && let Some(ty) = match qpath { + QPath::Resolved(ty, _) => ty, + QPath::TypeRelative(ty, _) => Some(ty), } + // (..::)RefCell:: -> T + && let TyKind::Path(qpath) = ty.kind + && let Some(ty) = qpath_generic_tys(&qpath).next() + { + let ty = snippet_with_context(cx, ty.span, stmt.span.ctxt(), "_", &mut app).0; + init_str.replacen("::", &format!("::<{ty}>::"), 1) + } else { + init_str }; (init.span, sugg) @@ -299,7 +281,7 @@ impl CtorKind { /// Trait bounds are not considered due to complexity (for now). fn callee_requires_refcell<'tcx>(cx: &LateContext<'tcx>, pat: &'tcx Pat<'tcx>, init: &'tcx Expr<'tcx>) -> bool { for_each_local_use_after_expr(cx, pat.hir_id, init.hir_id, |expr| { - // attach `&`s as much as possible + // Ignore `&`s because we want to know whether `expr` is passed to a function. let expr = { let mut expr = expr; while let Some(p) = get_parent_expr(cx, expr) { @@ -311,8 +293,7 @@ fn callee_requires_refcell<'tcx>(cx: &LateContext<'tcx>, pat: &'tcx Pat<'tcx>, i expr }; - let node = get_expr_use_site(cx.tcx, cx.typeck_results(), expr.span.ctxt(), expr); - let (def_id, i) = match node.use_node(cx) { + let (def_id, i) = match get_expr_use_site(cx.tcx, cx.typeck_results(), expr.span.ctxt(), expr).use_node(cx) { ExprUseNode::FnArg(path, i) if let Some(def_id) = path.res(cx).opt_def_id() => (def_id, i), ExprUseNode::MethodArg(hir_id, _, i) if let Some(def_id) = cx.typeck_results().type_dependent_def_id(hir_id) => @@ -331,7 +312,6 @@ fn callee_requires_refcell<'tcx>(cx: &LateContext<'tcx>, pat: &'tcx Pat<'tcx>, i .input(i) // Need early bound params but not late ones .skip_binder(); - if input_ty.peel_refs().is_diag_item(&cx.tcx, RefCell) { Break(()) } else { diff --git a/tests/ui/await_holding_refcell_ref.rs b/tests/ui/await_holding_refcell_ref.rs index 601b4e35a806..aa1ccc4ec65a 100644 --- a/tests/ui/await_holding_refcell_ref.rs +++ b/tests/ui/await_holding_refcell_ref.rs @@ -1,8 +1,8 @@ -#![allow(clippy::refcell_cell)] #![warn(clippy::await_holding_refcell_ref)] use std::cell::RefCell; +#[expect(clippy::refcell_cell, reason = "User may intentionally use `RefCell` over `Cell`.")] async fn bad(x: &RefCell) -> u32 { let b = x.borrow(); //~^ await_holding_refcell_ref @@ -10,6 +10,7 @@ async fn bad(x: &RefCell) -> u32 { baz().await } +#[expect(clippy::refcell_cell, reason = "User may intentionally use `RefCell` over `Cell`.")] async fn bad_mut(x: &RefCell) -> u32 { let b = x.borrow_mut(); //~^ await_holding_refcell_ref @@ -17,6 +18,7 @@ async fn bad_mut(x: &RefCell) -> u32 { baz().await } +#[expect(clippy::refcell_cell, reason = "User may intentionally use `RefCell` over `Cell`.")] async fn good(x: &RefCell) -> u32 { { let b = x.borrow_mut(); @@ -31,6 +33,7 @@ async fn baz() -> u32 { 42 } +#[expect(clippy::refcell_cell, reason = "User may intentionally use `RefCell` over `Cell`.")] async fn also_bad(x: &RefCell) -> u32 { let first = baz().await; @@ -44,6 +47,7 @@ async fn also_bad(x: &RefCell) -> u32 { first + second + third } +#[expect(clippy::refcell_cell, reason = "User may intentionally use `RefCell` over `Cell`.")] async fn less_bad(x: &RefCell) -> u32 { let first = baz().await; @@ -59,6 +63,7 @@ async fn less_bad(x: &RefCell) -> u32 { first + second + third } +#[expect(clippy::refcell_cell, reason = "User may intentionally use `RefCell` over `Cell`.")] async fn not_good(x: &RefCell) -> u32 { let first = baz().await; @@ -74,6 +79,7 @@ async fn not_good(x: &RefCell) -> u32 { first + second + third } +#[expect(clippy::refcell_cell, reason = "User may intentionally use `RefCell` over `Cell`.")] #[allow(clippy::manual_async_fn)] fn block_bad(x: &RefCell) -> impl std::future::Future + '_ { async move { diff --git a/tests/ui/await_holding_refcell_ref.stderr b/tests/ui/await_holding_refcell_ref.stderr index 5daefdf257ed..bed836ab617c 100644 --- a/tests/ui/await_holding_refcell_ref.stderr +++ b/tests/ui/await_holding_refcell_ref.stderr @@ -14,27 +14,27 @@ LL | baz().await = 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:14:9 + --> tests/ui/await_holding_refcell_ref.rs:15: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:17:11 + --> tests/ui/await_holding_refcell_ref.rs:18:11 | LL | baz().await | ^^^^^ error: this `RefCell` reference is held across an await point - --> tests/ui/await_holding_refcell_ref.rs:37:9 + --> tests/ui/await_holding_refcell_ref.rs:40: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:40:24 + --> tests/ui/await_holding_refcell_ref.rs:43: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:50:9 + --> tests/ui/await_holding_refcell_ref.rs:54: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:53:24 + --> tests/ui/await_holding_refcell_ref.rs:57:24 | LL | let second = baz().await; | ^^^^^ error: this `RefCell` reference is held across an await point - --> tests/ui/await_holding_refcell_ref.rs:66:13 + --> tests/ui/await_holding_refcell_ref.rs:71: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:69:15 + --> tests/ui/await_holding_refcell_ref.rs:74:15 | LL | baz().await | ^^^^^ error: this `RefCell` reference is held across an await point - --> tests/ui/await_holding_refcell_ref.rs:80:13 + --> tests/ui/await_holding_refcell_ref.rs:86: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:83:15 + --> tests/ui/await_holding_refcell_ref.rs:89:15 | LL | baz().await | ^^^^^ diff --git a/tests/ui/clone_on_copy.fixed b/tests/ui/clone_on_copy.fixed index 05c3649aebb3..ad0a4343146a 100644 --- a/tests/ui/clone_on_copy.fixed +++ b/tests/ui/clone_on_copy.fixed @@ -3,7 +3,6 @@ clippy::deref_addrof, clippy::needless_borrow, clippy::no_effect, - clippy::refcell_cell, clippy::unnecessary_operation )] #![expect(clippy::toplevel_ref_arg)] diff --git a/tests/ui/clone_on_copy.rs b/tests/ui/clone_on_copy.rs index 14be7fc7a83d..d5371eda0fcb 100644 --- a/tests/ui/clone_on_copy.rs +++ b/tests/ui/clone_on_copy.rs @@ -3,7 +3,6 @@ clippy::deref_addrof, clippy::needless_borrow, clippy::no_effect, - clippy::refcell_cell, clippy::unnecessary_operation )] #![expect(clippy::toplevel_ref_arg)] diff --git a/tests/ui/clone_on_copy.stderr b/tests/ui/clone_on_copy.stderr index ca6245e4e8d6..c1bf81152902 100644 --- a/tests/ui/clone_on_copy.stderr +++ b/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:14:5 + --> tests/ui/clone_on_copy.rs:13:5 | LL | 42.clone(); | ^^^^^^^^^^ help: try removing the `clone` call: `42` @@ -8,223 +8,223 @@ 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:19:5 + --> tests/ui/clone_on_copy.rs:18: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:23:5 + --> tests/ui/clone_on_copy.rs:22: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:27:5 + --> tests/ui/clone_on_copy.rs:26: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:42:5 + --> tests/ui/clone_on_copy.rs:41: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:53:5 + --> tests/ui/clone_on_copy.rs:52:5 | LL | x.clone()[0]; | ^^^^^^^^^ help: try dereferencing it: `(*x)` error: using `clone` on type `E` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:92:20 + --> tests/ui/clone_on_copy.rs:91:20 | LL | let _: E = a.clone(); | ^^^^^^^^^ help: try dereferencing it: `*****a` error: using `clone` on type `E` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:94:20 + --> tests/ui/clone_on_copy.rs:93:20 | LL | let _: E = E::clone(&a); | ^^^^^^^^^^^^ help: try dereferencing it: `*****a` error: using `clone` on type `E` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:96:20 + --> tests/ui/clone_on_copy.rs:95:20 | LL | let _: E = Clone::clone(&a); | ^^^^^^^^^^^^^^^^ help: try dereferencing it: `*****a` error: using `clone` on type `E` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:98:20 + --> tests/ui/clone_on_copy.rs:97:20 | LL | let _: E = ::clone(&a); | ^^^^^^^^^^^^^^^^^^^^^^^ help: try dereferencing it: `*****a` error: using `clone` on type `char` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:111:14 + --> tests/ui/clone_on_copy.rs:110: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:130:14 + --> tests/ui/clone_on_copy.rs:129:14 | LL | vec.push(42.clone()); | ^^^^^^^^^^ help: try removing the `clone` call: `42` error: using `clone` on type `Option` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:137:17 + --> tests/ui/clone_on_copy.rs:136:17 | LL | let value = opt.clone()?; | ^^^^^^^^^^^ help: try dereferencing it: `(*opt)` error: using `clone` on type `i32` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:186:17 + --> tests/ui/clone_on_copy.rs:185:17 | LL | let _ = i32::clone(&x); | ^^^^^^^^^^^^^^ help: try removing the `clone` call: `x` error: using `clone` on type `i32` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:188:17 + --> tests/ui/clone_on_copy.rs:187:17 | LL | let _ = Clone::clone(&x); | ^^^^^^^^^^^^^^^^ help: try removing the `clone` call: `x` error: using `clone` on type `i32` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:190:17 + --> tests/ui/clone_on_copy.rs:189:17 | LL | let _ = ::clone(&x); | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try removing the `clone` call: `x` error: using `clone` on type `&i32` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:196:17 + --> tests/ui/clone_on_copy.rs:195:17 | LL | let _ = Clone::clone(ref_ref); | ^^^^^^^^^^^^^^^^^^^^^ help: try dereferencing it: `*ref_ref` error: using `clone` on type `&i32` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:198:17 + --> tests/ui/clone_on_copy.rs:197:17 | LL | let _ = <&i32 as Clone>::clone(ref_ref); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try dereferencing it: `*ref_ref` error: using `clone` on type `i32` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:202:17 + --> tests/ui/clone_on_copy.rs:201:17 | LL | let _ = i32::clone(&*imm_ref); | ^^^^^^^^^^^^^^^^^^^^^ help: try removing the `clone` call: `*imm_ref` error: using `clone` on type `i32` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:204:17 + --> tests/ui/clone_on_copy.rs:203:17 | LL | let _ = Clone::clone(&*imm_ref); | ^^^^^^^^^^^^^^^^^^^^^^^ help: try removing the `clone` call: `*imm_ref` error: using `clone` on type `i32` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:206:17 + --> tests/ui/clone_on_copy.rs:205:17 | LL | let _ = ::clone(&*imm_ref); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try removing the `clone` call: `*imm_ref` error: using `clone` on type `i32` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:210:17 + --> tests/ui/clone_on_copy.rs:209:17 | LL | let _ = i32::clone(&*x_ref); | ^^^^^^^^^^^^^^^^^^^ help: try removing the `clone` call: `*x_ref` error: using `clone` on type `i32` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:212:17 + --> tests/ui/clone_on_copy.rs:211:17 | LL | let _ = Clone::clone(&*x_ref); | ^^^^^^^^^^^^^^^^^^^^^ help: try removing the `clone` call: `*x_ref` error: using `clone` on type `i32` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:214:17 + --> tests/ui/clone_on_copy.rs:213:17 | LL | let _ = ::clone(&*x_ref); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try removing the `clone` call: `*x_ref` error: using `clone` on type `i32` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:217:17 + --> tests/ui/clone_on_copy.rs:216:17 | LL | let _ = i32::clone(&x).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:219:17 + --> tests/ui/clone_on_copy.rs:218:17 | LL | let _ = Clone::clone(&x).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:221:17 + --> tests/ui/clone_on_copy.rs:220:17 | LL | let _ = ::clone(&x).rotate_left(1); | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try removing the `clone` call: `x` error: using `clone` on type `char` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:228:18 + --> tests/ui/clone_on_copy.rs:227:18 | LL | is_ascii(char::clone(&'z')); | ^^^^^^^^^^^^^^^^^ help: try removing the `clone` call: `'z'` error: using `clone` on type `char` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:230:18 + --> tests/ui/clone_on_copy.rs:229:18 | LL | is_ascii(Clone::clone(&'z')); | ^^^^^^^^^^^^^^^^^^ help: try removing the `clone` call: `'z'` error: using `clone` on type `char` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:232:18 + --> tests/ui/clone_on_copy.rs:231:18 | LL | is_ascii(::clone(&'z')); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try removing the `clone` call: `'z'` error: using `clone` on type `i32` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:237:17 + --> tests/ui/clone_on_copy.rs:236:17 | LL | let _ = Clone::clone(&m!(42)); | ^^^^^^^^^^^^^^^^^^^^^ help: try removing the `clone` call: `m!(42)` error: using `clone` on type `i32` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:241:17 + --> tests/ui/clone_on_copy.rs:240:17 | LL | let _ = Clone::clone(&*rc.borrow()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try removing the `clone` call: `*rc.borrow()` error: using `clone` on type `Bar` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:245:17 + --> tests/ui/clone_on_copy.rs:244:17 | LL | let _ = Clone::clone(&bar).field; | ^^^^^^^^^^^^^^^^^^ help: try removing the `clone` call: `bar` error: using `clone` on type `[i32; 2]` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:249:17 + --> tests/ui/clone_on_copy.rs:248:17 | LL | let _ = Clone::clone(&array)[0]; | ^^^^^^^^^^^^^^^^^^^^ help: try removing the `clone` call: `array` error: using `clone` on type `[u32; 2]` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:253:17 + --> tests/ui/clone_on_copy.rs:252:17 | LL | let _ = Clone::clone(&*wrapped)[0]; | ^^^^^^^^^^^^^^^^^^^^^^^ help: try dereferencing it: `(*wrapped)` error: using `clone` on type `Option` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:276:17 + --> tests/ui/clone_on_copy.rs:275:17 | LL | let _ = Option::clone(opt)?; | ^^^^^^^^^^^^^^^^^^ help: try dereferencing it: `(*opt)` error: using `clone` on type `Option` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:278:17 + --> tests/ui/clone_on_copy.rs:277:17 | LL | let _ = Clone::clone(opt)?; | ^^^^^^^^^^^^^^^^^ help: try dereferencing it: `(*opt)` error: using `clone` on type `Option` which implements the `Copy` trait - --> tests/ui/clone_on_copy.rs:280:17 + --> tests/ui/clone_on_copy.rs:279:17 | LL | let _ = as Clone>::clone(opt)?; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try dereferencing it: `(*opt)` From 95c7b73c56d43a6be2e28986ab0fe379ad8e124c Mon Sep 17 00:00:00 2001 From: qdot3 Date: Tue, 18 Aug 2026 09:37:45 +0900 Subject: [PATCH 14/16] refactor --- clippy_lints/src/refcell_cell.rs | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/clippy_lints/src/refcell_cell.rs b/clippy_lints/src/refcell_cell.rs index 4d4fb809f235..8e2cef9c3715 100644 --- a/clippy_lints/src/refcell_cell.rs +++ b/clippy_lints/src/refcell_cell.rs @@ -6,7 +6,7 @@ use clippy_utils::res::{MaybeDef as _, MaybeQPath as _, MaybeResPath as _}; use clippy_utils::source::{snippet_with_applicability, snippet_with_context}; use clippy_utils::ty::{approx_ty_size, is_copy, ty_from_hir_ty}; use clippy_utils::visitors::for_each_local_use_after_expr; -use clippy_utils::{ExprUseNode, get_expr_use_site, get_parent_expr, is_trait_impl_item, qpath_generic_tys, sym}; +use clippy_utils::{ExprUseNode, expr_use_sites, is_trait_impl_item, qpath_generic_tys, sym}; use rustc_errors::Applicability; use rustc_hir::def_id::LocalDefId; use rustc_hir::intravisit::FnKind; @@ -281,19 +281,14 @@ impl CtorKind { /// Trait bounds are not considered due to complexity (for now). fn callee_requires_refcell<'tcx>(cx: &LateContext<'tcx>, pat: &'tcx Pat<'tcx>, init: &'tcx Expr<'tcx>) -> bool { for_each_local_use_after_expr(cx, pat.hir_id, init.hir_id, |expr| { - // Ignore `&`s because we want to know whether `expr` is passed to a function. - let expr = { - let mut expr = expr; - while let Some(p) = get_parent_expr(cx, expr) { - match p.kind { - ExprKind::AddrOf(..) => expr = p, - _ => break, - } - } - expr + let Some(use_site) = expr_use_sites(cx.tcx, cx.typeck_results(), expr.span.ctxt(), expr) + // Ignore `&`s because we want to know whether `expr` is passed to a function. + .find(|use_site| !matches!(use_site.use_node(cx), ExprUseNode::AddrOf(..))) + else { + return Continue(()); }; - let (def_id, i) = match get_expr_use_site(cx.tcx, cx.typeck_results(), expr.span.ctxt(), expr).use_node(cx) { + let (def_id, i) = match use_site.use_node(cx) { ExprUseNode::FnArg(path, i) if let Some(def_id) = path.res(cx).opt_def_id() => (def_id, i), ExprUseNode::MethodArg(hir_id, _, i) if let Some(def_id) = cx.typeck_results().type_dependent_def_id(hir_id) => From d9f03d5d6db8c142506b4b011c579dd99964694f Mon Sep 17 00:00:00 2001 From: qdot3 Date: Tue, 18 Aug 2026 10:35:37 +0900 Subject: [PATCH 15/16] handle unwrap error --- clippy_lints/src/refcell_cell.rs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/clippy_lints/src/refcell_cell.rs b/clippy_lints/src/refcell_cell.rs index 8e2cef9c3715..877703b517de 100644 --- a/clippy_lints/src/refcell_cell.rs +++ b/clippy_lints/src/refcell_cell.rs @@ -295,18 +295,25 @@ fn callee_requires_refcell<'tcx>(cx: &LateContext<'tcx>, pat: &'tcx Pat<'tcx>, i { (def_id, i) }, + ExprUseNode::Callee => return Break(()), _ => return Continue(()), }; // Does callee require `RefCell`? - let input_ty = cx + let Some(input_ty) = cx .tcx .fn_sig(def_id) .instantiate_identity() - .skip_normalization() - .input(i) + .skip_norm_wip() // Need early bound params but not late ones - .skip_binder(); + .skip_binder() + .inputs() + .get(i) + else { + // This arm seems unreachable + return Break(()); + }; + if input_ty.peel_refs().is_diag_item(&cx.tcx, RefCell) { Break(()) } else { From e6d57f5c85e052a90ef37d144acbe25723d25bde Mon Sep 17 00:00:00 2001 From: qdot3 Date: Tue, 18 Aug 2026 12:00:42 +0900 Subject: [PATCH 16/16] fix conf --- CHANGELOG.md | 2 +- book/src/lint_configuration.md | 2 +- clippy_config/src/conf.rs | 2 +- clippy_lints/src/refcell_cell.rs | 11 ++++++++--- tests/ui-toml/refcell_cell/clippy.toml | 1 + tests/ui-toml/refcell_cell/tests.rs | 9 +++++++++ tests/ui-toml/refcell_cell/tests.stderr | 11 +++++++++++ .../ui-toml/toml_unknown_key/conf_unknown_key.stderr | 2 +- 8 files changed, 33 insertions(+), 7 deletions(-) create mode 100644 tests/ui-toml/refcell_cell/clippy.toml create mode 100644 tests/ui-toml/refcell_cell/tests.rs create mode 100644 tests/ui-toml/refcell_cell/tests.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 09ba79656658..23ab08bddddb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7723,7 +7723,7 @@ Released 2018-09-13 [`lint-commented-code`]: https://doc.rust-lang.org/clippy/lint_configuration.html#lint-commented-code [`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 -[`max_cheap-copy-size`]: https://doc.rust-lang.org/clippy/lint_configuration.html#max_cheap-copy-size +[`max-cheap-copy-size`]: https://doc.rust-lang.org/clippy/lint_configuration.html#max-cheap-copy-size [`max-fn-params-bools`]: https://doc.rust-lang.org/clippy/lint_configuration.html#max-fn-params-bools [`max-include-file-size`]: https://doc.rust-lang.org/clippy/lint_configuration.html#max-include-file-size [`max-struct-bools`]: https://doc.rust-lang.org/clippy/lint_configuration.html#max-struct-bools diff --git a/book/src/lint_configuration.md b/book/src/lint_configuration.md index 6711cc85bc25..ce29750ea7c8 100644 --- a/book/src/lint_configuration.md +++ b/book/src/lint_configuration.md @@ -810,7 +810,7 @@ be filtering for common types. * [`manual_let_else`](https://rust-lang.github.io/rust-clippy/main/index.html#manual_let_else) -## `max_cheap-copy-size` +## `max-cheap-copy-size` Maximum size in bytes for a `Copy` type to be considered cheap to copy. **Default Value:** `16` diff --git a/clippy_config/src/conf.rs b/clippy_config/src/conf.rs index b226f675f299..ee4da0bc7da9 100644 --- a/clippy_config/src/conf.rs +++ b/clippy_config/src/conf.rs @@ -599,7 +599,7 @@ define_Conf! { matches_for_let_else("matches-for-let-else"): MatchLintBehaviour = MatchLintBehaviour::WellKnownTypes, /// Maximum size in bytes for a `Copy` type to be considered cheap to copy. #[lints(refcell_cell)] - max_cheap_copy_size("max_cheap-copy-size"): u64 = 16, + max_cheap_copy_size("max-cheap-copy-size"): u64 = 16, /// The maximum number of bool parameters a function can have. /// Use `0` to lint on any function with a bool parameter. #[lints(fn_params_excessive_bools)] diff --git a/clippy_lints/src/refcell_cell.rs b/clippy_lints/src/refcell_cell.rs index 877703b517de..fee1bddca137 100644 --- a/clippy_lints/src/refcell_cell.rs +++ b/clippy_lints/src/refcell_cell.rs @@ -278,7 +278,10 @@ impl CtorKind { } } -/// Trait bounds are not considered due to complexity (for now). +/// Checks whether the given local variable must be a `RefCell`. +/// +/// This performs only simple analysis because a full analysis would be too expensive. +/// Trait bounds and indirect uses are not considered. fn callee_requires_refcell<'tcx>(cx: &LateContext<'tcx>, pat: &'tcx Pat<'tcx>, init: &'tcx Expr<'tcx>) -> bool { for_each_local_use_after_expr(cx, pat.hir_id, init.hir_id, |expr| { let Some(use_site) = expr_use_sites(cx.tcx, cx.typeck_results(), expr.span.ctxt(), expr) @@ -303,14 +306,16 @@ fn callee_requires_refcell<'tcx>(cx: &LateContext<'tcx>, pat: &'tcx Pat<'tcx>, i let Some(input_ty) = cx .tcx .fn_sig(def_id) + // Inspect the type as declared by the callee, without normalization. .instantiate_identity() .skip_norm_wip() - // Need early bound params but not late ones .skip_binder() .inputs() .get(i) else { - // This arm seems unreachable + // FIXME: This arm seems unreachable but is required to avoid a panic from + // an `Option::unwrap` inside `FnSig::input` when linting some crates on Linux. + // Why? return Break(()); }; diff --git a/tests/ui-toml/refcell_cell/clippy.toml b/tests/ui-toml/refcell_cell/clippy.toml new file mode 100644 index 000000000000..259eeec251c6 --- /dev/null +++ b/tests/ui-toml/refcell_cell/clippy.toml @@ -0,0 +1 @@ +max-cheap-copy-size = 64 \ No newline at end of file diff --git a/tests/ui-toml/refcell_cell/tests.rs b/tests/ui-toml/refcell_cell/tests.rs new file mode 100644 index 000000000000..70541e73c63b --- /dev/null +++ b/tests/ui-toml/refcell_cell/tests.rs @@ -0,0 +1,9 @@ +//@no-rustfix +#![warn(clippy::refcell_cell)] + +use std::cell::RefCell; + +fn main() { + let _ = RefCell::new([0_u32; 16]); //~ refcell_cell + let _ = RefCell::new([0_u32; 17]); +} diff --git a/tests/ui-toml/refcell_cell/tests.stderr b/tests/ui-toml/refcell_cell/tests.stderr new file mode 100644 index 000000000000..46408af08365 --- /dev/null +++ b/tests/ui-toml/refcell_cell/tests.stderr @@ -0,0 +1,11 @@ +error: using a `RefCell` for a `Copy` type + --> tests/ui-toml/refcell_cell/tests.rs:7:13 + | +LL | let _ = RefCell::new([0_u32; 16]); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Cell::new([0_u32; 16])` + | + = note: `-D clippy::refcell-cell` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::refcell_cell)]` + +error: aborting due to 1 previous error + diff --git a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr index 91a4fe329673..c648522be510 100644 --- a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr +++ b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr @@ -65,7 +65,7 @@ LL | foobar = 42 lint-commented-code literal-representation-threshold matches-for-let-else - max_cheap-copy-size + max-cheap-copy-size max-fn-params-bools max-include-file-size max-struct-bools