about summary refs log tree commit diff
diff options
context:
space:
mode:
authorGrzegorz Bartoszek <grzegorz.bartoszek@thaumatec.com>2019-01-20 16:15:00 +0100
committerGrzegorz Bartoszek <grzegorz.bartoszek@thaumatec.com>2019-01-20 16:15:00 +0100
commitadce3ef96666d0f2e142980e293fe4e14fc17faf (patch)
tree4b3db9281706e623c8612211d834ed9ce8f098d5
parente648adf0866a1cea7db6ce2d33ea86e442f25377 (diff)
downloadrust-adce3ef96666d0f2e142980e293fe4e14fc17faf.tar.gz
rust-adce3ef96666d0f2e142980e293fe4e14fc17faf.zip
needless bool lint suggestion is wrapped in brackets if it is an "else" clause of an "if-else" statement
-rw-r--r--clippy_lints/src/needless_bool.rs20
-rw-r--r--tests/ui/needless_bool.rs13
-rw-r--r--tests/ui/needless_bool.stderr13
3 files changed, 44 insertions, 2 deletions
diff --git a/clippy_lints/src/needless_bool.rs b/clippy_lints/src/needless_bool.rs
index 1dfc3f6501e..38879fcda95 100644
--- a/clippy_lints/src/needless_bool.rs
+++ b/clippy_lints/src/needless_bool.rs
@@ -67,17 +67,22 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessBool {
     fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) {
         use self::Expression::*;
         if let ExprKind::If(ref pred, ref then_block, Some(ref else_expr)) = e.node {
+
             let reduce = |ret, not| {
                 let mut applicability = Applicability::MachineApplicable;
                 let snip = Sugg::hir_with_applicability(cx, pred, "<predicate>", &mut applicability);
                 let snip = if not { !snip } else { snip };
 
-                let hint = if ret {
+                let mut hint = if ret {
                     format!("return {}", snip)
                 } else {
                     snip.to_string()
                 };
 
+                if parent_node_is_if_expr(&e, &cx) {
+                    hint = format!("{{ {} }}", hint);
+                }
+
                 span_lint_and_sugg(
                     cx,
                     NEEDLESS_BOOL,
@@ -119,6 +124,19 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessBool {
     }
 }
 
+fn parent_node_is_if_expr<'a, 'b>(expr: &Expr, cx: &LateContext<'a, 'b>) -> bool {
+    let parent_id = cx.tcx.hir().get_parent_node(expr.id);
+    let parent_node = cx.tcx.hir().get(parent_id);
+
+    if let rustc::hir::Node::Expr(e) = parent_node {
+        if let ExprKind::If(_,_,_) = e.node {
+           return true;
+        }
+    }
+
+    false
+}
+
 #[derive(Copy, Clone)]
 pub struct BoolComparison;
 
diff --git a/tests/ui/needless_bool.rs b/tests/ui/needless_bool.rs
index 87493ab8c3d..75705525790 100644
--- a/tests/ui/needless_bool.rs
+++ b/tests/ui/needless_bool.rs
@@ -141,3 +141,16 @@ fn needless_bool3(x: bool) {
     if x == true {};
     if x == false {};
 }
+
+fn needless_bool_in_the_suggestion_wraps_the_predicate_of_if_else_statement_in_brackets() {
+    let b = false;
+    let returns_bool = || false;
+
+    let x = if b {
+        true
+    } else if returns_bool() {
+        false
+    } else {
+        true
+    };
+}
diff --git a/tests/ui/needless_bool.stderr b/tests/ui/needless_bool.stderr
index c829bf97dd2..46734ea07a5 100644
--- a/tests/ui/needless_bool.stderr
+++ b/tests/ui/needless_bool.stderr
@@ -136,5 +136,16 @@ error: equality checks against false can be replaced by a negation
 LL |     if x == false {};
    |        ^^^^^^^^^^ help: try simplifying it as shown: `!x`
 
-error: aborting due to 15 previous errors
+error: this if-then-else expression returns a bool literal
+  --> $DIR/needless_bool.rs:151:12
+   |
+LL |       } else if returns_bool() {
+   |  ____________^
+LL | |         false
+LL | |     } else {
+LL | |         true
+LL | |     };
+   | |_____^ help: you can reduce it to: `{ !returns_bool() }`
+
+error: aborting due to 16 previous errors