]> git.lizzy.rs Git - rust.git/commitdiff
Merge pull request #1255 from Manishearth/cov
authorManish Goregaokar <manishsmail@gmail.com>
Mon, 3 Oct 2016 16:45:23 +0000 (22:15 +0530)
committerGitHub <noreply@github.com>
Mon, 3 Oct 2016 16:45:23 +0000 (22:15 +0530)
Improve test coverage

24 files changed:
clippy_lints/src/consts.rs
clippy_lints/src/format.rs
clippy_lints/src/shadow.rs
clippy_lints/src/utils/hir.rs
tests/aux/conf_french_blacklisted_name.toml [new file with mode: 0644]
tests/aux/conf_unknown_key.toml [new file with mode: 0644]
tests/aux/conf_whitelisted.toml [new file with mode: 0644]
tests/compile-fail/array_indexing.rs
tests/compile-fail/conf_bad_toml.rs [new file with mode: 0644]
tests/compile-fail/conf_bad_toml.toml [new file with mode: 0644]
tests/compile-fail/conf_bad_type.rs [new file with mode: 0644]
tests/compile-fail/conf_bad_type.toml [new file with mode: 0644]
tests/compile-fail/conf_french_blacklisted_name.rs
tests/compile-fail/conf_french_blacklisted_name.toml [deleted file]
tests/compile-fail/conf_non_existant.rs
tests/compile-fail/conf_path_non_string.rs [new file with mode: 0644]
tests/compile-fail/conf_unknown_key.rs
tests/compile-fail/conf_unknown_key.toml [deleted file]
tests/compile-fail/copies.rs
tests/compile-fail/shadow.rs
tests/run-pass/conf_unknown_key.rs [deleted file]
tests/run-pass/conf_unknown_key.toml [deleted file]
tests/run-pass/conf_whitelisted.rs [new file with mode: 0644]
util/cov.sh [new file with mode: 0755]

index fa3e6a15606080c937413bba5881e7d8d6201d61..19149671e5b07b3dfdd571355024ec4b4e51a61e 100644 (file)
@@ -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);
         }
     }
 }
index 349ce2cf8b819d0fde29bd7b85dea8c3e194b2be..a6e40bd05c06a196446fa914b43168d34ed6d948 100644 (file)
@@ -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) {
index 8e3a44031ab516cf1963d7ba191ad01e8f81ff43..2794cd14304b3f0e960f89f0afa8c0d3e082005e 100644 (file)
@@ -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"); });
     }
 }
