benhamilton requested changes to this revision. benhamilton added a comment. This revision now requires changes to proceed.
Can you add test cases for non-plural acronyms in the middle of the string and plural acronyms at the start/end of the string, please? ================ 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]*(" + ---------------- 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. ================ Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:144 + "[A-Z]?)?[a-z]+[a-z0-9]*(" + + AcronymsGroupRegex(EscapedAcronyms, true) + "|([A-Z][a-z0-9]+))*$"; } ---------------- Why do we not allow singular acronyms in the middle of the property name? I think we should allow singular and plural acronyms anywhere. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42947 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits