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

[Yan Zhang via Phabricator via cfe-commits]

Wizard added inline comments.


================
Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:142
 
-  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, false) +
+         "[A-Z]?)?[a-z]+[a-z0-9]*(" +
----------------
benhamilton wrote:
> Why do we not allow plural acronyms at the start of the property name? For example:
> 
> ```lang=objc
> @property(nonatomic) NSArray<NSString *> *URLsToFetch;
> ```
> 
> should be allowed.
> 
Hmm I was thinking that prefix should not have plural form. Will enable plural forms everywhere.


================
Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:144
+         "[A-Z]?)?[a-z]+[a-z0-9]*(" +
+         AcronymsGroupRegex(EscapedAcronyms, true) + "|([A-Z][a-z0-9]+))*$";
 }
----------------
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.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42947