Add replace_box lint (#14953)
Adds a new lint that detects `var = Default::default()` when `var` is `Box<T>` and `T` implements `Default`. changelog: [`replace_box`]: new lint
This commit is contained in:
commit
0a2eeceefc
7 changed files with 595 additions and 107 deletions
373
CHANGELOG.md
373
CHANGELOG.md
File diff suppressed because it is too large
Load diff
|
|
@ -655,6 +655,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
|
|||
crate::regex::REGEX_CREATION_IN_LOOPS_INFO,
|
||||
crate::regex::TRIVIAL_REGEX_INFO,
|
||||
crate::repeat_vec_with_capacity::REPEAT_VEC_WITH_CAPACITY_INFO,
|
||||
crate::replace_box::REPLACE_BOX_INFO,
|
||||
crate::reserve_after_initialization::RESERVE_AFTER_INITIALIZATION_INFO,
|
||||
crate::return_self_not_must_use::RETURN_SELF_NOT_MUST_USE_INFO,
|
||||
crate::returns::LET_AND_RETURN_INFO,
|
||||
|
|
|
|||
|
|
@ -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`
|
||||
}
|
||||
|
|
|
|||
130
clippy_lints/src/replace_box.rs
Normal file
130
clippy_lints/src/replace_box.rs
Normal file
|
|
@ -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
|
||||
}
|
||||
}
|
||||
72
tests/ui/replace_box.fixed
Normal file
72
tests/ui/replace_box.fixed
Normal file
|
|
@ -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();
|
||||
}
|
||||
72
tests/ui/replace_box.rs
Normal file
72
tests/ui/replace_box.rs
Normal file
|
|
@ -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();
|
||||
}
|
||||
52
tests/ui/replace_box.stderr
Normal file
52
tests/ui/replace_box.stderr
Normal file
|
|
@ -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
|
||||
|
||||
Loading…
Add table
Add a link
Reference in a new issue