Skip to content

Commit 9aa85dc

Browse files
borsbors
committed
Auto merge of #9511 - rust-lang:box-default, r=Alexendoo
add `box-default` lint This adds a `box-default` lint to suggest using `Box::default()` instead of `Box::new(Default::default())`, which offers less moving parts and potentially better performance according to [the perf book](https://nnethercote.github.io/perf-book/standard-library-types.html#box). --- changelog: add [`box_default`] lint
2 parents 257fb4b + 63f441e commit 9aa85dc

11 files changed

+197
-16
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -3759,6 +3759,7 @@ Released 2018-09-13
37593759
[`borrow_interior_mutable_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#borrow_interior_mutable_const
37603760
[`borrowed_box`]: https://rust-lang.github.io/rust-clippy/master/index.html#borrowed_box
37613761
[`box_collection`]: https://rust-lang.github.io/rust-clippy/master/index.html#box_collection
3762+
[`box_default`]: https://rust-lang.github.io/rust-clippy/master/index.html#box_default
37623763
[`box_vec`]: https://rust-lang.github.io/rust-clippy/master/index.html#box_vec
37633764
[`boxed_local`]: https://rust-lang.github.io/rust-clippy/master/index.html#boxed_local
37643765
[`branches_sharing_code`]: https://rust-lang.github.io/rust-clippy/master/index.html#branches_sharing_code

‎clippy_lints/src/box_default.rs

+61
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
use clippy_utils::{diagnostics::span_lint_and_help, is_default_equivalent, path_def_id};
2+
use rustc_hir::{Expr, ExprKind, QPath};
3+
use rustc_lint::{LateContext, LateLintPass, LintContext};
4+
use rustc_middle::lint::in_external_macro;
5+
use rustc_session::{declare_lint_pass, declare_tool_lint};
6+
use rustc_span::sym;
7+
8+
declare_clippy_lint! {
9+
/// ### What it does
10+
/// checks for `Box::new(T::default())`, which is better written as
11+
/// `Box::<T>::default()`.
12+
///
13+
/// ### Why is this bad?
14+
/// First, it's more complex, involving two calls instead of one.
15+
/// Second, `Box::default()` can be faster
16+
/// [in certain cases](https://nnethercote.github.io/perf-book/standard-library-types.html#box).
17+
///
18+
/// ### Known problems
19+
/// The lint may miss some cases (e.g. Box::new(String::from(""))).
20+
/// On the other hand, it will trigger on cases where the `default`
21+
/// code comes from a macro that does something different based on
22+
/// e.g. target operating system.
23+
///
24+
/// ### Example
25+
/// ```rust
26+
/// let x: Box<String> = Box::new(Default::default());
27+
/// ```
28+
/// Use instead:
29+
/// ```rust
30+
/// let x: Box<String> = Box::default();
31+
/// ```
32+
#[clippy::version = "1.65.0"]
33+
pub BOX_DEFAULT,
34+
perf,
35+
"Using Box::new(T::default()) instead of Box::default()"
36+
}
37+
38+
declare_lint_pass!(BoxDefault => [BOX_DEFAULT]);
39+
40+
impl LateLintPass<'_> for BoxDefault {
41+
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
42+
if let ExprKind::Call(box_new, [arg]) = expr.kind
43+
&& let ExprKind::Path(QPath::TypeRelative(ty, seg)) = box_new.kind
44+
&& let ExprKind::Call(..) = arg.kind
45+
&& !in_external_macro(cx.sess(), expr.span)
46+
&& expr.span.eq_ctxt(arg.span)
47+
&& seg.ident.name == sym::new
48+
&& path_def_id(cx, ty) == cx.tcx.lang_items().owned_box()
49+
&& is_default_equivalent(cx, arg)
50+
{
51+
span_lint_and_help(
52+
cx,
53+
BOX_DEFAULT,
54+
expr.span,
55+
"`Box::new(_)` of default value",
56+
None,
57+
"use `Box::default()` instead",
58+
);
59+
}
60+
}
61+
}

‎clippy_lints/src/lib.register_all.rs

