diff --git a/CHANGELOG.md b/CHANGELOG.md index 32cbbe801017..78c400ca5d3a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1074,6 +1074,7 @@ Released 2018-09-13 [`bool_comparison`]: https://rust-lang.github.io/rust-clippy/master/index.html#bool_comparison [`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_borrow`]: https://rust-lang.github.io/rust-clippy/master/index.html#box_borrow [`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 [`builtin_type_shadow`]: https://rust-lang.github.io/rust-clippy/master/index.html#builtin_type_shadow diff --git a/README.md b/README.md index 2181c296e9b2..7d6fcbc90986 100644 --- a/README.md +++ b/README.md @@ -5,7 +5,7 @@ A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code. -[There are 361 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) +[There are 362 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 1eac69678a64..1d14b4d59b94 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -785,6 +785,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &try_err::TRY_ERR, &types::ABSURD_EXTREME_COMPARISONS, &types::BORROWED_BOX, + &types::BOX_BORROW, &types::BOX_VEC, &types::CAST_LOSSLESS, &types::CAST_POSSIBLE_TRUNCATION, @@ -1355,6 +1356,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&try_err::TRY_ERR), LintId::of(&types::ABSURD_EXTREME_COMPARISONS), LintId::of(&types::BORROWED_BOX), + LintId::of(&types::BOX_BORROW), LintId::of(&types::BOX_VEC), LintId::of(&types::CAST_PTR_ALIGNMENT), LintId::of(&types::CAST_REF_TO_MUT), @@ -1647,6 +1649,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&redundant_clone::REDUNDANT_CLONE), LintId::of(&slow_vector_initialization::SLOW_VECTOR_INITIALIZATION), LintId::of(&trivially_copy_pass_by_ref::TRIVIALLY_COPY_PASS_BY_REF), + LintId::of(&types::BOX_BORROW), LintId::of(&types::BOX_VEC), LintId::of(&vec::USELESS_VEC), ]); diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index fbd3f37c5643..31fd335dc0ac 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -171,11 +171,38 @@ declare_clippy_lint! { "a borrow of a boxed type" } +declare_clippy_lint! { + /// **What it does:** Checks for use of `Box<&T>` anywhere in the code. + /// + /// **Why is this bad?** Any `Box<&T>` can also be a `&T`, which is more + /// general. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// ```rust,ignore + /// struct X { + /// values: Box<&T>, + /// } + /// ``` + /// + /// Better: + /// + /// ```rust,ignore + /// struct X { + /// values: &T, + /// } + /// ``` + pub BOX_BORROW, + perf, + "a box of borrowed type" +} + pub struct Types { vec_box_size_threshold: u64, } -impl_lint_pass!(Types => [BOX_VEC, VEC_BOX, OPTION_OPTION, LINKEDLIST, BORROWED_BOX]); +impl_lint_pass!(Types => [BOX_VEC, VEC_BOX, OPTION_OPTION, LINKEDLIST, BORROWED_BOX, BOX_BORROW]); impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Types { fn check_fn( @@ -236,6 +263,24 @@ fn match_type_parameter(cx: &LateContext<'_, '_>, qpath: &QPath<'_>, path: &[&st false } +/// Checks if `qpath` last segment type parameter is a reference +fn match_type_parameter_rptr(qpath: &QPath<'_>) -> bool { + let last = last_path_segment(qpath); + if_chain! { + if let Some(ref params) = last.args; + if !params.parenthesized; + if let Some(ty) = params.args.iter().find_map(|arg| match arg { + GenericArg::Type(ty) => Some(ty), + _ => None, + }); + if let TyKind::Rptr(ref _lifetime_opt, ref _mut) = ty.kind; + then { + return true; + } + } + false +} + impl Types { pub fn new(vec_box_size_threshold: u64) -> Self { Self { vec_box_size_threshold } @@ -267,6 +312,16 @@ impl Types { let res = qpath_res(cx, qpath, hir_id); if let Some(def_id) = res.opt_def_id() { if Some(def_id) == cx.tcx.lang_items().owned_box() { + if match_type_parameter_rptr(qpath) { + span_lint_and_help( + cx, + BOX_BORROW, + hir_ty.span, + "you seem to be trying to use `Box<&T>`. Consider using just `&T`", + "try `&T`", + ); + return; // don't recurse into the type + } if match_type_parameter(cx, qpath, &paths::VEC) { span_lint_and_help( cx, diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index fd948953ea23..ca202eb1158b 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -6,7 +6,7 @@ pub use lint::Lint; pub use lint::LINT_LEVELS; // begin lint list, do not remove this comment, it’s used in `update_lints` -pub const ALL_LINTS: [Lint; 361] = [ +pub const ALL_LINTS: [Lint; 362] = [ Lint { name: "absurd_extreme_comparisons", group: "correctness", @@ -98,6 +98,13 @@ pub const ALL_LINTS: [Lint; 361] = [ deprecation: None, module: "types", }, + Lint { + name: "box_borrow", + group: "perf", + desc: "a box of borrowed type", + deprecation: None, + module: "types", + }, Lint { name: "box_vec", group: "perf", diff --git a/tests/ui/box_vec.rs b/tests/ui/box_vec.rs index 87b67c23704c..bc5898c8ab0b 100644 --- a/tests/ui/box_vec.rs +++ b/tests/ui/box_vec.rs @@ -24,6 +24,8 @@ pub fn test_local_not_linted() { let _: Box>; } +pub fn test3(foo: Box<&T>) {} + fn main() { test(Box::new(Vec::new())); test2(Box::new(|v| println!("{:?}", v))); diff --git a/tests/ui/box_vec.stderr b/tests/ui/box_vec.stderr index fca12eddd573..0406bd2b1646 100644 --- a/tests/ui/box_vec.stderr +++ b/tests/ui/box_vec.stderr @@ -7,5 +7,14 @@ LL | pub fn test(foo: Box>) { = note: `-D clippy::box-vec` implied by `-D warnings` = help: `Vec` is already on the heap, `Box>` makes an extra allocation. -error: aborting due to previous error +error: you seem to be trying to use `Box<&T>`. Consider using just `&T` + --> $DIR/box_vec.rs:27:22 + | +LL | pub fn test3(foo: Box<&T>) {} + | ^^^^^^^ + | + = note: `-D clippy::box-borrow` implied by `-D warnings` + = help: try `&T` + +error: aborting due to 2 previous errors