[PATCH] D10933: Add new IdentifierNaming check

[Beren Minor]

berenm added inline comments.

================
Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:178
@@ +177,3 @@
+    if (NamingStyles[Typedef].isSet()) {
+      KindName = "typedef";
+      Style = NamingStyles[Typedef];
----------------
alexfh wrote:
> berenm wrote:
> > alexfh wrote:
> > > Would it be better to have these in a single array next to `StyleNames`? Also, this code could return a `StyleKind` (now `StyleConst`), and you would need to get the corresponding `NamingStyle` and `KindName` once.
> > > 
> > > This code would suddenly become much leaner.
> > The problem is that sometimes, the current code falls back to a more generic naming style, but the "kind name" is still trying to describe the original declaration.
> > 
> > For example, if no style is defined for methods, then it will try to use a more generic "function" style, but the warning will still be "invalid case style for method xxx".
> > 
> > Maybe this is superfluous and I can drop it. It don't see many cases anyway (my original code was covering more cases - too many - and it seemed sensible at that time).
> > The problem is that sometimes, the current code falls back to a more 
> > generic naming style, but the "kind name" is still trying to describe the 
> > original declaration.
> 
> I see. It might be possible to split the mapping of types to style kinds and handling missing styles. E.g. a function can return "Method" (which should be SK_Method according to [1], btw) and then a caller would check whether the corresponding `NamingStyle` is configured, and if needed fall back to a more generic category. WDYT?
> 
> 
> [1] http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly
I think it would be difficult to map the declaration types to style kinds without checking the styles that have been provided and falling back to another style at the same time.

For example, a constexpr method is currently selecting the styles in this preferred order :
constexpr method > [public/protected/private] method > method > function.

The order is debatable, but we cannot have the //constexpr method// to //public/protected/private method// fallback, if there is no style for constexpr methods, without reading again the method declaration.

It might be OK that the warning message do not use the exact identifier kind name, and one can even think it is better to have a warning message that tells the user which style it used instead of which kind of identifier was checked.


http://reviews.llvm.org/D10933