diff options
| -rw-r--r-- | clippy_lints/src/box_default.rs | 51 | ||||
| -rw-r--r-- | tests/ui/box_default.fixed | 85 | ||||
| -rw-r--r-- | tests/ui/box_default.rs | 75 | ||||
| -rw-r--r-- | tests/ui/box_default.stderr | 104 |
4 files changed, 122 insertions, 193 deletions
diff --git a/clippy_lints/src/box_default.rs b/clippy_lints/src/box_default.rs index e83f5c2a74c..c7b0006c8cb 100644 --- a/clippy_lints/src/box_default.rs +++ b/clippy_lints/src/box_default.rs @@ -1,6 +1,5 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::macros::macro_backtrace; -use clippy_utils::source::snippet_opt; use clippy_utils::ty::expr_sig; use clippy_utils::{is_default_equivalent, path_def_id}; 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_lint::{LateContext, LateLintPass, LintContext}; 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_span::sym; declare_clippy_lint! { /// ### What it does - /// checks for `Box::new(T::default())`, which is better written as - /// `Box::<T>::default()`. + /// checks for `Box::new(Default::default())`, which can be written as + /// `Box::default()`. /// /// ### Why is this bad? - /// First, it's more complex, involving two calls instead of one. - /// Second, `Box::default()` can be faster - /// [in certain cases](https://nnethercote.github.io/perf-book/standard-library-types.html#box). + /// `Box::default()` is equivalent and more concise. /// /// ### Example /// ```no_run @@ -34,7 +29,7 @@ declare_clippy_lint! { /// ``` #[clippy::version = "1.66.0"] pub BOX_DEFAULT, - perf, + style, "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()) // And the single argument to the call is another function call // 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 && !in_external_macro(cx.sess(), expr.span) // And the argument expression has the same context as the outer call expression // or that we are inside a `vec!` macro expansion && (expr.span.eq_ctxt(arg.span) || is_local_vec_expn(cx, arg, expr)) - // And the argument is equivalent to `Default::default()` - && is_default_equivalent(cx, arg) + // And the argument is `Default::default()` or the type is specified + && (is_plain_default(cx, arg_path) || (given_type(cx, expr) && is_default_equivalent(cx, arg))) { span_lint_and_sugg( cx, @@ -68,23 +63,7 @@ impl LateLintPass<'_> for BoxDefault { expr.span, "`Box::new(_)` of default value", "try", - if is_plain_default(cx, arg_path) || given_type(cx, expr) { - "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; - }, + "Box::default()".into(), 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 { 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) diff --git a/tests/ui/box_default.fixed b/tests/ui/box_default.fixed index fea7405c685..6c2896b3aa0 100644 --- a/tests/ui/box_default.fixed +++ b/tests/ui/box_default.fixed @@ -1,5 +1,5 @@ #![warn(clippy::box_default)] -#![allow(clippy::default_constructed_unit_structs)] +#![allow(clippy::boxed_local, clippy::default_constructed_unit_structs)] #[derive(Default)] struct ImplementsDefault; @@ -12,26 +12,50 @@ impl OwnDefault { } } -macro_rules! outer { - ($e: expr) => { - $e +macro_rules! default { + () => { + Default::default() + }; +} + +macro_rules! string_new { + () => { + String::new() + }; +} + +macro_rules! box_new { + ($e:expr) => { + Box::new($e) }; } fn main() { - let _string: Box<String> = Box::default(); - let _byte = Box::<u8>::default(); - let _vec = Box::<Vec::<u8>>::default(); - let _impl = Box::<ImplementsDefault>::default(); - let _impl2 = Box::<ImplementsDefault>::default(); - let _impl3: Box<ImplementsDefault> = Box::default(); - let _own = Box::new(OwnDefault::default()); // should not lint - let _in_macro = outer!(Box::<String>::default()); - let _string_default = outer!(Box::<String>::default()); - let _vec2: Box<Vec<ImplementsDefault>> = Box::default(); - let _vec3: Box<Vec<bool>> = Box::default(); - let _vec4: Box<_> = Box::<Vec<bool>>::default(); - let _more = ret_ty_fn(); + let string1: Box<String> = Box::default(); + let string2: Box<String> = Box::default(); + let impl1: Box<ImplementsDefault> = Box::default(); + let vec: Box<Vec<u8>> = Box::default(); + let byte: Box<u8> = Box::default(); + let vec2: Box<Vec<ImplementsDefault>> = Box::default(); + let vec3: Box<Vec<bool>> = Box::default(); + + let plain_default = Box::default(); + let _: Box<String> = plain_default; + + let _: Box<String> = Box::new(default!()); + 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()); issue_10381(); @@ -44,10 +68,9 @@ fn main() { } 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>) { issue_9621_dyn_trait(); } @@ -61,7 +84,7 @@ impl Read for ImplementsDefault { } fn issue_9621_dyn_trait() { - let _: Box<dyn Read> = Box::<ImplementsDefault>::default(); + let _: Box<dyn Read> = Box::new(ImplementsDefault::default()); issue_10089(); } @@ -70,7 +93,7 @@ fn issue_10089() { #[derive(Default)] 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>> { if i % 2 == 0 { - Some(Box::<Foo>::default()) + Some(Box::new(Foo::default())) } else { None } @@ -91,20 +114,6 @@ fn issue_10381() { 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()`, // removing the `outer::` segment. fn issue_11927() { @@ -116,7 +125,7 @@ fn issue_11927() { } fn foo() { - let _b = Box::<outer::Inner>::default(); - let _b = Box::<std::collections::HashSet::<i32>>::default(); + let _b = Box::new(outer::Inner::default()); + let _b = Box::new(std::collections::HashSet::<i32>::new()); } } diff --git a/tests/ui/box_default.rs b/tests/ui/box_default.rs index eecba7464ec..e19a62a9022 100644 --- a/tests/ui/box_default.rs +++ b/tests/ui/box_default.rs @@ -1,5 +1,5 @@ #![warn(clippy::box_default)] -#![allow(clippy::default_constructed_unit_structs)] +#![allow(clippy::boxed_local, clippy::default_constructed_unit_structs)] #[derive(Default)] struct ImplementsDefault; @@ -12,26 +12,50 @@ impl OwnDefault { } } -macro_rules! outer { - ($e: expr) => { - $e +macro_rules! default { + () => { + Default::default() + }; +} + +macro_rules! string_new { + () => { + String::new() + }; +} + +macro_rules! box_new { + ($e:expr) => { + Box::new($e) }; } fn main() { - let _string: Box<String> = Box::new(Default::default()); - let _byte = Box::new(u8::default()); - let _vec = Box::new(Vec::<u8>::new()); - let _impl = Box::new(ImplementsDefault::default()); - let _impl2 = Box::new(<ImplementsDefault as Default>::default()); - let _impl3: Box<ImplementsDefault> = Box::new(Default::default()); - let _own = Box::new(OwnDefault::default()); // should not lint - let _in_macro = outer!(Box::new(String::new())); - let _string_default = outer!(Box::new(String::from(""))); - let _vec2: Box<Vec<ImplementsDefault>> = Box::new(vec![]); - let _vec3: Box<Vec<bool>> = Box::new(Vec::from([])); - let _vec4: Box<_> = Box::new(Vec::from([false; 0])); - let _more = ret_ty_fn(); + let string1: Box<String> = Box::new(Default::default()); + let string2: Box<String> = Box::new(String::new()); + let impl1: Box<ImplementsDefault> = Box::new(Default::default()); + let vec: Box<Vec<u8>> = Box::new(Vec::new()); + let byte: Box<u8> = Box::new(u8::default()); + let vec2: Box<Vec<ImplementsDefault>> = Box::new(vec![]); + let vec3: Box<Vec<bool>> = Box::new(Vec::from([])); + + let plain_default = Box::new(Default::default()); + let _: Box<String> = plain_default; + + let _: Box<String> = Box::new(default!()); + 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())); issue_10381(); @@ -44,10 +68,9 @@ fn main() { } 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>) { issue_9621_dyn_trait(); } @@ -91,20 +114,6 @@ fn issue_10381() { 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()`, // removing the `outer::` segment. fn issue_11927() { diff --git a/tests/ui/box_default.stderr b/tests/ui/box_default.stderr index 8bb5917a627..f172a875dce 100644 --- a/tests/ui/box_default.stderr +++ b/tests/ui/box_default.stderr @@ -1,113 +1,59 @@ 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()` | = note: `-D clippy::box-default` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::box_default)]` 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()); - | ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::<u8>::default()` +LL | let string2: Box<String> = Box::new(String::new()); + | ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::default()` 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()); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::<Vec::<u8>>::default()` +LL | let impl1: Box<ImplementsDefault> = Box::new(Default::default()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::default()` 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()); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::<ImplementsDefault>::default()` +LL | let vec: Box<Vec<u8>> = Box::new(Vec::new()); + | ^^^^^^^^^^^^^^^^^^^^ help: try: `Box::default()` 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()); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::<ImplementsDefault>::default()` +LL | let byte: Box<u8> = Box::new(u8::default()); + | ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::default()` 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()); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::default()` +LL | let vec2: Box<Vec<ImplementsDefault>> = Box::new(vec![]); + | ^^^^^^^^^^^^^^^^ help: try: `Box::default()` 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())); - | ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::<String>::default()` +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:30:34 + --> tests/ui/box_default.rs:42:25 | -LL | let _string_default = outer!(Box::new(String::from(""))); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::<String>::default()` +LL | let plain_default = Box::new(Default::default()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::default()` error: `Box::new(_)` of default value - --> tests/ui/box_default.rs:31:46 - | -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 + --> tests/ui/box_default.rs:59:16 | LL | call_ty_fn(Box::new(u8::default())); | ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::default()` -error: `Box::new(_)` of default value - --> 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 +error: aborting due to 9 previous errors |