Add lint for b = Box::new(value)

Author: bluebear94

clippy_lints/src/default_box_assignments.rs

@@ -1,17 +1,18 @@
 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_to_local};
+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};
+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()` to a place of type `Box<T>`.
+    /// 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
@@ -46,7 +47,18 @@ impl LateLintPass<'_> for DefaultBoxAssignments {
                 return;
             }
 
-            if is_box_of_default(cx, lhs_ty) && is_default_call(cx, rhs) && !rhs.span.from_expansion() {
+            let Some(inner_ty) = get_box_inner_type(cx, lhs_ty) else {
+                return;
+            };
+
+            let Some(default_trait_id) = cx.tcx.get_diagnostic_item(sym::Default) else {
+                return;
+            };
+
+            if implements_trait(cx, inner_ty, default_trait_id, &[])
+                && is_default_call(cx, rhs)
+                && !rhs.span.from_expansion()
+            {
                 span_lint_and_then(
                     cx,
                     DEFAULT_BOX_ASSIGNMENTS,
@@ -68,21 +80,52 @@ impl LateLintPass<'_> for DefaultBoxAssignments {
                     },
                 );
             }
+
+            if inner_ty.is_sized(cx.tcx, cx.typing_env())
+                && let Some(rhs_inner) = get_new_call_value(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_applicability(cx, rhs_inner, "_", &mut app),
+                    );
+
+                    diag.note("this creates a needless allocation").span_suggestion(
+                        expr.span,
+                        "replace existing content with inner value instead",
+                        suggestion,
+                        app,
+                    );
+                });
+            }
         }
     }
 }
 
-fn is_box_of_default<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
+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)
-        && let Some(default_trait_id) = cx.tcx.get_diagnostic_item(sym::Default)
     {
-        implements_trait(cx, args.type_at(0), default_trait_id, &[])
+        Some(args.type_at(0))
     } else {
-        false
+        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_new_call_value<'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
+    }
+}

tests/ui/default_box_assignments.fixed

@@ -20,6 +20,9 @@ fn main() {
     // No lint for call originating in macro
     b = create_default!();
 
+    *b = 5;
+    //~^ 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();

tests/ui/default_box_assignments.rs

@@ -20,6 +20,9 @@ fn main() {
     // No lint for call originating in macro
     b = create_default!();
 
+    b = Box::new(5);
+    //~^ 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();

tests/ui/default_box_assignments.stderr

@@ -16,5 +16,13 @@ LL |     b = Box::default();
    |
    = note: this creates a needless allocation
 
-error: aborting due to 2 previous errors
+error: creating a new box
+  --> tests/ui/default_box_assignments.rs:23:5
+   |
+LL |     b = Box::new(5);
+   |     ^^^^^^^^^^^^^^^ help: replace existing content with inner value instead: `*b = 5`
+   |
+   = note: this creates a needless allocation
+
+error: aborting due to 3 previous errors