Skip to content
Open
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 @@ -5894,6 +5894,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
Expand Down
103 changes: 103 additions & 0 deletions clippy_lints/src/decimal_bit_mask.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
use clippy_utils::diagnostics::span_lint;
use clippy_utils::source::snippet_opt;
use rustc_hir::*;
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::declare_lint_pass;
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 literals make the bit pattern more explicit and easier to understand at a glance.
///
/// ### Example
/// ```rust,no_run
/// let a = 15 & 6; // Bit pattern is not immediately clear
/// ```
/// Use instead:
/// ```rust,no_run
/// let a = 0b1111 & 0b0110;
/// ```
#[clippy::version = "1.87.0"]
pub DECIMAL_BIT_MASK,
nursery,
"default lint description"
}

declare_lint_pass!(DecimalBitMask => [DECIMAL_BIT_MASK]);

impl<'tcx> LateLintPass<'tcx> for DecimalBitMask {
fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
if let ExprKind::Binary(
Spanned {
node: BinOpKind::BitAnd | BinOpKind::BitOr | BinOpKind::BitXor,
..
},
Expr {
kind: kind1,
span: span1,
..
},
Expr {
kind: kind2,
span: span2,
..
},
) = &e.kind
Comment on lines +39 to +49
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be a bit more concise to have these simply as left and right, and then use left.span, right.kind etc. throughout the code

{
if let ExprKind::Lit(_) = kind1
&& let Some(snippet) = snippet_opt(cx, *span1)
&& !snippet.starts_with("0b")
&& !snippet.starts_with("0x")
{
span_lint(
cx,
DECIMAL_BIT_MASK,
e.span,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can improve the output by pointing the lint at the exact operand that is decimal -- for example, here you would use span1 (or left.span, as per previous comment)

"Using decimal literal for bit mask. Consider using binary (0b...) or hexadecimal (0x...) notation for better readability.",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The second sentence should be emitted separately, as a help message -- see https://doc.rust-lang.org/clippy/development/emitting_lints.html#emitting-a-lint-1 for more info.

Try using span_lint_and_help here

);
}

if let ExprKind::Lit(_) = kind2
&& let Some(snippet) = snippet_opt(cx, *span2)
&& !snippet.starts_with("0b")
&& !snippet.starts_with("0x")
Comment on lines +65 to +67
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use SpanRangeExt::get_source_code here to avoid allocating a String (which is what snippet_opt does)

                && let Some(snippet) = span2.get_source_text(cx)

In fact, since you're only using the snippet to check its contents, you can get away with SpanRangeExt::check_source_code:

Suggested change
&& let Some(snippet) = snippet_opt(cx, *span2)
&& !snippet.starts_with("0b")
&& !snippet.starts_with("0x")
&& span2.check_source_text(cx, |src| !src.starts_with("0b") && !src.starts_with("0x"))

{
span_lint(
cx,
DECIMAL_BIT_MASK,
e.span,
"Using decimal literal for bit mask. Consider using binary (0b...) or hexadecimal (0x...) notation for better readability.",
);
}
}
if let ExprKind::AssignOp(
Spanned {
node: AssignOpKind::BitAndAssign | AssignOpKind::BitOrAssign | AssignOpKind::BitXorAssign,
..
},
_,
Expr {
kind: ExprKind::Lit(_),
span,
..
},
) = &e.kind
{
if let Some(snippet) = snippet_opt(cx, *span)
&& !snippet.starts_with("0b")
&& !snippet.starts_with("0x")
{
span_lint(
cx,
DECIMAL_BIT_MASK,
e.span,
"Using decimal literal for bit mask. Consider using binary (0b...) or hexadecimal (0x...) notation for better readability.",
);
}
}
}
}
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,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,
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,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;
Expand Down Expand Up @@ -830,6 +831,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));
// add lints here, do not remove this comment, it's used in `new_lint`
}
50 changes: 50 additions & 0 deletions tests/ui/decimal_bit_mask.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
#![allow(clippy::no_effect)]
#![warn(clippy::decimal_bit_mask)]
fn main() {
let mut x = 0;
// BAD: Bitwise operation, decimal literal, one literal
x & 99; //~ decimal_bit_mask
x | 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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although I annotated my test with //~ decimal_bit_mask, Clippy still reports the lint as an unmatched diagnostic and the test fails.
Any idea what I might be missing?

That's because in this case, you're firing the lint twice, once for each operand, so you'll need to add a second error mark. That's the easiest to do by bringing them to under the linted expression:

Suggested change
88 & 99; //~ decimal_bit_mask
88 & 99;
//~^ decimal_bit_mask
//~| decimal_bit_mask
  • ^ means the lint should happen on the line above the comment
  • | "attaches" the error mark to another one on a neighbouring line (in this case the //~^ decimal_bit_mask directly above), and will expect a lint at the same location (in this case, the line containing 88 & 99;)


// GOOD: Bitwise operation, binary/hex literal, one literal
x & 0b1010;
x | 0b1010;
x ^ 0b1010;
x &= 0b1010;
x |= 0b1010;
x ^= 0b1010;
x & 0xD;

// GOOD: Bitwise operation, binary/hex literal, two literals
0b1010 & 0b1101;
0xD ^ 0xF;

// GOOD: Numeric operations, any literal
x += 99;
x -= 0b1010;
x *= 0xD;
99 + 99;
0b1010 - 0b1101;
0xD * 0xD;

// GOOD: Bitwise operations, variables only
let y = 0;
x & y;
x &= y;
x + y;
x += y;
}
97 changes: 97 additions & 0 deletions tests/ui/decimal_bit_mask.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
error: Using decimal literal for bit mask. Consider using binary (0b...) or hexadecimal (0x...) notation for better readability.
--> tests/ui/decimal_bit_mask.rs:6:5
|
LL | x & 99;
| ^^^^^^
|
= 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 bit mask. Consider using binary (0b...) or hexadecimal (0x...) notation for better readability.
--> tests/ui/decimal_bit_mask.rs:7:5
|
LL | x | 99;
| ^^^^^^

error: Using decimal literal for bit mask. Consider using binary (0b...) or hexadecimal (0x...) notation for better readability.
--> tests/ui/decimal_bit_mask.rs:8:5
|
LL | x ^ 99;
| ^^^^^^

error: Using decimal literal for bit mask. Consider using binary (0b...) or hexadecimal (0x...) notation for better readability.
--> tests/ui/decimal_bit_mask.rs:9:5
|
LL | x &= 99;
| ^^^^^^^

error: Using decimal literal for bit mask. Consider using binary (0b...) or hexadecimal (0x...) notation for better readability.
--> tests/ui/decimal_bit_mask.rs:10:5
|
LL | x |= 99;
| ^^^^^^^

error: Using decimal literal for bit mask. Consider using binary (0b...) or hexadecimal (0x...) notation for better readability.
--> tests/ui/decimal_bit_mask.rs:11:5
|
LL | x ^= 99;
| ^^^^^^^

error: Using decimal literal for bit mask. Consider using binary (0b...) or hexadecimal (0x...) notation for better readability.
--> tests/ui/decimal_bit_mask.rs:14:5
|
LL | 0b1010 & 99;
| ^^^^^^^^^^^

error: Using decimal literal for bit mask. Consider using binary (0b...) or hexadecimal (0x...) notation for better readability.
--> tests/ui/decimal_bit_mask.rs:15:5
|
LL | 0b1010 | 99;
| ^^^^^^^^^^^

error: Using decimal literal for bit mask. Consider using binary (0b...) or hexadecimal (0x...) notation for better readability.
--> tests/ui/decimal_bit_mask.rs:16:5
|
LL | 0b1010 ^ 99;
| ^^^^^^^^^^^

error: Using decimal literal for bit mask. Consider using binary (0b...) or hexadecimal (0x...) notation for better readability.
--> tests/ui/decimal_bit_mask.rs:17:5
|
LL | 99 & 0b1010;
| ^^^^^^^^^^^

error: Using decimal literal for bit mask. Consider using binary (0b...) or hexadecimal (0x...) notation for better readability.
--> tests/ui/decimal_bit_mask.rs:18:5
|
LL | 99 | 0b1010;
| ^^^^^^^^^^^

error: Using decimal literal for bit mask. Consider using binary (0b...) or hexadecimal (0x...) notation for better readability.
--> tests/ui/decimal_bit_mask.rs:19:5
|
LL | 99 ^ 0b1010;
| ^^^^^^^^^^^

error: Using decimal literal for bit mask. Consider using binary (0b...) or hexadecimal (0x...) notation for better readability.
--> tests/ui/decimal_bit_mask.rs:20:5
|
LL | 0xD | 99;
| ^^^^^^^^

error: Using decimal literal for bit mask. Consider using binary (0b...) or hexadecimal (0x...) notation for better readability.
--> tests/ui/decimal_bit_mask.rs:21:5
|
LL | 88 & 99;
| ^^^^^^^

error: Using decimal literal for bit mask. Consider using binary (0b...) or hexadecimal (0x...) notation for better readability.
--> tests/ui/decimal_bit_mask.rs:21:5
|
LL | 88 & 99;
| ^^^^^^^
|
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`

error: aborting due to 15 previous errors

Loading