[PATCH] D10933: Add new IdentifierNaming check

[Alexander Kornienko]

alexfh added inline comments.

================
Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:87
@@ +86,3 @@
+  auto const fromString = [](StringRef Str) {
+    if (Str.equals("any") || Str.equals("aNy_CasE"))
+      return AnyCase;
----------------
berenm wrote:
> alexfh wrote:
> > alexfh wrote:
> > > This could be written nicer using llvm::StringSwitch.
> > Not sure why we would need alternative spellings of these options. That seems to just add complexity with no benefit.
> The idea was to provide a way easier to type than the full names. This may not be very useful, and maybe a single simple name for each case scheme would be enough.
Clang-tidy options are usually read from a file, so minimizing typing is not really important.

================
Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:178
@@ +177,3 @@
+    if (NamingStyles[Typedef].isSet()) {
+      KindName = "typedef";
+      Style = NamingStyles[Typedef];
----------------
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


http://reviews.llvm.org/D10933