-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Add a check for names starting with a digit #3542
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @brson (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
|
@bors: r+ Thanks! |
|
📌 Commit 73aa117 has been approved by |
|
⌛ Testing commit 73aa117 with merge 54f5248... |
|
💥 Test timed out |
|
@bors: retry
…On Sun, Jan 15, 2017 at 9:08 PM, bors ***@***.***> wrote:
💥 Test timed out
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#3542 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAD95Pz6sfCKHKxPGZ4w3-e45epRqImTks5rSvtEgaJpZM4LjvUb>
.
|
|
⌛ Testing commit 73aa117 with merge c20cda0... |
|
💥 Test timed out |
|
@bors: retry
…On Mon, Jan 16, 2017 at 5:08 PM, bors ***@***.***> wrote:
💥 Test timed out
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#3542 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAD95M9s3olhX2wpiOkC54x-_x7PMPR5ks5rTBSbgaJpZM4LjvUb>
.
|
Add a check for names starting with a digit According to Rust grammer https://doc.rust-lang.org/grammar.html#extern-crate-declarations for extern crate declarations, a crate name cannot start with a digit. But, currently this rule is not upheld by `cargo new` as creating a project like: `cargo new 2048` would create a project named 2048 which obviously won't compile with crate declaration like `extern crate 2048` by a consumer. This obviously is a rare case in practice, but its always good to check i guess. This PR adds a check to the `new` method and `bail`s out with a message for any names starting with a digit. PS: I noticed it while making a 2048 puzzle game as a library so thought it would be nice to add this check :)
|
☀️ Test successful - status-appveyor, status-travis |
| name) | ||
| } | ||
|
|
||
| if let Some(ref c) = name.chars().nth(0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be better style to use .next() here instead of .nth(0)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, char is Copy so we don't need the ref here
According to Rust grammer https://doc.rust-lang.org/grammar.html#extern-crate-declarations for extern crate declarations, a crate name cannot start with a digit.
But, currently this rule is not upheld by
cargo newas creating a project like:cargo new 2048would create a project named 2048 which obviously won't compile with crate declaration likeextern crate 2048by a consumer.This obviously is a rare case in practice, but its always good to check i guess.
This PR adds a check to the
newmethod andbails out with a message for any names starting with a digit.PS: I noticed it while making a 2048 puzzle game as a library so thought it would be nice to add this check :)