add closure arg check, also catch non-consts

clippy_lints/src/bytecount.rs

@@ -1,8 +1,8 @@
-use consts::{constant, Constant};
-use rustc_const_math::ConstInt;
 use rustc::hir::*;
 use rustc::lint::*;
-use utils::{match_type, paths, snippet, span_lint_and_sugg, walk_ptrs_ty};
+use rustc::ty;
+use syntax::ast::{Name, UintTy};
+use utils::{contains_name, match_type, paths, single_segment_path, snippet, span_lint_and_sugg, walk_ptrs_ty};
 
 /// **What it does:** Checks for naive byte counts
 ///
@@ -47,14 +47,24 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ByteCount {
         ], {
             let body = cx.tcx.hir.body(body_id);
             if_let_chain!([
+                body.arguments.len() == 1,
+                let Some(argname) = get_pat_name(&body.arguments[0].pat),
                 let ExprBinary(ref op, ref l, ref r) = body.value.node,
                 op.node == BiEq,
                 match_type(cx,
                            walk_ptrs_ty(cx.tables.expr_ty(&filter_args[0])),
                            &paths::SLICE_ITER),
-                let Some((Constant::Int(ConstInt::U8(needle)), _)) =
-                        constant(cx, l).or_else(|| constant(cx, r))
             ], {
+                let needle = match get_path_name(l) {
+                    Some(name) if check_arg(name, argname, r) => r,
+                    _ => match get_path_name(r) {
+                        Some(name) if check_arg(name, argname, l) => l,
+                        _ => { return; }
+                    }
+                };
+                if ty::TyUint(UintTy::U8) != walk_ptrs_ty(cx.tables.expr_ty(needle)).sty {
+                    return;
+                }
                 let haystack = if let ExprMethodCall(ref path, _, ref args) =
                         filter_args[0].node {
                     let p = path.name;
@@ -73,8 +83,33 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ByteCount {
                                    "Consider using the bytecount crate",
                                    format!("bytecount::count({}, {})",
                                             snippet(cx, haystack.span, ".."),
-                                            needle));
+                                            snippet(cx, needle.span, "..")));
             });
         });
     }
 }
+
+fn check_arg(name: Name, arg: Name, needle: &Expr) -> bool {
+    name == arg && !contains_name(name, needle)
+}
+
+fn get_pat_name(pat: &Pat) -> Option<Name> {
+    match pat.node {
+        PatKind::Binding(_, _, ref spname, _) => Some(spname.node),
+        PatKind::Path(ref qpath) => single_segment_path(qpath).map(|ps| ps.name),
+        PatKind::Box(ref p) | PatKind::Ref(ref p, _) => get_pat_name(&*p),
+        _ => None
+    }
+}
+
+fn get_path_name(expr: &Expr) -> Option<Name> {
+    match expr.node {
+        ExprBox(ref e) | ExprAddrOf(_, ref e) | ExprUnary(UnOp::UnDeref, ref e) => get_path_name(e),
+        ExprBlock(ref b) => if b.stmts.is_empty() {
+            b.expr.as_ref().and_then(|p| get_path_name(p))
+        } else { None },
+        ExprPath(ref qpath) => single_segment_path(qpath).map(|ps| ps.name),
+        _ => None
+    }
+}
+

clippy_lints/src/shadow.rs

@@ -1,10 +1,10 @@
 use reexport::*;
 use rustc::lint::*;
 use rustc::hir::*;
-use rustc::hir::intravisit::{Visitor, FnKind, NestedVisitorMap};
+use rustc::hir::intravisit::FnKind;
 use rustc::ty;
 use syntax::codemap::Span;
-use utils::{higher, in_external_macro, snippet, span_lint_and_then, iter_input_pats};
+use utils::{contains_name, higher, in_external_macro, snippet, span_lint_and_then, iter_input_pats};
 
 /// **What it does:** Checks for bindings that shadow other bindings already in
 /// scope, while just changing reference level or mutability.
@@ -261,7 +261,7 @@ fn lint_shadow<'a, 'tcx: 'a>(
                 ),
                 |db| { db.span_note(prev_span, "previous binding is here"); },
             );
