src/tools/clippy/clippy_lints/src/needless_bool.rs

   1 //! Checks for needless boolean results of if-else expressions
   2 //!
   3 //! This lint is **warn** by default
   4
   5 use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg};
   6 use clippy_utils::source::snippet_with_applicability;
   7 use clippy_utils::sugg::Sugg;
   8 use clippy_utils::{is_else_clause, is_expn_of};
   9 use rustc_ast::ast::LitKind;
  10 use rustc_errors::Applicability;
  11 use rustc_hir::{BinOpKind, Block, Expr, ExprKind, StmtKind, UnOp};
  12 use rustc_lint::{LateContext, LateLintPass};
  13 use rustc_session::{declare_lint_pass, declare_tool_lint};
  14 use rustc_span::source_map::Spanned;
  15 use rustc_span::Span;
  16
  17 declare_clippy_lint! {
  18     /// ### What it does
  19     /// Checks for expressions of the form `if c { true } else {
  20     /// false }` (or vice versa) and suggests using the condition directly.
  21     ///
  22     /// ### Why is this bad?
  23     /// Redundant code.
  24     ///
  25     /// ### Known problems
  26     /// Maybe false positives: Sometimes, the two branches are
  27     /// painstakingly documented (which we, of course, do not detect), so they *may*
  28     /// have some value. Even then, the documentation can be rewritten to match the
  29     /// shorter code.
  30     ///
  31     /// ### Example
  32     /// ```rust,ignore
  33     /// if x {
  34     ///     false
  35     /// } else {
  36     ///     true
  37     /// }
  38     /// ```
  39     /// Could be written as
  40     /// ```rust,ignore
  41     /// !x
  42     /// ```
  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 declare_clippy_lint! {
  49     /// ### What it does
  50     /// Checks for expressions of the form `x == true`,
  51     /// `x != true` and order comparisons such as `x < true` (or vice versa) and
  52     /// suggest using the variable directly.
  53     ///
  54     /// ### Why is this bad?
  55     /// Unnecessary code.
  56     ///
  57     /// ### Example
  58     /// ```rust,ignore
  59     /// if x == true {}
  60     /// if y == false {}
  61     /// ```
  62     /// use `x` directly:
  63     /// ```rust,ignore
  64     /// if x {}
  65     /// if !y {}
  66     /// ```
  67     pub BOOL_COMPARISON,
  68     complexity,
  69     "comparing a variable to a boolean, e.g., `if x == true` or `if x != true`"
  70 }
  71
  72 declare_lint_pass!(NeedlessBool => [NEEDLESS_BOOL]);
  73
  74 impl<'tcx> LateLintPass<'tcx> for NeedlessBool {
  75     fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
  76         use self::Expression::{Bool, RetBool};
  77         if e.span.from_expansion() {
  78             return;
  79         }
  80         if let ExprKind::If(pred, then_block, Some(else_expr)) = e.kind {
  81             let reduce = |ret, not| {
  82                 let mut applicability = Applicability::MachineApplicable;
  83                 let snip = Sugg::hir_with_applicability(cx, pred, "<predicate>", &mut applicability);
  84                 let mut snip = if not { !snip } else { snip };
  85
  86                 if ret {
  87                     snip = snip.make_return();
  88                 }
  89
  90                 if is_else_clause(cx.tcx, e) {
  91                     snip = snip.blockify();
  92                 }
  93
  94                 span_lint_and_sugg(
  95                     cx,
  96                     NEEDLESS_BOOL,
  97                     e.span,
  98                     "this if-then-else expression returns a bool literal",
  99                     "you can reduce it to",
 100                     snip.to_string(),
 101                     applicability,
 102                 );
 103             };
 104             if let ExprKind::Block(then_block, _) = then_block.kind {
 105                 match (fetch_bool_block(then_block), fetch_bool_expr(else_expr)) {
 106                     (RetBool(true), RetBool(true)) | (Bool(true), Bool(true)) => {
 107                         span_lint(
 108                             cx,
 109                             NEEDLESS_BOOL,
 110                             e.span,
 111                             "this if-then-else expression will always return true",
 112                         );
 113                     },
 114                     (RetBool(false), RetBool(false)) | (Bool(false), Bool(false)) => {
 115                         span_lint(
 116                             cx,
 117                             NEEDLESS_BOOL,
 118                             e.span,
 119                             "this if-then-else expression will always return false",
 120                         );
 121                     },
 122                     (RetBool(true), RetBool(false)) => reduce(true, false),
 123                     (Bool(true), Bool(false)) => reduce(false, false),
 124                     (RetBool(false), RetBool(true)) => reduce(true, true),
 125                     (Bool(false), Bool(true)) => reduce(false, true),
 126                     _ => (),
 127                 }
 128             } else {
 129                 panic!("IfExpr `then` node is not an `ExprKind::Block`");
 130             }
 131         }
 132     }
 133 }
 134
 135 declare_lint_pass!(BoolComparison => [BOOL_COMPARISON]);
 136
 137 impl<'tcx> LateLintPass<'tcx> for BoolComparison {
 138     fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
 139         if e.span.from_expansion() {
 140             return;
 141         }
 142
 143         if let ExprKind::Binary(Spanned { node, .. }, ..) = e.kind {
 144             let ignore_case = None::<(fn(_) -> _, &str)>;
 145             let ignore_no_literal = None::<(fn(_, _) -> _, &str)>;
 146             match node {
 147                 BinOpKind::Eq => {
 148                     let true_case = Some((|h| h, "equality checks against true are unnecessary"));
 149                     let false_case = Some((
 150                         |h: Sugg<'_>| !h,
 151                         "equality checks against false can be replaced by a negation",
 152                     ));
 153                     check_comparison(cx, e, true_case, false_case, true_case, false_case, ignore_no_literal);
 154                 },
 155                 BinOpKind::Ne => {
 156                     let true_case = Some((
 157                         |h: Sugg<'_>| !h,
 158                         "inequality checks against true can be replaced by a negation",
 159                     ));
 160                     let false_case = Some((|h| h, "inequality checks against false are unnecessary"));
 161                     check_comparison(cx, e, true_case, false_case, true_case, false_case, ignore_no_literal);
 162                 },
 163                 BinOpKind::Lt => check_comparison(
 164                     cx,
 165                     e,
 166                     ignore_case,
 167                     Some((|h| h, "greater than checks against false are unnecessary")),
 168                     Some((
 169                         |h: Sugg<'_>| !h,
 170                         "less than comparison against true can be replaced by a negation",
 171                     )),
 172                     ignore_case,
 173                     Some((
 174                         |l: Sugg<'_>, r: Sugg<'_>| (!l).bit_and(&r),
 175                         "order comparisons between booleans can be simplified",
 176                     )),
 177                 ),
 178                 BinOpKind::Gt => check_comparison(
 179                     cx,
 180                     e,
 181                     Some((
 182                         |h: Sugg<'_>| !h,
 183                         "less than comparison against true can be replaced by a negation",
 184                     )),
 185                     ignore_case,
 186                     ignore_case,
 187                     Some((|h| h, "greater than checks against false are unnecessary")),
 188                     Some((
 189                         |l: Sugg<'_>, r: Sugg<'_>| l.bit_and(&(!r)),
 190                         "order comparisons between booleans can be simplified",
 191                     )),
 192                 ),
 193                 _ => (),
 194             }
 195         }
 196     }
 197 }
 198
 199 struct ExpressionInfoWithSpan {
 200     one_side_is_unary_not: bool,
 201     left_span: Span,
 202     right_span: Span,
 203 }
 204
 205 fn is_unary_not(e: &Expr<'_>) -> (bool, Span) {
 206     if let ExprKind::Unary(UnOp::Not, operand) = e.kind {
 207         return (true, operand.span);
 208     }
 209     (false, e.span)
 210 }
 211
 212 fn one_side_is_unary_not<'tcx>(left_side: &'tcx Expr<'_>, right_side: &'tcx Expr<'_>) -> ExpressionInfoWithSpan {
 213     let left = is_unary_not(left_side);
 214     let right = is_unary_not(right_side);
 215
 216     ExpressionInfoWithSpan {
 217         one_side_is_unary_not: left.0 != right.0,
 218         left_span: left.1,
 219         right_span: right.1,
 220     }
 221 }
 222
 223 fn check_comparison<'a, 'tcx>(
 224     cx: &LateContext<'tcx>,
 225     e: &'tcx Expr<'_>,
 226     left_true: Option<(impl FnOnce(Sugg<'a>) -> Sugg<'a>, &str)>,
 227     left_false: Option<(impl FnOnce(Sugg<'a>) -> Sugg<'a>, &str)>,
 228     right_true: Option<(impl FnOnce(Sugg<'a>) -> Sugg<'a>, &str)>,
 229     right_false: Option<(impl FnOnce(Sugg<'a>) -> Sugg<'a>, &str)>,
 230     no_literal: Option<(impl FnOnce(Sugg<'a>, Sugg<'a>) -> Sugg<'a>, &str)>,
 231 ) {
 232     use self::Expression::{Bool, Other};
 233
 234     if let ExprKind::Binary(op, left_side, right_side) = e.kind {
 235         let (l_ty, r_ty) = (
 236             cx.typeck_results().expr_ty(left_side),
 237             cx.typeck_results().expr_ty(right_side),
 238         );
 239         if is_expn_of(left_side.span, "cfg").is_some() || is_expn_of(right_side.span, "cfg").is_some() {
 240             return;
 241         }
 242         if l_ty.is_bool() && r_ty.is_bool() {
 243             let mut applicability = Applicability::MachineApplicable;
 244
 245             if let BinOpKind::Eq = op.node {
 246                 let expression_info = one_side_is_unary_not(left_side, right_side);
 247                 if expression_info.one_side_is_unary_not {
 248                     span_lint_and_sugg(
 249                         cx,
 250                         BOOL_COMPARISON,
 251                         e.span,
 252                         "this comparison might be written more concisely",
 253                         "try simplifying it as shown",
 254                         format!(
 255                             "{} != {}",
 256                             snippet_with_applicability(cx, expression_info.left_span, "..", &mut applicability),
 257                             snippet_with_applicability(cx, expression_info.right_span, "..", &mut applicability)
 258                         ),
 259                         applicability,
 260                     );
 261                 }
 262             }
 263
 264             match (fetch_bool_expr(left_side), fetch_bool_expr(right_side)) {
 265                 (Bool(true), Other) => left_true.map_or((), |(h, m)| {
 266                     suggest_bool_comparison(cx, e, right_side, applicability, m, h);
 267                 }),
 268                 (Other, Bool(true)) => right_true.map_or((), |(h, m)| {
 269                     suggest_bool_comparison(cx, e, left_side, applicability, m, h);
 270                 }),
 271                 (Bool(false), Other) => left_false.map_or((), |(h, m)| {
 272                     suggest_bool_comparison(cx, e, right_side, applicability, m, h);
 273                 }),
 274                 (Other, Bool(false)) => right_false.map_or((), |(h, m)| {
 275                     suggest_bool_comparison(cx, e, left_side, applicability, m, h);
 276                 }),
 277                 (Other, Other) => no_literal.map_or((), |(h, m)| {
 278                     let left_side = Sugg::hir_with_applicability(cx, left_side, "..", &mut applicability);
 279                     let right_side = Sugg::hir_with_applicability(cx, right_side, "..", &mut applicability);
 280                     span_lint_and_sugg(
 281                         cx,
 282                         BOOL_COMPARISON,
 283                         e.span,
 284                         m,
 285                         "try simplifying it as shown",
 286                         h(left_side, right_side).to_string(),
 287                         applicability,
 288                     );
 289                 }),
 290                 _ => (),
 291             }
 292         }
 293     }
 294 }
 295
 296 fn suggest_bool_comparison<'a, 'tcx>(
 297     cx: &LateContext<'tcx>,
 298     e: &'tcx Expr<'_>,
 299     expr: &Expr<'_>,
 300     mut applicability: Applicability,
 301     message: &str,
 302     conv_hint: impl FnOnce(Sugg<'a>) -> Sugg<'a>,
 303 ) {
 304     let hint = if expr.span.from_expansion() {
 305         if applicability != Applicability::Unspecified {
 306             applicability = Applicability::MaybeIncorrect;
 307         }
 308         Sugg::hir_with_macro_callsite(cx, expr, "..")
 309     } else {
 310         Sugg::hir_with_applicability(cx, expr, "..", &mut applicability)
 311     };
 312     span_lint_and_sugg(
 313         cx,
 314         BOOL_COMPARISON,
 315         e.span,
 316         message,
 317         "try simplifying it as shown",
 318         conv_hint(hint).to_string(),
 319         applicability,
 320     );
 321 }
 322
 323 enum Expression {
 324     Bool(bool),
 325     RetBool(bool),
 326     Other,
 327 }
 328
 329 fn fetch_bool_block(block: &Block<'_>) -> Expression {
 330     match (&*block.stmts, block.expr.as_ref()) {
 331         (&[], Some(e)) => fetch_bool_expr(&**e),
 332         (&[ref e], None) => {
 333             if let StmtKind::Semi(e) = e.kind {
 334                 if let ExprKind::Ret(_) = e.kind {
 335                     fetch_bool_expr(e)
 336                 } else {
 337                     Expression::Other
 338                 }
 339             } else {
 340                 Expression::Other
 341             }
 342         },
 343         _ => Expression::Other,
 344     }
 345 }
 346
 347 fn fetch_bool_expr(expr: &Expr<'_>) -> Expression {
 348     match expr.kind {
 349         ExprKind::Block(block, _) => fetch_bool_block(block),
 350         ExprKind::Lit(ref lit_ptr) => {
 351             if let LitKind::Bool(value) = lit_ptr.node {
 352                 Expression::Bool(value)
 353             } else {
 354                 Expression::Other
 355             }
 356         },
 357         ExprKind::Ret(Some(expr)) => match fetch_bool_expr(expr) {
 358             Expression::Bool(value) => Expression::RetBool(value),
 359             _ => Expression::Other,
 360         },
 361         _ => Expression::Other,
 362     }
 363 }