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