diff --git a/CHANGELOG.md b/CHANGELOG.md index 78b81b5b74d6..dc93753a2130 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 4a350dca2993..de7d01ce0d1b 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/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, diff --git a/clippy_lints/src/operators/decimal_bitwise_operands.rs b/clippy_lints/src/operators/decimal_bitwise_operands.rs new file mode 100644 index 000000000000..8511f2151342 --- /dev/null +++ b/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), + ), + ); +} diff --git a/clippy_lints/src/operators/mod.rs b/clippy_lints/src/operators/mod.rs index 8db2cc1d3f57..53b8e9e5d5ae 100644 --- a/clippy_lints/src/operators/mod.rs +++ b/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); diff --git a/tests/ui/decimal_bitwise_operands.rs b/tests/ui/decimal_bitwise_operands.rs new file mode 100644 index 000000000000..f1c053bb4358 --- /dev/null +++ b/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 +} diff --git a/tests/ui/decimal_bitwise_operands.stderr b/tests/ui/decimal_bitwise_operands.stderr new file mode 100644 index 000000000000..1b2b7bb71b69 --- /dev/null +++ b/tests/ui/decimal_bitwise_operands.stderr @@ -0,0 +1,204 @@ +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:21:9 + | +LL | x & 9_8765_4321; + | ^^^^^^^^^^^ + | + = help: use binary (0b11_1010_1101_1110_0110_1000_1011_0001), hex (0x3ade_68b1), or octal (0o7_267_464_261) notation for better readability + = note: `-D clippy::decimal-bitwise-operands` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::decimal_bitwise_operands)]` + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:22:9 + | +LL | x & 100_i32; + | ^^^^^^^ + | + = help: use binary (0b110_0100_i32), hex (0x0064_i32), or octal (0o144_i32) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:23:23 + | +LL | x | (/* comment */99); + | ^^ + | + = help: use binary (0b110_0011), hex (0x0063), or octal (0o143) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:24:10 + | +LL | x ^ (99); + | ^^ + | + = help: use binary (0b110_0011), hex (0x0063), or octal (0o143) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:25:10 + | +LL | x &= 99; + | ^^ + | + = help: use binary (0b110_0011), hex (0x0063), or octal (0o143) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:26:12 + | +LL | x |= { 99 }; + | ^^ + | + = help: use binary (0b110_0011), hex (0x0063), or octal (0o143) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:27:14 + | +LL | x |= { { 99 } }; + | ^^ + | + = help: use binary (0b110_0011), hex (0x0063), or octal (0o143) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:30:9 + | +LL | 99 + | ^^ + | + = help: use binary (0b110_0011), hex (0x0063), or octal (0o143) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:32:11 + | +LL | x ^= (99); + | ^^ + | + = help: use binary (0b110_0011), hex (0x0063), or octal (0o143) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:35:14 + | +LL | 0b1010 & 99; + | ^^ + | + = help: use binary (0b110_0011), hex (0x0063), or octal (0o143) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:36:15 + | +LL | 0b1010 | (99); + | ^^ + | + = help: use binary (0b110_0011), hex (0x0063), or octal (0o143) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:37:28 + | +LL | 0b1010 ^ (/* comment */99); + | ^^ + | + = help: use binary (0b110_0011), hex (0x0063), or octal (0o143) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:38:5 + | +LL | 99 & 0b1010; + | ^^ + | + = help: use binary (0b110_0011), hex (0x0063), or octal (0o143) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:39:6 + | +LL | (99) | 0b1010; + | ^^ + | + = help: use binary (0b110_0011), hex (0x0063), or octal (0o143) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:40:19 + | +LL | (/* comment */99) ^ 0b1010; + | ^^ + | + = help: use binary (0b110_0011), hex (0x0063), or octal (0o143) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:41:13 + | +LL | 0xD | { 99 }; + | ^^ + | + = help: use binary (0b110_0011), hex (0x0063), or octal (0o143) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:42:5 + | +LL | 88 & 99; + | ^^ + | + = help: use binary (0b101_1000), hex (0x0058), or octal (0o130) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:42:10 + | +LL | 88 & 99; + | ^^ + | + = help: use binary (0b110_0011), hex (0x0063), or octal (0o143) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:45:15 + | +LL | 37 & 38 & 39; + | ^^ + | + = help: use binary (0b10_0111), hex (0x0027), or octal (0o47) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:45:5 + | +LL | 37 & 38 & 39; + | ^^ + | + = help: use binary (0b10_0101), hex (0x0025), or octal (0o45) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:45:10 + | +LL | 37 & 38 & 39; + | ^^ + | + = help: use binary (0b10_0110), hex (0x0026), or octal (0o46) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:80:10 + | +LL | x & !100; + | ^^^ + | + = help: use binary (0b110_0100), hex (0x0064), or octal (0o144) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:81:10 + | +LL | x & -100; + | ^^^ + | + = help: use binary (0b110_0100), hex (0x0064), or octal (0o144) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:82:10 + | +LL | x & (100 as i32); + | ^^^ + | + = help: use binary (0b110_0100), hex (0x0064), or octal (0o144) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:83:10 + | +LL | x & &100; + | ^^^ + | + = help: use binary (0b110_0100), hex (0x0064), or octal (0o144) notation for better readability + +error: aborting due to 25 previous errors +