[PATCH] D62045: Revise the google-objc-global-variable-declaration check to match the style guide.

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore accepted this revision.
stephanemoore marked 2 inline comments as done.
stephanemoore added a comment.

(sorry I forgot to send this earlier)

Two small things and then I think everything is good.



================
Comment at: clang-tools-extra/clang-tidy/google/GlobalVariableDeclarationCheck.cpp:29
 FixItHint generateFixItHint(const VarDecl *Decl, bool IsConst) {
+  if (IsConst && Decl->hasExternalStorage()) {
+    // No fix available if it is a extern global constant, since it is difficult
----------------
This condition is probably technically fine since I don't believe it's possible for variables with auto or register storage classes to get here but if we were worried about that then I think we could change the condition to something like this:
```
    if (IsConst && (Decl->getStorageClass() != SC_Static)) {
```


================
Comment at: clang-tools-extra/test/clang-tidy/google-objc-global-variable-declaration.m:16-18
 NSString* globalString = @"test";
 // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: non-const global variable 'globalString' must have a name which starts with 'g[A-Z]' [google-objc-global-variable-declaration]
 // CHECK-FIXES: NSString* gGlobalString = @"test";
----------------
This is definitely outside the scope of this commit but this fix seems like it would only be appropriate in an implementation file 🤔


================
Comment at: clang-tools-extra/test/clang-tidy/google-objc-global-variable-declaration.m:32
 
+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]
----------------
Isn't this a case where we would expect `kNotCap` as a suggested fixit?


================
Comment at: clang-tools-extra/test/clang-tidy/google-objc-global-variable-declaration.m:39
 
+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]
----------------
Isn't this a case where we would expect `kSecondNotCap` as a suggested fixit?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62045