add prefix with '_' support for property name. Corresponding apple dev doc: https://developer.apple.com/library/content/qa/qa1908/_index.html

Reviewers: benhamilton, hokein

Reviewed By: benhamilton

Subscribers: klimek, cfe-commits

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

llvm-svn: 323722

clang-tools-extra/clang-tidy/objc/PropertyDeclarationCheck.cpp

@@ -8,13 +8,14 @@
 //===----------------------------------------------------------------------===//
 
 #include "PropertyDeclarationCheck.h"
+#include <algorithm>
 #include "../utils/OptionsUtils.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "llvm/ADT/STLExtras.h"
+#include "clang/Basic/CharInfo.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Regex.h"
-#include <algorithm>
 
 using namespace clang::ast_matchers;
 
@@ -23,6 +24,16 @@ namespace tidy {
 namespace objc {
 
 namespace {
+
+// For StandardProperty the naming style is 'lowerCamelCase'.
+// For CategoryProperty especially in categories of system class,
+// to avoid naming conflict, the suggested naming style is
+// 'abc_lowerCamelCase' (adding lowercase prefix followed by '_').
+enum NamingStyle {
+  StandardProperty = 1,
+  CategoryProperty = 2,
+};
+
 /// The acronyms are from
 /// https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/CodingGuidelines/Articles/APIAbbreviations.html#//apple_ref/doc/uid/20001285-BCIHCGAE
 ///
@@ -84,22 +95,31 @@ constexpr llvm::StringLiteral DefaultSpecialAcronyms[] = {
     "XML",
 };
 
-/// For now we will only fix 'CamelCase' property to
-/// 'camelCase'. For other cases the users need to
+/// For now we will only fix 'CamelCase' or 'abc_CamelCase' property to
+/// 'camelCase' or 'abc_camelCase'. For other cases the users need to
 /// come up with a proper name by their own.
 /// FIXME: provide fix for snake_case to snakeCase
-FixItHint generateFixItHint(const ObjCPropertyDecl *Decl) {
-  if (isupper(Decl->getName()[0])) {
-    auto NewName = Decl->getName().str();
-    NewName[0] = tolower(NewName[0]);
-    return FixItHint::CreateReplacement(
-        CharSourceRange::getTokenRange(SourceRange(Decl->getLocation())),
-        llvm::StringRef(NewName));
+FixItHint generateFixItHint(const ObjCPropertyDecl *Decl, NamingStyle Style) {
+  auto Name = Decl->getName();
+  auto NewName = Decl->getName().str();
+  size_t Index = 0;
+  if (Style == CategoryProperty) {
+    Index = Name.find_first_of('_') + 1;
+    NewName.replace(0, Index - 1, Name.substr(0, Index - 1).lower());
+  }
+  if (Index < Name.size()) {
+    NewName[Index] = tolower(NewName[Index]);
+    if (NewName != Name) {
+      return FixItHint::CreateReplacement(
+          CharSourceRange::getTokenRange(SourceRange(Decl->getLocation())),
+          llvm::StringRef(NewName));
+    }
   }
   return FixItHint();
 }
 
-std::string validPropertyNameRegex(const std::vector<std::string> &EscapedAcronyms) {
+std::string validPropertyNameRegex(llvm::ArrayRef<std::string> EscapedAcronyms,
+                                   bool UsedInMatcher) {
   // Allow any of these names:
   // foo
   // fooBar
@@ -108,10 +128,31 @@ std::string validPropertyNameRegex(const std::vector<std::string> &EscapedAcrony
   // URL
   // URLString
   // bundleID
-  return std::string("::((") +
-      llvm::join(EscapedAcronyms.begin(), EscapedAcronyms.end(), "|") +
-      ")[A-Z]?)?[a-z]+[a-z0-9]*([A-Z][a-z0-9]+)*" + "(" +
-      llvm::join(EscapedAcronyms.begin(), EscapedAcronyms.end(), "|") + ")?$";
+  std::string StartMatcher = UsedInMatcher ? "::" : "^";
+
+  return StartMatcher + "((" +
+         llvm::join(EscapedAcronyms.begin(), EscapedAcronyms.end(), "|") +
+         ")[A-Z]?)?[a-z]+[a-z0-9]*([A-Z][a-z0-9]+)*" + "(" +
+         llvm::join(EscapedAcronyms.begin(), EscapedAcronyms.end(), "|") +
+         ")?$";
+}
+
+bool hasCategoryPropertyPrefix(llvm::StringRef PropertyName) {
+  auto RegexExp = llvm::Regex("^[a-zA-Z]+_[a-zA-Z0-9][a-zA-Z0-9_]+$");
+  return RegexExp.match(PropertyName);
+}
+
+bool prefixedPropertyNameValid(llvm::StringRef PropertyName,
+                               llvm::ArrayRef<std::string> Acronyms) {
+  size_t Start = PropertyName.find_first_of('_');
+  assert(Start != llvm::StringRef::npos && Start + 1 < PropertyName.size());
+  auto Prefix = PropertyName.substr(0, Start);
+  if (Prefix.lower() != Prefix) {
+    return false;
+  }
+  auto RegexExp =
+      llvm::Regex(llvm::StringRef(validPropertyNameRegex(Acronyms, false)));
+  return RegexExp.match(PropertyName.substr(Start + 1));
 }
 }  // namespace
 
@@ -120,10 +161,10 @@ PropertyDeclarationCheck::PropertyDeclarationCheck(StringRef Name,
     : ClangTidyCheck(Name, Context),
       SpecialAcronyms(
           utils::options::parseStringList(Options.get("Acronyms", ""))),
-      IncludeDefaultAcronyms(Options.get("IncludeDefaultAcronyms", true)) {}
+      IncludeDefaultAcronyms(Options.get("IncludeDefaultAcronyms", true)),
+      EscapedAcronyms() {}
 
 void PropertyDeclarationCheck::registerMatchers(MatchFinder *Finder) {
-  std::vector<std::string> EscapedAcronyms;
   if (IncludeDefaultAcronyms) {
     EscapedAcronyms.reserve(llvm::array_lengthof(DefaultSpecialAcronyms) +
                             SpecialAcronyms.size());
@@ -143,7 +184,7 @@ void PropertyDeclarationCheck::registerMatchers(MatchFinder *Finder) {
       objcPropertyDecl(
           // the property name should be in Lower Camel Case like
           // 'lowerCamelCase'
-          unless(matchesName(validPropertyNameRegex(EscapedAcronyms))))
+          unless(matchesName(validPropertyNameRegex(EscapedAcronyms, true))))
           .bind("property"),
       this);
 }
@@ -152,10 +193,26 @@ void PropertyDeclarationCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *MatchedDecl =
       Result.Nodes.getNodeAs<ObjCPropertyDecl>("property");
   assert(MatchedDecl->getName().size() > 0);
+  auto *DeclContext = MatchedDecl->getDeclContext();
+  auto *CategoryDecl = llvm::dyn_cast<ObjCCategoryDecl>(DeclContext);
+  if (CategoryDecl != nullptr &&
+      hasCategoryPropertyPrefix(MatchedDecl->getName())) {
+    if (!prefixedPropertyNameValid(MatchedDecl->getName(), EscapedAcronyms) ||
+        CategoryDecl->IsClassExtension()) {
+      NamingStyle Style = CategoryDecl->IsClassExtension() ? StandardProperty
+                                                           : CategoryProperty;
+      diag(MatchedDecl->getLocation(),
+           "property name '%0' not using lowerCamelCase style or not prefixed "
+           "in a category, according to the Apple Coding Guidelines")
+          << MatchedDecl->getName() << generateFixItHint(MatchedDecl, Style);
+    }
+    return;
+  }
   diag(MatchedDecl->getLocation(),
-       "property name '%0' should use lowerCamelCase style, according to "
-       "the Apple Coding Guidelines")
-      << MatchedDecl->getName() << generateFixItHint(MatchedDecl);
+       "property name '%0' not using lowerCamelCase style or not prefixed in "
+       "a category, according to the Apple Coding Guidelines")
+      << MatchedDecl->getName()
+      << generateFixItHint(MatchedDecl, StandardProperty);
 }
 
 void PropertyDeclarationCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {

clang-tools-extra/clang-tidy/objc/PropertyDeclarationCheck.h

@@ -36,6 +36,7 @@ public:
 private:
   const std::vector<std::string> SpecialAcronyms;
   const bool IncludeDefaultAcronyms;
+  std::vector<std::string> EscapedAcronyms;
 };
 
 } // namespace objc

clang-tools-extra/docs/clang-tidy/checks/objc-property-declaration.rst

@@ -32,6 +32,15 @@ https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/Codin
 
 The corresponding style rule: https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/CodingGuidelines/Articles/NamingIvarsAndTypes.html#//apple_ref/doc/uid/20001284-1001757
 
+The check will also accept property declared in category with a prefix of
+lowercase letters followed by a '_' to avoid naming conflict. For example:
+
+.. code-block:: objc
+@property(nonatomic, assign) int abc_lowerCamelCase;
+
+The corresponding style rule: https://developer.apple.com/library/content/qa/qa1908/_index.html
+
+
 Options
 -------
 

clang-tools-extra/test/clang-tidy/objc-property-declaration-additional.m

@@ -6,10 +6,10 @@
 
 @interface Foo
 @property(assign, nonatomic) int AbcNotRealPrefix;
-// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'AbcNotRealPrefix' should use lowerCamelCase style, according to the Apple Coding Guidelines [objc-property-declaration]
+// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'AbcNotRealPrefix' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
 // CHECK-FIXES: @property(assign, nonatomic) int abcNotRealPrefix;
 @property(assign, nonatomic) int ABCCustomPrefix;
 @property(strong, nonatomic) NSString *ABC_custom_prefix;
-// CHECK-MESSAGES: :[[@LINE-1]]:40: warning: property name 'ABC_custom_prefix' should use lowerCamelCase style, according to the Apple Coding Guidelines [objc-property-declaration]
+// CHECK-MESSAGES: :[[@LINE-1]]:40: warning: property name 'ABC_custom_prefix' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
 @property(assign, nonatomic) int GIFShouldIncludeStandardAcronym;
 @end

clang-tools-extra/test/clang-tidy/objc-property-declaration-custom.m

@@ -7,11 +7,11 @@
 
 @interface Foo
 @property(assign, nonatomic) int AbcNotRealPrefix;
-// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'AbcNotRealPrefix' should use lowerCamelCase style, according to the Apple Coding Guidelines [objc-property-declaration]
+// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'AbcNotRealPrefix' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
 // CHECK-FIXES: @property(assign, nonatomic) int abcNotRealPrefix;
 @property(assign, nonatomic) int ABCCustomPrefix;
 @property(strong, nonatomic) NSString *ABC_custom_prefix;
-// CHECK-MESSAGES: :[[@LINE-1]]:40: warning: property name 'ABC_custom_prefix' should use lowerCamelCase style, according to the Apple Coding Guidelines [objc-property-declaration]
+// CHECK-MESSAGES: :[[@LINE-1]]:40: warning: property name 'ABC_custom_prefix' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
 @property(assign, nonatomic) int GIFIgnoreStandardAcronym;
-// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'GIFIgnoreStandardAcronym' should use lowerCamelCase style, according to the Apple Coding Guidelines [objc-property-declaration]
+// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'GIFIgnoreStandardAcronym' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
 @end

clang-tools-extra/test/clang-tidy/objc-property-declaration.m

@@ -5,7 +5,7 @@
 
 @interface Foo
 @property(assign, nonatomic) int NotCamelCase;
-// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'NotCamelCase' should use lowerCamelCase style, according to the Apple Coding Guidelines [objc-property-declaration]
+// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'NotCamelCase' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
 // CHECK-FIXES: @property(assign, nonatomic) int notCamelCase;
 @property(assign, nonatomic) int camelCase;
 @property(strong, nonatomic) NSString *URLString;
@@ -13,5 +13,25 @@
 @property(strong, nonatomic) NSData *RGBABytes;
 @property(strong, nonatomic) UIViewController *notificationsVC;
 @property(strong, nonatomic) NSString *URL_string;
-// CHECK-MESSAGES: :[[@LINE-1]]:40: warning: property name 'URL_string' should use lowerCamelCase style, according to the Apple Coding Guidelines [objc-property-declaration]
+// CHECK-MESSAGES: :[[@LINE-1]]:40: warning: property name 'URL_string' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
 @end
+
+@interface Foo (Bar)
+@property(assign, nonatomic) int abc_NotCamelCase;
+// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'abc_NotCamelCase' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
+@property(assign, nonatomic) int abCD_camelCase;
+// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'abCD_camelCase' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
+// CHECK-FIXES: @property(assign, nonatomic) int abcd_camelCase;
+@property(assign, nonatomic) int abCD_NotCamelCase;
+// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'abCD_NotCamelCase' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
+// CHECK-FIXES: @property(assign, nonatomic) int abcd_notCamelCase;
+@property(assign, nonatomic) int wrongFormat_;
+// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'wrongFormat_' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
+@property(strong, nonatomic) NSString *URLStr;
+@property(assign, nonatomic) int abc_camelCase;
+@end
+
+@interface Foo ()
+@property(assign, nonatomic) int abc_inClassExtension;
+// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'abc_inClassExtension' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
+@end