[clang-tools-extra] r362279 - Revise the google-objc-global-variable-declaration check to match the style guide.

Stephane Moore via cfe-commits cfe-commits at lists.llvm.org
Fri May 31 16:41:15 PDT 2019


Author: stephanemoore
Date: Fri May 31 16:41:15 2019
New Revision: 362279

URL: http://llvm.org/viewvc/llvm-project?rev=362279&view=rev
Log:
Revise the google-objc-global-variable-declaration check to match the style guide.

Summary:
Revise the google-objc-global-variable-declaration check to match the style guide.

This commit updates the check as follows:
(1) Do not emit fixes for extern global constants.
(2) Allow the second character of prefixes for constants to be numeric (the new guideline is that global constants should generally be named with a prefix that begins with a capital letter followed by one or more capital letters or numbers).

https://google.github.io/styleguide/objcguide.html#prefixes

This is an amended re-submission of https://reviews.llvm.org/rG12e3726fadb0b2a4d8aeed0a2817b5159f9d029d.

Contributed By: yaqiji

Reviewers: Wizard, benhamilton, stephanemoore

Reviewed By: benhamilton, stephanemoore

Subscribers: mgorny, cfe-commits, yaqiji

Tags: #clang

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

Modified:
    clang-tools-extra/trunk/clang-tidy/google/GlobalVariableDeclarationCheck.cpp
    clang-tools-extra/trunk/test/clang-tidy/google-objc-global-variable-declaration.m

Modified: clang-tools-extra/trunk/clang-tidy/google/GlobalVariableDeclarationCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/GlobalVariableDeclarationCheck.cpp?rev=362279&r1=362278&r2=362279&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/google/GlobalVariableDeclarationCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/google/GlobalVariableDeclarationCheck.cpp Fri May 31 16:41:15 2019
@@ -23,29 +23,35 @@ namespace objc {
 
 namespace {
 
-AST_MATCHER(VarDecl, isLocalVariable) {
-  return Node.isLocalVarDecl();
-}
+AST_MATCHER(VarDecl, isLocalVariable) { return Node.isLocalVarDecl(); }
 
 FixItHint generateFixItHint(const VarDecl *Decl, bool IsConst) {
+  if (IsConst && (Decl->getStorageClass() != SC_Static)) {
+    // No fix available if it is not a static constant, since it is difficult
+    // to determine the proper fix in this case.
+    return FixItHint();
+  }
+
   char FC = Decl->getName()[0];
   if (!llvm::isAlpha(FC) || Decl->getName().size() == 1) {
     // No fix available if first character is not alphabetical character, or it
-    // is a single-character variable, since it is difficult to determine the 
+    // is a single-character variable, since it is difficult to determine the
     // proper fix in this case. Users should create a proper variable name by
     // their own.
     return FixItHint();
   }
   char SC = Decl->getName()[1];
   if ((FC == 'k' || FC == 'g') && !llvm::isAlpha(SC)) {
-    // No fix available if the prefix is correct but the second character is not
-    // alphabetical, since it is difficult to determine the proper fix in this
-    // case.
+    // No fix available if the prefix is correct but the second character is
+    // not alphabetical, since it is difficult to determine the proper fix in
+    // this case.
     return FixItHint();
   }
+
   auto NewName = (IsConst ? "k" : "g") +
                  llvm::StringRef(std::string(1, FC)).upper() +
                  Decl->getName().substr(1).str();
+
   return FixItHint::CreateReplacement(
       CharSourceRange::getTokenRange(SourceRange(Decl->getLocation())),
       llvm::StringRef(NewName));
@@ -71,7 +77,7 @@ void GlobalVariableDeclarationCheck::reg
       this);
   Finder->addMatcher(varDecl(hasGlobalStorage(), hasType(isConstQualified()),
                              unless(isLocalVariable()),
-                             unless(matchesName("::(k[A-Z]|[A-Z]{2,})")))
+                             unless(matchesName("::(k[A-Z])|([A-Z][A-Z0-9])")))
                          .bind("global_const"),
                      this);
 }

Modified: clang-tools-extra/trunk/test/clang-tidy/google-objc-global-variable-declaration.m
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/google-objc-global-variable-declaration.m?rev=362279&r1=362278&r2=362279&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/google-objc-global-variable-declaration.m (original)
+++ clang-tools-extra/trunk/test/clang-tidy/google-objc-global-variable-declaration.m Fri May 31 16:41:15 2019
@@ -1,10 +1,14 @@
 // RUN: %check_clang_tidy %s google-objc-global-variable-declaration %t
 
 @class NSString;
+
 static NSString* const myConstString = @"hello";
 // 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";
 
+extern NSString* const GlobalConstant = @"hey";
+// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: const global variable 'GlobalConstant' must have a name which starts with an appropriate prefix [google-objc-global-variable-declaration]
+
 static NSString* MyString = @"hi";
 // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: non-const global variable 'MyString' must have a name which starts with 'g[A-Z]' [google-objc-global-variable-declaration]
 // CHECK-FIXES: static NSString* gMyString = @"hi";
@@ -25,12 +29,25 @@ static NSString* const _notAlpha = @"Not
 // 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 notCap = @"NotBeginWithCap";
+// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: const global variable 'notCap' must have a name which starts with an appropriate prefix [google-objc-global-variable-declaration]
+// CHECK-FIXES: static NSString* const kNotCap = @"NotBeginWithCap";
+
 static NSString* const k_Alpha = @"SecondNotAlpha";
 // 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 SecondNotCap = @"SecondNotCapOrNumber";
+// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: const global variable 'SecondNotCap' must have a name which starts with an appropriate prefix [google-objc-global-variable-declaration]
+// CHECK-FIXES: static NSString* const kSecondNotCap = @"SecondNotCapOrNumber";
+
+extern NSString* Y2Bad;
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: non-const global variable 'Y2Bad' must have a name which starts with 'g[A-Z]' [google-objc-global-variable-declaration]
+// CHECK-FIXES: extern NSString* gY2Bad;
+
 static NSString* const kGood = @"hello";
 static NSString* const XYGood = @"hello";
+static NSString* const X1Good = @"hello";
 static NSString* gMyIntGood = 0;
 
 extern NSString* const GTLServiceErrorDomain;
@@ -42,8 +59,8 @@ enum GTLServiceError {
 
 @implementation Foo
 - (void)f {
-    int x = 0;
-    static int bar;
-    static const int baz = 42;
+  int x = 0;
+  static int bar;
+  static const int baz = 42;
 }
 @end




More information about the cfe-commits mailing list