+1
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
2121
LintId::of(booleans::NONMINIMAL_BOOL),
2222
LintId::of(booleans::OVERLY_COMPLEX_BOOL_EXPR),
2323
LintId::of(borrow_deref_ref::BORROW_DEREF_REF),
24+
LintId::of(box_default::BOX_DEFAULT),
2425
LintId::of(casts::CAST_ABS_TO_UNSIGNED),
2526
LintId::of(casts::CAST_ENUM_CONSTRUCTOR),
2627
LintId::of(casts::CAST_ENUM_TRUNCATION),

‎clippy_lints/src/lib.register_lints.rs

+1
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ store.register_lints(&[
6060
booleans::NONMINIMAL_BOOL,
6161
booleans::OVERLY_COMPLEX_BOOL_EXPR,
6262
borrow_deref_ref::BORROW_DEREF_REF,
63+
box_default::BOX_DEFAULT,
6364
cargo::CARGO_COMMON_METADATA,
6465
cargo::MULTIPLE_CRATE_VERSIONS,
6566
cargo::NEGATIVE_FEATURE_NAMES,

‎clippy_lints/src/lib.register_perf.rs

+1
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
// Manual edits will be overwritten.
44

55
store.register_group(true, "clippy::perf", Some("clippy_perf"), vec![
6+
LintId::of(box_default::BOX_DEFAULT),
67
LintId::of(entry::MAP_ENTRY),
78
LintId::of(escape::BOXED_LOCAL),
89
LintId::of(format_args::FORMAT_IN_FORMAT_ARGS),

‎clippy_lints/src/lib.rs

+16-14
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,7 @@ mod bool_assert_comparison;
181181
mod bool_to_int_with_if;
182182
mod booleans;
183183
mod borrow_deref_ref;
184+
mod box_default;
184185
mod cargo;
185186
mod casts;
186187
mod checked_conversions;
@@ -533,8 +534,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
533534
store.register_late_pass(|| Box::new(utils::internal_lints::CompilerLintFunctions::new()));
534535
store.register_late_pass(|| Box::new(utils::internal_lints::IfChainStyle));
535536
store.register_late_pass(|| Box::new(utils::internal_lints::InvalidPaths));
536-
store.register_late_pass(|| Box::new(utils::internal_lints::InterningDefinedSymbol::default()));
537-
store.register_late_pass(|| Box::new(utils::internal_lints::LintWithoutLintPass::default()));
537+
store.register_late_pass(|| Box::<utils::internal_lints::InterningDefinedSymbol>::default());
538+
store.register_late_pass(|| Box::<utils::internal_lints::LintWithoutLintPass>::default());
538539
store.register_late_pass(|| Box::new(utils::internal_lints::MatchTypeOnDiagItem));
539540
store.register_late_pass(|| Box::new(utils::internal_lints::OuterExpnDataPass));
540541
store.register_late_pass(|| Box::new(utils::internal_lints::MsrvAttrImpl));
@@ -627,10 +628,10 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
627628
msrv,
628629
))
629630
});
630-
store.register_late_pass(|| Box::new(shadow::Shadow::default()));
631+
store.register_late_pass(|| Box::<shadow::Shadow>::default());
631632
store.register_late_pass(|| Box::new(unit_types::UnitTypes));
632633
store.register_late_pass(|| Box::new(loops::Loops));
633-
store.register_late_pass(|| Box::new(main_recursion::MainRecursion::default()));
634+
store.register_late_pass(|| Box::<main_recursion::MainRecursion>::default());
634635
store.register_late_pass(|| Box::new(lifetimes::Lifetimes));
635636
store.register_late_pass(|| Box::new(entry::HashMapPass));
636637
store.register_late_pass(|| Box::new(minmax::MinMaxPass));
@@ -664,7 +665,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
664665
store.register_late_pass(|| Box::new(format::UselessFormat));
665666
store.register_late_pass(|| Box::new(swap::Swap));
666667
store.register_late_pass(|| Box::new(overflow_check_conditional::OverflowCheckConditional));
667-
store.register_late_pass(|| Box::new(new_without_default::NewWithoutDefault::default()));
668+
store.register_late_pass(|| Box::<new_without_default::NewWithoutDefault>::default());
668669
let disallowed_names = conf.disallowed_names.iter().cloned().collect::<FxHashSet<_>>();
669670
store.register_late_pass(move || Box::new(disallowed_names::DisallowedNames::new(disallowed_names.clone())));
670671
let too_many_arguments_threshold = conf.too_many_arguments_threshold;
@@ -703,7 +704,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
703704
store.register_late_pass(|| Box::new(ref_option_ref::RefOptionRef));
704705
store.register_late_pass(|| Box::new(infinite_iter::InfiniteIter));
705706
store.register_late_pass(|| Box::new(inline_fn_without_body::InlineFnWithoutBody));
706-
store.register_late_pass(|| Box::new(useless_conversion::UselessConversion::default()));
707+
store.register_late_pass(|| Box::<useless_conversion::UselessConversion>::default());
707708
store.register_late_pass(|| Box::new(implicit_hasher::ImplicitHasher));
708709
store.register_late_pass(|| Box::new(fallible_impl_from::FallibleImplFrom));
709710
store.register_late_pass(|| Box::new(question_mark::QuestionMark));
@@ -773,7 +774,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
773774
upper_case_acronyms_aggressive,
774775
))
775776
});
776-
store.register_late_pass(|| Box::new(default::Default::default()));
777+
store.register_late_pass(|| Box::<default::Default>::default());
777778
store.register_late_pass(move || Box::new(unused_self::UnusedSelf::new(avoid_breaking_exported_api)));
778779
store.register_late_pass(|| Box::new(mutable_debug_assertion::DebugAssertWithMutCall));
779780
store.register_late_pass(|| Box::new(exit::Exit));
@@ -796,7 +797,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
796797
store.register_early_pass(|| Box::new(option_env_unwrap::OptionEnvUnwrap));
797798
let warn_on_all_wildcard_imports = conf.warn_on_all_wildcard_imports;
798799
store.register_late_pass(move || Box::new(wildcard_imports::WildcardImports::new(warn_on_all_wildcard_imports)));
799-
store.register_late_pass(|| Box::new(redundant_pub_crate::RedundantPubCrate::default()));
800+
store.register_late_pass(|| Box::<redundant_pub_crate::RedundantPubCrate>::default());
800801
store.register_late_pass(|| Box::new(unnamed_address::UnnamedAddress));
801802
store.register_late_pass(move || Box::new(dereference::Dereferencing::new(msrv)));
802803
store.register_late_pass(|| Box::new(option_if_let_else::OptionIfLetElse));
@@ -814,7 +815,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
814815
});
815816
let macro_matcher = conf.standard_macro_braces.iter().cloned().collect::<FxHashSet<_>>();
816817
store.register_early_pass(move || Box::new(nonstandard_macro_braces::MacroBraces::new(&macro_matcher)));
817-
store.register_late_pass(|| Box::new(macro_use::MacroUseImports::default()));
818+
store.register_late_pass(|| Box::<macro_use::MacroUseImports>::default());
818819
store.register_late_pass(|| Box::new(pattern_type_mismatch::PatternTypeMismatch));
819820
store.register_late_pass(|| Box::new(unwrap_in_result::UnwrapInResult));
820821
store.register_late_pass(|| Box::new(semicolon_if_nothing_returned::SemicolonIfNothingReturned));
@@ -827,7 +828,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
827828
store.register_late_pass(|| Box::new(strings::StrToString));
828829
store.register_late_pass(|| Box::new(strings::StringToString));
829830
store.register_late_pass(|| Box::new(zero_sized_map_values::ZeroSizedMapValues));
830-
store.register_late_pass(|| Box::new(vec_init_then_push::VecInitThenPush::default()));
831+
store.register_late_pass(|| Box::<vec_init_then_push::VecInitThenPush>::default());
831832
store.register_late_pass(|| Box::new(redundant_slicing::RedundantSlicing));
832833
store.register_late_pass(|| Box::new(from_str_radix_10::FromStrRadix10));
833834
store.register_late_pass(move || Box::new(if_then_some_else_none::IfThenSomeElseNone::new(msrv)));
@@ -865,7 +866,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
865866
store.register_late_pass(move || Box::new(manual_bits::ManualBits::new(msrv)));
866867
store.register_late_pass(|| Box::new(default_union_representation::DefaultUnionRepresentation));
867868
store.register_early_pass(|| Box::new(doc_link_with_quotes::DocLinkWithQuotes));
868-
store.register_late_pass(|| Box::new(only_used_in_recursion::OnlyUsedInRecursion::default()));
869+
store.register_late_pass(|| Box::<only_used_in_recursion::OnlyUsedInRecursion>::default());
869870
let allow_dbg_in_tests = conf.allow_dbg_in_tests;
870871
store.register_late_pass(move || Box::new(dbg_macro::DbgMacro::new(allow_dbg_in_tests)));
871872
let cargo_ignore_publish = conf.cargo_ignore_publish;
@@ -874,7 +875,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
874875
ignore_publish: cargo_ignore_publish,
875876
})
876877
});
877-
store.register_late_pass(|| Box::new(write::Write::default()));
878+
store.register_late_pass(|| Box::<write::Write>::default());
878879
store.register_early_pass(|| Box::new(crate_in_macro_def::CrateInMacroDef));
879880
store.register_early_pass(|| Box::new(empty_structs_with_brackets::EmptyStructsWithBrackets));
880881
store.register_late_pass(|| Box::new(unnecessary_owned_empty_strings::UnnecessaryOwnedEmptyStrings));
@@ -884,7 +885,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
884885
store.register_late_pass(move || Box::new(large_include_file::LargeIncludeFile::new(max_include_file_size)));
885886
store.register_late_pass(|| Box::new(strings::TrimSplitWhitespace));
886887
store.register_late_pass(|| Box::new(rc_clone_in_vec_init::RcCloneInVecInit));
887-
store.register_early_pass(|| Box::new(duplicate_mod::DuplicateMod::default()));
888+
store.register_early_pass(|| Box::<duplicate_mod::DuplicateMod>::default());
888889
store.register_early_pass(|| Box::new(unused_rounding::UnusedRounding));
889890
store.register_early_pass(move || Box::new(almost_complete_letter_range::AlmostCompleteLetterRange::new(msrv)));
890891
store.register_late_pass(|| Box::new(swap_ptr_to_ref::SwapPtrToRef));
@@ -896,13 +897,14 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
896897
let verbose_bit_mask_threshold = conf.verbose_bit_mask_threshold;
897898
store.register_late_pass(move || Box::new(operators::Operators::new(verbose_bit_mask_threshold)));
898899
store.register_late_pass(|| Box::new(invalid_utf8_in_unchecked::InvalidUtf8InUnchecked));
899-
store.register_late_pass(|| Box::new(std_instead_of_core::StdReexports::default()));
900+
store.register_late_pass(|| Box::<std_instead_of_core::StdReexports>::default());
900901
store.register_late_pass(|| Box::new(manual_instant_elapsed::ManualInstantElapsed));
901902
store.register_late_pass(|| Box::new(partialeq_to_none::PartialeqToNone));
902903
store.register_late_pass(|| Box::new(manual_string_new::ManualStringNew));
903904
store.register_late_pass(|| Box::new(unused_peekable::UnusedPeekable));
904905
store.register_early_pass(|| Box::new(multi_assignments::MultiAssignments));
905906
store.register_late_pass(|| Box::new(bool_to_int_with_if::BoolToIntWithIf));
907+
store.register_late_pass(|| Box::new(box_default::BoxDefault));
906908
// add lints here, do not remove this comment, it's used in `new_lint`
907909
}
908910

‎src/docs.rs

+1
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ docs! {
4848
"borrow_interior_mutable_const",
4949
"borrowed_box",
5050
"box_collection",
51+
"box_default",
5152
"boxed_local",
5253
"branches_sharing_code",
5354
"builtin_type_shadow",

‎src/docs/box_default.txt

+23
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
### What it does
2+
checks for `Box::new(T::default())`, which is better written as
3+
`Box::<T>::default()`.
4+
5+
### Why is this bad?
6+
First, it's more complex, involving two calls instead of one.
7+
Second, `Box::default()` can be faster
8+
[in certain cases](https://nnethercote.github.io/perf-book/standard-library-types.html#box).
9+
10+
### Known problems
11+
The lint may miss some cases (e.g. Box::new(String::from(""))).
12+
On the other hand, it will trigger on cases where the `default`
13+
code comes from a macro that does something different based on
14+
e.g. target operating system.
15+
16+
### Example
17+
```
18+
let x: Box<String> = Box::new(Default::default());
19+
```
20+
Use instead:
21+
```
22+
let x: Box<String> = Box::default();
23+
```

‎tests/ui/box_collection.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ macro_rules! boxit {
1515
}
1616

1717
fn test_macro() {
18-
boxit!(Vec::new(), Vec<u8>);
18+
boxit!(vec![1], Vec<u8>);
1919
}
2020

2121
fn test1(foo: Box<Vec<bool>>) {}
@@ -50,7 +50,7 @@ fn test_local_not_linted() {
5050
pub fn pub_test(foo: Box<Vec<bool>>) {}
5151

5252
pub fn pub_test_ret() -> Box<Vec<bool>> {
53-
Box::new(Vec::new())
53+
Box::default()
5454
}
5555

5656
fn main() {}

‎tests/ui/box_default.rs

+31
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
#![warn(clippy::box_default)]
2+
3+
#[derive(Default)]
4+
struct ImplementsDefault;
5+
6+
struct OwnDefault;
7+
8+
impl OwnDefault {
9+
fn default() -> Self {
10+
Self
11+
}
12+
}
13+
14+
macro_rules! outer {
15+
($e: expr) => {
16+
$e
17+
};
18+
}
19+
20+
fn main() {
21+
let _string: Box<String> = Box::new(Default::default());
22+
let _byte = Box::new(u8::default());
23+
let _vec = Box::new(Vec::<u8>::new());
24+
let _impl = Box::new(ImplementsDefault::default());
25+
let _impl2 = Box::new(<ImplementsDefault as Default>::default());
26+
let _impl3: Box<ImplementsDefault> = Box::new(Default::default());
27+
let _own = Box::new(OwnDefault::default()); // should not lint
28+
let _in_macro = outer!(Box::new(String::new()));
29+
// false negative: default is from different expansion
30+
let _vec2: Box<Vec<ImplementsDefault>> = Box::new(vec![]);
31+
}

‎tests/ui/box_default.stderr

+59
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
error: `Box::new(_)` of default value
2+
--> $DIR/box_default.rs:21:32
3+
|
4+
LL | let _string: Box<String> = Box::new(Default::default());
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
6+
|
7+
= note: `-D clippy::box-default` implied by `-D warnings`
8+
= help: use `Box::default()` instead
9+
10+
error: `Box::new(_)` of default value
11+
--> $DIR/box_default.rs:22:17
12+
|
13+
LL | let _byte = Box::new(u8::default());
14+
| ^^^^^^^^^^^^^^^^^^^^^^^
15+
|
16+
= help: use `Box::default()` instead
17+
18+
error: `Box::new(_)` of default value
19+
--> $DIR/box_default.rs:23:16
20+
|
21+
LL | let _vec = Box::new(Vec::<u8>::new());
22+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
23+
|
24+
= help: use `Box::default()` instead
25+
26+
error: `Box::new(_)` of default value
27+
--> $DIR/box_default.rs:24:17
28+
|
29+
LL | let _impl = Box::new(ImplementsDefault::default());
30+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
31+
|
32+
= help: use `Box::default()` instead
33+
34+
error: `Box::new(_)` of default value
35+
--> $DIR/box_default.rs:25:18
36+
|
37+
LL | let _impl2 = Box::new(<ImplementsDefault as Default>::default());
38+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
39+
|
40+
= help: use `Box::default()` instead
41+
42+
error: `Box::new(_)` of default value
43+
--> $DIR/box_default.rs:26:42
44+
|
45+
LL | let _impl3: Box<ImplementsDefault> = Box::new(Default::default());
46+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
47+
|
48+
= help: use `Box::default()` instead
49+
50+
error: `Box::new(_)` of default value
51+
--> $DIR/box_default.rs:28:28
52+
|
53+
LL | let _in_macro = outer!(Box::new(String::new()));
54+
| ^^^^^^^^^^^^^^^^^^^^^^^
55+
|
56+
= help: use `Box::default()` instead
57+
58+
error: aborting due to 7 previous errors
59+

0 commit comments

Comments
 (0)