Add replace_box lint

bluebear94 · bluebear94 · commit a04fcb205ac6 · 2025-10-07T11:18:03.000-04:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs
@@ -90,6 +90,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
     crate::dbg_macro::DBG_MACRO_INFO,
     crate::default::DEFAULT_TRAIT_ACCESS_INFO,
     crate::default::FIELD_REASSIGN_WITH_DEFAULT_INFO,
+    crate::replace_box::REPLACE_BOX_INFO,
     crate::default_constructed_unit_structs::DEFAULT_CONSTRUCTED_UNIT_STRUCTS_INFO,
     crate::default_instead_of_iter_empty::DEFAULT_INSTEAD_OF_ITER_EMPTY_INFO,
     crate::default_numeric_fallback::DEFAULT_NUMERIC_FALLBACK_INFO,
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
@@ -323,6 +323,7 @@ mod ref_patterns;
 mod reference;
 mod regex;
 mod repeat_vec_with_capacity;
+mod replace_box;
 mod reserve_after_initialization;
 mod return_self_not_must_use;
 mod returns;
@@ -832,5 +833,6 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
     store.register_late_pass(|_| Box::new(coerce_container_to_any::CoerceContainerToAny));
     store.register_late_pass(|_| Box::new(toplevel_ref_arg::ToplevelRefArg));
     store.register_late_pass(|_| Box::new(volatile_composites::VolatileComposites));
