[PATCH] D62045: Modified global variable declaration to fit updated objc guide.

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore requested changes to this revision.
stephanemoore marked 4 inline comments as done.
stephanemoore added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/google/GlobalVariableDeclarationCheck.cpp:51-52
+
+  auto NewName = "g" + llvm::StringRef(std::string(1, FC)).upper()) +
+                           Decl->getName().substr(1).str());
+
----------------
benhamilton wrote:
> I don't see any guidance in the style guide about prefixing with `g` either. I assume we should remove that?
> 
Non-const static variable declaration names should have the prefix `g`:
"File scope or global variables have a prefix g."
https://google.github.io/styleguide/objcguide.html#variable-names

The prefixes section in the style guide (https://google.github.io/styleguide/objcguide.html#prefixes) is targeted towards prefixes for classes, protocols, global functions, and global constants (i.e., const qualified variable declaration names).


================
Comment at: clang-tools-extra/clang-tidy/google/GlobalVariableDeclarationCheck.cpp:79
                              unless(isLocalVariable()),
-                             unless(matchesName("::(k[A-Z]|[A-Z]{2,})")))
+                             unless(matchesName("::[A-Z][A-Z0-9].*")))
                          .bind("global_const"),
----------------
For legacy code, I think we still need to allow variable declarations of the following form:
```
static const int kFileCount = 12;
static const int kFOOFileCount = 12;
```


================
Comment at: clang-tools-extra/test/clang-tidy/google-objc-global-variable-declaration.m:26
 // 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";
 
----------------
Thanks for removing the fixit on this case 😄 The fix was not compliant with Google Objective-C style.


================
Comment at: clang-tools-extra/test/clang-tidy/google-objc-global-variable-declaration.m:30
-// 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";
 
----------------
Thanks for removing the fixit on this case 😄 The fix was not compliant with Google Objective-C style.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62045/new/

https://reviews.llvm.org/D62045