stephanemoore updated this revision to Diff 164568.
stephanemoore marked 3 inline comments as done.
stephanemoore edited the summary of this revision.
stephanemoore added a comment.
Implemented the following suggested changes:
• Restricted matching to function declarations that are not in expansions in
system headers.
• Extended the check to all function declarations rather than just function
definitions.
• Implemented a fixit hint for functions of static storage class.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51575
Files:
clang-tidy/google/CMakeLists.txt
clang-tidy/google/FunctionNamingCheck.cpp
clang-tidy/google/FunctionNamingCheck.h
clang-tidy/google/GoogleTidyModule.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/google-objc-function-naming.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/google-objc-function-naming.m
Index: test/clang-tidy/google-objc-function-naming.m
===================================================================
--- /dev/null
+++ test/clang-tidy/google-objc-function-naming.m
@@ -0,0 +1,49 @@
+// 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);
+
+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; }
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -118,6 +118,7 @@
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
Index: docs/clang-tidy/checks/google-objc-function-naming.rst
===================================================================
--- /dev/null
+++ 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 upper camel 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; }
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -93,6 +93,12 @@
Flags uses of ``absl::StrCat()`` to append to a ``std::string``. Suggests
``absl::StrAppend()`` should be used instead.
+- 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:`readability-magic-numbers
<clang-tidy/checks/readability-magic-numbers>` check.
Index: clang-tidy/google/GoogleTidyModule.cpp
===================================================================
--- clang-tidy/google/GoogleTidyModule.cpp
+++ 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 @@
"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>(
Index: clang-tidy/google/FunctionNamingCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/google/FunctionNamingCheck.h
@@ -0,0 +1,42 @@
+//===--- 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.
+///
+/// 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
Index: clang-tidy/google/FunctionNamingCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/google/FunctionNamingCheck.cpp
@@ -0,0 +1,130 @@
+//===--- 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();
+ }
+
+ auto Name = Decl->getName();
+ auto NewName = Decl->getName().str();
+
+ size_t Index = 0;
+ bool AtWordBoundary = true;
+ while (true) {
+ // Exit the loop once we reach the end of the string.
+ if (Index >= NewName.size()) break;
+
+ 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().ObjC1 && !getLangOpts().ObjC2) {
+ 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");
+ assert(MatchedDecl != nullptr);
+
+ // Only functions other than main should be matched.
+ assert(!MatchedDecl->isMain());
+
+ diag(MatchedDecl->getLocation(),
+ "function name '%0' not using function naming conventions described by "
+ "Google Objective-C style guide")
+ << MatchedDecl->getName() << generateFixItHint(MatchedDecl);
+}
+
+} // namespace objc
+} // namespace google
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/google/CMakeLists.txt
===================================================================
--- clang-tidy/google/CMakeLists.txt
+++ clang-tidy/google/CMakeLists.txt
@@ -6,6 +6,7 @@
DefaultArgumentsCheck.cpp
ExplicitConstructorCheck.cpp
ExplicitMakePairCheck.cpp
+ FunctionNamingCheck.cpp
GlobalNamesInHeadersCheck.cpp
GlobalVariableDeclarationCheck.cpp
GoogleTidyModule.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
