Skip to content

Add needless_bitwise_bool lint #7133

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 17, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,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;
Expand Down Expand Up @@ -833,6 +834,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,
Expand Down Expand Up @@ -1018,6 +1020,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);
Expand Down Expand Up @@ -1392,6 +1395,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),
Expand Down
86 changes: 86 additions & 0 deletions clippy_lints/src/needless_bitwise_bool.rs
Original file line number Diff line number Diff line change
@@ -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);
}
},
);
}
}
}
40 changes: 40 additions & 0 deletions tests/ui/needless_bitwise_bool.fixed
Original file line number Diff line number Diff line change
@@ -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
}
}
40 changes: 40 additions & 0 deletions tests/ui/needless_bitwise_bool.rs
Original file line number Diff line number Diff line change
@@ -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
}
}
10 changes: 10 additions & 0 deletions tests/ui/needless_bitwise_bool.stderr
Original file line number Diff line number Diff line change
@@ -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