diff options
| author | Andre Bogus <bogusandre@gmail.com> | 2022-09-21 23:43:49 +0200 |
|---|---|---|
| committer | Andre Bogus <bogusandre@gmail.com> | 2022-09-27 13:26:23 +0200 |
| commit | 63f441ec855f0de342ccc2b9ff90776c633ca9e6 (patch) | |
| tree | fbe233938f4ab9c9ae3ea1b19ed78e7ba31a4a8e | |
| parent | 7248d06384c6a90de58c04c1f46be88821278d8b (diff) | |
| download | rust-63f441ec855f0de342ccc2b9ff90776c633ca9e6.tar.gz rust-63f441ec855f0de342ccc2b9ff90776c633ca9e6.zip | |
add `box-default` lint
| -rw-r--r-- | CHANGELOG.md | 1 | ||||
| -rw-r--r-- | clippy_lints/src/box_default.rs | 61 | ||||
| -rw-r--r-- | clippy_lints/src/lib.register_all.rs | 1 | ||||
| -rw-r--r-- | clippy_lints/src/lib.register_lints.rs | 1 | ||||
| -rw-r--r-- | clippy_lints/src/lib.register_perf.rs | 1 | ||||
| -rw-r--r-- | clippy_lints/src/lib.rs | 30 | ||||
| -rw-r--r-- | src/docs.rs | 1 | ||||
| -rw-r--r-- | src/docs/box_default.txt | 23 | ||||
| -rw-r--r-- | tests/ui/box_collection.rs | 4 | ||||
| -rw-r--r-- | tests/ui/box_default.rs | 31 | ||||
| -rw-r--r-- | tests/ui/box_default.stderr | 59 |
11 files changed, 197 insertions, 16 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 044cbff4b78..f1e2e6f462d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3609,6 +3609,7 @@ Released 2018-09-13 [`borrow_interior_mutable_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#borrow_interior_mutable_const [`borrowed_box`]: https://rust-lang.github.io/rust-clippy/master/index.html#borrowed_box [`box_collection`]: https://rust-lang.github.io/rust-clippy/master/index.html#box_collection +[`box_default`]: https://rust-lang.github.io/rust-clippy/master/index.html#box_default [`box_vec`]: https://rust-lang.github.io/rust-clippy/master/index.html#box_vec [`boxed_local`]: https://rust-lang.github.io/rust-clippy/master/index.html#boxed_local [`branches_sharing_code`]: https://rust-lang.github.io/rust-clippy/master/index.html#branches_sharing_code diff --git a/clippy_lints/src/box_default.rs b/clippy_lints/src/box_default.rs new file mode 100644 index 00000000000..792183ac408 --- /dev/null +++ b/clippy_lints/src/box_default.rs @@ -0,0 +1,61 @@ +use clippy_utils::{diagnostics::span_lint_and_help, is_default_equivalent, path_def_id}; +use rustc_hir::{Expr, ExprKind, QPath}; +use rustc_lint::{LateContext, LateLintPass, LintContext}; +use rustc_middle::lint::in_external_macro; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::sym; + +declare_clippy_lint! { + /// ### What it does + /// checks for `Box::new(T::default())`, which is better written as + /// `Box::<T>::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). + /// + /// ### Known problems + /// The lint may miss some cases (e.g. Box::new(String::from(""))). + /// On the other hand, it will trigger on cases where the `default` + /// code comes from a macro that does something different based on + /// e.g. target operating system. + /// + /// ### Example + /// ```rust + /// let x: Box<String> = Box::new(Default::default()); + /// ``` + /// Use instead: + /// ```rust + /// let x: Box<String> = Box::default(); + /// ``` + #[clippy::version = "1.65.0"] + pub BOX_DEFAULT, + perf, + "Using Box::new(T::default()) instead of Box::default()" +} + +declare_lint_pass!(BoxDefault => [BOX_DEFAULT]); + +impl LateLintPass<'_> for BoxDefault { + fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { + if let ExprKind::Call(box_new, [arg]) = expr.kind + && let ExprKind::Path(QPath::TypeRelative(ty, seg)) = box_new.kind + && let ExprKind::Call(..) = arg.kind + && !in_external_macro(cx.sess(), expr.span) + && expr.span.eq_ctxt(arg.span) + && seg.ident.name == sym::new + && path_def_id(cx, ty) == cx.tcx.lang_items().owned_box() + && is_default_equivalent(cx, arg) + { + span_lint_and_help( + cx, + BOX_DEFAULT, + expr.span, + "`Box::new(_)` of default value", + None, + "use `Box::default()` instead", + ); + } + } +} diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs index ae57a9a24a7..33490d1ccc5 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/clippy_lints/src/lib.register_all.rs @@ -21,6 +21,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec. + +### Known problems +The lint may miss some cases (e.g. Box::new(String::from(""))). +On the other hand, it will trigger on cases where the `default` +code comes from a macro that does something different based on +e.g. target operating system. + +### Example +``` +let x: Box<String> = Box::new(Default::default()); +``` +Use instead: +``` +let x: Box<String> = Box::default(); +``` \ No newline at end of file diff --git a/tests/ui/box_collection.rs b/tests/ui/box_collection.rs index 0780c8f0586..4c9947b9ae7 100644 --- a/tests/ui/box_collection.rs +++ b/tests/ui/box_collection.rs @@ -15,7 +15,7 @@ macro_rules! boxit { } fn test_macro() { - boxit!(Vec::new(), Vec<u8>); + boxit!(vec![1], Vec<u8>); } fn test1(foo: Box<Vec<bool>>) {} @@ -50,7 +50,7 @@ fn test_local_not_linted() { pub fn pub_test(foo: Box<Vec<bool>>) {} pub fn pub_test_ret() -> Box<Vec<bool>> { - Box::new(Vec::new()) + Box::default() } fn main() {} diff --git a/tests/ui/box_default.rs b/tests/ui/box_default.rs new file mode 100644 index 00000000000..dc522705bc6 --- /dev/null +++ b/tests/ui/box_default.rs @@ -0,0 +1,31 @@ +#![warn(clippy::box_default)] + +#[derive(Default)] +struct ImplementsDefault; + +struct OwnDefault; + +impl OwnDefault { + fn default() -> Self { + Self + } +} + +macro_rules! outer { + ($e: expr) => { + $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())); + // false negative: default is from different expansion + let _vec2: Box<Vec<ImplementsDefault>> = Box::new(vec![]); +} diff --git a/tests/ui/box_default.stderr b/tests/ui/box_default.stderr new file mode 100644 index 00000000000..341766a502b --- /dev/null +++ b/tests/ui/box_default.stderr @@ -0,0 +1,59 @@ +error: `Box::new(_)` of default value + --> $DIR/box_default.rs:21:32 + | +LL | let _string: Box<String> = Box::new(Default::default()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::box-default` implied by `-D warnings` + = help: use `Box::default()` instead + +error: `Box::new(_)` of default value + --> $DIR/box_default.rs:22:17 + | +LL | let _byte = Box::new(u8::default()); + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: use `Box::default()` instead + +error: `Box::new(_)` of default value + --> $DIR/box_default.rs:23:16 + | +LL | let _vec = Box::new(Vec::<u8>::new()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: use `Box::default()` instead + +error: `Box::new(_)` of default value + --> $DIR/box_default.rs:24:17 + | +LL | let _impl = Box::new(ImplementsDefault::default()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: use `Box::default()` instead + +error: `Box::new(_)` of default value + --> $DIR/box_default.rs:25:18 + | +LL | let _impl2 = Box::new(<ImplementsDefault as Default>::default()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: use `Box::default()` instead + +error: `Box::new(_)` of default value + --> $DIR/box_default.rs:26:42 + | +LL | let _impl3: Box<ImplementsDefault> = Box::new(Default::default()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: use `Box::default()` instead + +error: `Box::new(_)` of default value + --> $DIR/box_default.rs:28:28 + | +LL | let _in_macro = outer!(Box::new(String::new())); + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: use `Box::default()` instead + +error: aborting due to 7 previous errors + |