[PATCH] D39829: add new check for property declaration

[Haojian Wu via Phabricator via cfe-commits]

hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.

Looks good to me. I (or @benhamilton) will commit the patch for you if @benhamilton is fine with it.



================
Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:22
+namespace {
+constexpr char DefaultSpecialAcronyms[] =
+    "ASCII;"
----------------
nit: add a comment documenting these are from https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/CodingGuidelines/Articles/APIAbbreviations.html#//apple_ref/doc/uid/20001285-BCIHCGAE.


================
Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:93
+       "property '%0' is not in proper format according to property naming "
+       "convention. It should be in the format of lowerCamelCase or has "
+       "special acronyms")
----------------
nit: clang-tidy message is not a complete sentence. just `convertion; it should`.


================
Comment at: docs/clang-tidy/checks/objc-property-declaration.rst:40
+
+   Semicolon-separated list of acronys that can be used as prefix
+   of property names.
----------------
s/acronys/acronyms


================
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
----------------
benhamilton wrote:
> Wizard wrote:
> > hokein wrote:
> > > Why does the check catch this case? Isn't `camelCase` a correct name?
> > This is CHECK-MESSAGES-NOT
> I think in that case you don't need an explicit CHECK (there is implicitly a "CHECK-MESSAGES-NOT" on every line for all warnings which fails the test if a warning is raised).
Sorry, I misread it as `CHECK-MESSAGES`. As Ben pointed out, we don't need explicit CHECK here (the default behavior is what we expected).


https://reviews.llvm.org/D39829