Add default_box_assignments lint

CHANGELOG.md

@@ -5706,6 +5706,7 @@ Released 2018-09-13
 [`debug_assert_with_mut_call`]: https://rust-lang.github.io/rust-clippy/master/index.html#debug_assert_with_mut_call
 [`decimal_literal_representation`]: https://rust-lang.github.io/rust-clippy/master/index.html#decimal_literal_representation
 [`declare_interior_mutable_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#declare_interior_mutable_const
+[`default_box_assignments`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_box_assignments
 [`default_constructed_unit_structs`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_constructed_unit_structs
 [`default_instead_of_iter_empty`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_instead_of_iter_empty
 [`default_numeric_fallback`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_numeric_fallback

clippy_lints/src/declared_lints.rs

@@ -93,6 +93,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
     crate::dbg_macro::DBG_MACRO_INFO,
     crate::default::DEFAULT_TRAIT_ACCESS_INFO,
     crate::default::FIELD_REASSIGN_WITH_DEFAULT_INFO,
+    crate::default_box_assignments::DEFAULT_BOX_ASSIGNMENTS_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,

clippy_lints/src/default_box_assignments.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.89.0"]
+    pub DEFAULT_BOX_ASSIGNMENTS,
+    perf,
+    "assigning a newly created box to `Box<T>` is inefficient"
+}
+declare_lint_pass!(DefaultBoxAssignments => [DEFAULT_BOX_ASSIGNMENTS]);
+
+impl LateLintPass<'_> for DefaultBoxAssignments {
+    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,
+                    DEFAULT_BOX_ASSIGNMENTS,
+                    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, DEFAULT_BOX_ASSIGNMENTS, 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
+    }
+}

clippy_lints/src/lib.rs

@@ -107,6 +107,7 @@ mod crate_in_macro_def;
 mod create_dir;
 mod dbg_macro;
 mod default;
+mod default_box_assignments;
 mod default_constructed_unit_structs;
 mod default_instead_of_iter_empty;
 mod default_numeric_fallback;
@@ -951,5 +952,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
     store.register_late_pass(|_| Box::new(cloned_ref_to_slice_refs::ClonedRefToSliceRefs::new(conf)));
     store.register_late_pass(|_| Box::new(infallible_try_from::InfallibleTryFrom));
     store.register_late_pass(|_| Box::new(coerce_container_to_any::CoerceContainerToAny));
+    store.register_late_pass(|_| Box::new(default_box_assignments::DefaultBoxAssignments));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }

tests/ui/default_box_assignments.fixed

@@ -0,0 +1,72 @@
+#![warn(clippy::default_box_assignments)]
+
+fn with_default<T: Default>(b: &mut Box<T>) {
+    *(*b) = T::default();
+    //~^ default_box_assignments
+}
+
+fn with_sized<T>(b: &mut Box<T>, t: T) {
+    *(*b) = t;
+    //~^ default_box_assignments
+}
+
+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();
+    //~^ default_box_assignments
+    *b = Default::default();
+    //~^ default_box_assignments
+
+    // 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;
+    //~^ default_box_assignments
+
+    *b = mac!(three);
+    //~^ default_box_assignments
+
+    // 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();
+}

tests/ui/default_box_assignments.rs

@@ -0,0 +1,72 @@
+#![warn(clippy::default_box_assignments)]
+
+fn with_default<T: Default>(b: &mut Box<T>) {
+    *b = Box::new(T::default());
+    //~^ default_box_assignments
+}
+
+fn with_sized<T>(b: &mut Box<T>, t: T) {
+    *b = Box::new(t);
+    //~^ default_box_assignments
+}
+
+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();
+    //~^ default_box_assignments
+    b = Box::default();
+    //~^ default_box_assignments
+
+    // 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);
+    //~^ default_box_assignments
+
+    b = Box::new(mac!(three));
+    //~^ default_box_assignments
+
+    // 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();
+}

tests/ui/default_box_assignments.stderr

@@ -0,0 +1,52 @@
+error: creating a new box
+  --> tests/ui/default_box_assignments.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::default-box-assignments` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::default_box_assignments)]`
+
+error: creating a new box
+  --> tests/ui/default_box_assignments.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/default_box_assignments.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/default_box_assignments.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/default_box_assignments.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/default_box_assignments.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
+