From fc5eac58948d5a93304bd2eaa8b566de7018e408 Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Wed, 1 Jun 2022 10:12:26 -0400 Subject: [PATCH] Move `NeedlessBitwiseBool` into `Operators` lint pass --- clippy_lints/src/lib.register_lints.rs | 2 +- clippy_lints/src/lib.register_pedantic.rs | 2 +- clippy_lints/src/lib.rs | 2 - clippy_lints/src/needless_bitwise_bool.rs | 85 ------------------- clippy_lints/src/operators/mod.rs | 32 +++++++ .../src/operators/needless_bitwise_bool.rs | 36 ++++++++ 6 files changed, 70 insertions(+), 89 deletions(-) delete mode 100644 clippy_lints/src/needless_bitwise_bool.rs create mode 100644 clippy_lints/src/operators/needless_bitwise_bool.rs diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index fea98c08e4a..d006aa4bce5 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -387,7 +387,6 @@ 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_borrowed_ref::NEEDLESS_BORROWED_REFERENCE, @@ -435,6 +434,7 @@ operators::MISREFACTORED_ASSIGN_OP, operators::MODULO_ARITHMETIC, operators::MODULO_ONE, + operators::NEEDLESS_BITWISE_BOOL, operators::OP_REF, operators::VERBOSE_BIT_MASK, option_env_unwrap::OPTION_ENV_UNWRAP, diff --git a/clippy_lints/src/lib.register_pedantic.rs b/clippy_lints/src/lib.register_pedantic.rs index 0c02c2bef56..4d4b89687d0 100644 --- a/clippy_lints/src/lib.register_pedantic.rs +++ b/clippy_lints/src/lib.register_pedantic.rs @@ -67,7 +67,6 @@ LintId::of(misc::USED_UNDERSCORE_BINDING), LintId::of(mismatching_type_param_order::MISMATCHING_TYPE_PARAM_ORDER), 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), @@ -75,6 +74,7 @@ LintId::of(non_expressive_names::MANY_SINGLE_CHAR_NAMES), LintId::of(non_expressive_names::SIMILAR_NAMES), LintId::of(operators::FLOAT_CMP), + LintId::of(operators::NEEDLESS_BITWISE_BOOL), LintId::of(operators::VERBOSE_BIT_MASK), LintId::of(pass_by_ref_or_value::LARGE_TYPES_PASSED_BY_VALUE), LintId::of(pass_by_ref_or_value::TRIVIALLY_COPY_PASS_BY_REF), diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 724674471f2..42d4abcde43 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -302,7 +302,6 @@ macro_rules! declare_clippy_lint { mod mutable_debug_assertion; mod mutex_atomic; mod needless_arbitrary_self_type; -mod needless_bitwise_bool; mod needless_bool; mod needless_borrowed_ref; mod needless_continue; @@ -569,7 +568,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: )) }); store.register_late_pass(|| Box::new(booleans::NonminimalBool)); - store.register_late_pass(|| Box::new(needless_bitwise_bool::NeedlessBitwiseBool)); store.register_late_pass(|| Box::new(enum_clike::UnportableVariant)); store.register_late_pass(|| Box::new(float_literal::FloatLiteral)); store.register_late_pass(|| Box::new(ptr::Ptr)); diff --git a/clippy_lints/src/needless_bitwise_bool.rs b/clippy_lints/src/needless_bitwise_bool.rs deleted file mode 100644 index 623d22bc9bd..00000000000 --- a/clippy_lints/src/needless_bitwise_bool.rs +++ /dev/null @@ -1,85 +0,0 @@ -use clippy_utils::diagnostics::span_lint_and_then; -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 {} - /// ``` - #[clippy::version = "1.54.0"] - 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 !expr.span.from_expansion(); - 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 suggestion_snippet(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option { - 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) = suggestion_snippet(cx, expr) { - diag.span_suggestion(expr.span, "try", sugg, Applicability::MachineApplicable); - } - }, - ); - } - } -} diff --git a/clippy_lints/src/operators/mod.rs b/clippy_lints/src/operators/mod.rs index 7db38f0fcfb..0c5b8f3da6a 100644 --- a/clippy_lints/src/operators/mod.rs +++ b/clippy_lints/src/operators/mod.rs @@ -18,6 +18,7 @@ mod misrefactored_assign_op; mod modulo_arithmetic; mod modulo_one; +mod needless_bitwise_bool; mod numeric_arithmetic; mod op_ref; mod verbose_bit_mask; @@ -641,6 +642,35 @@ "any modulo arithmetic statement" } +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 {} + /// ``` + #[clippy::version = "1.54.0"] + pub NEEDLESS_BITWISE_BOOL, + pedantic, + "Boolean expressions that use bitwise rather than lazy operators" +} + pub struct Operators { arithmetic_context: numeric_arithmetic::Context, verbose_bit_mask_threshold: u64, @@ -668,6 +698,7 @@ pub struct Operators { FLOAT_CMP_CONST, MODULO_ONE, MODULO_ARITHMETIC, + NEEDLESS_BITWISE_BOOL, ]); impl Operators { pub fn new(verbose_bit_mask_threshold: u64) -> Self { @@ -690,6 +721,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) { } erasing_op::check(cx, e, op.node, lhs, rhs); identity_op::check(cx, e, op.node, lhs, rhs); + needless_bitwise_bool::check(cx, e, op.node, lhs, rhs); } self.arithmetic_context.check_binary(cx, e, op.node, lhs, rhs); bit_mask::check(cx, e, op.node, lhs, rhs); diff --git a/clippy_lints/src/operators/needless_bitwise_bool.rs b/clippy_lints/src/operators/needless_bitwise_bool.rs new file mode 100644 index 00000000000..e902235a014 --- /dev/null +++ b/clippy_lints/src/operators/needless_bitwise_bool.rs @@ -0,0 +1,36 @@ +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::source::snippet_opt; +use rustc_errors::Applicability; +use rustc_hir::{BinOpKind, Expr, ExprKind}; +use rustc_lint::LateContext; + +use super::NEEDLESS_BITWISE_BOOL; + +pub(super) fn check(cx: &LateContext<'_>, e: &Expr<'_>, op: BinOpKind, lhs: &Expr<'_>, rhs: &Expr<'_>) { + let op_str = match op { + BinOpKind::BitAnd => "&&", + BinOpKind::BitOr => "||", + _ => return, + }; + if matches!( + rhs.kind, + ExprKind::Call(..) | ExprKind::MethodCall(..) | ExprKind::Binary(..) | ExprKind::Unary(..) + ) && cx.typeck_results().expr_ty(e).is_bool() + && !rhs.can_have_side_effects() + { + span_lint_and_then( + cx, + NEEDLESS_BITWISE_BOOL, + e.span, + "use of bitwise operator instead of lazy operator between booleans", + |diag| { + if let Some(lhs_snip) = snippet_opt(cx, lhs.span) + && let Some(rhs_snip) = snippet_opt(cx, rhs.span) + { + let sugg = format!("{} {} {}", lhs_snip, op_str, rhs_snip); + diag.span_suggestion(e.span, "try", sugg, Applicability::MachineApplicable); + } + }, + ); + } +}