Skip to content

Commit

Permalink
box_vec: dont use Box<&T>
Browse files Browse the repository at this point in the history
  • Loading branch information
jpospychala committed Mar 19, 2020
1 parent 23549a8 commit a35a8b2
Show file tree
Hide file tree
Showing 7 changed files with 144 additions and 3 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -1166,6 +1166,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_borrows`]: https://rust-lang.github.io/rust-clippy/master/index.html#box_borrows
[`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
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down
3 changes: 3 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -788,6 +788,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_BORROWS,
&types::BOX_VEC,
&types::CAST_LOSSLESS,
&types::CAST_POSSIBLE_TRUNCATION,
Expand Down Expand Up @@ -1358,6 +1359,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_BORROWS),
LintId::of(&types::BOX_VEC),
LintId::of(&types::CAST_PTR_ALIGNMENT),
LintId::of(&types::CAST_REF_TO_MUT),
Expand Down Expand Up @@ -1650,6 +1652,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_BORROWS),
LintId::of(&types::BOX_VEC),
LintId::of(&vec::USELESS_VEC),
]);
Expand Down
51 changes: 50 additions & 1 deletion clippy_lints/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
/// struct X {
/// values: Box<&T>,
/// }
/// ```
///
/// Better:
///
/// ```rust
/// struct X {
/// values: &T,
/// }
/// ```
pub BOX_BORROWS,
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_BORROWS]);

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Types {
fn check_fn(
Expand Down Expand Up @@ -257,6 +284,7 @@ impl Types {
/// The parameter `is_local` distinguishes the context of the type; types from
/// local bindings should only be checked for the `BORROWED_BOX` lint.
#[allow(clippy::too_many_lines)]
#[allow(clippy::cognitive_complexity)]
fn check_ty(&mut self, cx: &LateContext<'_, '_>, hir_ty: &hir::Ty<'_>, is_local: bool) {
if hir_ty.span.from_expansion() {
return;
Expand All @@ -267,6 +295,27 @@ 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_chain! {
let last = last_path_segment(qpath);
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(..) = ty.kind;
let ty_ty = hir_ty_to_ty(cx.tcx, ty);
then {
span_lint_and_help(
cx,
BOX_BORROWS,
hir_ty.span,
"usage of `Box<&T>`",
format!("try `{}`", ty_ty).as_str(),
);
return; // don't recurse into the type
}
}
if match_type_parameter(cx, qpath, &paths::VEC) {
span_lint_and_help(
cx,
Expand Down
9 changes: 8 additions & 1 deletion src/lintlist/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -98,6 +98,13 @@ pub const ALL_LINTS: [Lint; 361] = [
deprecation: None,
module: "types",
},
Lint {
name: "box_borrows",
group: "perf",
desc: "a box of borrowed type",
deprecation: None,
module: "types",
},
Lint {
name: "box_vec",
group: "perf",
Expand Down
30 changes: 30 additions & 0 deletions tests/ui/box_borrows.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
#![warn(clippy::all)]
#![allow(clippy::boxed_local, clippy::needless_pass_by_value)]
#![allow(clippy::blacklisted_name)]

pub struct MyStruct {}

pub struct SubT<T> {
foo: T,
}

pub enum MyEnum {
One,
Two,
}

pub fn test<T>(foo: Box<&T>) {}

pub fn test1(foo: Box<&usize>) {}

pub fn test2(foo: Box<&MyStruct>) {}

pub fn test3(foo: Box<&MyEnum>) {}

pub fn test4(foo: Box<&&MyEnum>) {}

pub fn test5(foo: Box<Box<&usize>>) {}

pub fn test6(foo: Box<SubT<&usize>>) {}

fn main() {}
51 changes: 51 additions & 0 deletions tests/ui/box_borrows.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
error: usage of `Box<&T>`
--> $DIR/box_borrows.rs:16:21
|
LL | pub fn test<T>(foo: Box<&T>) {}
| ^^^^^^^
|
= note: `-D clippy::box-borrows` implied by `-D warnings`
= help: try `&T`

error: usage of `Box<&T>`
--> $DIR/box_borrows.rs:18:19
|
LL | pub fn test1(foo: Box<&usize>) {}
| ^^^^^^^^^^^
|
= help: try `&usize`

error: usage of `Box<&T>`
--> $DIR/box_borrows.rs:20:19
|
LL | pub fn test2(foo: Box<&MyStruct>) {}
| ^^^^^^^^^^^^^^
|
= help: try `&MyStruct`

error: usage of `Box<&T>`
--> $DIR/box_borrows.rs:22:19
|
LL | pub fn test3(foo: Box<&MyEnum>) {}
| ^^^^^^^^^^^^
|
= help: try `&MyEnum`

error: usage of `Box<&T>`
--> $DIR/box_borrows.rs:24:19
|
LL | pub fn test4(foo: Box<&&MyEnum>) {}
| ^^^^^^^^^^^^^
|
= help: try `&&MyEnum`

error: usage of `Box<&T>`
--> $DIR/box_borrows.rs:26:23
|
LL | pub fn test5(foo: Box<Box<&usize>>) {}
| ^^^^^^^^^^^
|
= help: try `&usize`

error: aborting due to 6 previous errors

0 comments on commit a35a8b2

Please sign in to comment.