rust-lang / rust-clippy

A bunch of lints to catch common mistakes and improve your Rust code. Book: https://doc.rust-lang.org/clippy/
https://rust-lang.github.io/rust-clippy/
Other
11.1k stars 1.49k forks source link

Inconsistent underscores false positive? #6836

Open gilescope opened 3 years ago

gilescope commented 3 years ago

Ok so this is a fun one. cargo fmt doesn't understand astetics of verticals alas ( https://github.com/rust-lang/rustfmt/issues/4710 ), but with the odd trailing _ you can fool cargo fmt into doing the right thing and making things beautiful:

https://github.com/mimblewimble/grin/pull/3572/files

But clippy's not happy with inconsistent underscores. ( https://rust-lang.github.io/rust-clippy/master/index.html#inconsistent_digit_grouping )

Is there a better solution than a few trailing underscores? (leading underscores are banned) if not, could clippy not complain about trailing underscores please?

Tables come up a lot in programming so it would be nice to have good ways to format them.

camsteffen commented 3 years ago

I suppose we could have a simple "table heuristic": is this a list of numbers that occupies ~multiple lines but the first two numbers are on the same line~ a number of lines that is 1 < lines < N?

gilescope commented 3 years ago

Yes it’s a way to make all the numbers the same size in terms of character length so that cargo fmt aligns them nicely when it wraps them on multiple lines. It’s the nicest way of doing it with cargo fmt not skipped that I have found yet...

On Thu, 4 Mar 2021 at 16:35, Cameron Steffen notifications@github.com wrote:

I suppose we could have a simple "table heuristic": is this a list of numbers that occupies multiple lines but the first two numbers are on the same line?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/rust-lang/rust-clippy/issues/6836#issuecomment-790750553, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGEJCAAXB3SU3CPZWJOHKTTB6ZFHANCNFSM4YSAFOGA .

torfsen commented 7 months ago

I'd be interested in taking a look at this, but I'm not sure how the interaction with cargo fmt is supposed to work: as far as I understand, the exact way that cargo fmt will format such long lists of numbers depends on various rustfmt settings, e.g. short_array_element_width_threshold. Depending on those settings, a list of numbers may be formatted as one of the following:

Ideally, the trailing underscores for table-alignment should only be allowed in the second case. However, that would mean that the Clippy lint would depend on the exact formatting. I don't think any of the existing lints behaves that way, and I'm not sure whether it's a good idea to start doing this.

The only alternative I can think of is to allow trailing underscores in any list of numbers.

Perhaps the better "big picture" fix would be to add an option to rustfmt so that it formats such tables correctly in the first place without the need of the trailing underscores?