[PATCH] D90282: [clang-tidy] Add IgnoreShortNames config to identifier naming checks

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D90282#2393019 <https://reviews.llvm.org/D90282#2393019>, @smhc wrote:

> In D90282#2391360 <https://reviews.llvm.org/D90282#2391360>, @aaron.ballman wrote:
>
>> In D90282#2391005 <https://reviews.llvm.org/D90282#2391005>, @njames93 wrote:
>>
>>> Should this be a NamingStyle option instead.
>>> `{key: readability-identifier-naming.ParameterShortSizeThreshold, value: 2}`
>>> WDYT?
>>
>> I think that makes a lot of sense -- I can imagine wanting to enforce different identifier lengths depending on whether we're spelling a type name vs a local variable name, etc.
>
> I considered this but thought the configuration and documentation would be quite cumbersome. We could provide a global setting (as already done) and allow refining it further by type if needed? For what it's worth I only need it for local variables, I imagine that would be the main use case.
>
> Or should we simply add this threshold to every type of name, similar to how suffix, prefix and case style have been done?

I think length of names matters differently in different contexts. I think three-letter identifiers as a class name should probably be a rarity, but may be more reasonable as a local variable. I think one-letter local variables are generally a bad thing, except for loop induction variables. That sort of thing. (And we may someday want to add more specific naming categories like "loop induction variable".) So I think adding the threshold to all the different name kinds is a more flexible approach.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90282/new/

https://reviews.llvm.org/D90282