Lintcheck changes for 9cb6306

This comment will be updated if you push new changes

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?

r? @llogiq

rustbot has assigned @llogiq.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot note remove Feature-freeze

Could you please change the PR description to something like this?

"""
Add a new lint that detects the use of decimal literals as bit masks in bitwise operations. Using decimal literals for bit masks can obscure the intended bit pattern and reduce code readability. This lint encourages the use of binary (0b...) or hexadecimal (0x...) notation to make bit patterns explicit and easier to understand at a glance.

Example:

let masked = x & 15; // Bad: decimal literal as bit mask
let masked = x & 0b1111; // Good: bit pattern is explicit

changelog: [decimal_bit_mask]: new lint

Fixes #1775
"""

Motivation for the changes:

• the changelog message should fit on one line
• by using "Fixes", you make it so that when this PR is merged, the corresponding issue is automatically closed

If you look at Lintcheck results (displayed over at #15215 (comment)), you can see a couple of problems with the current implementation:

warning: Using decimal literal for bit mask. Consider using binary (0b...) or hexadecimal (0x...) notation for better readability.
   --> target/lintcheck/sources/sha2-0.11.0-pre.3/src/sha256/x86.rs:100:1
    |
100 | cpufeatures::new!(shani_cpuid, "sha", "sse2", "ssse3", "sse4.1");
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: `--force-warn clippy::decimal-bit-mask` implied by `--force-warn clippy::nursery`
    = note: this warning originates in the macro `$crate::__detect_target_features` which comes from the expansion of the macro `cpufeatures::new` (in Nightly builds, run with -Z macro-backtrace for more info)

Here, the lint fires on some binary operation coming deep from an expansion of cpufeatures::new!. Typically, you want to avoid linting things coming from expansion -- please use !e.span.from_expansion() to filter those out. See https://doc.rust-lang.org/clippy/development/macro_expansions.html#the-spanfrom_expansion-method for more information

warning: Using decimal literal for bit mask. Consider using binary (0b...) or hexadecimal (0x...) notation for better readability.
    --> target/lintcheck/sources/chrono-0.4.38/src/naive/date/mod.rs:1344:9
     |
1344 |         self.yof() & (0b1000) == 0
     |         ^^^^^^^^^^^^^^^^^^^^^

Here, 0b1000 isn't recognized as being a binary literal, because the way you check for that is by looking at the span of the literal, but here the span actually contains the parentheses surrounding the literal. Because of that, you'll need to first trim starting paren(s), then possibly some whitespace, and only then performing the actual checks.

warning: Using decimal literal for bit mask. Consider using binary (0b...) or hexadecimal (0x...) notation for better readability.
   --> target/lintcheck/sources/ahash-0.8.11/src/fallback_hash.rs:197:19
    |
197 |         let rot = (self.buffer & 63) as u32;
    |                   ^^^^^^^^^^^^^^^^^^
    |
    = note: `--force-warn clippy::decimal-bit-mask` implied by `--force-warn clippy::nursery`

Here, and in a couple more cases, the decimal operand is a power of 2, or (power of 2 - 1). I think it'd be appropriate to give them a pass. To get the value of a literal, you can go

if let ExprKind::Lit(lit) = left
    && let LitKind::Int(val, _) = lit

with val being your value.

Note that this will also include operands 0 and 1, which is desirable.

warning: Using decimal literal for bit mask. Consider using binary (0b...) or hexadecimal (0x...) notation for better readability.
   --> target/lintcheck/sources/libc-0.2.155/src/unix/linux_like/linux/gnu/b64/x86_64/mod.rs:416:32
    |
416 | pub const O_TMPFILE: ::c_int = 0o20000000 | O_DIRECTORY;
    |                                ^^^^^^^^^^^^^^^^^^^^^^^^

Octal operands probably also make sense in some contexts, so I'd give them a pass as well.

@ada4a Thank you for the very thorough review and the clear explanations of all the details! I’ve addressed all the issues, and the PR is ready for another round of review.

All comments addressed, ready for review.

I addressed the comment and updated the lint message to group digits with underscores. I used numeric_literal::format, which picks a reasonable digit group size for each radix.

Additional changes:

• Reused the existing logic for detecting decimals (NumericLiteral), instead of custom parsing
• Extended handling of bitwise operations to cover unary and cast expressions

@llogiq Sorry to hear that, good luck with everything!

@llogiq @ada4a Commits squashed and rebased. Thanks for the review!

@samueltardieu Thanks for your comments. I've addressed them – plus I added handling a literal with a reference (example: x ^ &100).

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

Alright. Can you please squash your commits?

🔥