diff --git a/clippy_lints/src/collapsible_if.rs b/clippy_lints/src/collapsible_if.rs index 85fdca1d4213..a55ca04f706a 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 { + // 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 { diff --git a/tests/ui/collapsible_if.rs b/tests/ui/collapsible_if.rs index a6df9109df92..1bc866010fd1 100644 --- a/tests/ui/collapsible_if.rs +++ b/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!"); + } + } } diff --git a/tests/ui/collapsible_if.stderr b/tests/ui/collapsible_if.stderr index 87c279cd7252..3f06dca54952 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