[clang-tidy/checks] Implement a clang-tidy check to verify Google Obj…

…ective-C function naming conventions 📜

Summary:
§1 Description

This check finds function names in function declarations in Objective-C files that do not follow the naming pattern described in the Google Objective-C Style Guide. Function names should be in UpperCamelCase and functions that are not of static storage class should have an appropriate prefix as described in the Google Objective-C Style Guide. The function `main` is a notable exception. Function declarations in expansions in system headers are ignored.

Example conforming function definitions:
```
static bool IsPositive(int i) { return i > 0; }
static bool ABIsPositive(int i) { return i > 0; }
bool ABIsNegative(int i) { return i < 0; }
```

A fixit hint is generated for functions of static storage class but otherwise the check does not generate a fixit hint because an appropriate prefix for the function cannot be determined.

§2 Test Notes
* Verified clang-tidy tests pass successfully.
* Used check_clang_tidy.py to verify expected output of processing google-objc-function-naming.m

Reviewers: benhamilton, hokein, Wizard, aaron.ballman

Reviewed By: benhamilton

Subscribers: Eugene.Zelenko, mgorny, xazax.hun, cfe-commits

Tags: #clang-tools-extra

Differential Revision: https://reviews.llvm.org/D51575

llvm-svn: 347132

clang-tools-extra/clang-tidy/google/CMakeLists.txt

@@ -6,6 +6,7 @@ add_clang_library(clangTidyGoogleModule
   DefaultArgumentsCheck.cpp
   ExplicitConstructorCheck.cpp
   ExplicitMakePairCheck.cpp
+  FunctionNamingCheck.cpp
   GlobalNamesInHeadersCheck.cpp
   GlobalVariableDeclarationCheck.cpp
   GoogleTidyModule.cpp

clang-tools-extra/clang-tidy/google/FunctionNamingCheck.cpp

@@ -0,0 +1,121 @@
+//===--- FunctionNamingCheck.cpp - clang-tidy -----------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "FunctionNamingCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "llvm/Support/Regex.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace google {
+namespace objc {
+
+namespace {
+
+std::string validFunctionNameRegex(bool RequirePrefix) {
+  // Allow the following name patterns for all functions:
+  // • ABFoo (prefix + UpperCamelCase)
+  // • ABURL (prefix + capitalized acronym/initialism)
+  //
+  // If no prefix is required, additionally allow the following name patterns:
+  // • Foo (UpperCamelCase)
+  // • URL (capitalized acronym/initialism)
+  //
+  // The function name following the prefix can contain standard and
+  // non-standard capitalized character sequences including acronyms,
+  // initialisms, and prefixes of symbols (e.g., UIColorFromNSString). For this
+  // reason, the regex only verifies that the function name after the prefix
+  // begins with a capital letter followed by an arbitrary sequence of
+  // alphanumeric characters.
+  //
+  // If a prefix is required, the regex checks for a capital letter followed by
+  // another capital letter or number that is part of the prefix and another
+  // capital letter or number that begins the name following the prefix.
+  std::string FunctionNameMatcher =
+      std::string(RequirePrefix ? "[A-Z][A-Z0-9]+" : "") + "[A-Z][a-zA-Z0-9]*";
+  return std::string("::(") + FunctionNameMatcher + ")$";
+}
+
+/// For now we will only fix functions of static storage class with names like
+/// 'functionName' or 'function_name' and convert them to 'FunctionName'. For
+/// other cases the user must determine an appropriate name on their own.
+FixItHint generateFixItHint(const FunctionDecl *Decl) {
+  // A fixit can be generated for functions of static storage class but
+  // otherwise the check cannot determine the appropriate function name prefix
+  // to use.
+  if (Decl->getStorageClass() != SC_Static)
+    return FixItHint();
+
+  StringRef Name = Decl->getName();
+  std::string NewName = Decl->getName().str();
+
+  size_t Index = 0;
+  bool AtWordBoundary = true;
+  while (Index < NewName.size()) {
+    char ch = NewName[Index];
+    if (isalnum(ch)) {
+      // Capitalize the first letter after every word boundary.
+      if (AtWordBoundary) {
+        NewName[Index] = toupper(NewName[Index]);
+        AtWordBoundary = false;
+      }
+
+      // Advance the index after every alphanumeric character.
+      Index++;
+    } else {
+      // Strip out any characters other than alphanumeric characters.
+      NewName.erase(Index, 1);
+      AtWordBoundary = true;
+    }
+  }
+
+  // Generate a fixit hint if the new name is different.
+  if (NewName != Name)
+    return FixItHint::CreateReplacement(
+        CharSourceRange::getTokenRange(SourceRange(Decl->getLocation())),
+        llvm::StringRef(NewName));
+
+  return FixItHint();
+}
+
+} // namespace
+
+void FunctionNamingCheck::registerMatchers(MatchFinder *Finder) {
+  // This check should only be applied to Objective-C sources.
+  if (!getLangOpts().ObjC)
+    return;
+
+  // Match function declarations that are not in system headers and are not
+  // main.
+  Finder->addMatcher(
+      functionDecl(
+          unless(isExpansionInSystemHeader()),
+          unless(anyOf(isMain(), matchesName(validFunctionNameRegex(true)),
+                       allOf(isStaticStorageClass(),
+                             matchesName(validFunctionNameRegex(false))))))
+          .bind("function"),
+      this);
+}
+
+void FunctionNamingCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *MatchedDecl = Result.Nodes.getNodeAs<FunctionDecl>("function");
+
+  diag(MatchedDecl->getLocation(),
+       "function name %0 not using function naming conventions described by "
+       "Google Objective-C style guide")
+      << MatchedDecl << generateFixItHint(MatchedDecl);
+}
+
+} // namespace objc
+} // namespace google
+} // namespace tidy
+} // namespace clang

