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

[benhamilton]

clang-tidy/objc/PropertyDeclarationCheck.cpp

62–83

Instead of bool isCategory, how about an enum NamingStyle:

enum NamingStyle {
  StandardProperty = 1,
  CategoryProperty = 2,
}

119

I think this is an off-by-one error, right? Change:

i < PropertyName.size() - 1

to

i < PropertyName.size()

120–122

Do we really want a leading _ to count? I think this might need to be a regular expression instead, something like:

^[a-zA-Z][a-zA-Z0-9]+_[a-zA-Z0-9][a-zA-Z0-9_]*$

137

I don't understand what this is doing. Why are we replacing things in a regex?

I don't think this is very maintainable. Can you rewrite it for legibility, please?

158

Is a class extension treated as a category? If so, we probably need to treat it specially.

e.g.:

Foo.h:

@interface Foo
@end

Foo.m:

@interface Foo ()
@property (nonatomic, readonly) int foo_bar;
@end

should not be allowed.

Can you handle this and add tests, please?

[Wizard]

clang-tidy/objc/PropertyDeclarationCheck.cpp

119

I was thinking of not letting go anything that ends with "_" otherwise I have to do more sanity check later.

120–122

Yes it is better to use a regex instead. I would use ^[a-z]+_[a-zA-Z0-9][a-zA-Z0-9_]+$ to make sure we are not having anything like foo_

137

It is a little awkward here because in the matcher, the regex use "::" to indicate matching start, but in llvm::regex it is the conventional "^". I will add some comments here.

[Wizard]

check for class extentsion

[Wizard]

clang-tidy/objc/PropertyDeclarationCheck.cpp

137

End up using a bool since it is not about naming style but whether we are using it in matcher or normal regex match.

[hokein]

clang-tidy/objc/PropertyDeclarationCheck.cpp

28

Please add documentation describing what these properties are.

57–58

Do we need to update the documentation here? it seems stale now?

88–96

Also add documentation for the method as well as parameters. It is a bit hard to follow the logic.

104

Does the comment still make sense? Seems you changed the regex below.

126

no need to pass const reference of llvm::StringRef. StringRef is cheap.

127

nit: use llvm::ArrayRef<std::string> would save a lot of keystroke.

129

we can use find_first_not_of or something similar.

160

Consider using const auto* CategoryDecl = dyn_cast<ObjCCategoryDecl*>(DeclContext), we can get rid of this cast, and NodeKind variable.

[Wizard]

update some documents and comments

clang-tidy/objc/PropertyDeclarationCheck.cpp

104

Yes they are actually the same. I am just replacing the prefix "::" with "^" according to the UsedInMatcher param.

126

The original property name I directly get from ast is a const. If I remove the const here, I will have to make a copy of the property name before calling this.
I prefer keep this const to save that copy :-)

160

Tried that before but I encountered 2 issues:

1. 'clang::DeclContext' is not polymorphic
2. cannot use dynamic_cast with -fno-rtti

which are preventing me from using dynamic_cast

[hokein]

clang-tidy/objc/PropertyDeclarationCheck.cpp

67

I think we save this code by using find_first_of and llvm::StringRef::tolower() function.

95

The same, ArrayRef.

121

The same: llvm::StringRef

126

Why? MatchedDecl->getName() returns llvm::StringRef. As the name indicates, StringRef is a const reference of String, using StringRef is sufficient.

The same to ArrayRef. So the function is like bool prefixedPropertyNameMatches(llvm::StringRef PropertyName, llvm::ArrayRef<std::string> Acronyms).

160

Sorry, I mean llvm::dyn_cast here, it should work.

163

Do we need it? I think you can pass SpecialAcronyms directly.

[Wizard]

clang-tidy/objc/PropertyDeclarationCheck.cpp

121

Cannot remove the const for the same reason.

126

If I remove the const in the method, compiler will have error of "candidate function not viable: expects an l-value for 1st argument". I believe it cannot accept a const reference as arg if the definition is var.

160

It is not working either. It says "error: static_cast from 'clang::Decl *' to 'const clang::ObjCCategoryDecl *const *' is not allowed" though I have no idea why it is regarded as a static cast.
I am using it like this:
const auto *CategoryDecl = llvm::dyn_cast<const ObjCCategoryDecl *>(DeclContext);