+    store.register_late_pass(|_| Box::new(replace_box::ReplaceBox));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
diff --git a/clippy_lints/src/replace_box.rs b/clippy_lints/src/replace_box.rs
@@ -0,0 +1,130 @@
+use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::sugg::Sugg;
+use clippy_utils::ty::implements_trait;
+use clippy_utils::{is_default_equivalent_call, local_is_initialized, path_def_id, path_to_local};
+use rustc_errors::Applicability;
+use rustc_hir::{Expr, ExprKind, LangItem, QPath};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty::{self, Ty};
+use rustc_session::declare_lint_pass;
+use rustc_span::sym;
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Detects assignments of `Default::default()` or `Box::new(value)`
+    /// to a place of type `Box<T>`.
+    ///
+    /// ### Why is this bad?
+    /// This incurs an extra heap allocation compared to assigning the boxed
+    /// storage.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// let mut b = Box::new(1u32);
+    /// b = Default::default();
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// let mut b = Box::new(1u32);
+    /// *b = Default::default();
+    /// ```
+    #[clippy::version = "1.92.0"]
+    pub REPLACE_BOX,
+    perf,
+    "assigning a newly created box to `Box<T>` is inefficient"
+}
+declare_lint_pass!(ReplaceBox => [REPLACE_BOX]);
+
+impl LateLintPass<'_> for ReplaceBox {
+    fn check_expr(&mut self, cx: &LateContext<'_>, expr: &'_ Expr<'_>) {
+        if let ExprKind::Assign(lhs, rhs, _) = &expr.kind
+            && !lhs.span.from_expansion()
+            && !rhs.span.from_expansion()
+        {
+            let lhs_ty = cx.typeck_results().expr_ty(lhs);
+
+            // No diagnostic for late-initialized locals
+            if let Some(local) = path_to_local(lhs)
+                && !local_is_initialized(cx, local)
+            {
+                return;
+            }
+
+            let Some(inner_ty) = get_box_inner_type(cx, lhs_ty) else {
+                return;
+            };
+
+            if let Some(default_trait_id) = cx.tcx.get_diagnostic_item(sym::Default)
+                && implements_trait(cx, inner_ty, default_trait_id, &[])
+                && is_default_call(cx, rhs)
+            {
+                span_lint_and_then(
+                    cx,
+                    REPLACE_BOX,
+                    expr.span,
+                    "creating a new box with default content",
+                    |diag| {
+                        let mut app = Applicability::MachineApplicable;
+                        let suggestion = format!(
+                            "{} = Default::default()",
+                            Sugg::hir_with_applicability(cx, lhs, "_", &mut app).deref()
+                        );
+
+                        diag.note("this creates a needless allocation").span_suggestion(
+                            expr.span,
+                            "replace existing content with default instead",
+                            suggestion,
+                            app,
+                        );
+                    },
+                );
+            }
+
+            if inner_ty.is_sized(cx.tcx, cx.typing_env())
+                && let Some(rhs_inner) = get_box_new_payload(cx, rhs)
+            {
+                span_lint_and_then(cx, REPLACE_BOX, expr.span, "creating a new box", |diag| {
+                    let mut app = Applicability::MachineApplicable;
+                    let suggestion = format!(
+                        "{} = {}",
+                        Sugg::hir_with_applicability(cx, lhs, "_", &mut app).deref(),
+                        Sugg::hir_with_context(cx, rhs_inner, expr.span.ctxt(), "_", &mut app),
+                    );
+
+                    diag.note("this creates a needless allocation").span_suggestion(
+                        expr.span,
+                        "replace existing content with inner value instead",
+                        suggestion,
+                        app,
+                    );
+                });
+            }
+        }
+    }
+}
+
+fn get_box_inner_type<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<Ty<'tcx>> {
+    if let ty::Adt(def, args) = ty.kind()
+        && cx.tcx.is_lang_item(def.did(), LangItem::OwnedBox)
+    {
+        Some(args.type_at(0))
+    } else {
+        None
+    }
+}
+
+fn is_default_call(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
+    matches!(expr.kind, ExprKind::Call(func, _args) if is_default_equivalent_call(cx, func, Some(expr)))
+}
+
+fn get_box_new_payload<'tcx>(cx: &LateContext<'_>, expr: &Expr<'tcx>) -> Option<&'tcx Expr<'tcx>> {
+    if let ExprKind::Call(box_new, [arg]) = expr.kind
+        && let ExprKind::Path(QPath::TypeRelative(ty, seg)) = box_new.kind
+        && seg.ident.name == sym::new
+        && path_def_id(cx, ty).is_some_and(|id| Some(id) == cx.tcx.lang_items().owned_box())
+    {
+        Some(arg)
+    } else {
+        None
+    }
+}
diff --git a/tests/ui/replace_box.fixed b/tests/ui/replace_box.fixed
@@ -0,0 +1,72 @@
+#![warn(clippy::replace_box)]
+
+fn with_default<T: Default>(b: &mut Box<T>) {
+    **b = T::default();
+    //~^ replace_box
+}
+
+fn with_sized<T>(b: &mut Box<T>, t: T) {
+    **b = t;
+    //~^ replace_box
+}
+
+fn with_unsized<const N: usize>(b: &mut Box<[u32]>) {
+    // No lint for assigning to Box<T> where T: !Default
+    *b = Box::new([42; N]);
+}
+
+macro_rules! create_default {
+    () => {
+        Default::default()
+    };
+}
+
+macro_rules! create_zero_box {
+    () => {
+        Box::new(0)
+    };
+}
+
+macro_rules! same {
+    ($v:ident) => {
+        $v
+    };
+}
+
+macro_rules! mac {
+    (three) => {
+        3u32
+    };
+}
+
+fn main() {
+    let mut b = Box::new(1u32);
+    *b = Default::default();
+    //~^ replace_box
+    *b = Default::default();
+    //~^ replace_box
+
+    // No lint for assigning to the storage
+    *b = Default::default();
+    *b = u32::default();
+
+    // No lint if either LHS or RHS originates in macro
+    b = create_default!();
+    b = create_zero_box!();
+    same!(b) = Default::default();
+
+    *b = 5;
+    //~^ replace_box
+
+    *b = mac!(three);
+    //~^ replace_box
+
+    // No lint for assigning to Box<T> where T: !Default
+    let mut b = Box::<str>::from("hi".to_string());
+    b = Default::default();
+
+    // No lint for late initializations
+    #[allow(clippy::needless_late_init)]
+    let bb: Box<u32>;
+    bb = Default::default();
+}
diff --git a/tests/ui/replace_box.rs b/tests/ui/replace_box.rs
@@ -0,0 +1,72 @@
+#![warn(clippy::replace_box)]
+
+fn with_default<T: Default>(b: &mut Box<T>) {
+    *b = Box::new(T::default());
+    //~^ replace_box
+}
+
+fn with_sized<T>(b: &mut Box<T>, t: T) {
+    *b = Box::new(t);
+    //~^ replace_box
+}
+
+fn with_unsized<const N: usize>(b: &mut Box<[u32]>) {
+    // No lint for assigning to Box<T> where T: !Default
+    *b = Box::new([42; N]);
+}
+
+macro_rules! create_default {
+    () => {
+        Default::default()
+    };
+}
+
+macro_rules! create_zero_box {
+    () => {
+        Box::new(0)
+    };
+}
+
+macro_rules! same {
+    ($v:ident) => {
+        $v
+    };
+}
+
+macro_rules! mac {
+    (three) => {
+        3u32
+    };
+}
+
+fn main() {
+    let mut b = Box::new(1u32);
+    b = Default::default();
+    //~^ replace_box
+    b = Box::default();
+    //~^ replace_box
+
+    // No lint for assigning to the storage
+    *b = Default::default();
+    *b = u32::default();
+
+    // No lint if either LHS or RHS originates in macro
+    b = create_default!();
+    b = create_zero_box!();
+    same!(b) = Default::default();
+
+    b = Box::new(5);
+    //~^ replace_box
+
+    b = Box::new(mac!(three));
+    //~^ replace_box
+
+    // No lint for assigning to Box<T> where T: !Default
+    let mut b = Box::<str>::from("hi".to_string());
+    b = Default::default();
+
+    // No lint for late initializations
+    #[allow(clippy::needless_late_init)]
+    let bb: Box<u32>;
+    bb = Default::default();
+}
diff --git a/tests/ui/replace_box.stderr b/tests/ui/replace_box.stderr
@@ -0,0 +1,52 @@
+error: creating a new box
+  --> tests/ui/replace_box.rs:4:5
+   |
+LL |     *b = Box::new(T::default());
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace existing content with inner value instead: `**b = T::default()`
+   |
+   = note: this creates a needless allocation
+   = note: `-D clippy::replace-box` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::replace_box)]`
+
+error: creating a new box
+  --> tests/ui/replace_box.rs:9:5
+   |
+LL |     *b = Box::new(t);
+   |     ^^^^^^^^^^^^^^^^ help: replace existing content with inner value instead: `**b = t`
+   |
+   = note: this creates a needless allocation
+
+error: creating a new box with default content
+  --> tests/ui/replace_box.rs:44:5
+   |
+LL |     b = Default::default();
+   |     ^^^^^^^^^^^^^^^^^^^^^^ help: replace existing content with default instead: `*b = Default::default()`
+   |
+   = note: this creates a needless allocation
+
+error: creating a new box with default content
+  --> tests/ui/replace_box.rs:46:5
+   |
+LL |     b = Box::default();
+   |     ^^^^^^^^^^^^^^^^^^ help: replace existing content with default instead: `*b = Default::default()`
+   |
+   = note: this creates a needless allocation
+
+error: creating a new box
+  --> tests/ui/replace_box.rs:58:5
+   |
+LL |     b = Box::new(5);
+   |     ^^^^^^^^^^^^^^^ help: replace existing content with inner value instead: `*b = 5`
+   |
+   = note: this creates a needless allocation
+
+error: creating a new box
+  --> tests/ui/replace_box.rs:61:5
+   |
+LL |     b = Box::new(mac!(three));
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace existing content with inner value instead: `*b = mac!(three)`
+   |
+   = note: this creates a needless allocation
+
+error: aborting due to 6 previous errors
+
