Skip to content
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

Incorrect camel case warning #57319

Closed
tcullum-gpsw opened this issue Jan 3, 2019 · 4 comments · Fixed by #58407
Closed

Incorrect camel case warning #57319

tcullum-gpsw opened this issue Jan 3, 2019 · 4 comments · Fixed by #58407
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut.

Comments

@tcullum-gpsw
Copy link

tcullum-gpsw commented Jan 3, 2019

Run the following program using Stable or Nightly:

https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=a8e0392ac3ef8c62c5cc66fddabeb564

The compiler tells us:

warning: type `userData` should have a camel case name such as `Userdata`
 --> src/main.rs:1:1
  |
1 | / struct userData
2 | | {
3 | |     name: String,
4 | |     age: u8,
5 | |     weight: f32,
6 | |     height: f32
7 | | }
  | |_^
  |
  = note: #[warn(non_camel_case_types)] on by default

This is incorrect/confusing in 3 ways:

1.) userData is in fact camelCase.

2.) UserData would be PascalCase and this is the convention I've seen

3.) Userdata is neither... So even if for some strange reason we're considering PascalCase as camel case, the recommendation is neither.

@varkor
Copy link
Member

varkor commented Jan 3, 2019

Ah, good catch: it's been noted in the style guide that "upper camel case" is a better term, as it's not so ambiguous (the term "camel case" is variably used for ThisStyle and thisStyle). The lint name itself (non_camel_case_types) is probably fine, but it would be good to clarify "upper camel case" in the lint message.

Userdata is neither...

Userdata is upper camel case — but it's considered to have a single component (i.e. userdata as a single word). I think it would be reasonable not to lowercase mid-word capitals like this.

This should be very straightforward to fix — you're welcome to open a pull request fixing this! (If you search for the message, you should find the location to make the change.)

@varkor varkor added the A-diagnostics Area: Messages for errors, warnings, and lints label Jan 3, 2019
@tcullum-gpsw
Copy link
Author

@varkor Do we want to specify Pascal Case which always exclusively refers to upper camel case to be least ambiguous? Or do we want to stick to "upper camel case?"

@varkor
Copy link
Member

varkor commented Jan 3, 2019

There was actually a whole discussion on this topic earlier this year and it was decided that "upper camel case" was clearer (though it seems no-one thought to change the compiler lints), so let's go with that.

@tcullum-gpsw
Copy link
Author

Great, will make a PR tonight, thanks.

@estebank estebank added A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. and removed A-diagnostics Area: Messages for errors, warnings, and lints labels Jan 7, 2019
Centril added a commit to Centril/rust that referenced this issue Feb 14, 2019
specify "upper camel case" in style lint

Also, fix an issue where internal upper case letters were converted to
lower case.

Fixes rust-lang#57319.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants