Fix wrong tests and improve some other

Cargo.toml

@@ -35,7 +35,7 @@ clippy_lints = { version = "0.0.74", path = "clippy_lints" }
 rustc-serialize = "0.3"
 
 [dev-dependencies]
-compiletest_rs = "0.1.0"
+compiletest_rs = "0.2.0"
 lazy_static = "0.1.15"
 regex = "0.1.56"
 

clippy_lints/src/loops.rs

@@ -290,11 +290,15 @@ impl LateLintPass for LoopsPass {
                        !is_iterator_used_after_while_let(cx, iter_expr) {
                         let iterator = snippet(cx, method_args[0].span, "_");
                         let loop_var = snippet(cx, pat_args[0].span, "_");
-                        span_help_and_lint(cx,
+                        span_lint_and_then(cx,
                                            WHILE_LET_ON_ITERATOR,
                                            expr.span,
                                            "this loop could be written as a `for` loop",
-                                           &format!("try\nfor {} in {} {{...}}", loop_var, iterator));
+                                           |db| {
+                        db.span_suggestion(expr.span,
+                                           "try",
+                                           format!("for {} in {} {{ .. }}", loop_var, iterator));
+                        });
                     }
                 }
             }
@@ -446,11 +450,11 @@ fn check_for_loop_reverse_range(cx: &LateContext, arg: &Expr, expr: &Expr) {
                                        expr.span,
                                        "this range is empty so this for loop will never run",
                                        |db| {
-                                           db.span_suggestion(expr.span,
+                                           db.span_suggestion(arg.span,
                                                               "consider using the following if \
                                                                you are attempting to iterate \
                                                                over this range in reverse",
-                                                              format!("({}..{}).rev()` ", end_snippet, start_snippet));
+                                                              format!("({}..{}).rev()", end_snippet, start_snippet));
                                        });
                 } else if eq && limits != ast::RangeLimits::Closed {
                     // if they are equal, it's also problematic - this loop
@@ -598,7 +602,7 @@ fn check_for_loop_over_map_kv(cx: &LateContext, pat: &Pat, arg: &Expr, body: &Ex
                                    |db| {
                                        db.span_suggestion(expr.span,
                                                           "use the corresponding method",
-                                                          format!("for {} in {}.{}() {{...}}",
+                                                          format!("for {} in {}.{}() {{ .. }}",
                                                                   snippet(cx, *pat_span, ".."),
                                                                   snippet(cx, arg_span, ".."),
                                                                   kind));

tests/compile-fail/absurd-extreme-comparisons.rs

@@ -8,15 +8,33 @@ fn main() {
 
     let u: u32 = 42;
 
-    u <= 0; //~ERROR this comparison involving the minimum or maximum element for this type contains a case that is always true or always false
-    u <= Z; //~ERROR this comparison involving
-    u < Z; //~ERROR this comparison involving
-    Z >= u; //~ERROR this comparison involving
-    Z > u; //~ERROR this comparison involving
-    u > std::u32::MAX; //~ERROR this comparison involving
-    u >= std::u32::MAX; //~ERROR this comparison involving
-    std::u32::MAX < u; //~ERROR this comparison involving
-    std::u32::MAX <= u; //~ERROR this comparison involving
+    u <= 0;
+    //~^ ERROR this comparison involving the minimum or maximum element for this type contains a case that is always true or always false
+    //~| HELP using u == 0 instead
+    u <= Z;
+    //~^ ERROR this comparison involving
+    //~| HELP using u == Z instead
+    u < Z;
+    //~^ ERROR this comparison involving
+    //~| HELP comparison is always false
+    Z >= u;
+    //~^ ERROR this comparison involving
+    //~| HELP using Z == u instead
+    Z > u;
+    //~^ ERROR this comparison involving
+    //~| HELP comparison is always false
+    u > std::u32::MAX;
+    //~^ ERROR this comparison involving
+    //~| HELP comparison is always false
+    u >= std::u32::MAX;
+    //~^ ERROR this comparison involving
+    //~| HELP using u == std::u32::MAX instead
+    std::u32::MAX < u;
+    //~^ ERROR this comparison involving
+    //~| HELP comparison is always false
+    std::u32::MAX <= u;
+    //~^ ERROR this comparison involving
+    //~| HELP using std::u32::MAX == u instead
 
     1-1 > u;
         //~^ ERROR this comparison involving
@@ -29,13 +47,23 @@ fn main() {
         //~| HELP because 12 - 2*6 is the minimum value for this type, the case where the two sides are not equal never occurs, consider using u == 12 - 2*6 instead
 
     let i: i8 = 0;
-    i < -127 - 1; //~ERROR this comparison involving
-    std::i8::MAX >= i; //~ERROR this comparison involving
-    3-7 < std::i32::MIN; //~ERROR this comparison involving
+    i < -127 - 1;
+    //~^ ERROR this comparison involving
+    //~| HELP comparison is always false
+    std::i8::MAX >= i;
+    //~^ ERROR this comparison involving
+    //~| HELP comparison is always true
+    3-7 < std::i32::MIN;
+    //~^ ERROR this comparison involving
+    //~| HELP comparison is always false
 
     let b = false;
-    b >= true; //~ERROR this comparison involving
-    false > b; //~ERROR this comparison involving
+    b >= true;
+    //~^ ERROR this comparison involving
+    //~| HELP using b == true instead
+    false > b;
+    //~^ ERROR this comparison involving
+    //~| HELP comparison is always false
 
     u > 0; // ok
 

tests/compile-fail/booleans.rs

@@ -52,29 +52,37 @@ fn equality_stuff() {
     let c: i32 = unimplemented!();
     let d: i32 = unimplemented!();
     let e: i32 = unimplemented!();
-    let _ = a == b && a != b; //~ ERROR this boolean expression contains a logic bug
+    let _ = a == b && a != b;
+    //~^ ERROR this boolean expression contains a logic bug
     //~| HELP this expression can be optimized out
     //~| HELP it would look like the following
     //~| SUGGESTION let _ = false;
-    let _ = a == b && c == 5 && a == b; //~ ERROR this boolean expression can be simplified
+    let _ = a == b && c == 5 && a == b;
+    //~^ ERROR this boolean expression can be simplified
     //~| HELP try
     //~| SUGGESTION let _ = a == b && c == 5;
-    let _ = a == b && c == 5 && b == a; //~ ERROR this boolean expression can be simplified
+    //~| HELP try
+    //~| SUGGESTION let _ = !(c != 5 || a != b);
+    let _ = a == b && c == 5 && b == a;
+    //~^ ERROR this boolean expression can be simplified
     //~| HELP try
     //~| SUGGESTION let _ = a == b && c == 5;
     //~| HELP try
     //~| SUGGESTION let _ = !(c != 5 || a != b);
-    let _ = a < b && a >= b; //~ ERROR this boolean expression contains a logic bug
+    let _ = a < b && a >= b;
+    //~^ ERROR this boolean expression contains a logic bug
     //~| HELP this expression can be optimized out
     //~| HELP it would look like the following
     //~| SUGGESTION let _ = false;
-    let _ = a > b && a <= b; //~ ERROR this boolean expression contains a logic bug
+    let _ = a > b && a <= b;
+    //~^ ERROR this boolean expression contains a logic bug
     //~| HELP this expression can be optimized out
     //~| HELP it would look like the following
     //~| SUGGESTION let _ = false;
     let _ = a > b && a == b;
 
-    let _ = a != b || !(a != b || c == d); //~ ERROR this boolean expression can be simplified
+    let _ = a != b || !(a != b || c == d);
+    //~^ ERROR this boolean expression can be simplified
     //~| HELP try
     //~| SUGGESTION let _ = c != d || a != b;
     //~| HELP try

tests/compile-fail/collapsible_if.rs

@@ -5,13 +5,19 @@
 fn main() {
     let x = "hello";
     let y = "world";
-    if x == "hello" { //~ERROR this if statement can be collapsed
+    if x == "hello" {
+    //~^ ERROR this if statement can be collapsed
+    //~| HELP try
+    //~| SUGGESTION if x == "hello" && y == "world" {
         if y == "world" {
             println!("Hello world!");
         }
     }
 
-    if x == "hello" || x == "world" { //~ERROR this if statement can be collapsed
+    if x == "hello" || x == "world" {
+    //~^ ERROR this if statement can be collapsed
+    //~| HELP try
+    //~| SUGGESTION if (x == "hello" || x == "world") && (y == "world" || y == "hello") {
         if y == "world" || y == "hello" {
             println!("Hello world!");
         }

tests/compile-fail/for_loop.rs

@@ -200,11 +200,17 @@ fn main() {
     }
 
     // testing that the empty range lint folds constants
-    for i in 10..5+4 { //~ERROR this range is empty so this for loop will never run
+    for i in 10..5+4 {
+    //~^ ERROR this range is empty so this for loop will never run
+    //~| HELP if you are attempting to iterate over this range in reverse
+    //~| SUGGESTION for i in (5+4..10).rev() {
         println!("{}", i);
     }
 
-    for i in (5+2)..(3-1) { //~ERROR this range is empty so this for loop will never run
+    for i in (5+2)..(3-1) {
+    //~^ ERROR this range is empty so this for loop will never run
+    //~| HELP if you are attempting to iterate over this range in reverse
+    //~| SUGGESTION for i in ((3-1)..(5+2)).rev() {
         println!("{}", i);
     }
 

tests/compile-fail/formatting.rs

@@ -11,24 +11,32 @@ fn foo() -> bool { true }
 fn main() {
     // weird `else if` formatting:
     if foo() {
-    } if foo() { //~ERROR this looks like an `else if` but the `else` is missing
+    } if foo() {
+    //~^ ERROR this looks like an `else if` but the `else` is missing
+    //~| NOTE add the missing `else` or
     }
 
     let _ = {
         if foo() {
-        } if foo() { //~ERROR this looks like an `else if` but the `else` is missing
+        } if foo() {
+        //~^ ERROR this looks like an `else if` but the `else` is missing
+        //~| NOTE add the missing `else` or
         }
         else {
         }
     };
 
     if foo() {
-    } else //~ERROR this is an `else if` but the formatting might hide it
+    } else
+    //~^ ERROR this is an `else if` but the formatting might hide it
+    //~| NOTE remove the `else` or
     if foo() { // the span of the above error should continue here
     }
 
     if foo() {
-    } //~ERROR this is an `else if` but the formatting might hide it
+    }
+    //~^ ERROR this is an `else if` but the formatting might hide it
+    //~| NOTE remove the `else` or
     else
     if foo() { // the span of the above error should continue here
     }

tests/compile-fail/let_return.rs

@@ -6,13 +6,13 @@
 
 fn test() -> i32 {
     let _y = 0; // no warning
-    let x = 5;   //~NOTE
+    let x = 5;   //~NOTE this expression can be directly returned
     x            //~ERROR returning the result of a let binding
 }
 
 fn test_inner() -> i32 {
     if true {
-        let x = 5;
+        let x = 5;   //~NOTE this expression can be directly returned
         x            //~ERROR returning the result of a let binding
     } else {
         0

tests/compile-fail/matches.rs

@@ -100,28 +100,43 @@ fn single_match_know_enum() {
 fn match_bool() {
     let test: bool = true;
 
-    match test {  //~ ERROR you seem to be trying to match on a boolean expression
+    match test {
+    //~^ ERROR you seem to be trying to match on a boolean expression
+    //~| HELP try
+    //~| SUGGESTION if test { 0 } else { 42 };
         true => 0,
         false => 42,
     };
 
     let option = 1;
-    match option == 1 {  //~ ERROR you seem to be trying to match on a boolean expression
+    match option == 1 {
+    //~^ ERROR you seem to be trying to match on a boolean expression
+    //~| HELP try
+    //~| SUGGESTION if option == 1 { 1 } else { 0 };
         true => 1,
         false => 0,
     };
 
-    match test { //~ ERROR you seem to be trying to match on a boolean expression
+    match test {
+    //~^ ERROR you seem to be trying to match on a boolean expression
+    //~| HELP try
+    //~^^ SUGGESTION if !test { println!("Noooo!"); };
         true => (),
         false => { println!("Noooo!"); }
     };
 
-    match test { //~ ERROR you seem to be trying to match on a boolean expression
+    match test {
+    //~^ ERROR you seem to be trying to match on a boolean expression
+    //~| HELP try
+    //~^^ SUGGESTION if !test { println!("Noooo!"); };
         false => { println!("Noooo!"); }
         _ => (),
     };
 
-    match test { //~ ERROR you seem to be trying to match on a boolean expression
+    match test {
+    //~^ ERROR you seem to be trying to match on a boolean expression
+    //~| HELP try
+    //~| SUGGESTION if test { println!("Yes!"); } else { println!("Noooo!"); };
         false => { println!("Noooo!"); }
         true => { println!("Yes!"); }
     };
@@ -216,7 +231,7 @@ fn overlapping() {
         11 ... 50 => println!("0 ... 10"),
         _ => (),
     }
-    
+
     if let None = Some(42) {
         // nothing
     } else if let None = Some(42) {

tests/compile-fail/methods.rs

@@ -356,13 +356,19 @@ fn starts_with() {
 
 fn use_extend_from_slice() {
     let mut v : Vec<&'static str> = vec![];
-    v.extend(&["Hello", "World"]); //~ERROR use of `extend`
+    v.extend(&["Hello", "World"]);
+    //~^ ERROR use of `extend`
+    //~| HELP try this
+    //~| SUGGESTION v.extend_from_slice(&["Hello", "World"]);
     v.extend(&vec!["Some", "more"]);
-    //~^ERROR use of `extend`
+    //~^ ERROR use of `extend`
     //~| HELP try this
     //~| SUGGESTION v.extend_from_slice(&vec!["Some", "more"]);
 
-    v.extend(vec!["And", "even", "more"].iter()); //~ERROR use of `extend`
+    v.extend(vec!["And", "even", "more"].iter());
+    //~^ ERROR use of `extend`
+    //~| HELP try this
+    //FIXME: the suggestion if broken because of the macro
     let o : Option<&'static str> = None;
     v.extend(o);
     v.extend(Some("Bye"));

tests/compile-fail/mut_mut.rs

@@ -38,5 +38,6 @@ fn main() {
         ***y + **x;
     }
 
-    let mut z = mut_ptr!(&mut 3u32); //~ NOTE in this expansion of mut_ptr!
+    let mut z = mut_ptr!(&mut 3u32);
+    //~^ NOTE in this expansion of mut_ptr!
 }

tests/compile-fail/needless_borrow.rs

@@ -10,7 +10,7 @@ fn x(y: &i32) -> i32 {
 fn main() {
     let a = 5;
     let b = x(&a);
-    let c = x(&&a); //~ ERROR: needless_borrow
+    let c = x(&&a); //~ ERROR: this expression borrows a reference that is immediately dereferenced by the compiler
     let s = &String::from("hi");
     let s_ident = f(&s); // should not error, because `&String` implements Copy, but `String` does not
     let g_val = g(&Vec::new()); // should not error, because `&Vec<T>` derefs to `&[T]`