diff --git a/clang-tools-extra/clang-tidy/google/CMakeLists.txt b/clang-tools-extra/clang-tidy/google/CMakeLists.txt --- a/clang-tools-extra/clang-tidy/google/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/google/CMakeLists.txt @@ -15,6 +15,7 @@ IntegerTypesCheck.cpp NonConstReferences.cpp OverloadedUnaryAndCheck.cpp + RequireCategoryMethodPrefixesCheck.cpp TodoCommentCheck.cpp UnnamedNamespaceInHeaderCheck.cpp UpgradeGoogletestCaseCheck.cpp diff --git a/clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp b/clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp --- a/clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp @@ -25,6 +25,7 @@ #include "IntegerTypesCheck.h" #include "NonConstReferences.h" #include "OverloadedUnaryAndCheck.h" +#include "RequireCategoryMethodPrefixesCheck.h" #include "TodoCommentCheck.h" #include "UnnamedNamespaceInHeaderCheck.h" #include "UpgradeGoogletestCaseCheck.h" @@ -39,6 +40,8 @@ class GoogleModule : public ClangTidyModule { public: void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { + CheckFactories.registerCheck( + "google-objc-require-category-method-prefixes"); CheckFactories.registerCheck( "google-build-explicit-make-pair"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.h b/clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.h new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.h @@ -0,0 +1,39 @@ +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_REQUIRECATEGORYMETHODPREFIXES_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_REQUIRECATEGORYMETHODPREFIXES_H + +#include "../ClangTidyCheck.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +namespace clang { +namespace tidy { +namespace google { +namespace objc { + +// Warn about missing prefixes in category method names. +class RequireCategoryMethodPrefixesCheck : public ClangTidyCheck { +public: + RequireCategoryMethodPrefixesCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + whitelisted_prefixes_(Options.get("WhitelistedPrefixes", "")) {} + + /// Make configuration of checker discoverable. + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + + void registerMatchers(ast_matchers::MatchFinder *finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &result) override; + +private: + /// Semicolon-separated list of whitelisted class prefixes. + /// Defaults to empty. + const std::string whitelisted_prefixes_; + + std::unique_ptr + matching_whitelisted_prefix(std::string class_name); +}; + +} // namespace objc +} // namespace google +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_REQUIRECATEGORYMETHODPREFIXES_H diff --git a/clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp b/clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp @@ -0,0 +1,107 @@ +#include "RequireCategoryMethodPrefixesCheck.h" + +#include "../ClangTidyOptions.h" +#include "../utils/OptionsUtils.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchers.h" + +using namespace clang::ast_matchers; // NOLINT: Too many names. + +namespace clang { +namespace tidy { +namespace google { +namespace objc { + +namespace { +const char *kCustomCategoryMethodIdentifier = "ThisIsACategoryMethod"; +} // anonymous namespace + +void RequireCategoryMethodPrefixesCheck::registerMatchers(MatchFinder *finder) { + // This check should be run only on Objective-C code. + if (!getLangOpts().ObjC) { + return; + } + + auto method_declaration = + objcMethodDecl().bind(kCustomCategoryMethodIdentifier); + finder->addMatcher(method_declaration, this); +} + +void RequireCategoryMethodPrefixesCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "WhitelistedPrefixes", whitelisted_prefixes_); +} + +std::unique_ptr +RequireCategoryMethodPrefixesCheck::matching_whitelisted_prefix( + std::string class_name) { + std::vector prefix_array = + utils::options::parseStringList(whitelisted_prefixes_); + + for (auto &iterator : prefix_array) { + const char *prefix = iterator.c_str(); + if (!strncmp(class_name.c_str(), prefix, strlen(prefix))) { + return llvm::make_unique(prefix); + } + } + return nullptr; +} + +void RequireCategoryMethodPrefixesCheck::check( + const MatchFinder::MatchResult &result) { + const auto *method_declaration = + result.Nodes.getNodeAs(kCustomCategoryMethodIdentifier); + if (!method_declaration) { + return; + } + std::string method_name = method_declaration->getNameAsString(); + auto owning_objc_class_interface = method_declaration->getClassInterface(); + if (!owning_objc_class_interface) { + return; + } + std::string class_name = owning_objc_class_interface->getNameAsString(); + + auto owning_objc_category = + dyn_cast(method_declaration->getDeclContext()); + // Bail early if the method is not in a category. + if (!owning_objc_category || owning_objc_category->isInvalidDecl()) { + return; + } + + // Bail early if the method is in a category on a class with a whitelisted + // prefix. + std::unique_ptr matching_prefix = + matching_whitelisted_prefix(class_name); + if (matching_prefix) { + return; + } + + // Check whether the method name has a proper prefix. + for (const char *pos = method_name.c_str(); pos && *pos; ++pos) { + // The characters a-z are allowed in a prefix. Go to the next + // character. + if (*pos >= 'a' && *pos <= 'z') { + continue; + } + + // The underscore character (_) terminates the prefix. As long as it is + // not the last character in the name, the declaration passes. + if (*pos == '_' && *(pos + 1) != '\0') { + return; + } + + // The name does not start with /[a-z]+_/. This declaration fails. + break; + } + // If the end of the string is reached or an illegal character appears + // before the terminating underscore, then the method name is not + // properly prefixed. Throw an error. + diag(method_declaration->getBeginLoc(), + "the category method %0 is not properly prefixed.") + << method_name; +} + +} // namespace objc +} // namespace google +} // namespace tidy +} // namespace clang diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -67,6 +67,13 @@ Improvements to clang-tidy -------------------------- +- New :doc:`google-objc-require-category-method-prefixes + ` check. + + Warns when Objective-C category method names are not properly prefixed (e.g. + gmo_methodName) unless the category is extending a class with a (configurable) + whitelisted prefix. + - New :doc:`linuxkernel-must-use-errs ` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst b/clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst new file mode 100644 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst @@ -0,0 +1,39 @@ +.. title:: clang-tidy - google-objc-require-category-method-prefixes + +google-objc-require-category-method-prefixes +=========================== + +Finds method declarations in Objective-C files that do not follow the pattern +described in the Google Objective-C Style Guide. + +The Google Objective-C style guide requires +`prefixes for methods http://go/objc-style#Category_Names`_ in categories on +classes that you don't control (for example, categories on Apple or third-party +framework classes or classes created by other teams) to prevent name collisions +when those frameworks are updated. + +This checker ensures that all methods in categories have some sort of prefix +(e.g. gmo_). It excludes categories on classes whose names have a whitelisted +three-letter prefix. + +You should set the clang option WhitelistedPrefixes to a semicolon-delimited +lits of class prefixes within your project if you want to be able to create +unprefixed category methods on classes whose names begin with those prefixes. + +For example, the following code sample is a properly prefixed method on a +non-owned class (NSObject): + +.. code-block:: objc + @interface NSObject (QEDMyCategory) + - (BOOL)qed_myCustomMethod; + @end + +If you whitelist the QED three-letter prefix, the following code sample +is also allowed: + +.. code-block:: objc + + @interface QEDMyClass (MyCategory) + - (BOOL)myCustomMethod; + @end + diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -229,6 +229,7 @@ google-objc-avoid-throwing-exception google-objc-function-naming google-objc-global-variable-declaration + google-objc-require-category-method-prefixes google-readability-avoid-underscore-in-googletest-name google-readability-braces-around-statements (redirects to readability-braces-around-statements) google-readability-casting diff --git a/clang-tools-extra/test/clang-tidy/google-objc-require-category-method-prefixes.m b/clang-tools-extra/test/clang-tidy/google-objc-require-category-method-prefixes.m new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/google-objc-require-category-method-prefixes.m @@ -0,0 +1,35 @@ +// RUN: %check_clang_tidy %s google-objc-require-category-method-prefixes %t -config="{CheckOptions: [{key: google-objc-require-category-method-prefixes.WhitelistedPrefixes, value: GMO}]}" + +@class NSString; + +@interface NSURL ++ (nullable instancetype)URLWithString:(NSString *)URLString; ++ (instancetype)alloc; +- (instancetype)init; +@end + +@interface NSURL (CustomExtension) + +- (void)unprefixedMethod; +- (void)unprefixed; +- (void)justprefixed_; + +@end + +// CHECK-MESSAGES: :[[@LINE-6]]:1: warning: the category method unprefixedMethod is not properly prefixed. [google-objc-require-category-method-prefixes] +// CHECK-MESSAGES: :[[@LINE-6]]:1: warning: the category method unprefixed is not properly prefixed. [google-objc-require-category-method-prefixes] +// CHECK-MESSAGES: :[[@LINE-6]]:1: warning: the category method justprefixed_ is not properly prefixed. [google-objc-require-category-method-prefixes] + +@interface NSURL (CustomExtension2) +- (void)gmo_prefixedMethod; +@end + +@interface GMOURL ++ (nullable instancetype)URLWithString:(NSString *)URLString; ++ (instancetype)alloc; +- (instancetype)init; +@end + +@interface GMOURL (CustomExtension3) +- (void)unprefixedMethodInWhitelistedClass; +@end