Fix camel case type warning for types with trailing underscores #54101
Conversation
|
(rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
r? @nikomatsakis cc @oli-obk @Manishearth It's not clear whether we should do this. |
|
I see no reason not to, there's no obvious way to "fix" this aside from inventing a new name |
| @@ -43,8 +43,6 @@ struct foo7 { | |||
| bar: isize, | |||
| } | |||
|
|
|||
| type __ = isize; //~ ERROR type `__` should have a camel case name such as `CamelCase` | |||
There was a problem hiding this comment.
Well... The former message was not wrong per se. But pretty much useless
We should probably just add underscore-only identifiers to the https://rust-lang-nursery.github.io/rust-clippy/master/index.html#just_underscores_and_digits lint in clippy
|
@osa1 your PR title says:
but I don't see any test for that, just leading underscores. Perhaps add some more tests? |
nikomatsakis
added
S-waiting-on-author
|
Otherwise r=me, I agree that this makes sense to me. |
Done! |
| struct __ {} | ||
| struct X_ {} | ||
| struct X__ {} | ||
| struct X___ {} |
There was a problem hiding this comment.
looking around, I couldn't find many tests for this lint really...
There was a problem hiding this comment.
e.g., to see whether X_X would fail
There was a problem hiding this comment.
Hmm I actually don't know the answer to that myself -- that is whether X_X is camel case or not. Btw there are a few more tests about this in lint-non-camel-case-types.rs.
|
Anyway, I'm inclined to r+ this PR -- @oli-obk, @Manishearth, do we think we need an FCP here? I guess we could. Maybe I'll nominate for lang-team to discuss on Thu. |
nikomatsakis
added
I-nominated
T-lang
|
I think this is a bug fix and can just be landed, but feel free to FCP. I
don't have a strong opinion on this.
…On Wed, Sep 12, 2018, 11:01 PM Ömer Sinan Ağacan ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/test/ui/lint/issue-54099-camel-case-underscore-types.rs
<#54101 (comment)>:
> @@ -0,0 +1,14 @@
+// compile-pass
+
+#![forbid(non_camel_case_types)]
+#![allow(dead_code)]
+
+// None of the following types should generate a warning
+struct _X {}
+struct __X {}
+struct __ {}
+struct X_ {}
+struct X__ {}
+struct X___ {}
Hmm I actually don't know the answer to that myself -- that is whether X_X
is camel case or not. Btw there are a few more tests about this in
lint-non-camel-case-types.rs.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#54101 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ABivSKbdrmsyPsa52TkRkSANCy_VYrcRks5uaUUBgaJpZM4WhdG4>
.
|
|
We chatted in the lang team mtg and decided "oh what the heck we'll FCP it". |
|
I believe @Centril's exact quote was "I have never cared less about anything" |
|
@rfcbot fcp merge I propose we allow any number of trailing or leading underscores because...why not. |
|
Team member @nikomatsakis has proposed to merge this. The next step is review by the rest of the tagged teams:
No concerns currently listed. Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
rfcbot
added
proposed-final-comment-period
|
🤷♂️ 👍 |
|
😪 💤 🛌 |
|
🔔 This is now entering its final comment period, as per the review above. 🔔 |
rfcbot
added
the
final-comment-period
rfcbot
removed
the
proposed-final-comment-period
|
Do we approve now, or wait some more? |
|
@eddyb Go ahead. |
|
@bors r=nikomatsakis |
|
📌 Commit 07646bb has been approved by |
bors
added
S-waiting-on-bors
Fix camel case type warning for types with trailing underscores Fixes #54099
|
☀️ Test successful - status-appveyor, status-travis |
rfcbot
added
finished-final-comment-period
Fixes #54099