Skip to content

Commit a4198c1

Browse files
ManishearthManishearth
authored
Merge pull request #1255 from Manishearth/cov
Improve test coverage
2 parents 8d00341 + 6800111 commit a4198c1

21 files changed

+194
-46
lines changed

‎clippy_lints/src/consts.rs

+4-15
Original file line numberDiff line numberDiff line change
@@ -54,27 +54,16 @@ pub enum Constant {
5454
}
5555

5656
impl Constant {
57-
/// convert to u64 if possible
57+
/// Convert to `u64` if possible.
5858
///
5959
/// # panics
6060
///
61-
/// if the constant could not be converted to u64 losslessly
61+
/// If the constant could not be converted to `u64` losslessly.
6262
fn as_u64(&self) -> u64 {
6363
if let Constant::Int(val) = *self {
64-
val.to_u64().expect("negative constant can't be casted to u64")
64+
val.to_u64().expect("negative constant can't be casted to `u64`")
6565
} else {
66-
panic!("Could not convert a {:?} to u64", self);
67-
}
68-
}
69-
70-
/// convert this constant to a f64, if possible
71-
#[allow(cast_precision_loss, cast_possible_wrap)]
72-
pub fn as_float(&self) -> Option<f64> {
73-
match *self {
74-
Constant::Float(ref s, _) => s.parse().ok(),
75-
Constant::Int(i) if i.is_negative() => Some(i.to_u64_unchecked() as i64 as f64),
76-
Constant::Int(i) => Some(i.to_u64_unchecked() as f64),
77-
_ => None,
66+
panic!("Could not convert a `{:?}` to `u64`", self);
7867
}
7968
}
8069
}

