[PATCH] D42947: Support special acronyms inside property names and allow plural forms

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton accepted this revision.
benhamilton added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:144
+         "[A-Z]?)?[a-z]+[a-z0-9]*(" +
+         AcronymsGroupRegex(EscapedAcronyms, true) + "|([A-Z][a-z0-9]+))*$";
 }
----------------
Wizard wrote:
> benhamilton wrote:
> > Why do we not allow singular acronyms in the middle of the property name?
> > 
> > I think we should allow singular and plural acronyms anywhere.
> Actually we do. AcronymsGroupRegex(EscapedAcronyms, true) will support both while AcronymsGroupRegex(EscapedAcronyms, false) only supports singular. Will update test cases.
Ah, this is why I find boolean flags to be confusing :) Anyway, it's gone now.


================
Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:139
 
-  return StartMatcher + "((" +
-         llvm::join(EscapedAcronyms.begin(), EscapedAcronyms.end(), "|") +
-         ")[A-Z]?)?[a-z]+[a-z0-9]*([A-Z][a-z0-9]+)*" + "(" +
-         llvm::join(EscapedAcronyms.begin(), EscapedAcronyms.end(), "|") +
-         ")?$";
+  return StartMatcher + "(" + AcronymsGroupRegex(EscapedAcronyms) +
+         "[A-Z]?)?[a-z]+[a-z0-9]*(" + AcronymsGroupRegex(EscapedAcronyms) +
----------------
Since `AcronymsGroupRegex()` is called twice with the same parameter, please store the result in a local variable instead of doing the work twice.


================
Comment at: test/clang-tidy/objc-property-declaration.m:12
 @property(strong, nonatomic) NSString *URLString;
 @property(strong, nonatomic) NSString *bundleID;
 @property(strong, nonatomic) NSData *RGBABytes;
----------------
Add a check for a plural at the end, please.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42947