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`,
  49 /// `x != true` and order comparisons such as `x < true` (or vice versa) and
  50 /// suggest using the variable directly.
  51 ///
  52 /// **Why is this bad?** Unnecessary code.
  53 ///
  54 /// **Known problems:** None.
  55 ///
  56 /// **Example:**
  57 /// ```rust
  58 /// if x == true {} // could be `if x { }`
  59 /// ```
  60 declare_clippy_lint! {
  61     pub BOOL_COMPARISON,
  62     complexity,
  63     "comparing a variable to a boolean, e.g. `if x == true` or `if x != true`"
  64 }
  65
  66 #[derive(Copy, Clone)]
  67 pub struct NeedlessBool;
  68
  69 impl LintPass for NeedlessBool {
  70     fn get_lints(&self) -> LintArray {
  71         lint_array!(NEEDLESS_BOOL)
  72     }
  73 }
  74
  75 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessBool {
  76     fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) {
  77         use self::Expression::*;
  78         if let ExprKind::If(ref pred, ref then_block, Some(ref else_expr)) = e.node {
  79             let reduce = |ret, not| {
  80                 let mut applicability = Applicability::MachineApplicable;
  81                 let snip = Sugg::hir_with_applicability(cx, pred, "<predicate>", &mut applicability);
  82                 let snip = if not { !snip } else { snip };
  83
  84                 let hint = if ret {
  85                     format!("return {}", snip)
  86                 } else {
  87                     snip.to_string()
  88                 };
  89
  90                 span_lint_and_sugg(
  91                     cx,
  92                     NEEDLESS_BOOL,
  93                     e.span,
  94                     "this if-then-else expression returns a bool literal",
  95                     "you can reduce it to",
  96                     hint,
  97                     applicability,
  98                 );
  99             };
 100             if let ExprKind::Block(ref then_block, _) = then_block.node {
 101                 match (fetch_bool_block(then_block), fetch_bool_expr(else_expr)) {
 102                     (RetBool(true), RetBool(true)) | (Bool(true), Bool(true)) => {
 103                         span_lint(
 104                             cx,
 105                             NEEDLESS_BOOL,
 106                             e.span,
 107                             "this if-then-else expression will always return true",
 108                         );
 109                     },
 110                     (RetBool(false), RetBool(false)) | (Bool(false), Bool(false)) => {
 111                         span_lint(
 112                             cx,
 113                             NEEDLESS_BOOL,
 114                             e.span,
 115                             "this if-then-else expression will always return false",
 116                         );
 117                     },
 118                     (RetBool(true), RetBool(false)) => reduce(true, false),
 119                     (Bool(true), Bool(false)) => reduce(false, false),
 120                     (RetBool(false), RetBool(true)) => reduce(true, true),
 121                     (Bool(false), Bool(true)) => reduce(false, true),
 122                     _ => (),
 123                 }
 124             } else {
 125                 panic!("IfExpr 'then' node is not an ExprKind::Block");
 126             }
 127         }
 128     }
 129 }
 130
 131 #[derive(Copy, Clone)]
 132 pub struct BoolComparison;
 133
 134 impl LintPass for BoolComparison {
 135     fn get_lints(&self) -> LintArray {
 136         lint_array!(BOOL_COMPARISON)
 137     }
 138 }
 139
 140 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BoolComparison {
 141     fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) {
 142         if in_macro(e.span) {
 143             return;
 144         }
 145
 146         if let ExprKind::Binary(Spanned { node, .. }, ..) = e.node {
 147             let ignore_case = None::<(fn(_) -> _, &str)>;
 148             let ignore_no_literal = None::<(fn(_, _) -> _, &str)>;
 149             match node {
 150                 BinOpKind::Eq => {
 151                     let true_case = Some((|h| h, "equality checks against true are unnecessary"));
 152                     let false_case = Some((
 153                         |h: Sugg<'_>| !h,
 154                         "equality checks against false can be replaced by a negation",
 155                     ));
 156                     check_comparison(cx, e, true_case, false_case, true_case, false_case, ignore_no_literal)
 157                 },
 158                 BinOpKind::Ne => {
 159                     let true_case = Some((
 160                         |h: Sugg<'_>| !h,
 161                         "inequality checks against true can be replaced by a negation",
 162                     ));
 163                     let false_case = Some((|h| h, "inequality checks against false are unnecessary"));
 164                     check_comparison(cx, e, true_case, false_case, true_case, false_case, ignore_no_literal)
 165                 },
 166                 BinOpKind::Lt => check_comparison(
 167                     cx,
 168                     e,
 169                     ignore_case,
 170                     Some((|h| h, "greater than checks against false are unnecessary")),
 171                     Some((
 172                         |h: Sugg<'_>| !h,
 173                         "less than comparison against true can be replaced by a negation",
 174                     )),
 175                     ignore_case,
 176                     Some((
 177                         |l: Sugg<'_>, r: Sugg<'_>| (!l).bit_and(&r),
 178                         "order comparisons between booleans can be simplified",
 179                     )),
 180                 ),
 181                 BinOpKind::Gt => check_comparison(
 182                     cx,
 183                     e,
 184                     Some((
 185                         |h: Sugg<'_>| !h,
 186                         "less than comparison against true can be replaced by a negation",
 187                     )),
 188                     ignore_case,
 189                     ignore_case,
 190                     Some((|h| h, "greater than checks against false are unnecessary")),
 191                     Some((
 192                         |l: Sugg<'_>, r: Sugg<'_>| l.bit_and(&(!r)),
 193                         "order comparisons between booleans can be simplified",
 194                     )),
 195                 ),
 196                 _ => (),
 197             }
 198         }
 199     }
 200 }
 201
 202 fn check_comparison<'a, 'tcx>(
 203     cx: &LateContext<'a, 'tcx>,
 204     e: &'tcx Expr,
 205     left_true: Option<(impl FnOnce(Sugg<'a>) -> Sugg<'a>, &str)>,
 206     left_false: Option<(impl FnOnce(Sugg<'a>) -> Sugg<'a>, &str)>,
 207     right_true: Option<(impl FnOnce(Sugg<'a>) -> Sugg<'a>, &str)>,
 208     right_false: Option<(impl FnOnce(Sugg<'a>) -> Sugg<'a>, &str)>,
 209     no_literal: Option<(impl FnOnce(Sugg<'a>, Sugg<'a>) -> Sugg<'a>, &str)>,
 210 ) {
 211     use self::Expression::*;
 212
 213     if let ExprKind::Binary(_, ref left_side, ref right_side) = e.node {
 214         let mut applicability = Applicability::MachineApplicable;
 215         match (fetch_bool_expr(left_side), fetch_bool_expr(right_side)) {
 216             (Bool(true), Other) => left_true.map_or((), |(h, m)| {
 217                 suggest_bool_comparison(cx, e, right_side, applicability, m, h)
 218             }),
 219             (Other, Bool(true)) => right_true.map_or((), |(h, m)| {
 220                 suggest_bool_comparison(cx, e, left_side, applicability, m, h)
 221             }),
 222             (Bool(false), Other) => left_false.map_or((), |(h, m)| {
 223                 suggest_bool_comparison(cx, e, right_side, applicability, m, h)
 224             }),
 225             (Other, Bool(false)) => right_false.map_or((), |(h, m)| {
 226                 suggest_bool_comparison(cx, e, left_side, applicability, m, h)
 227             }),
 228             (Other, Other) => no_literal.map_or((), |(h, m)| {
 229                 let (l_ty, r_ty) = (cx.tables.expr_ty(left_side), cx.tables.expr_ty(right_side));
 230                 if l_ty.is_bool() && r_ty.is_bool() {
 231                     let left_side = Sugg::hir_with_applicability(cx, left_side, "..", &mut applicability);
 232                     let right_side = Sugg::hir_with_applicability(cx, right_side, "..", &mut applicability);
 233                     span_lint_and_sugg(
 234                         cx,
 235                         BOOL_COMPARISON,
 236                         e.span,
 237                         m,
 238                         "try simplifying it as shown",
 239                         h(left_side, right_side).to_string(),
 240                         applicability,
 241                     )
 242                 }
 243             }),
 244             _ => (),
 245         }
 246     }
 247 }
 248
 249 fn suggest_bool_comparison<'a, 'tcx>(
 250     cx: &LateContext<'a, 'tcx>,
 251     e: &'tcx Expr,
 252     expr: &Expr,
 253     mut applicability: Applicability,
 254     message: &str,
 255     conv_hint: impl FnOnce(Sugg<'a>) -> Sugg<'a>,
 256 ) {
 257     let hint = Sugg::hir_with_applicability(cx, expr, "..", &mut applicability);
 258     span_lint_and_sugg(
 259         cx,
 260         BOOL_COMPARISON,
 261         e.span,
 262         message,
 263         "try simplifying it as shown",
 264         conv_hint(hint).to_string(),
 265         applicability,
 266     );
 267 }
 268
 269 enum Expression {
 270     Bool(bool),
 271     RetBool(bool),
 272     Other,
 273 }
 274
 275 fn fetch_bool_block(block: &Block) -> Expression {
 276     match (&*block.stmts, block.expr.as_ref()) {
 277         (&[], Some(e)) => fetch_bool_expr(&**e),
 278         (&[ref e], None) => {
 279             if let StmtKind::Semi(ref e, _) = e.node {
 280                 if let ExprKind::Ret(_) = e.node {
 281                     fetch_bool_expr(&**e)
 282                 } else {
 283                     Expression::Other
 284                 }
 285             } else {
 286                 Expression::Other
 287             }
 288         },
 289         _ => Expression::Other,
 290     }
 291 }
 292
 293 fn fetch_bool_expr(expr: &Expr) -> Expression {
 294     match expr.node {
 295         ExprKind::Block(ref block, _) => fetch_bool_block(block),
 296         ExprKind::Lit(ref lit_ptr) => {
 297             if let LitKind::Bool(value) = lit_ptr.node {
 298                 Expression::Bool(value)
 299             } else {
 300                 Expression::Other
 301             }
 302         },
 303         ExprKind::Ret(Some(ref expr)) => match fetch_bool_expr(expr) {
 304             Expression::Bool(value) => Expression::RetBool(value),
 305             _ => Expression::Other,
 306         },
 307         _ => Expression::Other,
 308     }
 309 }