Warn about crate name's format when creating new crate

[shnaramn]

What does this PR try to resolve?

Warns about a crate's name during creation (crate new ...) if it doesn't follow the preferred snake_case format.

Fixes #2708

The warning message uses the language mentioned in RFC 430.

How should we test and review this PR?

Verified existing tests succeeded with updates. Added new tests to verify fix.

Additional information

The link to API naming guidelines was not used since it still stays unclear for naming convention for crates.

[rustbot]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @weihanglo (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

[sanmai-NL]

The API naming guidelines are not referenced but I think that is rightly so, since it has a lower status than an RFC.

[ehuss]

Just some drive-by comments:

I'm reluctant to issue this warning on a binary crate, since that affects the executable name. I don't see a reason why that should be restricted, and I think it is probably wrong that rustc issues a warning for it.

This doesn't seem to handle - from what I can tell.

The rules that rustc uses for camel-case is fairly complicated. See to_camel_case. It is also kinda broken (see rust-lang/rust#116336). EDIT: I was confusing the wrong lint. is_snake_case is still a bit different from this implementation.

I'm reluctant to try to duplicate what rustc is trying to do, since keeping them in sync will be challenging and cause confusion when they are out of sync. I'd like to be very careful of whether or not this is something that cargo really needs to do, since a warning will be evident on the very first build, which I think is a minor delay compared to immediately with cargo new. I'd like to hear from other cargo team members about this.

[sanmai-NL]

@ehuss Then an alternative might be to automatically run a cargo build or otherwise let rustc check the metadata that Cargo feeds it as a final phase of cargo new.

If I provide an empty name, this happens:

$  cargo new --name '' blah
warning: compiling this new package may not work due to invalid workspace configuration

failed to parse manifest at `/Users/sanmai/blah/Cargo.toml`

Caused by:
  package name cannot be an empty string
     Created binary (application) `` package

I don't understand why Cargo would offer such a convenience subcommand cargo new and then produce an incompatible initial Cargo project. Even as, in this example, rustc seems incorrect, as you note.

To an end user using a convenience command, degrees of invalidness are not relevant: whether it's a rustc default lint or a compilation fault. In fact, which experienced Rust developer uses cargo new rather than using a more complete template (project directory and/or Cargo.toml)? If the answer is, cargo new is for novices (e.g., my students), then they should be assisted to the maximum extent possible to avoid faults.

[epage]

I'm reluctant to try to duplicate what rustc is trying to do, since keeping them in sync will be challenging and cause confusion when they are out of sync. I'd like to be very careful of whether or not this is something that cargo really needs to do, since a warning will be evident on the very first build, which I think is a minor delay compared to immediately with cargo new. I'd like to hear from other cargo team members about this.

Thinking about this and my side note, I wonder if a user-controlled warning about package names being snake or kebab case would be the right way to go, independent of rustc. Until we have #12235, we could have this be a Shell::warn in cargo new.

[shnaramn]

Friendly ping. Do the latest changes look good?

[epage]

@ehuss

I'm reluctant to issue this warning on a binary crate, since that affects the executable name. I don't see a reason why that should be restricted, and I think it is probably wrong that rustc issues a warning for it.

Somehow I overlooked this originally. Bin names should be kebab case and lib package names should be kebab or snake case. That just leaves GUI bins. That seems like a small enough of a slice that they can choose to ignore it (and in the future allow it if we make this a user-controlled lint)

I'm reluctant to try to duplicate what rustc is trying to do, since keeping them in sync will be challenging and cause confusion when they are out of sync. I'd like to be very careful of whether or not this is something that cargo really needs to do, since a warning will be evident on the very first build, which I think is a minor delay compared to immediately with cargo new. I'd like to hear from other cargo team members about this.

I think its worthwhile, especially with this simplified form.

• Since this is a scaffolding problem, it seems like it'd be nice to report it then rather than the user being surprised by a warning on first build.
• Also, if the API guidelines ever get updated to distinguish crate from package naming, we'd have something unique to check compared to rustc (since we convert - into _).

[epage]

@shnaramn could you clean up the commits for how you'd like them merged?

[epage]

@bors r+

[bors]

📌 Commit 901018c has been approved by epage

It is now in the queue for this repository.

[bors]

⌛ Testing commit 901018c with merge f5418fc...

[bors]

☀️ Test successful - checks-actions
Approved by: epage
Pushing f5418fc to master...