Merge pull request #3332 from lukasstevens/fix798

Check for comments in collapsible ifs

Author: flip1995

clippy_lints/src/collapsible_if.rs

@@ -112,9 +112,17 @@ fn check_if(cx: &EarlyContext<'_>, expr: &ast::Expr) {
     }
 }
 
+fn block_starts_with_comment(cx: &EarlyContext<'_>, expr: &ast::Block) -> bool {
+    // We trim all opening braces and whitespaces and then check if the next string is a comment.
+    let trimmed_block_text =
+        snippet_block(cx, expr.span, "..").trim_left_matches(|c: char| c.is_whitespace() || c == '{').to_owned();
+    trimmed_block_text.starts_with("//") || trimmed_block_text.starts_with("/*")
+}
+
 fn check_collapsible_maybe_if_let(cx: &EarlyContext<'_>, else_: &ast::Expr) {
     if_chain! {
         if let ast::ExprKind::Block(ref block, _) = else_.node;
+        if !block_starts_with_comment(cx, block);
         if let Some(else_) = expr_block(block);
         if !in_macro(else_.span);
         then {
@@ -135,6 +143,7 @@ fn check_collapsible_maybe_if_let(cx: &EarlyContext<'_>, else_: &ast::Expr) {
 
 fn check_collapsible_no_if_let(cx: &EarlyContext<'_>, expr: &ast::Expr, check: &ast::Expr, then: &ast::Block) {
     if_chain! {
+        if !block_starts_with_comment(cx, then);
         if let Some(inner) = expr_block(then);
         if let ast::ExprKind::If(ref check_inner, ref content, None) = inner.node;
         then {

tests/ui/collapsible_if.rs

@@ -151,4 +151,62 @@ fn main() {
     } else {
         assert!(true); // assert! is just an `if`
     }
+
+
+    // The following tests check for the fix of https://github.com/rust-lang-nursery/rust-clippy/issues/798
+    if x == "hello" {// Not collapsible
+        if y == "world" {
+            println!("Hello world!");
+        }
+    }
+
+    if x == "hello" { // Not collapsible
+        if y == "world" {
+            println!("Hello world!");
+        }
+    }
+
+    if x == "hello" {
+        // Not collapsible
+        if y == "world" {
+            println!("Hello world!");
+        }
+    }
+
+    if x == "hello" {
+        if y == "world" { // Collapsible
+            println!("Hello world!");
+        }
+    }
+
+    if x == "hello" {
+        print!("Hello ");
+    } else {
+        // Not collapsible
+        if y == "world" {
+            println!("world!")
+        }
+    }
+
+    if x == "hello" {
+        print!("Hello ");
+    } else {
+        // Not collapsible
+        if let Some(42) = Some(42) {
+            println!("world!")
+        }
+    }
+
+    if x == "hello" {
+        /* Not collapsible */
+        if y == "world" {
+            println!("Hello world!");
+        }
+    }
+
+    if x == "hello" { /* Not collapsible */
+        if y == "world" {
+            println!("Hello world!");
+        }
+    }
 }

tests/ui/collapsible_if.stderr

@@ -240,5 +240,21 @@ help: try
 122 | }
     |
 
-error: aborting due to 13 previous errors
+error: this if statement can be collapsed
+   --> $DIR/collapsible_if.rs:176:5
+    |
+176 | /     if x == "hello" {
+177 | |         if y == "world" { // Collapsible
+178 | |             println!("Hello world!");
+179 | |         }
+180 | |     }
+    | |_____^
+help: try
+    |
+176 |     if x == "hello" && y == "world" { // Collapsible
+177 |     println!("Hello world!");
+178 | }
+    |
+
+error: aborting due to 14 previous errors