Skip to content

Improve test coverage #1255

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Oct 3, 2016
19 changes: 4 additions & 15 deletions clippy_lints/src/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/shadow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"); });
}
}
Expand Down
64 changes: 51 additions & 13 deletions clippy_lints/src/utils/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ impl<'a, 'tcx: 'a> SpanlessEq<'a, 'tcx> {
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
}
Expand Down Expand Up @@ -85,7 +85,10 @@ impl<'a, 'tcx: 'a> SpanlessEq<'a, 'tcx> {
(&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)
}
Expand All @@ -106,8 +109,8 @@ impl<'a, 'tcx: 'a> SpanlessEq<'a, 'tcx> {
}
(&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),
Expand Down Expand Up @@ -138,6 +141,10 @@ impl<'a, 'tcx: 'a> SpanlessEq<'a, 'tcx> {
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) {
Expand Down Expand Up @@ -169,12 +176,33 @@ impl<'a, 'tcx: 'a> SpanlessEq<'a, 'tcx> {
}

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 {
Expand All @@ -199,6 +227,10 @@ impl<'a, 'tcx: 'a> SpanlessEq<'a, 'tcx> {
_ => 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)> {
Expand Down Expand Up @@ -445,10 +477,11 @@ impl<'a, 'tcx: 'a> SpanlessHash<'a, 'tcx> {
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;
Expand Down Expand Up @@ -495,10 +528,15 @@ impl<'a, 'tcx: 'a> SpanlessHash<'a, 'tcx> {

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;
Expand Down
File renamed without changes.
4 changes: 4 additions & 0 deletions tests/compile-fail/array_indexing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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..];
Expand All @@ -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
Expand Down
6 changes: 6 additions & 0 deletions tests/compile-fail/conf_bad_toml.rs
Original file line number Diff line number Diff line change
@@ -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() {}
2 changes: 2 additions & 0 deletions tests/compile-fail/conf_bad_toml.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
fn this_is_obviously(not: a, toml: file) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we place this in tests/aux?

}
6 changes: 6 additions & 0 deletions tests/compile-fail/conf_bad_type.rs
Original file line number Diff line number Diff line change
@@ -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() {}
1 change: 1 addition & 0 deletions tests/compile-fail/conf_bad_type.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
blacklisted-names = 42
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto regarding aux

2 changes: 1 addition & 1 deletion tests/compile-fail/conf_french_blacklisted_name.rs
Original file line number Diff line number Diff line change
@@ -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)]
Expand Down
2 changes: 1 addition & 1 deletion tests/compile-fail/conf_non_existant.rs
Original file line number Diff line number Diff line change
@@ -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() {}
6 changes: 6 additions & 0 deletions tests/compile-fail/conf_path_non_string.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#![feature(attr_literals)]
#![feature(plugin)]
#![plugin(clippy(conf_file=42))]
//~^ ERROR `conf_file` value must be a string

fn main() {}
2 changes: 1 addition & 1 deletion tests/compile-fail/conf_unknown_key.rs
Original file line number Diff line number Diff line change
@@ -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() {}
72 changes: 64 additions & 8 deletions tests/compile-fail/copies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,13 @@ fn if_same_then_else() -> Result<&'static str, ()> {
Foo { bar: 43 };
}

if true {
();
}
else {
()
}

if true {
0..10;
}
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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) {}
}
Expand Down
3 changes: 3 additions & 0 deletions tests/compile-fail/shadow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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); }
Expand Down
4 changes: 0 additions & 4 deletions tests/run-pass/conf_unknown_key.rs

This file was deleted.

4 changes: 4 additions & 0 deletions tests/run-pass/conf_whitelisted.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
#![feature(plugin)]
#![plugin(clippy(conf_file="./tests/aux/conf_whitelisted.toml"))]

fn main() {}
Loading