stephanemoore created this revision.
stephanemoore added reviewers: benhamilton, Wizard.
Herald added subscribers: cfe-commits, jfb.
§1 Description
This changes the objc-property-declaration check to allow arbitrary acronyms
and initialisms instead of using whitelisted acronyms. In Objective-C it is
relatively common to use project prefixes in property names for the purposes of
disambiguation. For example, the CIColor¹ and CGColor² properties on UIColor
both represent symbol prefixes being used in proeprty names outside of Apple's
accepted acronyms³. The union of Apple's accepted acronyms and all symbol
prefixes that might be used for disambiguation in property declarations
effectively allows for any arbitrary sequence of capital alphanumeric
characters to be acceptable in property declarations. This change updates the
check accordingly.
The test variants with custom configurations are deleted as part of this change
because their configurations no longer impact behavior. The acronym
configurations are currently preserved for backwards compatibility of check
configuration.
[1]
https://developer.apple.com/documentation/uikit/uicolor/1621951-cicolor?language=objc
[2]
https://developer.apple.com/documentation/uikit/uicolor/1621954-cgcolor?language=objc
[3]
https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/CodingGuidelines/Articles/APIAbbreviations.html#//apple_ref/doc/uid/20001285-BCIHCGAE
§2 Test Notes
Changes verified by:
• Running clang-tidy unit tests.
• Used check_clang_tidy.py to verify expected output of processing
objc-property-declaration.m
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51832
Files:
clang-tidy/objc/PropertyDeclarationCheck.cpp
test/clang-tidy/objc-property-declaration-additional.m
test/clang-tidy/objc-property-declaration-custom.m
test/clang-tidy/objc-property-declaration.m
Index: test/clang-tidy/objc-property-declaration.m
===================================================================
--- test/clang-tidy/objc-property-declaration.m
+++ test/clang-tidy/objc-property-declaration.m
@@ -2,6 +2,9 @@
@class NSData;
@class NSString;
@class UIViewController;
+@class CIColor;
+
+typedef void *CGColorRef;
@interface Foo
@property(assign, nonatomic) int NotCamelCase;
@@ -23,6 +26,9 @@
@property(assign, nonatomic) int enableGLAcceleration;
@property(assign, nonatomic) int ID;
@property(assign, nonatomic) int hasADog;
+@property(nonatomic, readonly) CGColorRef CGColor;
+@property(nonatomic, readonly) CIColor *CIColor;
+@property(nonatomic, copy) NSArray<NSString *> *IDs;
@end
@interface Foo (Bar)
Index: test/clang-tidy/objc-property-declaration-custom.m
===================================================================
--- test/clang-tidy/objc-property-declaration-custom.m
+++ /dev/null
@@ -1,18 +0,0 @@
-// RUN: %check_clang_tidy %s objc-property-declaration %t \
-// RUN: -config='{CheckOptions: \
-// RUN: [{key: objc-property-declaration.Acronyms, value: "ABC;TGIF"}, \
-// RUN: {key: objc-property-declaration.IncludeDefaultAcronyms, value: 0}]}' \
-// RUN: --
-@class NSString;
-
-@interface Foo
-@property(assign, nonatomic) int AbcNotRealPrefix;
-// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'AbcNotRealPrefix' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
-// CHECK-FIXES: @property(assign, nonatomic) int abcNotRealPrefix;
-@property(assign, nonatomic) int ABCCustomPrefix;
-@property(strong, nonatomic) NSString *ABC_custom_prefix;
-// CHECK-MESSAGES: :[[@LINE-1]]:40: warning: property name 'ABC_custom_prefix' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
-@property(assign, nonatomic) int GIFIgnoreStandardAcronym;
-// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'GIFIgnoreStandardAcronym' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
-@property(strong, nonatomic) NSString *TGIF;
-@end
Index: test/clang-tidy/objc-property-declaration-additional.m
===================================================================
--- test/clang-tidy/objc-property-declaration-additional.m
+++ /dev/null
@@ -1,15 +0,0 @@
-// RUN: %check_clang_tidy %s objc-property-declaration %t \
-// RUN: -config='{CheckOptions: \
-// RUN: [{key: objc-property-declaration.Acronyms, value: "ABC;TGIF"}]}' \
-// RUN: --
-@class NSString;
-
-@interface Foo
-@property(assign, nonatomic) int AbcNotRealPrefix;
-// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'AbcNotRealPrefix' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
-// CHECK-FIXES: @property(assign, nonatomic) int abcNotRealPrefix;
-@property(assign, nonatomic) int ABCCustomPrefix;
-@property(strong, nonatomic) NSString *ABC_custom_prefix;
-// CHECK-MESSAGES: :[[@LINE-1]]:40: warning: property name 'ABC_custom_prefix' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
-@property(assign, nonatomic) int GIFShouldIncludeStandardAcronym;
-@end
Index: clang-tidy/objc/PropertyDeclarationCheck.cpp
===================================================================
--- clang-tidy/objc/PropertyDeclarationCheck.cpp
+++ clang-tidy/objc/PropertyDeclarationCheck.cpp
@@ -34,91 +34,6 @@
CategoryProperty = 2,
};
-/// The acronyms are aggregated from multiple sources including
-/// https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/CodingGuidelines/Articles/APIAbbreviations.html#//apple_ref/doc/uid/20001285-BCIHCGAE
-///
-/// Keep this list sorted.
-constexpr llvm::StringLiteral DefaultSpecialAcronyms[] = {
- "[2-9]G",
- "ACL",
- "API",
- "AR",
- "ARGB",
- "ASCII",
- "AV",
- "BGRA",
- "CA",
- "CF",
- "CG",
- "CI",
- "CRC",
- "CV",
- "CMYK",
- "DNS",
- "FPS",
- "FTP",
- "GIF",
- "GL",
- "GPS",
- "GUID",
- "HD",
- "HDR",
- "HMAC",
- "HTML",
- "HTTP",
- "HTTPS",
- "HUD",
- "ID",
- "JPG",
- "JS",
- "LAN",
- "LZW",
- "MAC",
- "MD",
- "MDNS",
- "MIDI",
- "NS",
- "OS",
- "PDF",
- "PIN",
- "PNG",
- "POI",
- "PSTN",
- "PTR",
- "QA",
- "QOS",
- "RGB",
- "RGBA",
- "RGBX",
- "RIPEMD",
- "ROM",
- "RPC",
- "RTF",
- "RTL",
- "SC",
- "SDK",
- "SHA",
- "SQL",
- "SSO",
- "TCP",
- "TIFF",
- "TTS",
- "UI",
- "URI",
- "URL",
- "UUID",
- "VC",
- "VOIP",
- "VPN",
- "VR",
- "W",
- "WAN",
- "X",
- "XML",
- "Y",
- "Z",
-};
-
/// For now we will only fix 'CamelCase' or 'abc_CamelCase' property to
/// 'camelCase' or 'abc_camelCase'. For other cases the users need to
/// come up with a proper name by their own.
@@ -142,43 +57,39 @@
return FixItHint();
}
-std::string AcronymsGroupRegex(llvm::ArrayRef<std::string> EscapedAcronyms) {
- return "(" +
- llvm::join(EscapedAcronyms.begin(), EscapedAcronyms.end(), "s?|") +
- "s?)";
-}
-
-std::string validPropertyNameRegex(llvm::ArrayRef<std::string> EscapedAcronyms,
- bool UsedInMatcher) {
+std::string validPropertyNameRegex(bool UsedInMatcher) {
// Allow any of these names:
// foo
// fooBar
// url
// urlString
+ // ID
+ // IDs
// URL
// URLString
// bundleID
+ // CIColor
+ //
+ // Disallow names of this form:
+ // LongString
std::string StartMatcher = UsedInMatcher ? "::" : "^";
- std::string AcronymsMatcher = AcronymsGroupRegex(EscapedAcronyms);
- return StartMatcher + "(" + AcronymsMatcher + "[A-Z]?)?[a-z]+[a-z0-9]*(" +
- AcronymsMatcher + "|([A-Z][a-z0-9]+)|A|I)*$";
+ return StartMatcher + "([a-z]|[A-Z][A-Z0-9])[a-z0-9A-Z]*$";
}
bool hasCategoryPropertyPrefix(llvm::StringRef PropertyName) {
auto RegexExp = llvm::Regex("^[a-zA-Z]+_[a-zA-Z0-9][a-zA-Z0-9_]+$");
return RegexExp.match(PropertyName);
}
-bool prefixedPropertyNameValid(llvm::StringRef PropertyName,
- llvm::ArrayRef<std::string> Acronyms) {
+bool prefixedPropertyNameValid(llvm::StringRef PropertyName) {
size_t Start = PropertyName.find_first_of('_');
assert(Start != llvm::StringRef::npos && Start + 1 < PropertyName.size());
auto Prefix = PropertyName.substr(0, Start);
if (Prefix.lower() != Prefix) {
return false;
}
auto RegexExp =
- llvm::Regex(llvm::StringRef(validPropertyNameRegex(Acronyms, false)));
+ llvm::Regex(llvm::StringRef(validPropertyNameRegex(false)));
return RegexExp.match(PropertyName.substr(Start + 1));
}
} // namespace
@@ -196,26 +107,12 @@
if (!getLangOpts().ObjC1 && !getLangOpts().ObjC2) {
return;
}
- if (IncludeDefaultAcronyms) {
- EscapedAcronyms.reserve(llvm::array_lengthof(DefaultSpecialAcronyms) +
- SpecialAcronyms.size());
- // No need to regex-escape the default acronyms.
- EscapedAcronyms.insert(EscapedAcronyms.end(),
- std::begin(DefaultSpecialAcronyms),
- std::end(DefaultSpecialAcronyms));
- } else {
- EscapedAcronyms.reserve(SpecialAcronyms.size());
- }
- // In case someone defines a prefix which includes a regex
- // special character, regex-escape all the user-defined prefixes.
- std::transform(SpecialAcronyms.begin(), SpecialAcronyms.end(),
- std::back_inserter(EscapedAcronyms),
- [](const std::string &s) { return llvm::Regex::escape(s); });
+
Finder->addMatcher(
objcPropertyDecl(
// the property name should be in Lower Camel Case like
// 'lowerCamelCase'
- unless(matchesName(validPropertyNameRegex(EscapedAcronyms, true))))
+ unless(matchesName(validPropertyNameRegex(true))))
.bind("property"),
this);
}
@@ -227,14 +124,9 @@
auto *DeclContext = MatchedDecl->getDeclContext();
auto *CategoryDecl = llvm::dyn_cast<ObjCCategoryDecl>(DeclContext);
- auto AcronymsRegex =
- llvm::Regex("^" + AcronymsGroupRegex(EscapedAcronyms) + "$");
- if (AcronymsRegex.match(MatchedDecl->getName())) {
- return;
- }
if (CategoryDecl != nullptr &&
hasCategoryPropertyPrefix(MatchedDecl->getName())) {
- if (!prefixedPropertyNameValid(MatchedDecl->getName(), EscapedAcronyms) ||
+ if (!prefixedPropertyNameValid(MatchedDecl->getName()) ||
CategoryDecl->IsClassExtension()) {
NamingStyle Style = CategoryDecl->IsClassExtension() ? StandardProperty
: CategoryProperty;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
