Skip to content

Commit

Permalink
[clang-tidy/google] Improve the Objective-C global variable declarati…
Browse files Browse the repository at this point in the history
…on check 🔧

Summary:
The current Objective-C global variable declaration check restricts naming that is permitted by the Google Objective-C style guide.

The Objective-C style guide states the following:
"Global and file scope constants should have an appropriate prefix. [...] Constants may use a lowercase k prefix when appropriate"
http://google.github.io/styleguide/objcguide#constants

This change fixes the check to allow two or more capital letters as an appropriate prefix. This change intentionally avoids making a decision regarding whether to flag constants that use a two letter prefix (two letter prefixes are reserved by Apple¹ but many projects seem to violate this guideline).

This change eliminates an important category of false positives (constants prefixed with '[A-Z]{2,}') at the cost of introducing a less important category of false negatives (constants prefixed with only '[A-Z]'). The false positives are observed in standard recommended code while the false negatives occur in non-standard unrecommended code. The number of eliminated false positives is expected to be significantly larger than the number of exposed false negatives.

❧

(1)
"Two-letter prefixes like these are reserved by Apple for use in framework classes."
https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/ProgrammingWithObjectiveC/Conventions/Conventions.html

Reviewers: aaron.ballman, Wizard, hokein, benhamilton

Reviewed By: aaron.ballman, Wizard

Subscribers: aaron.ballman, cfe-commits

Differential Revision: https://reviews.llvm.org/D43581

llvm-svn: 326046
  • Loading branch information
ynzhang0509 committed Feb 25, 2018
1 parent ac24bb5 commit 2d83647
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 5 deletions.
Expand Up @@ -72,7 +72,7 @@ void GlobalVariableDeclarationCheck::registerMatchers(MatchFinder *Finder) {
this);
Finder->addMatcher(varDecl(hasGlobalStorage(), hasType(isConstQualified()),
unless(isLocalVariable()),
unless(matchesName("::k[A-Z]")))
unless(matchesName("::(k[A-Z]|[A-Z]{2,})")))
.bind("global_const"),
this);
}
Expand All @@ -88,7 +88,7 @@ void GlobalVariableDeclarationCheck::check(
if (const auto *Decl = Result.Nodes.getNodeAs<VarDecl>("global_const")) {
diag(Decl->getLocation(),
"const global variable '%0' must have a name which starts with "
"'k[A-Z]'")
"an appropriate prefix")
<< Decl->getName() << generateFixItHint(Decl, true);
}
}
Expand Down
Expand Up @@ -2,7 +2,7 @@

@class NSString;
static NSString* const myConstString = @"hello";
// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: const global variable 'myConstString' must have a name which starts with 'k[A-Z]' [google-objc-global-variable-declaration]
// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: const global variable 'myConstString' must have a name which starts with an appropriate prefix [google-objc-global-variable-declaration]
// CHECK-FIXES: static NSString* const kMyConstString = @"hello";

static NSString* MyString = @"hi";
Expand All @@ -22,16 +22,24 @@
// CHECK-FIXES: static NSString* gNoDef;

static NSString* const _notAlpha = @"NotBeginWithAlpha";
// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: const global variable '_notAlpha' must have a name which starts with 'k[A-Z]' [google-objc-global-variable-declaration]
// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: const global variable '_notAlpha' must have a name which starts with an appropriate prefix [google-objc-global-variable-declaration]
// CHECK-FIXES: static NSString* const _notAlpha = @"NotBeginWithAlpha";

static NSString* const k_Alpha = @"SecondNotAlpha";
// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: const global variable 'k_Alpha' must have a name which starts with 'k[A-Z]' [google-objc-global-variable-declaration]
// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: const global variable 'k_Alpha' must have a name which starts with an appropriate prefix [google-objc-global-variable-declaration]
// CHECK-FIXES: static NSString* const k_Alpha = @"SecondNotAlpha";

static NSString* const kGood = @"hello";
static NSString* const XYGood = @"hello";
static NSString* gMyIntGood = 0;

extern NSString* const GTLServiceErrorDomain;

enum GTLServiceError {
GTLServiceErrorQueryResultMissing = -3000,
GTLServiceErrorWaitTimedOut = -3001,
};

@implementation Foo
- (void)f {
int x = 0;
Expand Down

0 comments on commit 2d83647

Please sign in to comment.