[PATCH] D39829: add new check for property declaration

Ben Hamilton via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 9 08:42:13 PST 2017


benhamilton added inline comments.


================
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.
----------------
hokein wrote:
> 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).
I agree, removing a prefix is not a good idea. Warning is fine.

We could probably also change `snake_case` variables to `CamelCase` automatically. Not sure if it's worth doing in this review, but @Wizard can file a bug to follow up and add a TODO comment here mentioning the bug.


https://reviews.llvm.org/D39829





More information about the cfe-commits mailing list