check for borrowed box types

Author: scott-linder

CHANGELOG.md

@@ -275,6 +275,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

README.md

@@ -180,7 +180,7 @@ transparently:
 
 ## Lints
 
-There are 184 lints included in this crate:
+There are 185 lints included in this crate:
 
 name                                                                                                                   | default | triggers on
 -----------------------------------------------------------------------------------------------------------------------|---------|----------------------------------------------------------------------------------------------------------------------------------
@@ -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<Vec<T>>`, vector elements are already on the heap
 [boxed_local](https://github.com/Manishearth/rust-clippy/wiki#boxed_local)                                             | warn    | using `Box<T>` where unnecessary
 [builtin_type_shadow](https://github.com/Manishearth/rust-clippy/wiki#builtin_type_shadow)                             | warn    | shadowing a builtin type

clippy_lints/src/lib.rs

@@ -480,6 +480,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,

clippy_lints/src/types.rs

@@ -63,9 +63,25 @@ declare_lint! {
      structure like a VecDeque"
 }
 
+/// **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
+/// fn foo(bar: &Box<T>) { ... }
+/// ```
+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)
     }
 }
 
@@ -159,11 +175,28 @@ fn check_ty(cx: &LateContext, ast_ty: &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<T>`. Consider using just `&T`",
+                                               "replace `&Box<T>` 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);

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<bool>) { //~ ERROR you seem to be trying to use `&Box<T>`
+    println!("{:?}", foo)
+}
+
+fn main(){
+    test(&Box::new(false));
+}