needless bool lint suggestion is wrapped in brackets if it is an "else" clause of an "if-else" statement

Author: Grzegorz Bartoszek

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;
 

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
+    };
+}

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