index 41b82f7203b9ef6469f6b43c5c88aa3bccbf8660..bb9d0583ab1c6e4c3a4fb09bbbb1c0582f8097e0 100644 (file)
@@ -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;
diff --git a/tests/aux/conf_french_blacklisted_name.toml b/tests/aux/conf_french_blacklisted_name.toml
new file mode 100644 (file)
index 0000000..6abe5a3
--- /dev/null
@@ -0,0 +1 @@
+blacklisted-names = ["toto", "tata", "titi"]
diff --git a/tests/aux/conf_unknown_key.toml b/tests/aux/conf_unknown_key.toml
new file mode 100644 (file)
index 0000000..554b87c
--- /dev/null
@@ -0,0 +1,6 @@
+# that one is an error
+foobar = 42
+
+# that one is white-listed
+[third-party]
+clippy-feature = "nightly"
diff --git a/tests/aux/conf_whitelisted.toml b/tests/aux/conf_whitelisted.toml
new file mode 100644 (file)
index 0000000..9f87de2
--- /dev/null
@@ -0,0 +1,3 @@
+# this is ignored by Clippy, but allowed for other tools like clippy-service
+[third-party]
+clippy-feature = "nightly"
index dacb72ee8ac3e27624d4d44109018f6ad2b87fa8..c69144fe292045106ba9c8f6c7296839840768aa 100644 (file)
@@ -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
diff --git a/tests/compile-fail/conf_bad_toml.rs b/tests/compile-fail/conf_bad_toml.rs
new file mode 100644 (file)
index 0000000..b5ea6d9
--- /dev/null
@@ -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() {}
diff --git a/tests/compile-fail/conf_bad_toml.toml b/tests/compile-fail/conf_bad_toml.toml
new file mode 100644 (file)
index 0000000..823e01a
--- /dev/null
@@ -0,0 +1,2 @@
+fn this_is_obviously(not: a, toml: file) {
+}
diff --git a/tests/compile-fail/conf_bad_type.rs b/tests/compile-fail/conf_bad_type.rs
new file mode 100644 (file)
index 0000000..8dc3e4e
--- /dev/null
@@ -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() {}
diff --git a/tests/compile-fail/conf_bad_type.toml b/tests/compile-fail/conf_bad_type.toml
new file mode 100644 (file)
index 0000000..1686753
--- /dev/null
@@ -0,0 +1 @@
+blacklisted-names = 42
index b7e29eeef1f7f8ec1fd3716bd34fcc55a55fbaf0..716b338e1a4382432c729de1676ada03044509a3 100644 (file)
@@ -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)]
diff --git a/tests/compile-fail/conf_french_blacklisted_name.toml b/tests/compile-fail/conf_french_blacklisted_name.toml
deleted file mode 100644 (file)
index 6abe5a3..0000000
+++ /dev/null
@@ -1 +0,0 @@
-blacklisted-names = ["toto", "tata", "titi"]
index cf1024705caf843104d1da900b71f70aa589cc13..0a31fb16147b4edc59535584fc4697ee838940b5 100644 (file)
@@ -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() {}
diff --git a/tests/compile-fail/conf_path_non_string.rs b/tests/compile-fail/conf_path_non_string.rs
new file mode 100644 (file)
index 0000000..f26f581
--- /dev/null
@@ -0,0 +1,6 @@
+#![feature(attr_literals)]
+#![feature(plugin)]
+#![plugin(clippy(conf_file=42))]
+//~^ ERROR `conf_file` value must be a string
+
+fn main() {}
index 02131d94d527f5cb4840fe752c9a2331c39eac4c..e6041d8ed1c36680564f907768ef35ae4479b8a8 100644 (file)
@@ -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() {}
diff --git a/tests/compile-fail/conf_unknown_key.toml b/tests/compile-fail/conf_unknown_key.toml
deleted file mode 100644 (file)
index 554b87c..0000000
+++ /dev/null
@@ -1,6 +0,0 @@
-# that one is an error
-foobar = 42
-
-# that one is white-listed
-[third-party]
-clippy-feature = "nightly"
index 3c3e43931ed7bf4ccdcc4e7bfea3c991cccf808c..dba653a148ffcc5832f2c72f4f3145086823385b 100644 (file)
@@ -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 {
-        //~^NOTE same as this
-        foo();
-        42
-    }
-    else { //~ERROR this `if` has identical blocks
-        foo();
-        42
+    let _ = match 42 {
+        42 => {
+            //~^ NOTE same as this
+            //~| NOTE refactoring
+            foo();
+            let mut a = 42 + [23].len() as i32;
+            if true {
+                a += 7;
+            }
+            a = -31-a;
+            a
+        }
+        _ => { //~ERROR this `match` has identical arm bodies
+            foo();
+            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) {}
     }
index fae87cd97503aa6bc65034fe06fa812ddc8bccae..bf0bdd81863203d4d5b4c86cb29f65ad0e4f09cf 100644 (file)
@@ -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); }
diff --git a/tests/run-pass/conf_unknown_key.rs b/tests/run-pass/conf_unknown_key.rs
deleted file mode 100644 (file)
index bb186d4..0000000
+++ /dev/null
@@ -1,4 +0,0 @@
-#![feature(plugin)]
-#![plugin(clippy(conf_file="./tests/run-pass/conf_unknown_key.toml"))]
-
-fn main() {}
diff --git a/tests/run-pass/conf_unknown_key.toml b/tests/run-pass/conf_unknown_key.toml
deleted file mode 100644 (file)
index 9f87de2..0000000
+++ /dev/null
@@ -1,3 +0,0 @@
-# this is ignored by Clippy, but allowed for other tools like clippy-service
-[third-party]
-clippy-feature = "nightly"
diff --git a/tests/run-pass/conf_whitelisted.rs b/tests/run-pass/conf_whitelisted.rs
new file mode 100644 (file)
index 0000000..9b4bb41
--- /dev/null
@@ -0,0 +1,4 @@
+#![feature(plugin)]
+#![plugin(clippy(conf_file="./tests/aux/conf_whitelisted.toml"))]
+
+fn main() {}
diff --git a/util/cov.sh b/util/cov.sh
new file mode 100755 (executable)
index 0000000..3f9a6b0
--- /dev/null
@@ -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"/*