Auto merge of #7133 - arya-k:master, r=llogiq

Add `needless_bitwise_bool` lint

fixes #6827
fixes #1594

changelog: Add ``[`needless_bitwise_bool`]`` lint

Creates a new `bitwise_bool` lint to convert `x & y` to `x && y` when both `x` and `y` are booleans. I also had to adjust thh `needless_bool` lint slightly, and fix a couple failing dogfood tests. I made it a correctness lint as per flip1995's comment [here](rust-lang/rust-clippy#3385 (comment)), from a previous WIP attempt at this lint.

Author: bors

CHANGELOG.md

@@ -2549,6 +2549,7 @@ Released 2018-09-13
 [`mutex_integer`]: https://rust-lang.github.io/rust-clippy/master/index.html#mutex_integer
 [`naive_bytecount`]: https://rust-lang.github.io/rust-clippy/master/index.html#naive_bytecount
 [`needless_arbitrary_self_type`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_arbitrary_self_type
+[`needless_bitwise_bool`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_bitwise_bool
 [`needless_bool`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_bool
 [`needless_borrow`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow
 [`needless_borrowed_reference`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrowed_reference

clippy_lints/src/lib.rs

@@ -290,6 +290,7 @@ mod mut_reference;
 mod mutable_debug_assertion;
 mod mutex_atomic;
 mod needless_arbitrary_self_type;
+mod needless_bitwise_bool;
 mod needless_bool;
 mod needless_borrow;
 mod needless_borrowed_ref;
@@ -836,6 +837,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         mutex_atomic::MUTEX_ATOMIC,
         mutex_atomic::MUTEX_INTEGER,
         needless_arbitrary_self_type::NEEDLESS_ARBITRARY_SELF_TYPE,
+        needless_bitwise_bool::NEEDLESS_BITWISE_BOOL,
         needless_bool::BOOL_COMPARISON,
         needless_bool::NEEDLESS_BOOL,
         needless_borrow::NEEDLESS_BORROW,
@@ -1022,6 +1024,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     let type_complexity_threshold = conf.type_complexity_threshold;
     store.register_late_pass(move || box types::Types::new(vec_box_size_threshold, type_complexity_threshold));
     store.register_late_pass(|| box booleans::NonminimalBool);
+    store.register_late_pass(|| box needless_bitwise_bool::NeedlessBitwiseBool);
     store.register_late_pass(|| box eq_op::EqOp);
     store.register_late_pass(|| box enum_clike::UnportableVariant);
     store.register_late_pass(|| box float_literal::FloatLiteral);
@@ -1397,6 +1400,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(misc::USED_UNDERSCORE_BINDING),
         LintId::of(misc_early::UNSEPARATED_LITERAL_SUFFIX),
         LintId::of(mut_mut::MUT_MUT),
+        LintId::of(needless_bitwise_bool::NEEDLESS_BITWISE_BOOL),
         LintId::of(needless_continue::NEEDLESS_CONTINUE),
         LintId::of(needless_for_each::NEEDLESS_FOR_EACH),
         LintId::of(needless_pass_by_value::NEEDLESS_PASS_BY_VALUE),

clippy_lints/src/needless_bitwise_bool.rs

@@ -0,0 +1,86 @@
+use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::in_macro;
+use clippy_utils::source::snippet_opt;
+use if_chain::if_chain;
+use rustc_errors::Applicability;
+use rustc_hir::{BinOpKind, Expr, ExprKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty;
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+
+declare_clippy_lint! {
+    /// **What it does:**
+    /// Checks for uses of bitwise and/or operators between booleans, where performance may be improved by using
+    /// a lazy and.
+    ///
+    /// **Why is this bad?**
+    /// The bitwise operators do not support short-circuiting, so it may hinder code performance.
+    /// Additionally, boolean logic "masked" as bitwise logic is not caught by lints like `unnecessary_fold`
+    ///
+    /// **Known problems:**
+    /// This lint evaluates only when the right side is determined to have no side effects. At this time, that
+    /// determination is quite conservative.
+    ///
+    /// **Example:**
+    ///
+    /// ```rust
+    /// let (x,y) = (true, false);
+    /// if x & !y {} // where both x and y are booleans
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// let (x,y) = (true, false);
+    /// if x && !y {}
+    /// ```
+    pub NEEDLESS_BITWISE_BOOL,
+    pedantic,
+    "Boolean expressions that use bitwise rather than lazy operators"
+}
+
+declare_lint_pass!(NeedlessBitwiseBool => [NEEDLESS_BITWISE_BOOL]);
+
+fn is_bitwise_operation(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
+    let ty = cx.typeck_results().expr_ty(expr);
+    if_chain! {
+        if !in_macro(expr.span);
+        if let (&ExprKind::Binary(ref op, _, right), &ty::Bool) = (&expr.kind, &ty.kind());
+        if op.node == BinOpKind::BitAnd || op.node == BinOpKind::BitOr;
+        if let ExprKind::Call(..) | ExprKind::MethodCall(..) | ExprKind::Binary(..) | ExprKind::Unary(..) = right.kind;
+        if !right.can_have_side_effects();
+        then {
+            return true;
+        }
+    }
+    false
+}
+
+fn suggession_snippet(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<String> {
+    if let ExprKind::Binary(ref op, left, right) = expr.kind {
+        if let (Some(l_snippet), Some(r_snippet)) = (snippet_opt(cx, left.span), snippet_opt(cx, right.span)) {
+            let op_snippet = match op.node {
+                BinOpKind::BitAnd => "&&",
+                _ => "||",
+            };
+            return Some(format!("{} {} {}", l_snippet, op_snippet, r_snippet));
+        }
+    }
+    None
+}
+
+impl LateLintPass<'_> for NeedlessBitwiseBool {
+    fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
+        if is_bitwise_operation(cx, expr) {
+            span_lint_and_then(
+                cx,
+                NEEDLESS_BITWISE_BOOL,
+                expr.span,
+                "use of bitwise operator instead of lazy operator between booleans",
+                |diag| {
+                    if let Some(sugg) = suggession_snippet(cx, expr) {
+                        diag.span_suggestion(expr.span, "try", sugg, Applicability::MachineApplicable);
+                    }
+                },
+            );
+        }
+    }
+}

tests/ui/needless_bitwise_bool.fixed

@@ -0,0 +1,40 @@
+// run-rustfix
+
+#![warn(clippy::needless_bitwise_bool)]
+
+fn returns_bool() -> bool {
+    true
+}
+
+const fn const_returns_bool() -> bool {
+    false
+}
+
+fn main() {
+    let (x, y) = (false, true);
+    if x & y {
+        println!("true")
+    }
+    if returns_bool() & x {
+        println!("true")
+    }
+    if !returns_bool() & returns_bool() {
+        println!("true")
+    }
+    if y && !x {
+        println!("true")
+    }
+
+    // BELOW: lints we hope to catch as `Expr::can_have_side_effects` improves.
+    if y & !const_returns_bool() {
+        println!("true") // This is a const function, in an UnOp
+    }
+
+    if y & "abcD".is_empty() {
+        println!("true") // This is a const method call
+    }
+
+    if y & (0 < 1) {
+        println!("true") // This is a BinOp with no side effects
+    }
+}

tests/ui/needless_bitwise_bool.rs

@@ -0,0 +1,40 @@
+// run-rustfix
+
+#![warn(clippy::needless_bitwise_bool)]
+
+fn returns_bool() -> bool {
+    true
+}
+
+const fn const_returns_bool() -> bool {
+    false
+}
+
+fn main() {
+    let (x, y) = (false, true);
+    if x & y {
+        println!("true")
+    }
+    if returns_bool() & x {
+        println!("true")
+    }
+    if !returns_bool() & returns_bool() {
+        println!("true")
+    }
+    if y & !x {
+        println!("true")
+    }
+
+    // BELOW: lints we hope to catch as `Expr::can_have_side_effects` improves.
+    if y & !const_returns_bool() {
+        println!("true") // This is a const function, in an UnOp
+    }
+
+    if y & "abcD".is_empty() {
+        println!("true") // This is a const method call
+    }
+
+    if y & (0 < 1) {
+        println!("true") // This is a BinOp with no side effects
+    }
+}

tests/ui/needless_bitwise_bool.stderr

@@ -0,0 +1,10 @@
+error: use of bitwise operator instead of lazy operator between booleans
+  --> $DIR/needless_bitwise_bool.rs:24:8
+   |
+LL |     if y & !x {
+   |        ^^^^^^ help: try: `y && !x`
+   |
+   = note: `-D clippy::needless-bitwise-bool` implied by `-D warnings`
+
+error: aborting due to previous error
+