clippy_lints/src/needless_bool.rs

   1 // Copyright 2014-2018 The Rust Project Developers. See the COPYRIGHT
   2 // file at the top-level directory of this distribution.
   3 //
   4 // Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
   5 // http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
   6 // <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
   7 // option. This file may not be copied, modified, or distributed
   8 // except according to those terms.
   9
  10 //! Checks for needless boolean results of if-else expressions
  11 //!
  12 //! This lint is **warn** by default
  13
  14 use crate::rustc::hir::*;
  15 use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
  16 use crate::rustc::{declare_tool_lint, lint_array};
  17 use crate::rustc_errors::Applicability;
  18 use crate::syntax::ast::LitKind;
  19 use crate::syntax::source_map::Spanned;
  20 use crate::utils::sugg::Sugg;
  21 use crate::utils::{in_macro, span_lint, span_lint_and_sugg};
  22
  23 /// **What it does:** Checks for expressions of the form `if c { true } else {
  24 /// false }`
  25 /// (or vice versa) and suggest using the condition directly.
  26 ///
  27 /// **Why is this bad?** Redundant code.
  28 ///
  29 /// **Known problems:** Maybe false positives: Sometimes, the two branches are
  30 /// painstakingly documented (which we of course do not detect), so they *may*
  31 /// have some value. Even then, the documentation can be rewritten to match the
  32 /// shorter code.
  33 ///
  34 /// **Example:**
  35 /// ```rust
  36 /// if x {
  37 ///     false
  38 /// } else {
  39 ///     true
  40 /// }
  41 /// ```
  42 declare_clippy_lint! {
  43     pub NEEDLESS_BOOL,
  44     complexity,
  45     "if-statements with plain booleans in the then- and else-clause, e.g. `if p { true } else { false }`"
  46 }
  47
  48 /// **What it does:** Checks for expressions of the form `x == true` and
  49 /// `x != true` (or vice versa) and suggest using the variable directly.
  50 ///
  51 /// **Why is this bad?** Unnecessary code.
  52 ///
  53 /// **Known problems:** None.
  54 ///
  55 /// **Example:**
  56 /// ```rust
  57 /// if x == true {} // could be `if x { }`
  58 /// ```
  59 declare_clippy_lint! {
  60     pub BOOL_COMPARISON,
  61     complexity,
  62     "comparing a variable to a boolean, e.g. `if x == true` or `if x != true`"
  63 }
  64
  65 #[derive(Copy, Clone)]
  66 pub struct NeedlessBool;
  67
  68 impl LintPass for NeedlessBool {
  69     fn get_lints(&self) -> LintArray {
  70         lint_array!(NEEDLESS_BOOL)
  71     }
  72 }
  73
  74 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessBool {
  75     fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) {
  76         use self::Expression::*;
  77         if let ExprKind::If(ref pred, ref then_block, Some(ref else_expr)) = e.node {
  78             let reduce = |ret, not| {
  79                 let mut applicability = Applicability::MachineApplicable;
  80                 let snip = Sugg::hir_with_applicability(cx, pred, "<predicate>", &mut applicability);
  81                 let snip = if not { !snip } else { snip };
  82
  83                 let hint = if ret {
  84                     format!("return {}", snip)
  85                 } else {
  86                     snip.to_string()
  87                 };
  88
  89                 span_lint_and_sugg(
  90                     cx,
  91                     NEEDLESS_BOOL,
  92                     e.span,
  93                     "this if-then-else expression returns a bool literal",
  94                     "you can reduce it to",
  95                     hint,
  96                     applicability,
  97                 );
  98             };
  99             if let ExprKind::Block(ref then_block, _) = then_block.node {
 100                 match (fetch_bool_block(then_block), fetch_bool_expr(else_expr)) {
 101                     (RetBool(true), RetBool(true)) | (Bool(true), Bool(true)) => {
 102                         span_lint(
 103                             cx,
 104                             NEEDLESS_BOOL,
 105                             e.span,
 106                             "this if-then-else expression will always return true",
 107                         );
 108                     },
 109                     (RetBool(false), RetBool(false)) | (Bool(false), Bool(false)) => {
 110                         span_lint(
 111                             cx,
 112                             NEEDLESS_BOOL,
 113                             e.span,
 114                             "this if-then-else expression will always return false",
 115                         );
 116                     },
 117                     (RetBool(true), RetBool(false)) => reduce(true, false),
 118                     (Bool(true), Bool(false)) => reduce(false, false),
 119                     (RetBool(false), RetBool(true)) => reduce(true, true),
 120                     (Bool(false), Bool(true)) => reduce(false, true),
 121                     _ => (),
 122                 }
 123             } else {
 124                 panic!("IfExpr 'then' node is not an ExprKind::Block");
 125             }
 126         }
 127     }
 128 }
 129
 130 #[derive(Copy, Clone)]
 131 pub struct BoolComparison;
 132
 133 impl LintPass for BoolComparison {
 134     fn get_lints(&self) -> LintArray {
 135         lint_array!(BOOL_COMPARISON)
 136     }
 137 }
 138
 139 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BoolComparison {
 140     fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) {
 141         if in_macro(e.span) {
 142             return;
 143         }
 144
 145         if let ExprKind::Binary(Spanned { node, .. }, ..) = e.node {
 146             match node {
 147                 BinOpKind::Eq => check_comparison(
 148                     cx,
 149                     e,
 150                     "equality checks against true are unnecessary",
 151                     "equality checks against false can be replaced by a negation",
 152                     |h| h,
 153                     |h| !h,
 154                 ),
 155                 BinOpKind::Ne => check_comparison(
 156                     cx,
 157                     e,
 158                     "inequality checks against true can be replaced by a negation",
 159                     "inequality checks against false are unnecessary",
 160                     |h| !h,
 161                     |h| h,
 162                 ),
 163                 _ => (),
 164             }
 165         }
 166     }
 167 }
 168
 169 fn check_comparison<'a, 'tcx>(
 170     cx: &LateContext<'a, 'tcx>,
 171     e: &'tcx Expr,
 172     true_message: &str,
 173     false_message: &str,
 174     true_hint: impl FnOnce(Sugg<'_>) -> Sugg<'_>,
 175     false_hint: impl FnOnce(Sugg<'_>) -> Sugg<'_>,
 176 ) {
 177     use self::Expression::*;
 178
 179     if let ExprKind::Binary(_, ref left_side, ref right_side) = e.node {
 180         let applicability = Applicability::MachineApplicable;
 181         match (fetch_bool_expr(left_side), fetch_bool_expr(right_side)) {
 182             (Bool(true), Other) => suggest_bool_comparison(cx, e, right_side, applicability, true_message, true_hint),
 183             (Other, Bool(true)) => suggest_bool_comparison(cx, e, left_side, applicability, true_message, true_hint),
 184             (Bool(false), Other) => {
 185                 suggest_bool_comparison(cx, e, right_side, applicability, false_message, false_hint)
 186             },
 187             (Other, Bool(false)) => suggest_bool_comparison(cx, e, left_side, applicability, false_message, false_hint),
 188             _ => (),
 189         }
 190     }
 191 }
 192
 193 fn suggest_bool_comparison<'a, 'tcx>(
 194     cx: &LateContext<'a, 'tcx>,
 195     e: &'tcx Expr,
 196     expr: &Expr,
 197     mut applicability: Applicability,
 198     message: &str,
 199     conv_hint: impl FnOnce(Sugg<'_>) -> Sugg<'_>,
 200 ) {
 201     let hint = Sugg::hir_with_applicability(cx, expr, "..", &mut applicability);
 202     span_lint_and_sugg(
 203         cx,
 204         BOOL_COMPARISON,
 205         e.span,
 206         message,
 207         "try simplifying it as shown",
 208         conv_hint(hint).to_string(),
 209         applicability,
 210     );
 211 }
 212
 213 enum Expression {
 214     Bool(bool),
 215     RetBool(bool),
 216     Other,
 217 }
 218
 219 fn fetch_bool_block(block: &Block) -> Expression {
 220     match (&*block.stmts, block.expr.as_ref()) {
 221         (&[], Some(e)) => fetch_bool_expr(&**e),
 222         (&[ref e], None) => {
 223             if let StmtKind::Semi(ref e, _) = e.node {
 224                 if let ExprKind::Ret(_) = e.node {
 225                     fetch_bool_expr(&**e)
 226                 } else {
 227                     Expression::Other
 228                 }
 229             } else {
 230                 Expression::Other
 231             }
 232         },
 233         _ => Expression::Other,
 234     }
 235 }
 236
 237 fn fetch_bool_expr(expr: &Expr) -> Expression {
 238     match expr.node {
 239         ExprKind::Block(ref block, _) => fetch_bool_block(block),
 240         ExprKind::Lit(ref lit_ptr) => {
 241             if let LitKind::Bool(value) = lit_ptr.node {
 242                 Expression::Bool(value)
 243             } else {
 244                 Expression::Other
 245             }
 246         },
 247         ExprKind::Ret(Some(ref expr)) => match fetch_bool_expr(expr) {
 248             Expression::Bool(value) => Expression::RetBool(value),
 249             _ => Expression::Other,
 250         },
 251         _ => Expression::Other,
 252     }
 253 }