[Wizard]

resolve comments

[benhamilton]

clang-tidy/objc/PropertyDeclarationCheck.cpp

126

I think hokein@ is saying you should change:

const llvm::StringRef &

to:

llvm::StringRef

Note removing both const and &, not just removing const. This is because there is no need to pass these by reference, they are already a reference.

Same with const llvm::ArrayRef &, it should just be llvm::ArrayRef.

132

No need to construct another llvm::StringRef(), StringRef::substr() returns a StringRef already.

160

You definitely don't want to use a C-style cast. My guess is adding const is not part of what dyn_cast<> does, so you can probably remove that from the dyn_cast<>. (You can of course assign to a const pointer if you want.)

[Wizard]

resolve comments and fix some logic

[benhamilton]

clang-tidy/objc/PropertyDeclarationCheck.cpp

129

Can you also check that Start + 1 < PropertyName.length() (e.g., the string doesn't end with a _)?

I know the regular expression shouldn't match this, so an assert is probably good. (There is logic later which tries to access Start + 1 without checking.)

135

Don't need to explicitly wrap in llvm::StringRef, that class has an implicit constructor from std::string.

https://github.com/llvm-mirror/llvm/blob/master/include/llvm/ADT/StringRef.h#L95

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

15–31

Can we add checks for properties whose names end with _?

[Wizard]

add more tests

[benhamilton]

Thanks!

[Wizard]

clang-tidy/objc/PropertyDeclarationCheck.cpp

160

Finally figured out that I cannot put pointer type in dyn_cast<>. It needs to be "llvm::dyn_cast<ObjCCategoryDecl>(DeclContext)" even though DeclContext is a pointer. I have to say it is weird in C++...

163

It is not about acronyms but determine how to generate the fix. It will save me from checking the naming format again when generating the fix hint.

[Wizard]

merge changes and add EscapedAcronyms to store the actual acronyms during runtime.

[hokein]

clang-tidy/objc/PropertyDeclarationCheck.cpp

126

Yeah, this is what I mean, thanks for the clarifying.

160

Yeah, llvm::dyn_cast doesn't need to specify the pointer, sorry for my beginning comment...

clang-tidy/objc/PropertyDeclarationCheck.cpp

 //===--- PropertyDeclarationCheck.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 "PropertyDeclarationCheck.h"
+#include <algorithm>
 #include "../utils/OptionsUtils.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Basic/CharInfo.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Regex.h"
-#include <algorithm>
 
 using namespace clang::ast_matchers;
 
 namespace clang {
 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
 constexpr char DefaultSpecialAcronyms[] =
     "ASCII;"
     "PDF;"
     "XML;"
     "HTML;"
     "URL;"
     "RTF;"
     "HTTP;"
     "TIFF;"
     "JPG;"
     "PNG;"
     "GIF;"
     "LZW;"
     "ROM;"
     "RGB;"
     "CMYK;"
     "MIDI;"
     "FTP;"
     "ID";
 
-/// 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]);
+FixItHint generateFixItHint(const ObjCPropertyDecl *Decl, NamingStyle Style) {
+  auto Name = Decl->getName();
+  size_t Index = 0;
+  if (Style == CategoryProperty) {
+    for (size_t i = 0; i < Name.size(); ++i) {
+      if (Name[i] == '_') {
+        Index = i + 1;
+        break;
+      }
+      if (isUppercase(Name[i])) {
+        NewName[i] = tolower(Name[i]);
+      }
+    }
+  }
+  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> &Acronyms) {
+// This method returns the regex to match lowerCamelCase
+// property names with special acronyms.
+// Due to the feature in ast matcher, we need to use '::'
+// to match the start when it is used in matchers.
+// Acronyms is the list of special acronyms we need to recognize,
+// and UsedInMatcher param indicates whether the regex will be used
+// in matchers or not.
+std::string validPropertyNameRegex(const std::vector<std::string> &Acronyms,
+                                   bool UsedInMatcher) {
   std::vector<std::string> EscapedAcronyms;
   EscapedAcronyms.reserve(Acronyms.size());
   // In case someone defines a custom prefix which includes a regex
   // special character, escape all the prefixes.
   std::transform(Acronyms.begin(), Acronyms.end(),
-                 std::back_inserter(EscapedAcronyms), [](const std::string& s) {
-                   return llvm::Regex::escape(s); });
+                 std::back_inserter(EscapedAcronyms),
+                 [](const std::string &s) { return llvm::Regex::escape(s); });
   // Allow any of these names:
   // foo
   // fooBar
   // url
   // urlString
   // URL
   // URLString
   // bundleID
-  return std::string("::((") +
+  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(), "|") + ")?$";
+         llvm::join(EscapedAcronyms.begin(), EscapedAcronyms.end(), "|") +
+         ")?$";
+}
+
+bool hasCategoryPropertyPrefix(const llvm::StringRef &PropertyName) {
+  auto RegexExp = llvm::Regex("^[a-z]+_[a-zA-Z0-9][a-zA-Z0-9_]+$");
+  return RegexExp.match(PropertyName);
+}
+
+bool prefixedPropertyNameMatches(const llvm::StringRef &PropertyName,
+                                 const llvm::ArrayRef<std::string> &Acronyms) {
+  size_t Start = PropertyName.find_first_of('_');
+  assert(Start != llvm::StringRef::npos);
+  auto RegexExp =
+      llvm::Regex(llvm::StringRef(validPropertyNameRegex(Acronyms, false)));
+  return RegexExp.match(llvm::StringRef(PropertyName.substr(Start + 1)));
 }
 }  // namespace
 
 PropertyDeclarationCheck::PropertyDeclarationCheck(StringRef Name,
                                                    ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
       SpecialAcronyms(utils::options::parseStringList(
           Options.get("Acronyms", DefaultSpecialAcronyms))) {}
 
 void PropertyDeclarationCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
       objcPropertyDecl(
           // the property name should be in Lower Camel Case like
           // 'lowerCamelCase'
-          unless(matchesName(validPropertyNameRegex(SpecialAcronyms))))
+          unless(matchesName(validPropertyNameRegex(SpecialAcronyms, true))))
           .bind("property"),
       this);
 }
 
 void PropertyDeclarationCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *MatchedDecl =
       Result.Nodes.getNodeAs<ObjCPropertyDecl>("property");
   assert(MatchedDecl->getName().size() > 0);
