[PATCH] D39829: add new check for property declaration

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.


================
Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:22
+
+/// we will do best effort to generate a fix, however, if the
+/// case can not be solved with a simple fix (e.g. CamelCase
----------------
I think we need to update the comment according to the new change (it is a bit ambiguous to users).  We only fix "camelCase" currently.


================
Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:25
+/// is a simple fix), we will leave the fix to the user.
+/// TODO: provide fix for snake_case to snakeCase
+FixItHint generateFixItHint(const ObjCPropertyDecl *Decl) {
----------------
nit: llvm uses `FIXME`.


================
Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:27
+FixItHint generateFixItHint(const ObjCPropertyDecl *Decl) {
+  if (isupper(Decl->getName()[0])) {
+    auto NewName = Decl->getName().str();
----------------
nit: I would add an assert to make sure the name is not empty.


================
Comment at: docs/ReleaseNotes.rst:63
+
+  FIXME: add release notes.
+
----------------
nit:  this can be removed.


================
Comment at: docs/clang-tidy/checks/objc-property-declaration.rst:7
+Finds property declarations in Objective-C files that do not follow the pattern
+of property names in Google's Objective-C Style Guide. The property name should
+be in the format of Lower Camel Case.
----------------
benhamilton wrote:
> hokein wrote:
> > Google style? but the link you provided is Apple.
> Google's Objective-C style guide is a list of additions on top of Apple's Objective-C style guide.
> 
> Property naming standards are defined in Apple's style guide, and not changed by Google's.
I see, thanks for the clarification, I think the "Apple" would be clearer.


================
Comment at: docs/clang-tidy/checks/objc-property-declaration.rst:22
+
+The check will do best effort to give a fix, however, in some cases it is
+difficult to give a proper fix since the property name could be complicated.
----------------
The same.


================
Comment at: test/clang-tidy/objc-property-declaration.m:8
+ at property(assign, nonatomic) int camelCase;
+// CHECK-MESSAGES-NOT: :[[@LINE-1]]:34: warning: property 'camelCase' is not in proper format according to property naming convention [objc-property-declaration]
+ at end
----------------
Why does the check catch this case? Isn't `camelCase` a correct name?


https://reviews.llvm.org/D39829