-        } else if contains_self(name, expr) {
+        } else if contains_name(name, expr) {
             span_lint_and_then(
                 cx,
                 SHADOW_REUSE,
@@ -391,27 +391,3 @@ fn path_eq_name(name: Name, path: &Path) -> bool {
     !path.is_global() && path.segments.len() == 1 && path.segments[0].name.as_str() == name.as_str()
 }
 
-struct ContainsSelf {
-    name: Name,
-    result: bool,
-}
-
-impl<'tcx> Visitor<'tcx> for ContainsSelf {
-    fn visit_name(&mut self, _: Span, name: Name) {
-        if self.name == name {
-            self.result = true;
-        }
-    }
-    fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> {
-        NestedVisitorMap::None
-    }
-}
-
-fn contains_self(name: Name, expr: &Expr) -> bool {
-    let mut cs = ContainsSelf {
-        name: name,
-        result: false,
-    };
-    cs.visit_expr(expr);
-    cs.result
-}

clippy_lints/src/utils/mod.rs

@@ -3,6 +3,7 @@ use rustc::hir;
 use rustc::hir::*;
 use rustc::hir::def_id::{DefId, CRATE_DEF_INDEX};
 use rustc::hir::def::Def;
+use rustc::hir::intravisit::{NestedVisitorMap, Visitor};
 use rustc::hir::map::Node;
 use rustc::lint::{LintContext, Level, LateContext, Lint};
 use rustc::session::Session;
@@ -393,6 +394,33 @@ pub fn get_item_name(cx: &LateContext, expr: &Expr) -> Option<Name> {
     }
 }
 
+struct ContainsName {
+    name: Name,
+    result: bool,
+}
+
+impl<'tcx> Visitor<'tcx> for ContainsName {
+    fn visit_name(&mut self, _: Span, name: Name) {
+        if self.name == name {
+            self.result = true;
+        }
+    }
+    fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> {
+        NestedVisitorMap::None
+    }
+}
+
+/// check if an `Expr` contains a certain name
+pub fn contains_name(name: Name, expr: &Expr) -> bool {
+    let mut cn = ContainsName {
+        name: name,
+        result: false,
+    };
+    cn.visit_expr(expr);
+    cn.result
+}
+
+
 /// Convert a span to a code snippet if available, otherwise use default.
 ///
 /// # Example

tests/ui/bytecount.rs

@@ -1,6 +1,7 @@
 #![feature(plugin)]
 #![plugin(clippy)]
 
+#[deny(naive_bytecount)]
 fn main() {
     let x = vec![0_u8; 16];
 
@@ -11,4 +12,16 @@ fn main() {
     let _ = x.iter().filter(|a| **a > 0).count(); // not an equality count, OK.
 
     let _ = x.iter().map(|a| a + 1).filter(|&a| a < 15).count(); // not a slice
+
+    let b = 0;
+
+    let _ = x.iter().filter(|_| b > 0).count(); // woah there
+
+    let _ = x.iter().filter(|_a| b == b + 1).count(); // nothing to see here, move along
+
+    let _ = x.iter().filter(|a| b + 1 == **a).count(); // naive byte count
+
+    let y = vec![0_u16; 3];
+
+    let _ = y.iter().filter(|&&a| a == 0).count(); // naive count, but not bytes
 }

tests/ui/bytecount.stderr

@@ -1,16 +1,26 @@
 error: You appear to be counting bytes the naive way
- --> $DIR/bytecount.rs:7:13
+ --> $DIR/bytecount.rs:8:13
   |
-7 |     let _ = x.iter().filter(|&&a| a == 0).count(); // naive byte count
+8 |     let _ = x.iter().filter(|&&a| a == 0).count(); // naive byte count
   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Consider using the bytecount crate: `bytecount::count(x, 0)`
   |
-  = note: `-D naive-bytecount` implied by `-D warnings`
+note: lint level defined here
+ --> $DIR/bytecount.rs:4:8
+  |
+4 | #[deny(naive_bytecount)]
+  |        ^^^^^^^^^^^^^^^
 
 error: You appear to be counting bytes the naive way
- --> $DIR/bytecount.rs:9:13
+  --> $DIR/bytecount.rs:10:13
    |
-9 |     let _ = (&x[..]).iter().filter(|&a| *a == 0).count(); // naive byte count
+10 |     let _ = (&x[..]).iter().filter(|&a| *a == 0).count(); // naive byte count
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Consider using the bytecount crate: `bytecount::count((&x[..]), 0)`
 
-error: aborting due to 2 previous errors
+error: You appear to be counting bytes the naive way
+  --> $DIR/bytecount.rs:22:13
+   |
+22 |     let _ = x.iter().filter(|a| b + 1 == **a).count(); // naive byte count
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Consider using the bytecount crate: `bytecount::count(x, b + 1)`
+
+error: aborting due to 3 previous errors