[PATCH] D10933: Add new IdentifierNaming check

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

Sorry for the delay. A few more comments, otherwise looks really nice.


================
Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:166
@@ +165,3 @@
+  static llvm::Regex Splitter(
+      "(([a-z0-9A-Z]*)(_+)|([A-Z]?[a-z0-9]+)([A-Z]|$)|([A-Z]+)([A-Z]|$))");
+
----------------
Why do you need the outermost parentheses?

================
Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:181
@@ +180,3 @@
+        Substr = Substr.substr(Groups[0].size());
+
+      } else if (Groups[4].size() > 0) {
----------------
nit: Please remove the empty lines before closing braces.

================
Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:533
@@ +532,3 @@
+      if (!IgnoreFailedSplit) {
+        diag(Decl->getLocStart(), "unable to split words for %0 '%1'")
+            << KindName << Name;
----------------
That doesn't seem like a useful warning message. If the tool didn't manage to understand the naming style, it may be interesting to the tool developer, but not to a user. Please make this a DEBUG(llvm::dbgs() << ...).

================
Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:539
@@ +538,3 @@
+      SourceRange Range =
+          clang::DeclarationNameInfo(Decl->getDeclName(), Decl->getLocation())
+              .getSourceRange();
----------------
`clang::` is superfluous here, the code is already inside the `clang` namespace.

================
Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:554
@@ +553,3 @@
+void IdentifierNamingCheck::onEndOfTranslationUnit() {
+  for (const auto &P : NamingCheckFailures) {
+    SourceRange DeclRange =
----------------
What does `P` stand for? I'd prefer a more meaningful name for the loop variable.


http://reviews.llvm.org/D10933