[PATCH] D51575: [clang-tidy/checks] Implement a clang-tidy check to verify Google Objective-C function naming conventions 📜

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang-tidy/google/FunctionNamingCheck.cpp:55
+  // to use.
+  if (Decl->getStorageClass() != SC_Static) {
+    return FixItHint();
----------------
Elide braces.


================
Comment at: clang-tidy/google/FunctionNamingCheck.cpp:59-60
+
+  auto Name = Decl->getName();
+  auto NewName = Decl->getName().str();
+
----------------
Do not use `auto` as the type is not mentioned in the initialization.


================
Comment at: clang-tidy/google/FunctionNamingCheck.cpp:66
+    // Exit the loop once we reach the end of the string.
+    if (Index >= NewName.size()) break;
+
----------------
Please make this the while loop condition rather than an explicit break.


================
Comment at: clang-tidy/google/FunctionNamingCheck.cpp:86
+  // Generate a fixit hint if the new name is different.
+  if (NewName != Name) {
+    return FixItHint::CreateReplacement(
----------------
Elide braces.


================
Comment at: clang-tidy/google/FunctionNamingCheck.cpp:98
+  // This check should only be applied to Objective-C sources.
+  if (!getLangOpts().ObjC1 && !getLangOpts().ObjC2) {
+    return;
----------------
Elide braces. Also, didn't we get rid of the distinction between ObjcC1 and ObjC2?


================
Comment at: clang-tidy/google/FunctionNamingCheck.cpp:116
+  const auto *MatchedDecl = Result.Nodes.getNodeAs<FunctionDecl>("function");
+  assert(MatchedDecl != nullptr);
+
----------------
No need for this assertion. The `check()` function cannot be called without this being nonnull.


================
Comment at: clang-tidy/google/FunctionNamingCheck.cpp:119
+  // Only functions other than main should be matched.
+  assert(!MatchedDecl->isMain());
+
----------------
I don't think this assert adds value.


================
Comment at: clang-tidy/google/FunctionNamingCheck.cpp:122-124
+       "function name '%0' not using function naming conventions described by "
+       "Google Objective-C style guide")
+      << MatchedDecl->getName() << generateFixItHint(MatchedDecl);
----------------
You can drop the explicit quotes around %0 and instead pass in `MatchedDecl` rather than `MatchedDecl->getName()`.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51575