diff --git a/clippy_lints/src/collapsible_if.rs b/clippy_lints/src/collapsible_if.rs index 2921bc2769c..350a3cc3ef2 100644 --- a/clippy_lints/src/collapsible_if.rs +++ b/clippy_lints/src/collapsible_if.rs @@ -13,11 +13,10 @@ //! This lint is **warn** by default use rustc::lint::*; -use std::borrow::Cow; -use syntax::codemap::Spanned; use syntax::ast; -use utils::{in_macro, snippet, snippet_block, span_lint_and_then}; +use utils::{in_macro, snippet_block, span_lint_and_then}; +use utils::sugg::Sugg; /// **What it does:** This lint checks for nested `if`-statements which can be collapsed by /// `&&`-combining their conditions and for `else { if .. }` expressions that can be collapsed to @@ -103,31 +102,17 @@ fn check_collapsible_no_if_let( return; } span_lint_and_then(cx, COLLAPSIBLE_IF, expr.span, "this if statement can be collapsed", |db| { + let lhs = Sugg::ast(cx, check, ".."); + let rhs = Sugg::ast(cx, check_inner, ".."); db.span_suggestion(expr.span, "try", - format!("if {} && {} {}", - check_to_string(cx, check), - check_to_string(cx, check_inner), + format!("if {} {}", + lhs.and(&rhs), snippet_block(cx, content.span, ".."))); }); }} } -fn requires_brackets(e: &ast::Expr) -> bool { - match e.node { - ast::ExprKind::Binary(Spanned { node: n, .. }, _, _) if n == ast::BinOpKind::Eq => false, - _ => true, - } -} - -fn check_to_string(cx: &EarlyContext, e: &ast::Expr) -> Cow<'static, str> { - if requires_brackets(e) { - format!("({})", snippet(cx, e.span, "..")).into() - } else { - snippet(cx, e.span, "..") - } -} - fn single_stmt_of_block(block: &ast::Block) -> Option<&ast::Expr> { if block.stmts.len() == 1 && block.expr.is_none() { if let ast::StmtKind::Expr(ref expr, _) = block.stmts[0].node { diff --git a/tests/compile-fail/collapsible_if.rs b/tests/compile-fail/collapsible_if.rs index ea2ef284f38..f7e6f15b121 100644 --- a/tests/compile-fail/collapsible_if.rs +++ b/tests/compile-fail/collapsible_if.rs @@ -23,6 +23,42 @@ fn main() { } } + 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!"); + } + } + + 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!"); + } + } + + 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!"); + } + } + + if 42 == 1337 { + //~^ ERROR this if statement can be collapsed + //~| HELP try + //~| SUGGESTION if 42 == 1337 && 'a' != 'A' { + if 'a' != 'A' { + println!("world!") + } + } + // Collaspe `else { if .. }` to `else if ..` if x == "hello" { print!("Hello ");