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

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.


================
Comment at: clang-tidy/google/FunctionNamingCheck.cpp:57
+      functionDecl(
+          isDefinition(),
+          unless(anyOf(isMain(), matchesName(validFunctionNameRegex(true)),
----------------
stephanemoore wrote:
> hokein wrote:
> > any reason why we restrict to definitions only? I think we can consider declarations too.
> I restricted the check to function definitions because function declarations are sometimes associated with functions outside the control of the author. I have personally observed unfortunate cases where functions declared in the iOS SDK had incorrect—or seemingly incorrect—availability attributes that caused fatal dyld assertions during application startup. The check currently intends to avoid flagging function declarations because of the rare circumstances where an inflexible function declaration without a corresponding function definition is required.
> 
> I have added a comment explaining why only function definitions are flagged.
> 
> I am still open to discussion though.
Thanks for the explanations.

I have a concern about the heuristic using here, it seems fragile -- if there is an inline function defined in a base header, the check will still give a warning to it if the source file `.m` #includes this header; it also limits the scope of the check, I think this check is flagged mostly on file-local functions (e.g. static functions, functions defined in anonymous namespace).

Flagging on function declaration doesn't seem too problematic (we already have a similar check `objc-property-declaration` does the same thing) -- our internal review system shows check warnings on changed code, if user's code includes a common header which violates the check, warnings on the header will not be shown; for violations in iOS SDK, we can use some filtering matchers (`isExpansionInSystemHeader` maybe work) to ignore all functions from these files.




Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51575