-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Generalize async_idents
to all new keywords
#53685
Conversation
r? @varkor (rust_highfive has picked a reviewer for you, use r? to override) |
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.
Looks good to me. A couple of tiny comments, but r=me after that (or immediately if this is especially time-sensitive).
src/librustc_lint/builtin.rs
Outdated
Edition::Edition2015 => { | ||
match &ident.as_str()[..] { | ||
"async" | | ||
"try" => 2018, |
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.
Feels like this should be using Edition::Edition2018
, which already implements Display
.
@@ -0,0 +1,21 @@ | |||
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT |
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.
You should just be able to remove this copyright header now (and with the other tests).
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.
Ah ok, good to know for the future!
(this'll be a hard habit to shake for me...)
src/librustc_lint/builtin.rs
Outdated
} | ||
|
||
/// Checks for uses of `async` as an identifier | ||
/// Checks for uses of future edtion keywords used as an identifier |
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.
nit: "edition"
src/librustc_lint/builtin.rs
Outdated
Edition::Edition2015 => { | ||
match &ident.as_str()[..] { | ||
"async" | | ||
"try" => 2018, |
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.
Was await
also reserved?
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.
It should be if it isn't already
cd5bbe5
to
9e96419
Compare
@bors: r=varkor |
📌 Commit 9e96419 has been approved by |
Generalize `async_idents` to all new keywords This commit generalizes the existing `async_idents` lint to easily encompass other identifiers that will be keywords in future editions. The new lint is called `keyword_idents` and the old `async_idents` lint is registered as renamed to this new lint. As a proof of concept the `try` keyword was added to this list as it looks to be listed as a keyword in the 2018 edition only. The `await` keyword was not added as it's not listed as a keyword yet. Closes rust-lang#53077
⌛ Testing commit 9e96419 with merge 9119395c08158c67dffe36fbe0e0062c1ad577c7... |
💔 Test failed - status-travis |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
This commit generalizes the existing `async_idents` lint to easily encompass other identifiers that will be keywords in future editions. The new lint is called `keyword_idents` and the old `async_idents` lint is registered as renamed to this new lint. As a proof of concept the `try` keyword was added to this list as it looks to be listed as a keyword in the 2018 edition only. The `await` keyword was not added as it's not listed as a keyword yet. Closes rust-lang#53077
@bors: r=varkor |
💡 This pull request was already approved, no need to approve it again.
|
📌 Commit 9e96419 has been approved by |
9e96419
to
003cab2
Compare
@bors: r=varkor |
📌 Commit 003cab2 has been approved by |
Generalize `async_idents` to all new keywords This commit generalizes the existing `async_idents` lint to easily encompass other identifiers that will be keywords in future editions. The new lint is called `keyword_idents` and the old `async_idents` lint is registered as renamed to this new lint. As a proof of concept the `try` keyword was added to this list as it looks to be listed as a keyword in the 2018 edition only. The `await` keyword was not added as it's not listed as a keyword yet. Closes rust-lang#53077
Generalize `async_idents` to all new keywords This commit generalizes the existing `async_idents` lint to easily encompass other identifiers that will be keywords in future editions. The new lint is called `keyword_idents` and the old `async_idents` lint is registered as renamed to this new lint. As a proof of concept the `try` keyword was added to this list as it looks to be listed as a keyword in the 2018 edition only. The `await` keyword was not added as it's not listed as a keyword yet. Closes rust-lang#53077
Generalize `async_idents` to all new keywords This commit generalizes the existing `async_idents` lint to easily encompass other identifiers that will be keywords in future editions. The new lint is called `keyword_idents` and the old `async_idents` lint is registered as renamed to this new lint. As a proof of concept the `try` keyword was added to this list as it looks to be listed as a keyword in the 2018 edition only. The `await` keyword was not added as it's not listed as a keyword yet. Closes #53077
☀️ Test successful - status-appveyor, status-travis |
This commit generalizes the existing
async_idents
lint to easily encompassother identifiers that will be keywords in future editions. The new lint is
called
keyword_idents
and the oldasync_idents
lint is registered as renamedto this new lint.
As a proof of concept the
try
keyword was added to this list as it looks to belisted as a keyword in the 2018 edition only. The
await
keyword was not addedas it's not listed as a keyword yet.
Closes #53077