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
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