+  const auto *DeclContext = MatchedDecl->getDeclContext();
+  auto NodeKind = DeclContext->getDeclKind();
+  if (NodeKind == Decl::ObjCCategory &&
+      hasCategoryPropertyPrefix(MatchedDecl->getName())) {
+    const auto *CategoryDecl = (ObjCCategoryDecl *)(DeclContext);
+    if (!prefixedPropertyNameMatches(
+            MatchedDecl->getName(),
+            llvm::ArrayRef<std::string>(SpecialAcronyms)) ||
+        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) {
   Options.store(Opts, "Acronyms",
                 utils::options::serializeStringList(SpecialAcronyms));
 }
 
 }  // namespace objc
 }  // namespace tidy
 }  // namespace clang

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

 the check of Lower Camel Case according to the guide:
 https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/CodingGuidelines/Articles/NamingBasics.html#//apple_ref/doc/uid/20001281-1002931-BBCFHEAB
 
 For a full list of well-known acronyms:
 https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/CodingGuidelines/Articles/APIAbbreviations.html#//apple_ref/doc/uid/20001285-BCIHCGAE
 
 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
 -------
 
 .. option:: Acronyms
 
    Semicolon-separated list of acronyms that can be used as prefix
    of property names.
 
    Defaults to `ASCII;PDF;XML;HTML;URL;RTF;HTTP;TIFF;JPG;PNG;GIF;LZW;ROM;RGB;CMYK;MIDI;FTP`.

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

 // RUN: %check_clang_tidy %s objc-property-declaration %t \
 // RUN: -config='{CheckOptions: \
 // RUN:  [{key: objc-property-declaration.Acronyms, value: "ABC;TGIF"}]}' \
 // RUN: --
 @class NSString;
 
 @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]
 @end

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

 // RUN: %check_clang_tidy %s objc-property-declaration %t
 @class NSString;
 
 @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;
 @property(strong, nonatomic) NSString *bundleID;
 @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 abc_camelCase;
+@end
+
+@interface Foo ()
+@property(assign, nonatomic) int abc_camelCase;
+// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'abc_camelCase' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
+@end
 No newline at end of file