[PATCH] D39829: add new check for property declaration

Yan Zhang via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 9 13:13:34 PST 2017


Wizard 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.
----------------
benhamilton wrote:
> 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.
Will do


https://reviews.llvm.org/D39829





More information about the cfe-commits mailing list