Move box_default to style, do not suggest turbofishes

`Box::default()` had its `#[rustc_box]` attribute removed in 1.69 so is
no longer a perf related lint

The lint is moved to style but no longer produces suggestions containing
turbofishes, as they're often longer/more annoying to type
This commit is contained in:
Alex Macleod 2024-03-31 13:22:04 +00:00
parent 797d50dfe6
commit 0f63fa8c33
4 changed files with 122 additions and 193 deletions

View File

@ -1,6 +1,5 @@
use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::macros::macro_backtrace; use clippy_utils::macros::macro_backtrace;
use clippy_utils::source::snippet_opt;
use clippy_utils::ty::expr_sig; use clippy_utils::ty::expr_sig;
use clippy_utils::{is_default_equivalent, path_def_id}; use clippy_utils::{is_default_equivalent, path_def_id};
use rustc_errors::Applicability; use rustc_errors::Applicability;
@ -9,20 +8,16 @@ use rustc_hir::intravisit::{walk_ty, Visitor};
use rustc_hir::{Block, Expr, ExprKind, Local, Node, QPath, Ty, TyKind}; use rustc_hir::{Block, Expr, ExprKind, Local, Node, QPath, Ty, TyKind};
use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::lint::in_external_macro; use rustc_middle::lint::in_external_macro;
use rustc_middle::ty::print::with_forced_trimmed_paths;
use rustc_middle::ty::IsSuggestable;
use rustc_session::declare_lint_pass; use rustc_session::declare_lint_pass;
use rustc_span::sym; use rustc_span::sym;
declare_clippy_lint! { declare_clippy_lint! {
/// ### What it does /// ### What it does
/// checks for `Box::new(T::default())`, which is better written as /// checks for `Box::new(Default::default())`, which can be written as
/// `Box::<T>::default()`. /// `Box::default()`.
/// ///
/// ### Why is this bad? /// ### Why is this bad?
/// First, it's more complex, involving two calls instead of one. /// `Box::default()` is equivalent and more concise.
/// Second, `Box::default()` can be faster
/// [in certain cases](https://nnethercote.github.io/perf-book/standard-library-types.html#box).
/// ///
/// ### Example /// ### Example
/// ```no_run /// ```no_run
@ -34,7 +29,7 @@ declare_clippy_lint! {
/// ``` /// ```
#[clippy::version = "1.66.0"] #[clippy::version = "1.66.0"]
pub BOX_DEFAULT, pub BOX_DEFAULT,
perf, style,
"Using Box::new(T::default()) instead of Box::default()" "Using Box::new(T::default()) instead of Box::default()"
} }
@ -53,14 +48,14 @@ impl LateLintPass<'_> for BoxDefault {
&& path_def_id(cx, ty).map_or(false, |id| Some(id) == cx.tcx.lang_items().owned_box()) && path_def_id(cx, ty).map_or(false, |id| Some(id) == cx.tcx.lang_items().owned_box())
// And the single argument to the call is another function call // And the single argument to the call is another function call
// This is the `T::default()` of `Box::new(T::default())` // This is the `T::default()` of `Box::new(T::default())`
&& let ExprKind::Call(arg_path, inner_call_args) = arg.kind && let ExprKind::Call(arg_path, _) = arg.kind
// And we are not in a foreign crate's macro // And we are not in a foreign crate's macro
&& !in_external_macro(cx.sess(), expr.span) && !in_external_macro(cx.sess(), expr.span)
// And the argument expression has the same context as the outer call expression // And the argument expression has the same context as the outer call expression
// or that we are inside a `vec!` macro expansion // or that we are inside a `vec!` macro expansion
&& (expr.span.eq_ctxt(arg.span) || is_local_vec_expn(cx, arg, expr)) && (expr.span.eq_ctxt(arg.span) || is_local_vec_expn(cx, arg, expr))
// And the argument is equivalent to `Default::default()` // And the argument is `Default::default()` or the type is specified
&& is_default_equivalent(cx, arg) && (is_plain_default(cx, arg_path) || (given_type(cx, expr) && is_default_equivalent(cx, arg)))
{ {
span_lint_and_sugg( span_lint_and_sugg(
cx, cx,
@ -68,23 +63,7 @@ impl LateLintPass<'_> for BoxDefault {
expr.span, expr.span,
"`Box::new(_)` of default value", "`Box::new(_)` of default value",
"try", "try",
if is_plain_default(cx, arg_path) || given_type(cx, expr) { "Box::default()".into(),
"Box::default()".into()
} else if let Some(arg_ty) = cx.typeck_results().expr_ty(arg).make_suggestable(cx.tcx, true) {
// Check if we can copy from the source expression in the replacement.
// We need the call to have no argument (see `explicit_default_type`).
if inner_call_args.is_empty()
&& let Some(ty) = explicit_default_type(arg_path)
&& let Some(s) = snippet_opt(cx, ty.span)
{
format!("Box::<{s}>::default()")
} else {
// Otherwise, use the inferred type's formatting.
with_forced_trimmed_paths!(format!("Box::<{arg_ty}>::default()"))
}
} else {
return;
},
Applicability::MachineApplicable, Applicability::MachineApplicable,
); );
} }
@ -103,20 +82,6 @@ fn is_plain_default(cx: &LateContext<'_>, arg_path: &Expr<'_>) -> bool {
} }
} }
// Checks whether the call is of the form `A::B::f()`. Returns `A::B` if it is.
//
// In the event we have this kind of construct, it's easy to use `A::B` as a replacement in the
// quickfix. `f` must however have no parameter. Should `f` have some, then some of the type of
// `A::B` may be inferred from the arguments. This would be the case for `Vec::from([0; false])`,
// where the argument to `from` allows inferring this is a `Vec<bool>`
fn explicit_default_type<'a>(arg_path: &'a Expr<'_>) -> Option<&'a Ty<'a>> {
if let ExprKind::Path(QPath::TypeRelative(ty, _)) = &arg_path.kind {
Some(ty)
} else {
None
}
}
fn is_local_vec_expn(cx: &LateContext<'_>, expr: &Expr<'_>, ref_expr: &Expr<'_>) -> bool { fn is_local_vec_expn(cx: &LateContext<'_>, expr: &Expr<'_>, ref_expr: &Expr<'_>) -> bool {
macro_backtrace(expr.span).next().map_or(false, |call| { macro_backtrace(expr.span).next().map_or(false, |call| {
cx.tcx.is_diagnostic_item(sym::vec_macro, call.def_id) && call.span.eq_ctxt(ref_expr.span) cx.tcx.is_diagnostic_item(sym::vec_macro, call.def_id) && call.span.eq_ctxt(ref_expr.span)

View File

@ -1,5 +1,5 @@
#![warn(clippy::box_default)] #![warn(clippy::box_default)]
#![allow(clippy::default_constructed_unit_structs)] #![allow(clippy::boxed_local, clippy::default_constructed_unit_structs)]
#[derive(Default)] #[derive(Default)]
struct ImplementsDefault; struct ImplementsDefault;
@ -12,26 +12,50 @@ impl OwnDefault {
} }
} }
macro_rules! outer { macro_rules! default {
($e: expr) => { () => {
$e Default::default()
};
}
macro_rules! string_new {
() => {
String::new()
};
}
macro_rules! box_new {
($e:expr) => {
Box::new($e)
}; };
} }
fn main() { fn main() {
let _string: Box<String> = Box::default(); let string1: Box<String> = Box::default();
let _byte = Box::<u8>::default(); let string2: Box<String> = Box::default();
let _vec = Box::<Vec::<u8>>::default(); let impl1: Box<ImplementsDefault> = Box::default();
let _impl = Box::<ImplementsDefault>::default(); let vec: Box<Vec<u8>> = Box::default();
let _impl2 = Box::<ImplementsDefault>::default(); let byte: Box<u8> = Box::default();
let _impl3: Box<ImplementsDefault> = Box::default(); let vec2: Box<Vec<ImplementsDefault>> = Box::default();
let _own = Box::new(OwnDefault::default()); // should not lint let vec3: Box<Vec<bool>> = Box::default();
let _in_macro = outer!(Box::<String>::default());
let _string_default = outer!(Box::<String>::default()); let plain_default = Box::default();
let _vec2: Box<Vec<ImplementsDefault>> = Box::default(); let _: Box<String> = plain_default;
let _vec3: Box<Vec<bool>> = Box::default();
let _vec4: Box<_> = Box::<Vec<bool>>::default(); let _: Box<String> = Box::new(default!());
let _more = ret_ty_fn(); let _: Box<String> = Box::new(string_new!());
let _: Box<String> = box_new!(Default::default());
let _: Box<String> = box_new!(String::new());
let _: Box<String> = box_new!(default!());
let _: Box<String> = box_new!(string_new!());
let own: Box<OwnDefault> = Box::new(OwnDefault::default()); // should not lint
// Do not suggest where a turbofish would be required
let impl2 = Box::new(ImplementsDefault::default());
let impl3 = Box::new(<ImplementsDefault as Default>::default());
let vec4: Box<_> = Box::new(Vec::from([false; 0]));
let more = ret_ty_fn();
call_ty_fn(Box::default()); call_ty_fn(Box::default());
issue_10381(); issue_10381();
@ -44,10 +68,9 @@ fn main() {
} }
fn ret_ty_fn() -> Box<bool> { fn ret_ty_fn() -> Box<bool> {
Box::<bool>::default() Box::new(bool::default()) // Could lint, currently doesn't
} }
#[allow(clippy::boxed_local)]
fn call_ty_fn(_b: Box<u8>) { fn call_ty_fn(_b: Box<u8>) {
issue_9621_dyn_trait(); issue_9621_dyn_trait();
} }
@ -61,7 +84,7 @@ impl Read for ImplementsDefault {
} }
fn issue_9621_dyn_trait() { fn issue_9621_dyn_trait() {
let _: Box<dyn Read> = Box::<ImplementsDefault>::default(); let _: Box<dyn Read> = Box::new(ImplementsDefault::default());
issue_10089(); issue_10089();
} }
@ -70,7 +93,7 @@ fn issue_10089() {
#[derive(Default)] #[derive(Default)]
struct WeirdPathed; struct WeirdPathed;
let _ = Box::<WeirdPathed>::default(); let _ = Box::new(WeirdPathed::default());
}; };
} }
@ -82,7 +105,7 @@ fn issue_10381() {
fn maybe_get_bar(i: u32) -> Option<Box<dyn Bar>> { fn maybe_get_bar(i: u32) -> Option<Box<dyn Bar>> {
if i % 2 == 0 { if i % 2 == 0 {
Some(Box::<Foo>::default()) Some(Box::new(Foo::default()))
} else { } else {
None None
} }
@ -91,20 +114,6 @@ fn issue_10381() {
assert!(maybe_get_bar(2).is_some()); assert!(maybe_get_bar(2).is_some());
} }
#[allow(unused)]
fn issue_11868() {
fn foo(_: &mut Vec<usize>) {}
macro_rules! bar {
($baz:expr) => {
Box::leak(Box::new($baz))
};
}
foo(bar!(vec![]));
foo(bar!(vec![1]));
}
// Issue #11927: The quickfix for the `Box::new` suggests replacing with `Box::<Inner>::default()`, // Issue #11927: The quickfix for the `Box::new` suggests replacing with `Box::<Inner>::default()`,
// removing the `outer::` segment. // removing the `outer::` segment.
fn issue_11927() { fn issue_11927() {
@ -116,7 +125,7 @@ fn issue_11927() {
} }
fn foo() { fn foo() {
let _b = Box::<outer::Inner>::default(); let _b = Box::new(outer::Inner::default());
let _b = Box::<std::collections::HashSet::<i32>>::default(); let _b = Box::new(std::collections::HashSet::<i32>::new());
} }
} }

View File

@ -1,5 +1,5 @@
#![warn(clippy::box_default)] #![warn(clippy::box_default)]
#![allow(clippy::default_constructed_unit_structs)] #![allow(clippy::boxed_local, clippy::default_constructed_unit_structs)]
#[derive(Default)] #[derive(Default)]
struct ImplementsDefault; struct ImplementsDefault;
@ -12,26 +12,50 @@ impl OwnDefault {
} }
} }
macro_rules! outer { macro_rules! default {
($e: expr) => { () => {
$e Default::default()
};
}
macro_rules! string_new {
() => {
String::new()
};
}
macro_rules! box_new {
($e:expr) => {
Box::new($e)
}; };
} }
fn main() { fn main() {
let _string: Box<String> = Box::new(Default::default()); let string1: Box<String> = Box::new(Default::default());
let _byte = Box::new(u8::default()); let string2: Box<String> = Box::new(String::new());
let _vec = Box::new(Vec::<u8>::new()); let impl1: Box<ImplementsDefault> = Box::new(Default::default());
let _impl = Box::new(ImplementsDefault::default()); let vec: Box<Vec<u8>> = Box::new(Vec::new());
let _impl2 = Box::new(<ImplementsDefault as Default>::default()); let byte: Box<u8> = Box::new(u8::default());
let _impl3: Box<ImplementsDefault> = Box::new(Default::default()); let vec2: Box<Vec<ImplementsDefault>> = Box::new(vec![]);
let _own = Box::new(OwnDefault::default()); // should not lint let vec3: Box<Vec<bool>> = Box::new(Vec::from([]));
let _in_macro = outer!(Box::new(String::new()));
let _string_default = outer!(Box::new(String::from(""))); let plain_default = Box::new(Default::default());
let _vec2: Box<Vec<ImplementsDefault>> = Box::new(vec![]); let _: Box<String> = plain_default;
let _vec3: Box<Vec<bool>> = Box::new(Vec::from([]));
let _vec4: Box<_> = Box::new(Vec::from([false; 0])); let _: Box<String> = Box::new(default!());
let _more = ret_ty_fn(); let _: Box<String> = Box::new(string_new!());
let _: Box<String> = box_new!(Default::default());
let _: Box<String> = box_new!(String::new());
let _: Box<String> = box_new!(default!());
let _: Box<String> = box_new!(string_new!());
let own: Box<OwnDefault> = Box::new(OwnDefault::default()); // should not lint
// Do not suggest where a turbofish would be required
let impl2 = Box::new(ImplementsDefault::default());
let impl3 = Box::new(<ImplementsDefault as Default>::default());
let vec4: Box<_> = Box::new(Vec::from([false; 0]));
let more = ret_ty_fn();
call_ty_fn(Box::new(u8::default())); call_ty_fn(Box::new(u8::default()));
issue_10381(); issue_10381();
@ -44,10 +68,9 @@ fn main() {
} }
fn ret_ty_fn() -> Box<bool> { fn ret_ty_fn() -> Box<bool> {
Box::new(bool::default()) Box::new(bool::default()) // Could lint, currently doesn't
} }
#[allow(clippy::boxed_local)]
fn call_ty_fn(_b: Box<u8>) { fn call_ty_fn(_b: Box<u8>) {
issue_9621_dyn_trait(); issue_9621_dyn_trait();
} }
@ -91,20 +114,6 @@ fn issue_10381() {
assert!(maybe_get_bar(2).is_some()); assert!(maybe_get_bar(2).is_some());
} }
#[allow(unused)]
fn issue_11868() {
fn foo(_: &mut Vec<usize>) {}
macro_rules! bar {
($baz:expr) => {
Box::leak(Box::new($baz))
};
}
foo(bar!(vec![]));
foo(bar!(vec![1]));
}
// Issue #11927: The quickfix for the `Box::new` suggests replacing with `Box::<Inner>::default()`, // Issue #11927: The quickfix for the `Box::new` suggests replacing with `Box::<Inner>::default()`,
// removing the `outer::` segment. // removing the `outer::` segment.
fn issue_11927() { fn issue_11927() {

View File

@ -1,113 +1,59 @@
error: `Box::new(_)` of default value error: `Box::new(_)` of default value
--> tests/ui/box_default.rs:22:32 --> tests/ui/box_default.rs:34:32
| |
LL | let _string: Box<String> = Box::new(Default::default()); LL | let string1: Box<String> = Box::new(Default::default());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::default()` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::default()`
| |
= note: `-D clippy::box-default` implied by `-D warnings` = note: `-D clippy::box-default` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::box_default)]` = help: to override `-D warnings` add `#[allow(clippy::box_default)]`
error: `Box::new(_)` of default value error: `Box::new(_)` of default value
--> tests/ui/box_default.rs:23:17 --> tests/ui/box_default.rs:35:32
| |
LL | let _byte = Box::new(u8::default()); LL | let string2: Box<String> = Box::new(String::new());
| ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::<u8>::default()` | ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::default()`
error: `Box::new(_)` of default value error: `Box::new(_)` of default value
--> tests/ui/box_default.rs:24:16 --> tests/ui/box_default.rs:36:41
| |
LL | let _vec = Box::new(Vec::<u8>::new()); LL | let impl1: Box<ImplementsDefault> = Box::new(Default::default());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::<Vec::<u8>>::default()` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::default()`
error: `Box::new(_)` of default value error: `Box::new(_)` of default value
--> tests/ui/box_default.rs:25:17 --> tests/ui/box_default.rs:37:29
| |
LL | let _impl = Box::new(ImplementsDefault::default()); LL | let vec: Box<Vec<u8>> = Box::new(Vec::new());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::<ImplementsDefault>::default()` | ^^^^^^^^^^^^^^^^^^^^ help: try: `Box::default()`
error: `Box::new(_)` of default value error: `Box::new(_)` of default value
--> tests/ui/box_default.rs:26:18 --> tests/ui/box_default.rs:38:25
| |
LL | let _impl2 = Box::new(<ImplementsDefault as Default>::default()); LL | let byte: Box<u8> = Box::new(u8::default());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::<ImplementsDefault>::default()` | ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::default()`
error: `Box::new(_)` of default value error: `Box::new(_)` of default value
--> tests/ui/box_default.rs:27:42 --> tests/ui/box_default.rs:39:45
| |
LL | let _impl3: Box<ImplementsDefault> = Box::new(Default::default()); LL | let vec2: Box<Vec<ImplementsDefault>> = Box::new(vec![]);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::default()` | ^^^^^^^^^^^^^^^^ help: try: `Box::default()`
error: `Box::new(_)` of default value error: `Box::new(_)` of default value
--> tests/ui/box_default.rs:29:28 --> tests/ui/box_default.rs:40:32
| |
LL | let _in_macro = outer!(Box::new(String::new())); LL | let vec3: Box<Vec<bool>> = Box::new(Vec::from([]));
| ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::<String>::default()` | ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::default()`
error: `Box::new(_)` of default value error: `Box::new(_)` of default value
--> tests/ui/box_default.rs:30:34 --> tests/ui/box_default.rs:42:25
| |
LL | let _string_default = outer!(Box::new(String::from(""))); LL | let plain_default = Box::new(Default::default());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::<String>::default()` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::default()`
error: `Box::new(_)` of default value error: `Box::new(_)` of default value
--> tests/ui/box_default.rs:31:46 --> tests/ui/box_default.rs:59:16
|
LL | let _vec2: Box<Vec<ImplementsDefault>> = Box::new(vec![]);
| ^^^^^^^^^^^^^^^^ help: try: `Box::default()`
error: `Box::new(_)` of default value
--> tests/ui/box_default.rs:32:33
|
LL | let _vec3: Box<Vec<bool>> = Box::new(Vec::from([]));
| ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::default()`
error: `Box::new(_)` of default value
--> tests/ui/box_default.rs:33:25
|
LL | let _vec4: Box<_> = Box::new(Vec::from([false; 0]));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::<Vec<bool>>::default()`
error: `Box::new(_)` of default value
--> tests/ui/box_default.rs:35:16
| |
LL | call_ty_fn(Box::new(u8::default())); LL | call_ty_fn(Box::new(u8::default()));
| ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::default()` | ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::default()`
error: `Box::new(_)` of default value error: aborting due to 9 previous errors
--> tests/ui/box_default.rs:47:5
|
LL | Box::new(bool::default())
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::<bool>::default()`
error: `Box::new(_)` of default value
--> tests/ui/box_default.rs:64:28
|
LL | let _: Box<dyn Read> = Box::new(ImplementsDefault::default());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::<ImplementsDefault>::default()`
error: `Box::new(_)` of default value
--> tests/ui/box_default.rs:73:17
|
LL | let _ = Box::new(WeirdPathed::default());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::<WeirdPathed>::default()`
error: `Box::new(_)` of default value
--> tests/ui/box_default.rs:85:18
|
LL | Some(Box::new(Foo::default()))
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::<Foo>::default()`
error: `Box::new(_)` of default value
--> tests/ui/box_default.rs:119:18
|
LL | let _b = Box::new(outer::Inner::default());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::<outer::Inner>::default()`
error: `Box::new(_)` of default value
--> tests/ui/box_default.rs:120:18
|
LL | let _b = Box::new(std::collections::HashSet::<i32>::new());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::<std::collections::HashSet::<i32>>::default()`
error: aborting due to 18 previous errors