From 711cad188abfc36e0a3d178ece2af41487ec5a45 Mon Sep 17 00:00:00 2001 From: scott-linder Date: Tue, 31 Jan 2017 23:19:33 -0500 Subject: [PATCH] check for borrowed box types --- CHANGELOG.md | 1 + README.md | 1 + clippy_lints/src/lib.rs | 1 + clippy_lints/src/types.rs | 39 +++++++++++++++++++++++++++++--- tests/compile-fail/borrow_box.rs | 14 ++++++++++++ 5 files changed, 53 insertions(+), 3 deletions(-) create mode 100644 tests/compile-fail/borrow_box.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 837f160f770d..d2b5f6f2fe7c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -364,6 +364,7 @@ All notable changes to this project will be documented in this file. [`block_in_if_condition_expr`]: https://github.com/Manishearth/rust-clippy/wiki#block_in_if_condition_expr [`block_in_if_condition_stmt`]: https://github.com/Manishearth/rust-clippy/wiki#block_in_if_condition_stmt [`bool_comparison`]: https://github.com/Manishearth/rust-clippy/wiki#bool_comparison +[`borrowed_box`]: https://github.com/Manishearth/rust-clippy/wiki#borrowed_box [`box_vec`]: https://github.com/Manishearth/rust-clippy/wiki#box_vec [`boxed_local`]: https://github.com/Manishearth/rust-clippy/wiki#boxed_local [`builtin_type_shadow`]: https://github.com/Manishearth/rust-clippy/wiki#builtin_type_shadow diff --git a/README.md b/README.md index c5b28812e944..1cd64745f123 100644 --- a/README.md +++ b/README.md @@ -194,6 +194,7 @@ name [block_in_if_condition_expr](https://github.com/Manishearth/rust-clippy/wiki#block_in_if_condition_expr) | warn | braces that can be eliminated in conditions, e.g. `if { true } ...` [block_in_if_condition_stmt](https://github.com/Manishearth/rust-clippy/wiki#block_in_if_condition_stmt) | warn | complex blocks in conditions, e.g. `if { let x = true; x } ...` [bool_comparison](https://github.com/Manishearth/rust-clippy/wiki#bool_comparison) | warn | comparing a variable to a boolean, e.g. `if x == true` +[borrowed_box](https://github.com/Manishearth/rust-clippy/wiki#borrowed_box) | warn | a borrow of a boxed type [box_vec](https://github.com/Manishearth/rust-clippy/wiki#box_vec) | warn | usage of `Box>`, vector elements are already on the heap [boxed_local](https://github.com/Manishearth/rust-clippy/wiki#boxed_local) | warn | using `Box` where unnecessary [builtin_type_shadow](https://github.com/Manishearth/rust-clippy/wiki#builtin_type_shadow) | warn | shadowing a builtin type diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index e212aaeaf7b8..70b2e0c43856 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -506,6 +506,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { transmute::USELESS_TRANSMUTE, transmute::WRONG_TRANSMUTE, types::ABSURD_EXTREME_COMPARISONS, + types::BORROWED_BOX, types::BOX_VEC, types::CHAR_LIT_AS_U8, types::LET_UNIT_VALUE, diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index f608c7808a0b..a073e7e98523 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -65,9 +65,25 @@ structure like a VecDeque" } +/// **What it does:** Checks for use of `&Box` anywhere in the code. +/// +/// **Why is this bad?** Any `&Box` can also be a `&T`, which is more general. +/// +/// **Known problems:** None. +/// +/// **Example:** +/// ```rust +/// fn foo(bar: &Box) { ... } +/// ``` +declare_lint! { + pub BORROWED_BOX, + Warn, + "a borrow of a boxed type" +} + impl LintPass for TypePass { fn get_lints(&self) -> LintArray { - lint_array!(BOX_VEC, LINKEDLIST) + lint_array!(BOX_VEC, LINKEDLIST, BORROWED_BOX) } } @@ -161,11 +177,28 @@ fn check_ty(cx: &LateContext, ast_ty: &hir::Ty) { }, } }, + TyRptr(_, MutTy { ref ty, .. }) => { + match ty.node { + TyPath(ref qpath) => { + let def = cx.tables.qpath_def(qpath, ast_ty.id); + if let Some(def_id) = opt_def_id(def) { + if Some(def_id) == cx.tcx.lang_items.owned_box() { + span_help_and_lint(cx, + BOX_VEC, + ast_ty.span, + "you seem to be trying to use `&Box`. Consider using just `&T`", + "replace `&Box` with simply `&T`"); + return; // don't recurse into the type + } + } + }, + _ => check_ty(cx, ty), + } + }, // recurse TySlice(ref ty) | TyArray(ref ty, _) | - TyPtr(MutTy { ref ty, .. }) | - TyRptr(_, MutTy { ref ty, .. }) => check_ty(cx, ty), + TyPtr(MutTy { ref ty, .. }) => check_ty(cx, ty), TyTup(ref tys) => { for ty in tys { check_ty(cx, ty); diff --git a/tests/compile-fail/borrow_box.rs b/tests/compile-fail/borrow_box.rs new file mode 100644 index 000000000000..54dee4f90161 --- /dev/null +++ b/tests/compile-fail/borrow_box.rs @@ -0,0 +1,14 @@ +#![feature(plugin)] +#![plugin(clippy)] + +#![deny(clippy)] +#![allow(boxed_local)] +#![allow(blacklisted_name)] + +pub fn test(foo: &Box) { //~ ERROR you seem to be trying to use `&Box` + println!("{:?}", foo) +} + +fn main(){ + test(&Box::new(false)); +}