[PATCH] D10933: Add new IdentifierNaming check

[Alexander Kornienko via cfe-commits]

alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.

Looks good with a couple of comments. Tell me if you need me to submit the patch for you, once you address the comments.

Thank you for the awesome new check!


================
Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:153
@@ +152,3 @@
+  else
+    Matches = false;
+
----------------
nit: Maybe remove the bool variable and just `return false;` instead of `Matches = false;`?

```
  if (Name.startswith(Style.Prefix))
    Name = Name.drop_front(Style.Prefix.size());
  else
    return false;

  if (Name.endswith(Style.Suffix))
    Name = Name.drop_back(Style.Suffix.size());
  else
    return false;

  return Matchers[static_cast<size_t>(Style.Case)].match(Name);
```

================
Comment at: test/clang-tidy/readability-identifier-naming.cpp:70
@@ +69,3 @@
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for namespace 'FOO_NS' [readability-identifier-naming]
+// CHECK-FIXES: foo_ns
+inline namespace InlineNamespace {
----------------
Looking at the tests once again, I find that they should be more strict. E.g. the presence of `foo_ns` doesn't say anything about whether the replacement was done in the right place (e.g. if the code replaces a wrong token and the result is `foo_ns FOO_NS {`, the test would pass). Please add more context to the CHECK-FIXES lines, maybe even whole lines including start- and end-of-line anchors (`{{^}}` and `{{$}}` respectively).

Thanks!


http://reviews.llvm.org/D10933