Merge pull request #1255 from Manishearth/cov

Improve test coverage

clippy_lints/src/consts.rs

@@ -54,27 +54,16 @@ pub enum Constant {
 }
 
 impl Constant {
-    /// convert to u64 if possible
+    /// Convert to `u64` if possible.
     ///
     /// # panics
     ///
-    /// if the constant could not be converted to u64 losslessly
+    /// If the constant could not be converted to `u64` losslessly.
     fn as_u64(&self) -> u64 {
         if let Constant::Int(val) = *self {
-            val.to_u64().expect("negative constant can't be casted to u64")
+            val.to_u64().expect("negative constant can't be casted to `u64`")
         } else {
-            panic!("Could not convert a {:?} to u64", self);
-        }
-    }
-
-    /// convert this constant to a f64, if possible
-    #[allow(cast_precision_loss, cast_possible_wrap)]
-    pub fn as_float(&self) -> Option<f64> {
-        match *self {
-            Constant::Float(ref s, _) => s.parse().ok(),
-            Constant::Int(i) if i.is_negative() => Some(i.to_u64_unchecked() as i64 as f64),
-            Constant::Int(i) => Some(i.to_u64_unchecked() as f64),
-            _ => None,
+            panic!("Could not convert a `{:?}` to `u64`", self);
         }
     }
 }

clippy_lints/src/format.rs

