[PATCH] D39829: add new check for property declaration

Haojian Wu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 9 02:59:30 PST 2017


hokein added a comment.

Next time, please remember to add `cfe-commits` to the subscriber.



================
Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:14
+
+#include <ctype.h>
+
----------------
Do we need this header?


================
Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:25
+/// we will do best effort to generate a fix, however, if the
+/// case can not be solved with a simple fix (e.g. remove prefix or change first
+/// character), we will leave the fix to the user.
----------------
I might miss some background context. 

The fix of the check seems to me that it does more things it should. It removes all the non-alphabetical prefix characters, I'd be conservative of the fix here (just fix the case "CamelCase", and only give a warning for other cases).


================
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.
----------------
Google style? but the link you provided is Apple.


================
Comment at: docs/clang-tidy/checks/objc-property-declaration.rst:27
+The corresponding style rule: https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/CodingGuidelines/Articles/NamingIvarsAndTypes.html#//apple_ref/doc/uid/20001284-1001757
\ No newline at end of file

----------------
nit: add a newline.


https://reviews.llvm.org/D39829





More information about the cfe-commits mailing list