diff --git a/clippy_lints/src/consts.rs b/clippy_lints/src/consts.rs index fa3e6a15606..19149671e5b 100644 --- a/clippy_lints/src/consts.rs +++ b/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 { - 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); } } } diff --git a/clippy_lints/src/format.rs b/clippy_lints/src/format.rs index 349ce2cf8b8..a6e40bd05c0 100644 --- a/clippy_lints/src/format.rs +++ b/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) { diff --git a/clippy_lints/src/shadow.rs b/clippy_lints/src/shadow.rs index 8e3a44031ab..2794cd14304 100644 --- a/clippy_lints/src/shadow.rs +++ b/clippy_lints/src/shadow.rs @@ -262,7 +262,7 @@ fn lint_shadow(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"); }); } } diff --git a/clippy_lints/src/utils/hir.rs b/clippy_lints/src/utils/hir.rs index 41b82f7203b..bb9d0583ab1 100644 --- a/clippy_lints/src/utils/hir.rs +++ b/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; diff --git a/tests/compile-fail/conf_french_blacklisted_name.toml b/tests/aux/conf_french_blacklisted_name.toml similarity index 100% rename from tests/compile-fail/conf_french_blacklisted_name.toml rename to tests/aux/conf_french_blacklisted_name.toml diff --git a/tests/compile-fail/conf_unknown_key.toml b/tests/aux/conf_unknown_key.toml similarity index 100% rename from tests/compile-fail/conf_unknown_key.toml rename to tests/aux/conf_unknown_key.toml diff --git a/tests/run-pass/conf_unknown_key.toml b/tests/aux/conf_whitelisted.toml similarity index 100% rename from tests/run-pass/conf_unknown_key.toml rename to tests/aux/conf_whitelisted.toml diff --git a/tests/compile-fail/array_indexing.rs b/tests/compile-fail/array_indexing.rs index dacb72ee8ac..c69144fe292 100644 --- a/tests/compile-fail/array_indexing.rs +++ b/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 diff --git a/tests/compile-fail/conf_bad_toml.rs b/tests/compile-fail/conf_bad_toml.rs new file mode 100644 index 00000000000..b5ea6d96ef5 --- /dev/null +++ b/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() {} diff --git a/tests/compile-fail/conf_bad_toml.toml b/tests/compile-fail/conf_bad_toml.toml new file mode 100644 index 00000000000..823e01a33b9 --- /dev/null +++ b/tests/compile-fail/conf_bad_toml.toml @@ -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 index 00000000000..8dc3e4ec2e6 --- /dev/null +++ b/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() {} diff --git a/tests/compile-fail/conf_bad_type.toml b/tests/compile-fail/conf_bad_type.toml new file mode 100644 index 00000000000..168675394d7 --- /dev/null +++ b/tests/compile-fail/conf_bad_type.toml @@ -0,0 +1 @@ +blacklisted-names = 42 diff --git a/tests/compile-fail/conf_french_blacklisted_name.rs b/tests/compile-fail/conf_french_blacklisted_name.rs index b7e29eeef1f..716b338e1a4 100644 --- a/tests/compile-fail/conf_french_blacklisted_name.rs +++ b/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)] diff --git a/tests/compile-fail/conf_non_existant.rs b/tests/compile-fail/conf_non_existant.rs index cf1024705ca..0a31fb16147 100644 --- a/tests/compile-fail/conf_non_existant.rs +++ b/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() {} diff --git a/tests/compile-fail/conf_path_non_string.rs b/tests/compile-fail/conf_path_non_string.rs new file mode 100644 index 00000000000..f26f581f5c0 --- /dev/null +++ b/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() {} diff --git a/tests/compile-fail/conf_unknown_key.rs b/tests/compile-fail/conf_unknown_key.rs index 02131d94d52..e6041d8ed1c 100644 --- a/tests/compile-fail/conf_unknown_key.rs +++ b/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() {} diff --git a/tests/compile-fail/copies.rs b/tests/compile-fail/copies.rs index 3c3e43931ed..dba653a148f 100644 --- a/tests/compile-fail/copies.rs +++ b/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 { - //~^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::(42); + if true { + break; + } else { + continue; + } + } + } + else { //~ERROR this `if` has identical blocks + for _ in &[42] { + let foo: &Option<_> = &Some::(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:: {} + } + else { + if let Some(42) = None {} + } + if true { if let Some(a) = Some(42) {} } diff --git a/tests/compile-fail/shadow.rs b/tests/compile-fail/shadow.rs index fae87cd9750..bf0bdd81863 100644 --- a/tests/compile-fail/shadow.rs +++ b/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); } diff --git a/tests/run-pass/conf_unknown_key.rs b/tests/run-pass/conf_unknown_key.rs deleted file mode 100644 index bb186d47630..00000000000 --- a/tests/run-pass/conf_unknown_key.rs +++ /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_whitelisted.rs b/tests/run-pass/conf_whitelisted.rs new file mode 100644 index 00000000000..9b4bb4155d3 --- /dev/null +++ b/tests/run-pass/conf_whitelisted.rs @@ -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 index 00000000000..3f9a6b06f72 --- /dev/null +++ b/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"/*