@@ -81,7 +81,7 @@ pub fn get_argument_fmtstr_parts<'a, 'b>(cx: &LateContext<'a, 'b>, expr: &'a Exp
         let Some(NodeItem(decl)) = cx.tcx.map.find(decl.id),
         decl.name.as_str() == "__STATIC_FMTSTR",
         let ItemStatic(_, _, ref expr) = decl.node,
-        let ExprAddrOf(_, ref expr) = expr.node, // &[""]
+        let ExprAddrOf(_, ref expr) = expr.node, // &["…", "…", …]
         let ExprVec(ref exprs) = expr.node,
     ], {
         let mut result = Vec::new();
@@ -99,7 +99,7 @@ pub fn get_argument_fmtstr_parts<'a, 'b>(cx: &LateContext<'a, 'b>, expr: &'a Exp
 
 /// Checks if the expressions matches
 /// ```rust
-/// { static __STATIC_FMTSTR: &[""] = _; __STATIC_FMTSTR }
+/// { static __STATIC_FMTSTR: … = &["…", "…", …]; __STATIC_FMTSTR }
 /// ```
 fn check_static_str(cx: &LateContext, expr: &Expr) -> bool {
     if let Some(expr) = get_argument_fmtstr_parts(cx, expr) {

clippy_lints/src/shadow.rs

@@ -262,7 +262,7 @@ fn lint_shadow<T>(cx: &LateContext, name: Name, span: Span, pattern_span: Span,
         span_lint_and_then(cx,
                            SHADOW_UNRELATED,
                            span,
-                           &format!("{} shadows a previous declaration", snippet(cx, pattern_span, "_")),
+                           &format!("`{}` shadows a previous declaration", snippet(cx, pattern_span, "_")),
                            |db| { db.span_note(prev_span, "previous binding is here"); });
     }
 }

clippy_lints/src/utils/hir.rs

@@ -37,8 +37,8 @@ pub fn eq_stmt(&self, left: &Stmt, right: &Stmt) -> bool {
         match (&left.node, &right.node) {
             (&StmtDecl(ref l, _), &StmtDecl(ref r, _)) => {
                 if let (&DeclLocal(ref l), &DeclLocal(ref r)) = (&l.node, &r.node) {
-                    // TODO: tys
-                    l.ty.is_none() && r.ty.is_none() && both(&l.init, &r.init, |l, r| self.eq_expr(l, r))
+                    both(&l.ty, &r.ty, |l, r| self.eq_ty(l, r)) &&
+                    both(&l.init, &r.init, |l, r| self.eq_expr(l, r))
                 } else {
                     false
                 }
@@ -85,7 +85,10 @@ pub fn eq_expr(&self, left: &Expr, right: &Expr) -> bool {
             (&ExprCall(ref l_fun, ref l_args), &ExprCall(ref r_fun, ref r_args)) => {
                 !self.ignore_fn && self.eq_expr(l_fun, r_fun) && self.eq_exprs(l_args, r_args)
             }
-            (&ExprCast(ref lx, ref lt), &ExprCast(ref rx, ref rt)) => self.eq_expr(lx, rx) && self.eq_ty(lt, rt),
+            (&ExprCast(ref lx, ref lt), &ExprCast(ref rx, ref rt)) |
+            (&ExprType(ref lx, ref lt), &ExprType(ref rx, ref rt)) => {
+                self.eq_expr(lx, rx) && self.eq_ty(lt, rt)
+            }
             (&ExprField(ref l_f_exp, ref l_f_ident), &ExprField(ref r_f_exp, ref r_f_ident)) => {
                 l_f_ident.node == r_f_ident.node && self.eq_expr(l_f_exp, r_f_exp)
             }
@@ -106,8 +109,8 @@ pub fn eq_expr(&self, left: &Expr, right: &Expr) -> bool {
             }
             (&ExprMethodCall(ref l_name, ref l_tys, ref l_args),
              &ExprMethodCall(ref r_name, ref r_tys, ref r_args)) => {
-                // TODO: tys
-                !self.ignore_fn && l_name.node == r_name.node && l_tys.is_empty() && r_tys.is_empty() &&
+                !self.ignore_fn && l_name.node == r_name.node &&
+                over(l_tys, r_tys, |l, r| self.eq_ty(l, r)) &&
                 self.eq_exprs(l_args, r_args)
             }
             (&ExprRepeat(ref le, ref ll), &ExprRepeat(ref re, ref rl)) => self.eq_expr(le, re) && self.eq_expr(ll, rl),
@@ -138,6 +141,10 @@ fn eq_field(&self, left: &Field, right: &Field) -> bool {
         left.name.node == right.name.node && self.eq_expr(&left.expr, &right.expr)
     }
 
+    fn eq_lifetime(&self, left: &Lifetime, right: &Lifetime) -> bool {
+        left.name == right.name
+    }
+
     /// Check whether two patterns are the same.
     pub fn eq_pat(&self, left: &Pat, right: &Pat) -> bool {
         match (&left.node, &right.node) {
@@ -169,12 +176,33 @@ pub fn eq_pat(&self, left: &Pat, right: &Pat) -> bool {
     }
 
     fn eq_path(&self, left: &Path, right: &Path) -> bool {
+        left.global == right.global &&
+        over(&left.segments, &right.segments, |l, r| self.eq_path_segment(l, r))
+    }
+
+    fn eq_path_parameters(&self, left: &PathParameters, right: &PathParameters) -> bool {
+        match (left, right) {
+            (&AngleBracketedParameters(ref left), &AngleBracketedParameters(ref right)) => {
+                over(&left.lifetimes, &right.lifetimes, |l, r| self.eq_lifetime(l, r)) &&
+                over(&left.types, &right.types, |l, r| self.eq_ty(l, r)) &&
+                over(&left.bindings, &right.bindings, |l, r| self.eq_type_binding(l, r))
+            }
+            (&ParenthesizedParameters(ref left), &ParenthesizedParameters(ref right)) => {
+                over(&left.inputs, &right.inputs, |l, r| self.eq_ty(l, r)) &&
+                both(&left.output, &right.output, |l, r| self.eq_ty(l, r))
+            }
+            (&AngleBracketedParameters(_), &ParenthesizedParameters(_)) |
+            (&ParenthesizedParameters(_), &AngleBracketedParameters(_)) => {
+                false
+            }
+        }
+    }
+
+    fn eq_path_segment(&self, left: &PathSegment, right: &PathSegment) -> bool {
         // The == of idents doesn't work with different contexts,
         // we have to be explicit about hygiene
-        left.global == right.global &&
-        over(&left.segments,
-             &right.segments,
-             |l, r| l.name.as_str() == r.name.as_str() && l.parameters == r.parameters)
+        left.name.as_str() == right.name.as_str() &&
+        self.eq_path_parameters(&left.parameters, &right.parameters)
     }
 
     fn eq_qself(&self, left: &QSelf, right: &QSelf) -> bool {
@@ -199,6 +227,10 @@ fn eq_ty(&self, left: &Ty, right: &Ty) -> bool {
             _ => false,
         }
     }
+
+    fn eq_type_binding(&self, left: &TypeBinding, right: &TypeBinding) -> bool {
+        left.name == right.name && self.eq_ty(&left.ty, &right.ty)
+    }
 }
 
 fn swap_binop<'a>(binop: BinOp_, lhs: &'a Expr, rhs: &'a Expr) -> Option<(BinOp_, &'a Expr, &'a Expr)> {
@@ -445,10 +477,11 @@ pub fn hash_expr(&mut self, e: &Expr) {
                 self.hash_expr(le);
                 li.node.hash(&mut self.s);
             }
-            ExprType(_, _) => {
+            ExprType(ref e, ref _ty) => {
                 let c: fn(_, _) -> _ = ExprType;
                 c.hash(&mut self.s);
-                // what’s an ExprType anyway?
+                self.hash_expr(e);
+                // TODO: _ty
             }
             ExprUnary(lop, ref le) => {
                 let c: fn(_, _) -> _ = ExprUnary;
@@ -495,10 +528,15 @@ pub fn hash_path(&mut self, p: &Path) {
 
     pub fn hash_stmt(&mut self, b: &Stmt) {
         match b.node {
-            StmtDecl(ref _decl, _) => {
+            StmtDecl(ref decl, _) => {
                 let c: fn(_, _) -> _ = StmtDecl;
                 c.hash(&mut self.s);
-                // TODO: decl
+
+                if let DeclLocal(ref local) = decl.node {
+                    if let Some(ref init) = local.init {
+                        self.hash_expr(init);
+                    }
+                }
             }
             StmtExpr(ref expr, _) => {
                 let c: fn(_, _) -> _ = StmtExpr;

tests/compile-fail/array_indexing.rs

@@ -14,6 +14,7 @@ fn main() {
     &x[1..5]; //~ERROR: range is out of bounds
     &x[0..3];
     &x[0...4]; //~ERROR: range is out of bounds
+    &x[...4]; //~ERROR: range is out of bounds
     &x[..];
     &x[1..];
     &x[4..];
@@ -26,15 +27,18 @@ fn main() {
     &y[1..2]; //~ERROR: slicing may panic
     &y[..];
     &y[0...4]; //~ERROR: slicing may panic
+    &y[...4]; //~ERROR: slicing may panic
 
     let empty: [i8; 0] = [];
     empty[0]; //~ERROR: const index is out of bounds
     &empty[1..5]; //~ERROR: range is out of bounds
     &empty[0...4]; //~ERROR: range is out of bounds
+    &empty[...4]; //~ERROR: range is out of bounds
     &empty[..];
     &empty[0..];
     &empty[0..0];
     &empty[0...0]; //~ERROR: range is out of bounds
+    &empty[...0]; //~ERROR: range is out of bounds
     &empty[..0];
     &empty[1..]; //~ERROR: range is out of bounds
     &empty[..4]; //~ERROR: range is out of bounds

tests/compile-fail/conf_bad_toml.rs

@@ -0,0 +1,6 @@
+// error-pattern: error reading Clippy's configuration file
+
+#![feature(plugin)]
+#![plugin(clippy(conf_file="./tests/compile-fail/conf_bad_toml.toml"))]
+
+fn main() {}

tests/compile-fail/conf_bad_toml.toml

@@ -0,0 +1,2 @@
+fn this_is_obviously(not: a, toml: file) {
+}

tests/compile-fail/conf_bad_type.rs

@@ -0,0 +1,6 @@
+// error-pattern: error reading Clippy's configuration file: `blacklisted-names` is expected to be a `Vec < String >` but is a `integer`
+
+#![feature(plugin)]
+#![plugin(clippy(conf_file="./tests/compile-fail/conf_bad_type.toml"))]
+
+fn main() {}

tests/compile-fail/conf_bad_type.toml

@@ -0,0 +1 @@
+blacklisted-names = 42

tests/compile-fail/conf_french_blacklisted_name.rs

@@ -1,5 +1,5 @@
 #![feature(plugin)]
-#![plugin(clippy(conf_file="./tests/compile-fail/conf_french_blacklisted_name.toml"))]
+#![plugin(clippy(conf_file="./tests/aux/conf_french_blacklisted_name.toml"))]
 
 #![allow(dead_code)]
 #![allow(single_match)]

tests/compile-fail/conf_non_existant.rs

@@ -1,6 +1,6 @@
 // error-pattern: error reading Clippy's configuration file
 
 #![feature(plugin)]
-#![plugin(clippy(conf_file="./tests/compile-fail/non_existant_conf.toml"))]
+#![plugin(clippy(conf_file="./tests/aux/non_existant_conf.toml"))]
 
 fn main() {}

tests/compile-fail/conf_path_non_string.rs

@@ -0,0 +1,6 @@
+#![feature(attr_literals)]
+#![feature(plugin)]
+#![plugin(clippy(conf_file=42))]
+//~^ ERROR `conf_file` value must be a string
+
+fn main() {}

tests/compile-fail/conf_unknown_key.rs

@@ -1,6 +1,6 @@
 // error-pattern: error reading Clippy's configuration file: unknown key `foobar`
 
 #![feature(plugin)]
-#![plugin(clippy(conf_file="./tests/compile-fail/conf_unknown_key.toml"))]
+#![plugin(clippy(conf_file="./tests/aux/conf_unknown_key.toml"))]
 
 fn main() {}

tests/compile-fail/copies.rs

@@ -48,6 +48,13 @@ fn if_same_then_else() -> Result<&'static str, ()> {
         Foo { bar: 43 };
     }
 
+    if true {
+        ();
+    }
+    else {
+        ()
+    }
+
     if true {
         0..10;
     }
@@ -63,14 +70,27 @@ fn if_same_then_else() -> Result<&'static str, ()> {
         foo();
     }
 
-    let _ = if true {
+    let _ = match 42 {
+        42 => {
             //~^ NOTE same as this
+            //~| NOTE refactoring
             foo();
-        42
+            let mut a = 42 + [23].len() as i32;
+            if true {
+                a += 7;
             }
-    else { //~ERROR this `if` has identical blocks
+            a = -31-a;
+            a
+        }
+        _ => { //~ERROR this `match` has identical arm bodies
             foo();
-        42
+            let mut a = 42 + [23].len() as i32;
+            if true {
+                a += 7;
+            }
+            a = -31-a;
+            a
+        }
     };
 
     if true {
@@ -85,6 +105,28 @@ fn if_same_then_else() -> Result<&'static str, ()> {
         42
     };
 
+    if true {
+        //~^NOTE same as this
+        for _ in &[42] {
+            let foo: &Option<_> = &Some::<u8>(42);
+            if true {
+                break;
+            } else {
+                continue;
+            }
+        }
+    }
+    else { //~ERROR this `if` has identical blocks
+        for _ in &[42] {
+            let foo: &Option<_> = &Some::<u8>(42);
+            if true {
+                break;
+            } else {
+                continue;
+            }
+        }
+    }
+
     if true {
         //~^NOTE same as this
         let bar = if true {
@@ -167,6 +209,20 @@ fn if_same_then_else() -> Result<&'static str, ()> {
         if let (.., 1, 3) = (1, 2, 3) {}
     }
 
+    if true {
+        if let Some(42) = None {}
+    }
+    else {
+        if let Option::Some(42) = None {}
+    }
+
+    if true {
+        if let Some(42) = None::<u8> {}
+    }
+    else {
+        if let Some(42) = None {}
+    }
+
     if true {
         if let Some(a) = Some(42) {}
     }

tests/compile-fail/shadow.rs

@@ -20,6 +20,9 @@ fn main() {
     let y = 1;
     let x = y; //~ERROR `x` is shadowed by `y`
 
+    let x; //~ERROR `x` shadows a previous declaration
+    x = 42;
+
     let o = Some(1_u8);
 
     if let Some(p) = o { assert_eq!(1, p); }

tests/run-pass/conf_unknown_key.rs

@@ -1,4 +0,0 @@
-#![feature(plugin)]
-#![plugin(clippy(conf_file="./tests/run-pass/conf_unknown_key.toml"))]
-
-fn main() {}

tests/run-pass/conf_whitelisted.rs

@@ -0,0 +1,4 @@
+#![feature(plugin)]
+#![plugin(clippy(conf_file="./tests/aux/conf_whitelisted.toml"))]
+
+fn main() {}

util/cov.sh

@@ -0,0 +1,37 @@
+#!/usr/bin/bash
+
+# This run `kcov` on Clippy. The coverage report will be at
+# `./target/cov/index.html`.
+# `compile-test` is special. `kcov` does not work directly on it so these files
+# are compiled manually.
+
+tests=$(find tests/ -maxdepth 1 -name '*.rs' ! -name compile-test.rs -exec basename {} .rs \;)
+tmpdir=$(mktemp -d)
+
+cargo test --no-run --verbose
+
+for t in $tests; do
+  kcov \
+    --verify \
+    --include-path="$(pwd)/src,$(pwd)/clippy_lints/src" \
+    "$tmpdir/$t" \
+    cargo test --test "$t"
+done
+
+for t in ./tests/compile-fail/*.rs; do
+  kcov \
+    --verify \
+    --include-path="$(pwd)/src,$(pwd)/clippy_lints/src" \
+    "$tmpdir/compile-fail-$(basename "$t")" \
+    cargo run -- -L target/debug -L target/debug/deps -Z no-trans "$t"
+done
+
+for t in ./tests/run-pass/*.rs; do
+  kcov \
+    --verify \
+    --include-path="$(pwd)/src,$(pwd)/clippy_lints/src" \
+    "$tmpdir/run-pass-$(basename "$t")" \
+    cargo run -- -L target/debug -L target/debug/deps -Z no-trans "$t"
+done
+
+kcov --verify --merge target/cov "$tmpdir"/*