Check for comments in collapsible ifs

3 files changed, 71 insertions, 1 deletions

diff --git a/clippy_lints/src/collapsible_if.rs b/clippy_lints/src/collapsible_if.rs
index 85fdca1d421..67ef1048299 100644
--- a/clippy_lints/src/collapsible_if.rs
+++ b/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 {
+    // The zeroth character in the trimmed block text is "{", which marks the beginning of the block.
+    // Therefore, we check if the first string after that is a comment, i.e. starts with //.
+    let trimmed_block_text = snippet_block(cx, expr.span, "..").trim_left().to_owned();
+    trimmed_block_text[1..trimmed_block_text.len()].trim_left().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 {
diff --git a/tests/ui/collapsible_if.rs b/tests/ui/collapsible_if.rs
index a6df9109df9..c186d9e577f 100644
--- a/tests/ui/collapsible_if.rs
+++ b/tests/ui/collapsible_if.rs
@@ -151,4 +151,49 @@ 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!")
+        }
+    }
 }
diff --git a/tests/ui/collapsible_if.stderr b/tests/ui/collapsible_if.stderr
index 87c279cd725..3f06dca5495 100644
--- a/tests/ui/collapsible_if.stderr
+++ b/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