benhamilton created this revision. benhamilton added reviewers: Wizard, hokein, klimek. Herald added subscribers: cfe-commits, xazax.hun.
google-objc-global-variable-declaration currently triggers on valid code like: - (void)foo { static dispatch_once_t onceToken; dispatch_once(&onceToken, ^{ /* ... */ }); } The Google Objective-C style guide says: http://google.github.io/styleguide/objcguide.html#common-variable-names > File scope or global variables (as opposed to constants) declared > outside the scope of a method or function should be rare, and should > have the prefix g. which is meant to insinuate that static variables inside a method or function don't need a special name. Test Plan: `make -j12 check-clang-tools` Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41789 Files: clang-tidy/google/GlobalVariableDeclarationCheck.cpp test/clang-tidy/google-objc-global-variable-declaration.m Index: test/clang-tidy/google-objc-global-variable-declaration.m =================================================================== --- test/clang-tidy/google-objc-global-variable-declaration.m +++ test/clang-tidy/google-objc-global-variable-declaration.m @@ -30,12 +30,12 @@ // CHECK-FIXES: static NSString* const k_Alpha = @"SecondNotAlpha"; static NSString* const kGood = @"hello"; -// CHECK-MESSAGES-NOT: :[[@LINE-1]]:24: warning: const global variable 'kGood' must have a name which starts with 'k[A-Z]' [google-objc-global-variable-declaration] static NSString* gMyIntGood = 0; -// CHECK-MESSAGES-NOT: :[[@LINE-1]]:18: warning: non-const global variable 'gMyIntGood' must have a name which starts with 'g[A-Z]' [google-objc-global-variable-declaration] + @implementation Foo - (void)f { int x = 0; -// CHECK-MESSAGES-NOT: :[[@LINE-1]]:9: warning: non-const global variable 'gMyIntGood' must have a name which starts with 'g[A-Z]' [google-objc-global-variable-declaration] + static int bar; + static const int baz = 42; } @end Index: clang-tidy/google/GlobalVariableDeclarationCheck.cpp =================================================================== --- clang-tidy/google/GlobalVariableDeclarationCheck.cpp +++ clang-tidy/google/GlobalVariableDeclarationCheck.cpp @@ -24,6 +24,10 @@ namespace { +AST_MATCHER(VarDecl, isLocalVariable) { + return Node.isLocalVarDecl(); +} + FixItHint generateFixItHint(const VarDecl *Decl, bool IsConst) { char FC = Decl->getName()[0]; if (!llvm::isAlpha(FC) || Decl->getName().size() == 1) { @@ -57,12 +61,17 @@ // need to add two matchers since we need to bind different ids to distinguish // constants and variables. Since bind() can only be called on node matchers, // we cannot make it in one matcher. + // + // Note that hasGlobalStorage() matches static variables declared locally + // inside a function or method, so we need to exclude those with + // isLocalVariable(). Finder->addMatcher( varDecl(hasGlobalStorage(), unless(hasType(isConstQualified())), - unless(matchesName("::g[A-Z]"))) + unless(isLocalVariable()), unless(matchesName("::g[A-Z]"))) .bind("global_var"), this); Finder->addMatcher(varDecl(hasGlobalStorage(), hasType(isConstQualified()), + unless(isLocalVariable()), unless(matchesName("::k[A-Z]"))) .bind("global_const"), this);
Index: test/clang-tidy/google-objc-global-variable-declaration.m =================================================================== --- test/clang-tidy/google-objc-global-variable-declaration.m +++ test/clang-tidy/google-objc-global-variable-declaration.m @@ -30,12 +30,12 @@ // CHECK-FIXES: static NSString* const k_Alpha = @"SecondNotAlpha"; static NSString* const kGood = @"hello"; -// CHECK-MESSAGES-NOT: :[[@LINE-1]]:24: warning: const global variable 'kGood' must have a name which starts with 'k[A-Z]' [google-objc-global-variable-declaration] static NSString* gMyIntGood = 0; -// CHECK-MESSAGES-NOT: :[[@LINE-1]]:18: warning: non-const global variable 'gMyIntGood' must have a name which starts with 'g[A-Z]' [google-objc-global-variable-declaration] + @implementation Foo - (void)f { int x = 0; -// CHECK-MESSAGES-NOT: :[[@LINE-1]]:9: warning: non-const global variable 'gMyIntGood' must have a name which starts with 'g[A-Z]' [google-objc-global-variable-declaration] + static int bar; + static const int baz = 42; } @end Index: clang-tidy/google/GlobalVariableDeclarationCheck.cpp =================================================================== --- clang-tidy/google/GlobalVariableDeclarationCheck.cpp +++ clang-tidy/google/GlobalVariableDeclarationCheck.cpp @@ -24,6 +24,10 @@ namespace { +AST_MATCHER(VarDecl, isLocalVariable) { + return Node.isLocalVarDecl(); +} + FixItHint generateFixItHint(const VarDecl *Decl, bool IsConst) { char FC = Decl->getName()[0]; if (!llvm::isAlpha(FC) || Decl->getName().size() == 1) { @@ -57,12 +61,17 @@ // need to add two matchers since we need to bind different ids to distinguish // constants and variables. Since bind() can only be called on node matchers, // we cannot make it in one matcher. + // + // Note that hasGlobalStorage() matches static variables declared locally + // inside a function or method, so we need to exclude those with + // isLocalVariable(). Finder->addMatcher( varDecl(hasGlobalStorage(), unless(hasType(isConstQualified())), - unless(matchesName("::g[A-Z]"))) + unless(isLocalVariable()), unless(matchesName("::g[A-Z]"))) .bind("global_var"), this); Finder->addMatcher(varDecl(hasGlobalStorage(), hasType(isConstQualified()), + unless(isLocalVariable()), unless(matchesName("::k[A-Z]"))) .bind("global_const"), this);
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits