Warn about C-style octal literals

[GrigorenkoPV]

Uplifts clippy::zero_prefixed_literal as leading_zeros_in_decimal_literals, warn-by-default.

---

The leading_zeros_in_decimal_literals lint detects decimal integral literals with leading zeros.

Example

fn is_executable(unix_mode: u32) -> bool {
    unix_mode & 0111 != 0

Explanation

In some languages (including the infamous C language and most of its family), a leading zero marks an octal constant. In Rust however, a 0o prefix is used instead.
Thus, a leading zero can be confusing for both the writer and a reader.

In Rust:

fn main() {
    let a = 0123;
    println!("{}", a);
}

prints 123, while in C:

#include <stdio.h>

int main() {
    int a = 0123;
    printf("%d\n", a);
}

prints 83 (as 83 == 0o123 while 123 == 0o173).

---

Notes

Compared to the Clippy lint, this will not warn if the literal is unambiguous, i.e. it has 8's or 9's in it (clearly not octal) or it is <10 (in this case the value does not depend on the base).

Closes #33448
r? @ghost

[GrigorenkoPV]

@rustbot ready

r? @Urgau

[rustbot]

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

[rustbot]

The Miri subtree was changed

cc @rust-lang/miri

[Urgau]

This looks fine to me. Let's nominate it for T-lang fcp (which could unfortunatly take months, due to T-lang backlog).

I've taken the liberty to adjust the PR description to include the lint description as well, in order to clearly see what the lint does. Feel free to update it if you update the one in the PR.

@rustbot labels -S-waiting-on-review +S-waiting-on-team +I-lang-nominated

[nikomatsakis]

I feel that targeting this lint specifically at unix permission bits would make sense -- something like `0[0-7][0-7][0-7]`.

…

On Tue, Jan 14, 2025, at 1:53 AM, Josh Triplett wrote: @traviscross <https://github.com/traviscross> I can appreciate that. On the other hand, UNIX is the gift that keeps on giving, and octal lives on in permission bits, file modes, and (as a consequence) Git repository internals, among other places. Because of that, octal remains *just* common enough to be a potential hazard. If the consequence is that we need to write one `allow` if we actually *want* to use leading zeroes (or switch to using leading spaces with a `rustfmt::skip`), that seems reasonable. The failure mode of "my personal style produces a warning and I need to change it or add an `allow`" seems less hazardous than the failure mode of "my permissions are wrong in a way that's extremely difficult to spot when looking at the code". — Reply to this email directly, view it on GitHub <#131309 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABF4ZSB7CZ2Q35Y2HFEJ2L2KSX6JAVCNFSM6AAAAABPNXCQFGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKOBZGE3DCMRVGQ>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[nikomatsakis]

To be honest you could probably go even narrower, like `0[75310][75310][75310]` or something. I feel like the vast majority of bits are something like `0755` and not arbitrary combinations. I think users would appreciate a narrowly tailored lint that can explain exactly why it is triggering.

…

On Tue, Jan 14, 2025, at 5:24 AM, Niko Matsakis wrote: I feel that targeting this lint specifically at unix permission bits would make sense -- something like `0[0-7][0-7][0-7]`. On Tue, Jan 14, 2025, at 1:53 AM, Josh Triplett wrote: > > > @traviscross <https://github.com/traviscross> I can appreciate that. On the other hand, UNIX is the gift that keeps on giving, and octal lives on in permission bits, file modes, and (as a consequence) Git repository internals, among other places. Because of that, octal remains *just* common enough to be a potential hazard. > > If the consequence is that we need to write one `allow` if we actually *want* to use leading zeroes (or switch to using leading spaces with a `rustfmt::skip`), that seems reasonable. The failure mode of "my personal style produces a warning and I need to change it or add an `allow`" seems less hazardous than the failure mode of "my permissions are wrong in a way that's extremely difficult to spot when looking at the code". > > > > — > Reply to this email directly, view it on GitHub <#131309 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABF4ZSB7CZ2Q35Y2HFEJ2L2KSX6JAVCNFSM6AAAAABPNXCQFGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKOBZGE3DCMRVGQ>. > You are receiving this because you were mentioned.Message ID: ***@***.***> > >

[joshtriplett]

I feel that targeting this lint specifically at unix permission bits would make sense -- something like 0[0-7][0-7][0-7].

077 is also likely to be permissions, and 0100644 is likely to be a file mode.

[joshtriplett]

So, proposal:

I think we should do two separate FCPs here. First, let's do an FCP for approving the lint, as allow-by-default. Then, separately, we can work on consensus for making it warn.

[nikomatsakis]

I'm ok with the lint, but as an alternative, I think a lint described as detecting "unix file permissions and modes" (vs any use of leading 0) might be a better framing that would be more likely to be warn-by-default.

…

On Tue, Jan 14, 2025, at 8:26 AM, Josh Triplett wrote: So, proposal: I think we should do two separate FCPs here. First, let's do an FCP for approving the lint, as allow-by-default. Then, separately, we can work on consensus for making it warn. — Reply to this email directly, view it on GitHub <#131309 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABF4ZSNDG4B7WVZDH7NPZT2KUGBLAVCNFSM6AAAAABPNXCQFGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKOBZHEYDONJYGQ>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[joshtriplett]

Could @rust-lang/lang folks who don't object to having the lint as allow-by-default please check a box at #131309 (comment) ? When that's done we can do a separate PR and FCP for whether to warn by default, either with the current lint or one with additional scope reduction to specific number lengths.

[GrigorenkoPV]

Does an allow-by-default rustc lint has better discoverability than a warn-by-default clippy lint?

[nikomatsakis]

@rfcbot reviewed

[nikomatsakis]

@rfcbot concern allow-by-default

Per @joshtriplett's comment, the FCP has changed to allow-by-default, but the OP on the PR (and afaik the impl) is still warn-by-default. Filing this concern until those are updated.

[nikomatsakis]

Regarding the lint, I think my ideal would probably be to have this lint as allow-by-default and a more narrowly targeted one as allow-by-default, with a "subtyping" relationship -- I guess this means a leading_zero lint group or something?

[bors]

☔ The latest upstream changes (presumably #136070) made this pull request unmergeable. Please resolve the merge conflicts.

[bors]

☔ The latest upstream changes (presumably #127541) made this pull request unmergeable. Please resolve the merge conflicts.

[bors]

☔ The latest upstream changes (presumably #137348) made this pull request unmergeable. Please resolve the merge conflicts.

[bors]

☔ The latest upstream changes (presumably #137752) made this pull request unmergeable. Please resolve the merge conflicts.