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

type parameters expect camel case, but shouting case is also common #60570

Open
ExpHP opened this issue May 5, 2019 · 5 comments
Open

type parameters expect camel case, but shouting case is also common #60570

ExpHP opened this issue May 5, 2019 · 5 comments
Labels
A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@ExpHP
Copy link
Contributor

ExpHP commented May 5, 2019

I am not sure why type parameters are checked to be upper camel case.

Type parameter names are not part of public API. Shouting case for type parameters is a convention followed by such popular crates as rayon, and it is the only way I know of to make something like the following not look confusing:

#[derive(Debug, Clone, PartialEq)]
pub enum Data<
    INTEGER = Vec<i32>,
    CHARACTER = Vec<u8>,
    COMPLEX = Vec<(f64, f64)>,
> {
    Integer(INTEGER),
    Character(CHARACTER),
    Complex(COMPLEX),
}

Furthermore, due to limitations of the check for upper camel case, you could write a whole crate using shouting case and not even know about the lint, because it will only hit you when you write an underscore:

pub fn zip<
    INTEGER_1, CHARACTER_1, COMPLEX_1,
    INTEGER_2, CHARACTER_2, COMPLEX_2,
>(
    data_1: Data<INTEGER_1, CHARACTER_1, COMPLEX_1>,
    data_2: Data<INTEGER_2, CHARACTER_2, COMPLEX_2>,
) -> Data<
    (INTEGER_1, INTEGER_2),
    (CHARACTER_1, CHARACTER_2),
    (COMPLEX_1, COMPLEX_2),
> { unimplemented!() }
warning: type parameter `INTEGER_1` should have a camel case name
 --> lib.rs:3:9
  |
3 |         INTEGER_1, CHARACTER_1, COMPLEX_1,
  |         ^^^^^^^^^ help: convert the identifier to camel case: `Integer1`
  |
  = note: #[warn(non_camel_case_types)] on by default

Given how rarely underscores should be needed, it's tempting to just change this to INTEGER1 so that it slips by unnoticed like the rest of the crate.

@Centril Centril added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue. A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. labels May 5, 2019
@Centril
Copy link
Contributor

Centril commented May 5, 2019

Triage: Classifying as:

  1. A bug in the non_camel_case_types lint (T-compiler)
  2. A request for a change to not lint on this (T-lang)

@ExpHP
Copy link
Contributor Author

ExpHP commented May 5, 2019

I think it is difficult to call this a bug. It is a limitation of allowing the "words" in camel case to be one letter long. (which is especially likely to occur in type parameters)

I mean, to us, upper camel case means:

  • Prefer Integer to INTEGER. (i.e. it applies to words)
  • Prefer Html to HTML. (i.e. acronyms are one word)

But consider the following:

pub fn zip<A, B, IA, IB>(a: IA, b: IB) -> impl Iterator<Item=(A, B)>
where
    IA: IntoIterator<Item=A>,
    IB: IntoIterator<Item=B>,
{ ... }

Here, I would assert that IA is, in fact, not an acronym, and it is in fact an upper camel case name composed of two separate words---one of which happens to be one letter long (A), and the other of which has been abbreviated to one letter for brevity (I). From here it is not a long shot to see that an identifier of 3 capital letters could still correctly be camel case, and putting any upper bound on this (even as a heuristic) would feel arbitrary.

@Centril
Copy link
Contributor

Centril commented May 6, 2019

I think it is difficult to call this a bug. It is a limitation of allowing the "words" in camel case to be one letter long. (which is especially likely to occur in type parameters)

Oh wow! I didn't consider this. 😂

@traviscross
Copy link
Contributor

@rustbot labels +I-lang-nominated

Nominating as this should be discussed in the same breath as #116389.

@rustbot rustbot added the I-lang-nominated Nominated for discussion during a lang team meeting. label Nov 13, 2023
@joshtriplett
Copy link
Member

joshtriplett commented Nov 15, 2023

We discussed this in today's @rust-lang/lang meeting. In general, we felt like the compiler doesn't have enough information to know whether a string of all-caps characters is camel case of one-letter words/abbreviations or whether it's all-caps.

I don't think we should encourage INTEGER_1, but I do think it's completely valid to write arbitrarily-long strings of capital letters in "camel case". If someone writes IFAClient (example made up on the spot), we have no way of knowing if that's "IFA Client" (in which case officially by our rules you're "supposed" to write IfaClient), or "I F A Client" (e.g. Iterator of Futures of Async-wrapped Client). And since we can't distinguish those, we should be more permissive.

With that in mind, if someone proposed a PR that loosened the rules for CamelCase to permit ABCDECase, the team would be receptive to that.

@traviscross traviscross removed the I-lang-nominated Nominated for discussion during a lang team meeting. label Nov 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants