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::{get_parent_node, 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, HirId, Node, 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 fn condition_needs_parentheses(e: &Expr<'_>) -> bool {
  78     let mut inner = e;
  79     while let ExprKind::Binary(_, i, _)
  80     | ExprKind::Call(i, _)
  81     | ExprKind::Cast(i, _)
  82     | ExprKind::Type(i, _)
  83     | ExprKind::Index(i, _) = inner.kind
  84     {
  85         if matches!(
  86             i.kind,
  87             ExprKind::Block(..)
  88                 | ExprKind::ConstBlock(..)
  89                 | ExprKind::If(..)
  90                 | ExprKind::Loop(..)
  91                 | ExprKind::Match(..)
  92         ) {
  93             return true;
  94         }
  95         inner = i;
  96     }
  97     false
  98 }
  99
 100 fn is_parent_stmt(cx: &LateContext<'_>, id: HirId) -> bool {
 101     matches!(
 102         get_parent_node(cx.tcx, id),
 103         Some(Node::Stmt(..) | Node::Block(Block { stmts: &[], .. }))
 104     )
 105 }
 106
 107 impl<'tcx> LateLintPass<'tcx> for NeedlessBool {
 108     fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
 109         use self::Expression::{Bool, RetBool};
 110         if e.span.from_expansion() {
 111             return;
 112         }
 113         if let Some(higher::If {
 114             cond,
 115             then,
 116             r#else: Some(r#else),
 117         }) = higher::If::hir(e)
 118         {
 119             let reduce = |ret, not| {
 120                 let mut applicability = Applicability::MachineApplicable;
 121                 let snip = Sugg::hir_with_applicability(cx, cond, "<predicate>", &mut applicability);
 122                 let mut snip = if not { !snip } else { snip };
 123
 124                 if ret {
 125                     snip = snip.make_return();
 126                 }
 127
 128                 if is_else_clause(cx.tcx, e) {
 129                     snip = snip.blockify();
 130                 }
 131
 132                 if condition_needs_parentheses(cond) && is_parent_stmt(cx, e.hir_id) {
 133                     snip = snip.maybe_par();
 134                 }
 135
 136                 span_lint_and_sugg(
 137                     cx,
 138                     NEEDLESS_BOOL,
 139                     e.span,
 140                     "this if-then-else expression returns a bool literal",
 141                     "you can reduce it to",
 142                     snip.to_string(),
 143                     applicability,
 144                 );
 145             };
 146             if let ExprKind::Block(then, _) = then.kind {
 147                 match (fetch_bool_block(then), fetch_bool_expr(r#else)) {
 148                     (RetBool(true), RetBool(true)) | (Bool(true), Bool(true)) => {
 149                         span_lint(
 150                             cx,
 151                             NEEDLESS_BOOL,
 152                             e.span,
 153                             "this if-then-else expression will always return true",
 154                         );
 155                     },
 156                     (RetBool(false), RetBool(false)) | (Bool(false), Bool(false)) => {
 157                         span_lint(
 158                             cx,
 159                             NEEDLESS_BOOL,
 160                             e.span,
 161                             "this if-then-else expression will always return false",
 162                         );
 163                     },
 164                     (RetBool(true), RetBool(false)) => reduce(true, false),
 165                     (Bool(true), Bool(false)) => reduce(false, false),
 166                     (RetBool(false), RetBool(true)) => reduce(true, true),
 167                     (Bool(false), Bool(true)) => reduce(false, true),
 168                     _ => (),
 169                 }
 170             } else {
 171                 panic!("IfExpr `then` node is not an `ExprKind::Block`");
 172             }
 173         }
 174     }
 175 }
 176
 177 declare_lint_pass!(BoolComparison => [BOOL_COMPARISON]);
 178
 179 impl<'tcx> LateLintPass<'tcx> for BoolComparison {
 180     fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
 181         if e.span.from_expansion() {
 182             return;
 183         }
 184
 185         if let ExprKind::Binary(Spanned { node, .. }, ..) = e.kind {
 186             let ignore_case = None::<(fn(_) -> _, &str)>;
 187             let ignore_no_literal = None::<(fn(_, _) -> _, &str)>;
 188             match node {
 189                 BinOpKind::Eq => {
 190                     let true_case = Some((|h| h, "equality checks against true are unnecessary"));
 191                     let false_case = Some((
 192                         |h: Sugg<'_>| !h,
 193                         "equality checks against false can be replaced by a negation",
 194                     ));
 195                     check_comparison(cx, e, true_case, false_case, true_case, false_case, ignore_no_literal);
 196                 },
 197                 BinOpKind::Ne => {
 198                     let true_case = Some((
 199                         |h: Sugg<'_>| !h,
 200                         "inequality checks against true can be replaced by a negation",
 201                     ));
 202                     let false_case = Some((|h| h, "inequality checks against false are unnecessary"));
 203                     check_comparison(cx, e, true_case, false_case, true_case, false_case, ignore_no_literal);
 204                 },
 205                 BinOpKind::Lt => check_comparison(
 206                     cx,
 207                     e,
 208                     ignore_case,
 209                     Some((|h| h, "greater than checks against false are unnecessary")),
 210                     Some((
 211                         |h: Sugg<'_>| !h,
 212                         "less than comparison against true can be replaced by a negation",
 213                     )),
 214                     ignore_case,
 215                     Some((
 216                         |l: Sugg<'_>, r: Sugg<'_>| (!l).bit_and(&r),
 217                         "order comparisons between booleans can be simplified",
 218                     )),
 219                 ),
 220                 BinOpKind::Gt => check_comparison(
 221                     cx,
 222                     e,
 223                     Some((
 224                         |h: Sugg<'_>| !h,
 225                         "less than comparison against true can be replaced by a negation",
 226                     )),
 227                     ignore_case,
 228                     ignore_case,
 229                     Some((|h| h, "greater than checks against false are unnecessary")),
 230                     Some((
 231                         |l: Sugg<'_>, r: Sugg<'_>| l.bit_and(&(!r)),
 232                         "order comparisons between booleans can be simplified",
 233                     )),
 234                 ),
 235                 _ => (),
 236             }
 237         }
 238     }
 239 }
 240
 241 struct ExpressionInfoWithSpan {
 242     one_side_is_unary_not: bool,
 243     left_span: Span,
 244     right_span: Span,
 245 }
 246
 247 fn is_unary_not(e: &Expr<'_>) -> (bool, Span) {
 248     if let ExprKind::Unary(UnOp::Not, operand) = e.kind {
 249         return (true, operand.span);
 250     }
 251     (false, e.span)
 252 }
 253
 254 fn one_side_is_unary_not<'tcx>(left_side: &'tcx Expr<'_>, right_side: &'tcx Expr<'_>) -> ExpressionInfoWithSpan {
 255     let left = is_unary_not(left_side);
 256     let right = is_unary_not(right_side);
 257
 258     ExpressionInfoWithSpan {
 259         one_side_is_unary_not: left.0 != right.0,
 260         left_span: left.1,
 261         right_span: right.1,
 262     }
 263 }
 264
 265 fn check_comparison<'a, 'tcx>(
 266     cx: &LateContext<'tcx>,
 267     e: &'tcx Expr<'_>,
 268     left_true: Option<(impl FnOnce(Sugg<'a>) -> Sugg<'a>, &str)>,
 269     left_false: Option<(impl FnOnce(Sugg<'a>) -> Sugg<'a>, &str)>,
 270     right_true: Option<(impl FnOnce(Sugg<'a>) -> Sugg<'a>, &str)>,
 271     right_false: Option<(impl FnOnce(Sugg<'a>) -> Sugg<'a>, &str)>,
 272     no_literal: Option<(impl FnOnce(Sugg<'a>, Sugg<'a>) -> Sugg<'a>, &str)>,
 273 ) {
 274     use self::Expression::{Bool, Other};
 275
 276     if let ExprKind::Binary(op, left_side, right_side) = e.kind {
 277         let (l_ty, r_ty) = (
 278             cx.typeck_results().expr_ty(left_side),
 279             cx.typeck_results().expr_ty(right_side),
 280         );
 281         if is_expn_of(left_side.span, "cfg").is_some() || is_expn_of(right_side.span, "cfg").is_some() {
 282             return;
 283         }
 284         if l_ty.is_bool() && r_ty.is_bool() {
 285             let mut applicability = Applicability::MachineApplicable;
 286
 287             if op.node == BinOpKind::Eq {
 288                 let expression_info = one_side_is_unary_not(left_side, right_side);
 289                 if expression_info.one_side_is_unary_not {
 290                     span_lint_and_sugg(
 291                         cx,
 292                         BOOL_COMPARISON,
 293                         e.span,
 294                         "this comparison might be written more concisely",
 295                         "try simplifying it as shown",
 296                         format!(
 297                             "{} != {}",
 298                             snippet_with_applicability(cx, expression_info.left_span, "..", &mut applicability),
 299                             snippet_with_applicability(cx, expression_info.right_span, "..", &mut applicability)
 300                         ),
 301                         applicability,
 302                     );
 303                 }
 304             }
 305
 306             match (fetch_bool_expr(left_side), fetch_bool_expr(right_side)) {
 307                 (Bool(true), Other) => left_true.map_or((), |(h, m)| {
 308                     suggest_bool_comparison(cx, e, right_side, applicability, m, h);
 309                 }),
 310                 (Other, Bool(true)) => right_true.map_or((), |(h, m)| {
 311                     suggest_bool_comparison(cx, e, left_side, applicability, m, h);
 312                 }),
 313                 (Bool(false), Other) => left_false.map_or((), |(h, m)| {
 314                     suggest_bool_comparison(cx, e, right_side, applicability, m, h);
 315                 }),
 316                 (Other, Bool(false)) => right_false.map_or((), |(h, m)| {
 317                     suggest_bool_comparison(cx, e, left_side, applicability, m, h);
 318                 }),
 319                 (Other, Other) => no_literal.map_or((), |(h, m)| {
 320                     let left_side = Sugg::hir_with_applicability(cx, left_side, "..", &mut applicability);
 321                     let right_side = Sugg::hir_with_applicability(cx, right_side, "..", &mut applicability);
 322                     span_lint_and_sugg(
 323                         cx,
 324                         BOOL_COMPARISON,
 325                         e.span,
 326                         m,
 327                         "try simplifying it as shown",
 328                         h(left_side, right_side).to_string(),
 329                         applicability,
 330                     );
 331                 }),
 332                 _ => (),
 333             }
 334         }
 335     }
 336 }
 337
 338 fn suggest_bool_comparison<'a, 'tcx>(
 339     cx: &LateContext<'tcx>,
 340     e: &'tcx Expr<'_>,
 341     expr: &Expr<'_>,
 342     mut applicability: Applicability,
 343     message: &str,
 344     conv_hint: impl FnOnce(Sugg<'a>) -> Sugg<'a>,
 345 ) {
 346     let hint = if expr.span.from_expansion() {
 347         if applicability != Applicability::Unspecified {
 348             applicability = Applicability::MaybeIncorrect;
 349         }
 350         Sugg::hir_with_macro_callsite(cx, expr, "..")
 351     } else {
 352         Sugg::hir_with_applicability(cx, expr, "..", &mut applicability)
 353     };
 354     span_lint_and_sugg(
 355         cx,
 356         BOOL_COMPARISON,
 357         e.span,
 358         message,
 359         "try simplifying it as shown",
 360         conv_hint(hint).to_string(),
 361         applicability,
 362     );
 363 }
 364
 365 enum Expression {
 366     Bool(bool),
 367     RetBool(bool),
 368     Other,
 369 }
 370
 371 fn fetch_bool_block(block: &Block<'_>) -> Expression {
 372     match (&*block.stmts, block.expr.as_ref()) {
 373         (&[], Some(e)) => fetch_bool_expr(&**e),
 374         (&[ref e], None) => {
 375             if let StmtKind::Semi(e) = e.kind {
 376                 if let ExprKind::Ret(_) = e.kind {
 377                     fetch_bool_expr(e)
 378                 } else {
 379                     Expression::Other
 380                 }
 381             } else {
 382                 Expression::Other
 383             }
 384         },
 385         _ => Expression::Other,
 386     }
 387 }
 388
 389 fn fetch_bool_expr(expr: &Expr<'_>) -> Expression {
 390     match expr.kind {
 391         ExprKind::Block(block, _) => fetch_bool_block(block),
 392         ExprKind::Lit(ref lit_ptr) => {
 393             if let LitKind::Bool(value) = lit_ptr.node {
 394                 Expression::Bool(value)
 395             } else {
 396                 Expression::Other
 397             }
 398         },
 399         ExprKind::Ret(Some(expr)) => match fetch_bool_expr(expr) {
 400             Expression::Bool(value) => Expression::RetBool(value),
 401             _ => Expression::Other,
 402         },
 403         _ => Expression::Other,
 404     }
 405 }