‎clippy_lints/src/format.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ pub fn get_argument_fmtstr_parts<'a, 'b>(cx: &LateContext<'a, 'b>, expr: &'a Exp
8181
let Some(NodeItem(decl)) = cx.tcx.map.find(decl.id),
8282
decl.name.as_str() == "__STATIC_FMTSTR",
8383
let ItemStatic(_, _, ref expr) = decl.node,
84-
let ExprAddrOf(_, ref expr) = expr.node, // &[""]
84+
let ExprAddrOf(_, ref expr) = expr.node, // &["…", "…", …]
8585
let ExprVec(ref exprs) = expr.node,
8686
], {
8787
let mut result = Vec::new();
@@ -99,7 +99,7 @@ pub fn get_argument_fmtstr_parts<'a, 'b>(cx: &LateContext<'a, 'b>, expr: &'a Exp
9999

100100
/// Checks if the expressions matches
101101
/// ```rust
102-
/// { static __STATIC_FMTSTR: &[""] = _; __STATIC_FMTSTR }
102+
/// { static __STATIC_FMTSTR: … = &["…", "…", …]; __STATIC_FMTSTR }
103103
/// ```
104104
fn check_static_str(cx: &LateContext, expr: &Expr) -> bool {
105105
if let Some(expr) = get_argument_fmtstr_parts(cx, expr) {

‎clippy_lints/src/shadow.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,7 @@ fn lint_shadow<T>(cx: &LateContext, name: Name, span: Span, pattern_span: Span,
262262
span_lint_and_then(cx,
263263
SHADOW_UNRELATED,
264264
span,
265-
&format!("{} shadows a previous declaration", snippet(cx, pattern_span, "_")),
265+
&format!("`{}` shadows a previous declaration", snippet(cx, pattern_span, "_")),
266266
|db| { db.span_note(prev_span, "previous binding is here"); });
267267
}
268268
}

‎clippy_lints/src/utils/hir.rs

+51-13
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,8 @@ impl<'a, 'tcx: 'a> SpanlessEq<'a, 'tcx> {
3737
match (&left.node, &right.node) {
3838
(&StmtDecl(ref l, _), &StmtDecl(ref r, _)) => {
3939
if let (&DeclLocal(ref l), &DeclLocal(ref r)) = (&l.node, &r.node) {
40-
// TODO: tys
41-
l.ty.is_none() && r.ty.is_none() && both(&l.init, &r.init, |l, r| self.eq_expr(l, r))
40+
both(&l.ty, &r.ty, |l, r| self.eq_ty(l, r)) &&
41+
both(&l.init, &r.init, |l, r| self.eq_expr(l, r))
4242
} else {
4343
false
4444
}
@@ -85,7 +85,10 @@ impl<'a, 'tcx: 'a> SpanlessEq<'a, 'tcx> {
8585
(&ExprCall(ref l_fun, ref l_args), &ExprCall(ref r_fun, ref r_args)) => {
8686
!self.ignore_fn && self.eq_expr(l_fun, r_fun) && self.eq_exprs(l_args, r_args)
8787
}
88-
(&ExprCast(ref lx, ref lt), &ExprCast(ref rx, ref rt)) => self.eq_expr(lx, rx) && self.eq_ty(lt, rt),
88+
(&ExprCast(ref lx, ref lt), &ExprCast(ref rx, ref rt)) |
89+
(&ExprType(ref lx, ref lt), &ExprType(ref rx, ref rt)) => {
90+
self.eq_expr(lx, rx) && self.eq_ty(lt, rt)
91+
}
8992
(&ExprField(ref l_f_exp, ref l_f_ident), &ExprField(ref r_f_exp, ref r_f_ident)) => {
9093
l_f_ident.node == r_f_ident.node && self.eq_expr(l_f_exp, r_f_exp)
9194
}
@@ -106,8 +109,8 @@ impl<'a, 'tcx: 'a> SpanlessEq<'a, 'tcx> {
106109
}
107110
(&ExprMethodCall(ref l_name, ref l_tys, ref l_args),
108111
&ExprMethodCall(ref r_name, ref r_tys, ref r_args)) => {
109-
// TODO: tys
110-
!self.ignore_fn && l_name.node == r_name.node && l_tys.is_empty() && r_tys.is_empty() &&
112+
!self.ignore_fn && l_name.node == r_name.node &&
113+
over(l_tys, r_tys, |l, r| self.eq_ty(l, r)) &&
111114
self.eq_exprs(l_args, r_args)
112115
}
113116
(&ExprRepeat(ref le, ref ll), &ExprRepeat(ref re, ref rl)) => self.eq_expr(le, re) && self.eq_expr(ll, rl),
@@ -138,6 +141,10 @@ impl<'a, 'tcx: 'a> SpanlessEq<'a, 'tcx> {
138141
left.name.node == right.name.node && self.eq_expr(&left.expr, &right.expr)
139142
}
140143

144+
fn eq_lifetime(&self, left: &Lifetime, right: &Lifetime) -> bool {
145+
left.name == right.name
146+
}
147+
141148
/// Check whether two patterns are the same.
142149
pub fn eq_pat(&self, left: &Pat, right: &Pat) -> bool {
143150
match (&left.node, &right.node) {
@@ -169,12 +176,33 @@ impl<'a, 'tcx: 'a> SpanlessEq<'a, 'tcx> {
169176
}
170177

171178
fn eq_path(&self, left: &Path, right: &Path) -> bool {
179+
left.global == right.global &&
180+
over(&left.segments, &right.segments, |l, r| self.eq_path_segment(l, r))
181+
}
182+
183+
fn eq_path_parameters(&self, left: &PathParameters, right: &PathParameters) -> bool {
184+
match (left, right) {
185+
(&AngleBracketedParameters(ref left), &AngleBracketedParameters(ref right)) => {
186+
over(&left.lifetimes, &right.lifetimes, |l, r| self.eq_lifetime(l, r)) &&
187+
over(&left.types, &right.types, |l, r| self.eq_ty(l, r)) &&
188+
over(&left.bindings, &right.bindings, |l, r| self.eq_type_binding(l, r))
189+
}
190+
(&ParenthesizedParameters(ref left), &ParenthesizedParameters(ref right)) => {
191+
over(&left.inputs, &right.inputs, |l, r| self.eq_ty(l, r)) &&
192+
both(&left.output, &right.output, |l, r| self.eq_ty(l, r))
193+
}
194+
(&AngleBracketedParameters(_), &ParenthesizedParameters(_)) |
195+
(&ParenthesizedParameters(_), &AngleBracketedParameters(_)) => {
196+
false
197+
}
198+
}
199+
}
200+
201+
fn eq_path_segment(&self, left: &PathSegment, right: &PathSegment) -> bool {
172202
// The == of idents doesn't work with different contexts,
173203
// we have to be explicit about hygiene
174-
left.global == right.global &&
175-
over(&left.segments,
176-
&right.segments,
177-
|l, r| l.name.as_str() == r.name.as_str() && l.parameters == r.parameters)
204+
left.name.as_str() == right.name.as_str() &&
205+
self.eq_path_parameters(&left.parameters, &right.parameters)
178206
}
179207

180208
fn eq_qself(&self, left: &QSelf, right: &QSelf) -> bool {
@@ -199,6 +227,10 @@ impl<'a, 'tcx: 'a> SpanlessEq<'a, 'tcx> {
199227
_ => false,
200228
}
201229
}
230+
231+
fn eq_type_binding(&self, left: &TypeBinding, right: &TypeBinding) -> bool {
232+
left.name == right.name && self.eq_ty(&left.ty, &right.ty)
233+
}
202234
}
203235

204236
fn swap_binop<'a>(binop: BinOp_, lhs: &'a Expr, rhs: &'a Expr) -> Option<(BinOp_, &'a Expr, &'a Expr)> {
@@ -445,10 +477,11 @@ impl<'a, 'tcx: 'a> SpanlessHash<'a, 'tcx> {
445477
self.hash_expr(le);
446478
li.node.hash(&mut self.s);
447479
}
448-
ExprType(_, _) => {
480+
ExprType(ref e, ref _ty) => {
449481
let c: fn(_, _) -> _ = ExprType;
450482
c.hash(&mut self.s);
451-
// what’s an ExprType anyway?
483+
self.hash_expr(e);
484+
// TODO: _ty
452485
}
453486
ExprUnary(lop, ref le) => {
454487
let c: fn(_, _) -> _ = ExprUnary;
@@ -495,10 +528,15 @@ impl<'a, 'tcx: 'a> SpanlessHash<'a, 'tcx> {
495528

496529
pub fn hash_stmt(&mut self, b: &Stmt) {
497530
match b.node {
498-
StmtDecl(ref _decl, _) => {
531+
StmtDecl(ref decl, _) => {
499532
let c: fn(_, _) -> _ = StmtDecl;
500533
c.hash(&mut self.s);
501-
// TODO: decl
534+
535+
if let DeclLocal(ref local) = decl.node {
536+
if let Some(ref init) = local.init {
537+
self.hash_expr(init);
538+
}
539+
}
502540
}
503541
StmtExpr(ref expr, _) => {
504542
let c: fn(_, _) -> _ = StmtExpr;

‎tests/compile-fail/conf_french_blacklisted_name.toml renamed to ‎tests/aux/conf_french_blacklisted_name.toml

File renamed without changes.

‎tests/compile-fail/conf_unknown_key.toml renamed to ‎tests/aux/conf_unknown_key.toml

File renamed without changes.

‎tests/run-pass/conf_unknown_key.toml renamed to ‎tests/aux/conf_whitelisted.toml

File renamed without changes.

‎tests/compile-fail/array_indexing.rs

+4
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ fn main() {
1414
&x[1..5]; //~ERROR: range is out of bounds
1515
&x[0..3];
1616
&x[0...4]; //~ERROR: range is out of bounds
17+
&x[...4]; //~ERROR: range is out of bounds
1718
&x[..];
1819
&x[1..];
1920
&x[4..];
@@ -26,15 +27,18 @@ fn main() {
2627
&y[1..2]; //~ERROR: slicing may panic
2728
&y[..];
2829
&y[0...4]; //~ERROR: slicing may panic
30+
&y[...4]; //~ERROR: slicing may panic
2931

3032
let empty: [i8; 0] = [];
3133
empty[0]; //~ERROR: const index is out of bounds
3234
&empty[1..5]; //~ERROR: range is out of bounds
3335
&empty[0...4]; //~ERROR: range is out of bounds
36+
&empty[...4]; //~ERROR: range is out of bounds
3437
&empty[..];
3538
&empty[0..];
3639
&empty[0..0];
3740
&empty[0...0]; //~ERROR: range is out of bounds
41+
&empty[...0]; //~ERROR: range is out of bounds
3842
&empty[..0];
3943
&empty[1..]; //~ERROR: range is out of bounds
4044
&empty[..4]; //~ERROR: range is out of bounds

‎tests/compile-fail/conf_bad_toml.rs

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

‎tests/compile-fail/conf_bad_toml.toml

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

‎tests/compile-fail/conf_bad_type.rs

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

‎tests/compile-fail/conf_bad_type.toml

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
blacklisted-names = 42

‎tests/compile-fail/conf_french_blacklisted_name.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#![feature(plugin)]
2-
#![plugin(clippy(conf_file="./tests/compile-fail/conf_french_blacklisted_name.toml"))]
2+
#![plugin(clippy(conf_file="./tests/aux/conf_french_blacklisted_name.toml"))]
33

44
#![allow(dead_code)]
55
#![allow(single_match)]

‎tests/compile-fail/conf_non_existant.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// error-pattern: error reading Clippy's configuration file
22

33
#![feature(plugin)]
4-
#![plugin(clippy(conf_file="./tests/compile-fail/non_existant_conf.toml"))]
4+
#![plugin(clippy(conf_file="./tests/aux/non_existant_conf.toml"))]
55

66
fn main() {}

‎tests/compile-fail/conf_path_non_string.rs

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
#![feature(attr_literals)]
2+
#![feature(plugin)]
3+
#![plugin(clippy(conf_file=42))]
4+
//~^ ERROR `conf_file` value must be a string
5+
6+
fn main() {}

‎tests/compile-fail/conf_unknown_key.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// error-pattern: error reading Clippy's configuration file: unknown key `foobar`
22

33
#![feature(plugin)]
4-
#![plugin(clippy(conf_file="./tests/compile-fail/conf_unknown_key.toml"))]
4+
#![plugin(clippy(conf_file="./tests/aux/conf_unknown_key.toml"))]
55

66
fn main() {}

‎tests/compile-fail/copies.rs

+64-8
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,13 @@ fn if_same_then_else() -> Result<&'static str, ()> {
4848
Foo { bar: 43 };
4949
}
5050

51+
if true {
52+
();
53+
}
54+
else {
55+
()
56+
}
57+
5158
if true {
5259
0..10;
5360
}
@@ -63,14 +70,27 @@ fn if_same_then_else() -> Result<&'static str, ()> {
6370
foo();
6471
}
6572

66-
let _ = if true {
67-
//~^NOTE same as this
68-
foo();
69-
42
70-
}
71-
else { //~ERROR this `if` has identical blocks
72-
foo();
73-
42
73+
let _ = match 42 {
74+
42 => {
75+
//~^ NOTE same as this
76+
//~| NOTE refactoring
77+
foo();
78+
let mut a = 42 + [23].len() as i32;
79+
if true {
80+
a += 7;
81+
}
82+
a = -31-a;
83+
a
84+
}
85+
_ => { //~ERROR this `match` has identical arm bodies
86+
foo();
87+
let mut a = 42 + [23].len() as i32;
88+
if true {
89+
a += 7;
90+
}
91+
a = -31-a;
92+
a
93+
}
7494
};
7595

7696
if true {
@@ -85,6 +105,28 @@ fn if_same_then_else() -> Result<&'static str, ()> {
85105
42
86106
};
87107

108+
if true {
109+
//~^NOTE same as this
110+
for _ in &[42] {
111+
let foo: &Option<_> = &Some::<u8>(42);
112+
if true {
113+
break;
114+
} else {
115+
continue;
116+
}
117+
}
118+
}
119+
else { //~ERROR this `if` has identical blocks
120+
for _ in &[42] {
121+
let foo: &Option<_> = &Some::<u8>(42);
122+
if true {
123+
break;
124+
} else {
125+
continue;
126+
}
127+
}
128+
}
129+
88130
if true {
89131
//~^NOTE same as this
90132
let bar = if true {
@@ -167,6 +209,20 @@ fn if_same_then_else() -> Result<&'static str, ()> {
167209
if let (.., 1, 3) = (1, 2, 3) {}
168210
}
169211

212+
if true {
213+
if let Some(42) = None {}
214+
}
215+
else {
216+
if let Option::Some(42) = None {}
217+
}
218+
219+
if true {
220+
if let Some(42) = None::<u8> {}
221+
}
222+
else {
223+
if let Some(42) = None {}
224+
}
225+
170226
if true {
171227
if let Some(a) = Some(42) {}
172228
}

‎tests/compile-fail/shadow.rs

+3
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@ fn main() {
2020
let y = 1;
2121
let x = y; //~ERROR `x` is shadowed by `y`
2222

23+
let x; //~ERROR `x` shadows a previous declaration
24+
x = 42;
25+
2326
let o = Some(1_u8);
2427

2528
if let Some(p) = o { assert_eq!(1, p); }

‎tests/run-pass/conf_unknown_key.rs

-4
This file was deleted.

‎tests/run-pass/conf_whitelisted.rs

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

0 commit comments

Comments
 (0)