diff --git a/CHANGELOG.md b/CHANGELOG.md index 6cb2755be0ee..29da8b090ad2 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_bit_mask`]: https://rust-lang.github.io/rust-clippy/master/index.html#decimal_bit_mask [`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/decimal_bit_mask.rs b/clippy_lints/src/decimal_bit_mask.rs new file mode 100644 index 000000000000..62a44e8f7edc --- /dev/null +++ b/clippy_lints/src/decimal_bit_mask.rs @@ -0,0 +1,106 @@ +use clippy_utils::diagnostics::span_lint_and_help; +use clippy_utils::source::SpanRangeExt; +use rustc_data_structures::packed::Pu128; +use rustc_hir::{AssignOpKind, BinOpKind, Expr, ExprKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::declare_lint_pass; +use rustc_span::Span; +use rustc_span::source_map::Spanned; + +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 or 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.87.0"] + pub DECIMAL_BIT_MASK, + nursery, + "use binary, hex or octal literals for bitwise operations" +} + +declare_lint_pass!(DecimalBitMask => [DECIMAL_BIT_MASK]); + +fn check_bitwise_binary_expr(cx: &LateContext<'_>, e: &Expr<'_>) { + if let ExprKind::Binary( + Spanned { + node: BinOpKind::BitAnd | BinOpKind::BitOr | BinOpKind::BitXor, + .. + }, + left, + right, + ) = &e.kind + { + for expr in [left, right] { + if let ExprKind::Lit(_) = &expr.kind + && !is_not_decimal_number(cx, expr.span) + && !is_power_of_twoish(expr) + { + emit_lint(cx, expr.span); + } + } + } +} + +fn check_bitwise_assign_expr(cx: &LateContext<'_>, e: &Expr<'_>) { + if let ExprKind::AssignOp( + Spanned { + node: AssignOpKind::BitAndAssign | AssignOpKind::BitOrAssign | AssignOpKind::BitXorAssign, + .. + }, + _, + expr @ Expr { + kind: ExprKind::Lit(lit), + .. + }, + ) = &e.kind + && !is_power_of_twoish(expr) + && !is_not_decimal_number(cx, lit.span) + { + emit_lint(cx, lit.span); + } +} + +fn is_not_decimal_number(cx: &LateContext<'_>, span: Span) -> bool { + span.check_source_text(cx, |src| src.contains("0b") || src.contains("0x") || src.contains("0o")) +} + +fn is_power_of_twoish(expr: &Expr<'_>) -> bool { + if let ExprKind::Lit(lit) = &expr.kind + && let rustc_ast::LitKind::Int(Pu128(val), _) = lit.node + { + return val.is_power_of_two() || val.wrapping_add(1).is_power_of_two(); + } + false +} + +fn emit_lint(cx: &LateContext<'_>, span: Span) { + span_lint_and_help( + cx, + DECIMAL_BIT_MASK, + span, + "using decimal literal for bitwise operation", + None, + "use binary (0b...) or hex (0x...) or octal (0o...) notation for better readability", + ); +} + +impl<'tcx> LateLintPass<'tcx> for DecimalBitMask { + fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) { + if e.span.from_expansion() { + return; + } + check_bitwise_binary_expr(cx, e); + check_bitwise_assign_expr(cx, e); + } +} diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index a754eea31165..1ef4d725daf5 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -88,6 +88,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[ crate::crate_in_macro_def::CRATE_IN_MACRO_DEF_INFO, crate::create_dir::CREATE_DIR_INFO, crate::dbg_macro::DBG_MACRO_INFO, + crate::decimal_bit_mask::DECIMAL_BIT_MASK_INFO, crate::default::DEFAULT_TRAIT_ACCESS_INFO, crate::default::FIELD_REASSIGN_WITH_DEFAULT_INFO, crate::default_constructed_unit_structs::DEFAULT_CONSTRUCTED_UNIT_STRUCTS_INFO, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 033f85e70ef0..12205ff6926e 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -96,6 +96,7 @@ mod copy_iterator; mod crate_in_macro_def; mod create_dir; mod dbg_macro; +mod decimal_bit_mask; mod default; mod default_constructed_unit_structs; mod default_instead_of_iter_empty; @@ -816,6 +817,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co store.register_late_pass(|_| Box::new(cloned_ref_to_slice_refs::ClonedRefToSliceRefs::new(conf))); store.register_late_pass(|_| Box::new(infallible_try_from::InfallibleTryFrom)); store.register_late_pass(|_| Box::new(coerce_container_to_any::CoerceContainerToAny)); + store.register_late_pass(|_| Box::new(decimal_bit_mask::DecimalBitMask)); store.register_late_pass(|_| Box::new(toplevel_ref_arg::ToplevelRefArg)); store.register_late_pass(|_| Box::new(volatile_composites::VolatileComposites)); store.register_late_pass(|_| Box::new(replace_box::ReplaceBox::default())); diff --git a/tests/ui/decimal_bit_mask.rs b/tests/ui/decimal_bit_mask.rs new file mode 100644 index 000000000000..d0c22169b08c --- /dev/null +++ b/tests/ui/decimal_bit_mask.rs @@ -0,0 +1,101 @@ +#![allow(clippy::erasing_op)] +#![allow(clippy::no_effect)] +#![warn(clippy::decimal_bit_mask)] + +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 & 99; //~ decimal_bit_mask + x | (/* comment */99); //~ decimal_bit_mask + x ^ (99); //~ decimal_bit_mask + x &= 99; //~ decimal_bit_mask + x |= 99; //~ decimal_bit_mask + x ^= (99); //~ decimal_bit_mask + + // BAD: Bitwise operation, decimal literal, two literals + 0b1010 & 99; //~ decimal_bit_mask + 0b1010 | 99; //~ decimal_bit_mask + 0b1010 ^ 99; //~ decimal_bit_mask + 99 & 0b1010; //~ decimal_bit_mask + 99 | 0b1010; //~ decimal_bit_mask + 99 ^ 0b1010; //~ decimal_bit_mask + 0xD | 99; //~ decimal_bit_mask + 88 & 99; + //~^ decimal_bit_mask + //~| decimal_bit_mask + + // 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; + + // 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: 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_bit_mask.stderr b/tests/ui/decimal_bit_mask.stderr new file mode 100644 index 000000000000..233f81b4c72a --- /dev/null +++ b/tests/ui/decimal_bit_mask.stderr @@ -0,0 +1,124 @@ +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bit_mask.rs:16:9 + | +LL | x & 99; + | ^^ + | + = help: use binary (0b...) or hex (0x...) or octal (0o...) notation for better readability + = note: `-D clippy::decimal-bit-mask` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::decimal_bit_mask)]` + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bit_mask.rs:17:9 + | +LL | x | (/* comment */99); + | ^^^^^^^^^^^^^^^^^ + | + = help: use binary (0b...) or hex (0x...) or octal (0o...) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bit_mask.rs:18:9 + | +LL | x ^ (99); + | ^^^^ + | + = help: use binary (0b...) or hex (0x...) or octal (0o...) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bit_mask.rs:19:10 + | +LL | x &= 99; + | ^^ + | + = help: use binary (0b...) or hex (0x...) or octal (0o...) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bit_mask.rs:20:10 + | +LL | x |= 99; + | ^^ + | + = help: use binary (0b...) or hex (0x...) or octal (0o...) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bit_mask.rs:21:11 + | +LL | x ^= (99); + | ^^ + | + = help: use binary (0b...) or hex (0x...) or octal (0o...) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bit_mask.rs:24:14 + | +LL | 0b1010 & 99; + | ^^ + | + = help: use binary (0b...) or hex (0x...) or octal (0o...) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bit_mask.rs:25:14 + | +LL | 0b1010 | 99; + | ^^ + | + = help: use binary (0b...) or hex (0x...) or octal (0o...) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bit_mask.rs:26:14 + | +LL | 0b1010 ^ 99; + | ^^ + | + = help: use binary (0b...) or hex (0x...) or octal (0o...) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bit_mask.rs:27:5 + | +LL | 99 & 0b1010; + | ^^ + | + = help: use binary (0b...) or hex (0x...) or octal (0o...) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bit_mask.rs:28:5 + | +LL | 99 | 0b1010; + | ^^ + | + = help: use binary (0b...) or hex (0x...) or octal (0o...) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bit_mask.rs:29:5 + | +LL | 99 ^ 0b1010; + | ^^ + | + = help: use binary (0b...) or hex (0x...) or octal (0o...) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bit_mask.rs:30:11 + | +LL | 0xD | 99; + | ^^ + | + = help: use binary (0b...) or hex (0x...) or octal (0o...) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bit_mask.rs:31:5 + | +LL | 88 & 99; + | ^^ + | + = help: use binary (0b...) or hex (0x...) or octal (0o...) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bit_mask.rs:31:10 + | +LL | 88 & 99; + | ^^ + | + = help: use binary (0b...) or hex (0x...) or octal (0o...) notation for better readability + +error: aborting due to 15 previous errors + diff --git a/tests/ui/explicit_write_in_test.stderr b/tests/ui/explicit_write_in_test.stderr deleted file mode 100644 index e69de29bb2d1..000000000000