New lint: decimal_bitwise_operands

Author: Artur-Sulej

CHANGELOG.md

@@ -6280,6 +6280,7 @@ Released 2018-09-13
 [`cyclomatic_complexity`]: https://rust-lang.github.io/rust-clippy/master/index.html#cyclomatic_complexity
 [`dbg_macro`]: https://rust-lang.github.io/rust-clippy/master/index.html#dbg_macro
 [`debug_assert_with_mut_call`]: https://rust-lang.github.io/rust-clippy/master/index.html#debug_assert_with_mut_call
+[`decimal_bitwise_operands`]: https://rust-lang.github.io/rust-clippy/master/index.html#decimal_bitwise_operands
 [`decimal_literal_representation`]: https://rust-lang.github.io/rust-clippy/master/index.html#decimal_literal_representation
 [`declare_interior_mutable_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#declare_interior_mutable_const
 [`default_constructed_unit_structs`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_constructed_unit_structs

clippy_lints/src/declared_lints.rs

@@ -578,6 +578,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
     crate::operators::ASSIGN_OP_PATTERN_INFO,
     crate::operators::BAD_BIT_MASK_INFO,
     crate::operators::CMP_OWNED_INFO,
+    crate::operators::DECIMAL_BITWISE_OPERANDS_INFO,
     crate::operators::DOUBLE_COMPARISONS_INFO,
     crate::operators::DURATION_SUBSEC_INFO,
     crate::operators::EQ_OP_INFO,

clippy_lints/src/operators/decimal_bitwise_operands.rs

@@ -0,0 +1,76 @@
+use clippy_utils::diagnostics::span_lint_and_help;
+use clippy_utils::numeric_literal;
+use clippy_utils::numeric_literal::NumericLiteral;
+use clippy_utils::source::SpanRangeExt;
+use rustc_ast::LitKind;
+use rustc_data_structures::packed::Pu128;
+use rustc_hir::{BinOpKind, Expr, ExprKind};
+use rustc_lint::LateContext;
+use rustc_span::Span;
+
+use super::DECIMAL_BITWISE_OPERANDS;
+
+pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, op: BinOpKind, left: &'tcx Expr<'_>, right: &'tcx Expr<'_>) {
+    if !matches!(op, BinOpKind::BitAnd | BinOpKind::BitOr | BinOpKind::BitXor) {
+        return;
+    }
+
+    for expr in [left, right] {
+        check_expr(cx, expr);
+    }
+}
+
+fn check_expr(cx: &LateContext<'_>, expr: &Expr<'_>) {
+    match &expr.kind {
+        ExprKind::Block(block, _) => {
+            if let Some(block_expr) = block.expr {
+                check_expr(cx, block_expr);
+            }
+        },
+        ExprKind::Cast(cast_expr, _) => {
+            check_expr(cx, cast_expr);
+        },
+        ExprKind::Unary(_, unary_expr) => {
+            check_expr(cx, unary_expr);
+        },
+        ExprKind::AddrOf(_, _, addr_of_expr) => {
+            check_expr(cx, addr_of_expr);
+        },
+        ExprKind::Lit(lit) => {
+            if let LitKind::Int(Pu128(val), _) = lit.node
+                && !is_single_digit(val)
+                && !is_power_of_twoish(val)
+                && let Some(src) = lit.span.get_source_text(cx)
+                && let Some(num_lit) = NumericLiteral::from_lit_kind(&src, &lit.node)
+                && num_lit.is_decimal()
+            {
+                emit_lint(cx, lit.span, num_lit.suffix, val);
+            }
+        },
+        _ => (),
+    }
+}
+
+fn is_power_of_twoish(val: u128) -> bool {
+    val.is_power_of_two() || val.wrapping_add(1).is_power_of_two()
+}
+
+fn is_single_digit(val: u128) -> bool {
+    val <= 9
+}
+
+fn emit_lint(cx: &LateContext<'_>, span: Span, suffix: Option<&str>, val: u128) {
+    span_lint_and_help(
+        cx,
+        DECIMAL_BITWISE_OPERANDS,
+        span,
+        "using decimal literal for bitwise operation",
+        None,
+        format!(
+            "use binary ({}), hex ({}), or octal ({}) notation for better readability",
+            numeric_literal::format(&format!("{val:#b}"), suffix, false),
+            numeric_literal::format(&format!("{val:#x}"), suffix, false),
+            numeric_literal::format(&format!("{val:#o}"), suffix, false),
+        ),
+    );
+}

clippy_lints/src/operators/mod.rs

@@ -3,6 +3,7 @@ mod assign_op_pattern;
 mod bit_mask;
 mod cmp_owned;
 mod const_comparisons;
+mod decimal_bitwise_operands;
 mod double_comparison;
 mod duration_subsec;
 mod eq_op;
@@ -935,6 +936,28 @@ declare_clippy_lint! {
     "use of disallowed default division and remainder operations"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for decimal literals used as bit masks in bitwise operations.
+    ///
+    /// ### Why is this bad?
+    /// Using decimal literals for bit masks can make the code less readable and obscure the intended bit pattern.
+    /// Binary, hexadecimal, or octal literals make the bit pattern more explicit and easier to understand at a glance.
+    ///
+    /// ### Example
+    /// ```rust,no_run
+    /// let a = 14 & 6; // Bit pattern is not immediately clear
+    /// ```
+    /// Use instead:
+    /// ```rust,no_run
+    /// let a = 0b1110 & 0b0110;
+    /// ```
+    #[clippy::version = "1.93.0"]
+    pub DECIMAL_BITWISE_OPERANDS,
+    pedantic,
+    "use binary, hex, or octal literals for bitwise operations"
+}
+
 pub struct Operators {
     arithmetic_context: numeric_arithmetic::Context,
     verbose_bit_mask_threshold: u64,
@@ -984,6 +1007,7 @@ impl_lint_pass!(Operators => [
     MANUAL_IS_MULTIPLE_OF,
     MANUAL_DIV_CEIL,
     INVALID_UPCAST_COMPARISONS,
+    DECIMAL_BITWISE_OPERANDS
 ]);
 
 impl<'tcx> LateLintPass<'tcx> for Operators {
@@ -1003,6 +1027,7 @@ impl<'tcx> LateLintPass<'tcx> for Operators {
                     needless_bitwise_bool::check(cx, e, op.node, lhs, rhs);
                     manual_midpoint::check(cx, e, op.node, lhs, rhs, self.msrv);
                     manual_is_multiple_of::check(cx, e, op.node, lhs, rhs, self.msrv);
+                    decimal_bitwise_operands::check(cx, op.node, lhs, rhs);
                 }
                 self.arithmetic_context.check_binary(cx, e, op.node, lhs, rhs);
                 bit_mask::check(cx, e, op.node, lhs, rhs);
@@ -1028,6 +1053,9 @@ impl<'tcx> LateLintPass<'tcx> for Operators {
             },
             ExprKind::AssignOp(op, lhs, rhs) => {
                 let bin_op = op.node.into();
+                if !e.span.from_expansion() {
+                    decimal_bitwise_operands::check(cx, bin_op, lhs, rhs);
+                }
                 self.arithmetic_context.check_binary(cx, e, bin_op, lhs, rhs);
                 misrefactored_assign_op::check(cx, e, bin_op, lhs, rhs);
                 modulo_arithmetic::check(cx, e, bin_op, lhs, rhs, false);

tests/ui/decimal_bitwise_operands.rs

@@ -0,0 +1,133 @@
+#![allow(
+    clippy::erasing_op,
+    clippy::no_effect,
+    clippy::unnecessary_operation,
+    clippy::unnecessary_cast,
+    clippy::op_ref
+)]
+#![warn(clippy::decimal_bitwise_operands)]
+
+macro_rules! bitwise_op {
+    ($x:expr, $y:expr) => {
+        $x & $y;
+    };
+}
+
+pub const SOME_CONST: i32 = 12345;
+
+fn main() {
+    let mut x = 0;
+    // BAD: Bitwise operation, decimal literal, one literal
+    x & 9_8765_4321; //~ decimal_bitwise_operands
+    x & 100_i32; //~ decimal_bitwise_operands
+    x | (/* comment */99); //~ decimal_bitwise_operands
+    x ^ (99); //~ decimal_bitwise_operands
+    x &= 99; //~ decimal_bitwise_operands
+    x |= { 99 }; //~ decimal_bitwise_operands
+    x |= { { 99 } }; //~ decimal_bitwise_operands
+    x |= {
+        0b1000;
+        99 //~ decimal_bitwise_operands
+    };
+    x ^= (99); //~ decimal_bitwise_operands
+
+    // BAD: Bitwise operation, decimal literal, two literals
+    0b1010 & 99; //~ decimal_bitwise_operands
+    0b1010 | (99); //~ decimal_bitwise_operands
+    0b1010 ^ (/* comment */99); //~ decimal_bitwise_operands
+    99 & 0b1010; //~ decimal_bitwise_operands
+    (99) | 0b1010; //~ decimal_bitwise_operands
+    (/* comment */99) ^ 0b1010; //~ decimal_bitwise_operands
+    0xD | { 99 }; //~ decimal_bitwise_operands
+    88 & 99;
+    //~^ decimal_bitwise_operands
+    //~| decimal_bitwise_operands
+    37 & 38 & 39;
+    //~^ decimal_bitwise_operands
+    //~| decimal_bitwise_operands
+    //~| decimal_bitwise_operands
+
+    // GOOD: Bitwise operation, binary/hex/octal literal, one literal
+    x & 0b1010;
+    x | 0b1010;
+    x ^ 0b1010;
+    x &= 0b1010;
+    x |= 0b1010;
+    x ^= 0b1010;
+    x & 0xD;
+    x & 0o77;
+    x | 0o123;
+    x ^ 0o377;
+    x &= 0o777;
+    x |= 0o7;
+    x ^= 0o70;
+
+    // GOOD: Bitwise operation, binary/hex/octal literal, two literals
+    0b1010 & 0b1101;
+    0xD ^ 0xF;
+    0o377 ^ 0o77;
+    0b1101 ^ 0xFF;
+
+    // GOOD: Numeric operation, any literal
+    x += 99;
+    x -= 0b1010;
+    x *= 0xD;
+    99 + 99;
+    0b1010 - 0b1101;
+    0xD * 0xD;
+
+    // BAD: Unary, cast and reference, decimal literal
+    x & !100; //~ decimal_bitwise_operands
+    x & -100; //~ decimal_bitwise_operands
+    x & (100 as i32); //~ decimal_bitwise_operands
+    x & &100; //~ decimal_bitwise_operands
+
+    // GOOD: Unary, cast and reference, non-decimal literal
+    x & !0b1101;
+    x & -0xD;
+    x & (0o333 as i32);
+    x & &0b1010;
+
+    // GOOD: Bitwise operation, variables only
+    let y = 0;
+    x & y;
+    x &= y;
+    x + y;
+    x += y;
+
+    // GOOD: Macro expansion (should be ignored)
+    bitwise_op!(x, 123);
+    bitwise_op!(0b1010, 123);
+
+    // GOOD: Using const (should be ignored)
+    x & SOME_CONST;
+    x |= SOME_CONST;
+
+    // GOOD: Parenthesized binary/hex literal (should not trigger lint)
+    x & (0b1111);
+    x |= (0b1010);
+    x ^ (/* comment */0b1100);
+    (0xFF) & x;
+
+    // GOOD: Power of two and power of two minus one
+    x & 16; // 2^4
+    x | (31); // 2^5 - 1
+    x ^ 0x40; // 2^6 (hex)
+    x ^= 7; // 2^3 - 1
+
+    // GOOD: Bitwise operation, single digit decimal literal
+    5 & 9;
+    x ^ 6;
+    x ^= 7;
+
+    // GOOD: More complex expressions
+    (x + 1) & 0xFF;
+    (x * 2) | (y & 0xF);
+    (x ^ y) & 0b11110000;
+    x | (1 << 9);
+
+    // GOOD: Special cases
+    x & 0; // All bits off
+    x | !0; // All bits on
+    x ^ 1; // Toggle LSB
+}