Don't lint snake-case on executable crate name

[GilShoshan94]

Fix #45127

Hi,
First PR on Rust, I tried my best to follow "Rust Compiler Development Guide".

I tested it with a custom toolchain on a dummy bin crate with one submodule and it seems to work.
The lint non_snake_case ignore the binary crate name but not the module name.

Thank you @Manishearth and @jyn514 for the guidance !

[rustbot]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @oli-obk (or someone else) soon.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

• @rustbot author: the review is finished, PR author should check the comments and take action accordingly
• @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

[jyn514]

Could you please add a UI test so this doesn't regress? There's instructions in https://rustc-dev-guide.rust-lang.org/tests/ui.html#ui-tests.

[GilShoshan94]

@jyn514 Yes, I will read that right now.

[jyn514]

oh lol looks like there was already tests for this - you can probably just change those to check-pass :)

[est31]

I think on linux, the snake or dash case naming style is very common. I can count few binaries that don't have snake case. Edit: concrete numbers: compgen -c | wc -l has output 2280, and compgen -c | egrep "[A-Z]" | wc -l outputs 12.

[jyn514]

@est31 please keep bikeshedding of whether we should do this or not to #45127

[est31]

@jyn514 I don't really want to discuss this further, I just wanted to give the linux point of view. I understand that windows and Mac OS have different points of view and ultimately it doesn't matter that much, so one can probably have this PR.

[GilShoshan94]

@jyn514 I added the UI tests and extend them to check every crate_type to insure I modified it only for the bin case.

[workingjubilee]

The dominant naming convention on Linux is alllowercasemashedtogetherwithoutrestraint except with the caveat that it usually also is constrained to about 6 letters... notably, the binary produced by the Rust build system is named rustc, not RustC, rustC, rust-c, or rust_c. This is the same "naming convention", if you want to call it that, used to entitle memcpy (which might otherwise be mem_copy, perhaps), wcstombs (wcs-to-mbs), and so on. The fact that this is not currently linted on is incidental, because the lint is too mechanical and simplistically implemented to detect that these are still notionally separate, likewise with rustfmt, etc. Of course, none of this "commentary" is actionable by the PR author, so I hope "not a discussion" discussion comments genuinely do end and not continue further.

[GilShoshan94]

Hi @workingjubilee,

I am not sure if I understood your comment fully (especially the last line, English is not my first language)...

The discussion occured there #45127.

I see you mention on Linux, so I remind everyone that on the frontpage of the Rust website is mentioned cross-platform.

And on other OS, binary names have other conventions.

If you wish to keep such a lint on the binary name, make a PR to add that.

Here the main issue is that the only solution left for the people requiring to name their binary differently is to use #![allow(non_snake_case)] at the top-level, that disables the lint for the entire crate, which is not desirable at all.
(Or you have a script that rename your binary after compilation...)

[workingjubilee]

Oh, I agree with the intent and implementation of this PR (not that I've reviewed it closely), no worry there.

[oli-obk]

@bors r+

[bors]

📌 Commit e4dcf0c has been approved by oli-obk

It is now in the queue for this repository.

[compiler-errors]

This failed during rollup: #111464 (comment)

@bors r-

[GilShoshan94]

Not sure if I understood the output correctly, but it seems expected since the PR has for goal to not lint in the case where the crate_type include bin (code).

I am not familiar with "rollup".
Where can I learn more about it and see what I should do in order to fix that please ?
Also, any direction on what to do is appreciated.

[jyn514]

@GilShoshan94 here is the error:

diff of stderr:

+ warning: dropping unsupported crate type `dylib` for target `wasm32-unknown-unknown`
+ 
1 error: crate `NonSnakeCase` should have a snake case name
3    |

10 LL | #![deny(non_snake_case)]
11    |         ^^^^^^^^^^^^^^
---
To only update this specific test, also pass `--test-args lint/lint-non-snake-case-crate-dylib.rs`

error: 1 errors occurred comparing output.

Notice how it says "warning: dropping unsupported crate type dylib for target wasm32-unknown-unknown". You can likely replicate it locally by running x test tests/ui/lint/lint-non-snake-case-crate-dylib.rs --target wasm32-unknown-unknown.

See https://forge.rust-lang.org/release/rollups.html for more information about rollups.

[GilShoshan94]

Thank you, I will look into this when I can.

[Dylan-DPC]

@GilShoshan94 any updates on this?

[GilShoshan94]

@Dylan-DPC No sorry, I had no time available, lots of work and personal stuff. I also moved city... I hope I will have more time end of the year to contribute.

[Dylan-DPC]

yeah sure that's fine. I am going to close this for the time being. Whenever you get the time you can work on it and reöpen and push to it or create a new pr with it and we can take it forward from there. Thanks