diff --git a/CHANGELOG.md b/CHANGELOG.md index e691ec9412f0..1713fda031f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -899,6 +899,7 @@ All notable changes to this project will be documented in this file. [`useless_let_if_seq`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_let_if_seq [`useless_transmute`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_transmute [`useless_vec`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_vec +[`vec_box`]: https://rust-lang.github.io/rust-clippy/master/index.html#vec_box [`verbose_bit_mask`]: https://rust-lang.github.io/rust-clippy/master/index.html#verbose_bit_mask [`while_immutable_condition`]: https://rust-lang.github.io/rust-clippy/master/index.html#while_immutable_condition [`while_let_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#while_let_loop diff --git a/README.md b/README.md index f42771709fb7..5c5d32e4a89d 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code. -[There are 290 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) +[There are 291 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you: diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index a862d774174b..f4b5edee84c5 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -766,6 +766,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { types::UNIT_ARG, types::UNIT_CMP, types::UNNECESSARY_CAST, + types::VEC_BOX, unicode::ZERO_WIDTH_SPACE, unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME, unused_io_amount::UNUSED_IO_AMOUNT, @@ -931,6 +932,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { types::TYPE_COMPLEXITY, types::UNIT_ARG, types::UNNECESSARY_CAST, + types::VEC_BOX, unused_label::UNUSED_LABEL, zero_div_zero::ZERO_DIVIDED_BY_ZERO, ]); diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index 820f2fdf32db..dfa4cfdcf94f 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -68,6 +68,34 @@ declare_clippy_lint! { "usage of `Box>`, vector elements are already on the heap" } +/// **What it does:** Checks for use of `Vec>` where T: Sized anywhere in the code. +/// +/// **Why is this bad?** `Vec` already keeps its contents in a separate area on +/// the heap. So if you `Box` its contents, you just add another level of indirection. +/// +/// **Known problems:** Vec> makes sense if T is a large type (see #3530, +/// 1st comment). +/// +/// **Example:** +/// ```rust +/// struct X { +/// values: Vec>, +/// } +/// ``` +/// +/// Better: +/// +/// ```rust +/// struct X { +/// values: Vec, +/// } +/// ``` +declare_clippy_lint! { + pub VEC_BOX, + complexity, + "usage of `Vec>` where T: Sized, vector elements are already on the heap" +} + /// **What it does:** Checks for use of `Option>` in function signatures and type /// definitions /// @@ -148,7 +176,7 @@ declare_clippy_lint! { impl LintPass for TypePass { fn get_lints(&self) -> LintArray { - lint_array!(BOX_VEC, OPTION_OPTION, LINKEDLIST, BORROWED_BOX) + lint_array!(BOX_VEC, VEC_BOX, OPTION_OPTION, LINKEDLIST, BORROWED_BOX) } } @@ -238,6 +266,43 @@ fn check_ty(cx: &LateContext<'_, '_>, ast_ty: &hir::Ty, is_local: bool) { ); return; // don't recurse into the type } + } else if match_def_path(cx.tcx, def_id, &paths::VEC) { + if_chain! { + // Get the _ part of Vec<_> + if let Some(ref last) = last_path_segment(qpath).args; + if let Some(ty) = last.args.iter().find_map(|arg| match arg { + GenericArg::Type(ty) => Some(ty), + GenericArg::Lifetime(_) => None, + }); + // ty is now _ at this point + if let TyKind::Path(ref ty_qpath) = ty.node; + let def = cx.tables.qpath_def(ty_qpath, ty.hir_id); + if let Some(def_id) = opt_def_id(def); + if Some(def_id) == cx.tcx.lang_items().owned_box(); + // At this point, we know ty is Box, now get T + if let Some(ref last) = last_path_segment(ty_qpath).args; + if let Some(ty) = last.args.iter().find_map(|arg| match arg { + GenericArg::Type(ty) => Some(ty), + GenericArg::Lifetime(_) => None, + }); + if let TyKind::Path(ref ty_qpath) = ty.node; + let def = cx.tables.qpath_def(ty_qpath, ty.hir_id); + if let Some(def_id) = opt_def_id(def); + let boxed_type = cx.tcx.type_of(def_id); + if boxed_type.is_sized(cx.tcx.at(ty.span), cx.param_env); + then { + span_lint_and_sugg( + cx, + VEC_BOX, + ast_ty.span, + "`Vec` is already on the heap, the boxing is unnecessary.", + "try", + format!("Vec<{}>", boxed_type), + Applicability::MaybeIncorrect, + ); + return; // don't recurse into the type + } + } } else if match_def_path(cx.tcx, def_id, &paths::OPTION) { if match_type_parameter(cx, qpath, &paths::OPTION) { span_lint( diff --git a/tests/ui/vec_box_sized.rs b/tests/ui/vec_box_sized.rs new file mode 100644 index 000000000000..d740f95edfe2 --- /dev/null +++ b/tests/ui/vec_box_sized.rs @@ -0,0 +1,17 @@ +struct SizedStruct { + _a: i32, +} + +struct UnsizedStruct { + _a: [i32], +} + +struct StructWithVecBox { + sized_type: Vec>, +} + +struct StructWithVecBoxButItsUnsized { + unsized_type: Vec>, +} + +fn main() {} diff --git a/tests/ui/vec_box_sized.stderr b/tests/ui/vec_box_sized.stderr new file mode 100644 index 000000000000..7f4bdfb5aed9 --- /dev/null +++ b/tests/ui/vec_box_sized.stderr @@ -0,0 +1,10 @@ +error: `Vec` is already on the heap, the boxing is unnecessary. + --> $DIR/vec_box_sized.rs:10:14 + | +10 | sized_type: Vec>, + | ^^^^^^^^^^^^^^^^^^^^^ help: try: `Vec` + | + = note: `-D clippy::vec-box` implied by `-D warnings` + +error: aborting due to previous error +