clang-tools-extra/clang-tidy/google/FunctionNamingCheck.h

@@ -0,0 +1,43 @@
+//===--- FunctionNamingCheck.h - clang-tidy ---------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_OBJC_FUNCTION_NAMING_CHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_OBJC_FUNCTION_NAMING_CHECK_H
+
+#include "../ClangTidy.h"
+#include "llvm/ADT/StringRef.h"
+
+namespace clang {
+namespace tidy {
+namespace google {
+namespace objc {
+
+/// Finds function names that do not conform to the recommendations of the
+/// Google Objective-C Style Guide. Function names should be in upper camel case
+/// including capitalized acronyms and initialisms. Functions that are not of
+/// static storage class must also have an appropriate prefix. The function
+/// `main` is an exception. Note that this check does not apply to Objective-C
+/// method or property declarations.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/google-objc-function-naming.html
+class FunctionNamingCheck : public ClangTidyCheck {
+public:
+  FunctionNamingCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace objc
+} // namespace google
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_OBJC_FUNCTION_NAMING_CHECK_H

clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp

@@ -18,6 +18,7 @@
 #include "DefaultArgumentsCheck.h"
 #include "ExplicitConstructorCheck.h"
 #include "ExplicitMakePairCheck.h"
+#include "FunctionNamingCheck.h"
 #include "GlobalNamesInHeadersCheck.h"
 #include "GlobalVariableDeclarationCheck.h"
 #include "IntegerTypesCheck.h"
@@ -50,6 +51,8 @@ class GoogleModule : public ClangTidyModule {
         "google-global-names-in-headers");
     CheckFactories.registerCheck<objc::AvoidThrowingObjCExceptionCheck>(
         "google-objc-avoid-throwing-exception");
+    CheckFactories.registerCheck<objc::FunctionNamingCheck>(
+        "google-objc-function-naming");
     CheckFactories.registerCheck<objc::GlobalVariableDeclarationCheck>(
         "google-objc-global-variable-declaration");
     CheckFactories.registerCheck<runtime::IntegerTypesCheck>(

clang-tools-extra/docs/ReleaseNotes.rst

@@ -123,6 +123,12 @@ Improvements to clang-tidy
   Finds macro usage that is considered problematic because better language
   constructs exist for the task.
 
+- New :doc:`google-objc-function-naming
+  <clang-tidy/checks/google-objc-function-naming>` check.
+
+  Checks that function names in function declarations comply with the naming
+  conventions described in the Google Objective-C Style Guide.
+
 - New :doc:`misc-non-private-member-variables-in-classes
   <clang-tidy/checks/misc-non-private-member-variables-in-classes>` check.
 

clang-tools-extra/docs/clang-tidy/checks/google-objc-function-naming.rst

@@ -0,0 +1,27 @@
+.. title:: clang-tidy - google-objc-function-naming
+
+google-objc-function-naming
+===========================
+
+Finds function declarations in Objective-C files that do not follow the pattern
+described in the Google Objective-C Style Guide.
+
+The corresponding style guide rule can be found here:
+https://google.github.io/styleguide/objcguide.html#function-names
+
+All function names should be in Pascal case. Functions whose storage class is
+not static should have an appropriate prefix.
+
+The following code sample does not follow this pattern:
+
+.. code-block:: objc
+
+  static bool is_positive(int i) { return i > 0; }
+  bool IsNegative(int i) { return i < 0; }
+
+The sample above might be corrected to the following code:
+
+.. code-block:: objc
+
+  static bool IsPositive(int i) { return i > 0; }
+  bool *ABCIsNegative(int i) { return i < 0; }

clang-tools-extra/docs/clang-tidy/checks/list.rst

@@ -124,6 +124,7 @@ Clang-Tidy Checks
    google-explicit-constructor
    google-global-names-in-headers
    google-objc-avoid-throwing-exception
+   google-objc-function-naming
    google-objc-global-variable-declaration
    google-readability-braces-around-statements (redirects to readability-braces-around-statements) <google-readability-braces-around-statements>
    google-readability-casting

clang-tools-extra/test/clang-tidy/google-objc-function-naming.m

@@ -0,0 +1,52 @@
+// RUN: %check_clang_tidy %s google-objc-function-naming %t
+
+typedef _Bool bool;
+
+static bool ispositive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'ispositive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool Ispositive(int a) { return a > 0; }
+
+static bool is_positive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'is_positive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
+
+static bool isPositive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'isPositive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
+
+static bool Is_Positive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'Is_Positive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
+
+static bool IsPositive(int a) { return a > 0; }
+
+bool ispalindrome(const char *str);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ispalindrome' not using function naming conventions described by Google Objective-C style guide
+
+static const char *md5(const char *str) { return 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: function name 'md5' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static const char *Md5(const char *str) { return 0; }
+
+static const char *MD5(const char *str) { return 0; }
+
+static const char *URL(void) { return "https://clang.llvm.org/"; }
+
+static const char *DEFURL(void) { return "https://clang.llvm.org/"; }
+
+static const char *DEFFooURL(void) { return "https://clang.llvm.org/"; }
+
+static const char *StringFromNSString(id str) { return ""; }
+
+void ABLog_String(const char *str);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ABLog_String' not using function naming conventions described by Google Objective-C style guide
+
+void ABLogString(const char *str);
+
+bool IsPrime(int a);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'IsPrime' not using function naming conventions described by Google Objective-C style guide
+
+const char *ABURL(void) { return "https://clang.llvm.org/"; }
+
+const char *ABFooURL(void) { return "https://clang.llvm.org/"; }
+
+int main(int argc, const char **argv) { return 0; }