[PATCH] D62045: Revise the google-objc-global-variable-declaration check to match the style guide.
gribozavr added a comment. Sorry for jumping in late, but renaming the declaration is not enough -- usages should also be updated; otherwise the developer is better off using a refactoring in their IDE or even a textual search and replace. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62045/new/ https://reviews.llvm.org/D62045 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D62045: Revise the google-objc-global-variable-declaration check to match the style guide.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL362279: Revise the google-objc-global-variable-declaration
check to match the style… (authored by stephanemoore, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D62045?vs=201973=202508#toc
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62045/new/
https://reviews.llvm.org/D62045
Files:
clang-tools-extra/trunk/clang-tidy/google/GlobalVariableDeclarationCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/google-objc-global-variable-declaration.m
Index: clang-tools-extra/trunk/clang-tidy/google/GlobalVariableDeclarationCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/google/GlobalVariableDeclarationCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/google/GlobalVariableDeclarationCheck.cpp
@@ -23,29 +23,35 @@
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 @@
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);
}
Index: clang-tools-extra/trunk/test/clang-tidy/google-objc-global-variable-declaration.m
===
--- clang-tools-extra/trunk/test/clang-tidy/google-objc-global-variable-declaration.m
+++ clang-tools-extra/trunk/test/clang-tidy/google-objc-global-variable-declaration.m
@@ -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 @@
// 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:
[PATCH] D62045: Revise the google-objc-global-variable-declaration check to match the style guide.
stephanemoore accepted this revision. stephanemoore marked an inline comment as done. stephanemoore added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/test/clang-tidy/google-objc-global-variable-declaration.m:46 +// 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; + This is interesting as it respects the Google Objective-C Style Guide as it is currently written. I think the truth might actually be that there are no guidelines for non-const extern variables as they are incredibly rare and presumably ubiquitously discouraged. In that respect, I am not sure that the fix here represents what would actually be recommended for Google Objective-C. With that said, the fix might be reasonable to emit based on the expectation that it would be exceedingly rare and it's unclear what better guidance we would provide. I think it's fine to continue allowing this existing behavior and continue monitoring. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62045/new/ https://reviews.llvm.org/D62045 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D62045: Revise the google-objc-global-variable-declaration check to match the style guide.
yaqiji updated this revision to Diff 201973.
yaqiji added a comment.
Added to check no fixit is generated.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62045/new/
https://reviews.llvm.org/D62045
Files:
clang-tools-extra/clang-tidy/google/GlobalVariableDeclarationCheck.cpp
clang-tools-extra/test/clang-tidy/google-objc-global-variable-declaration.m
Index: clang-tools-extra/test/clang-tidy/google-objc-global-variable-declaration.m
===
--- clang-tools-extra/test/clang-tidy/google-objc-global-variable-declaration.m
+++ clang-tools-extra/test/clang-tidy/google-objc-global-variable-declaration.m
@@ -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 @@
// 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 @@
@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
Index: clang-tools-extra/clang-tidy/google/GlobalVariableDeclarationCheck.cpp
===
--- clang-tools-extra/clang-tidy/google/GlobalVariableDeclarationCheck.cpp
+++ clang-tools-extra/clang-tidy/google/GlobalVariableDeclarationCheck.cpp
@@ -23,29 +23,35 @@
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
[PATCH] D62045: Revise the google-objc-global-variable-declaration check to match the style guide.
yaqiji updated this revision to Diff 201972.
yaqiji added a comment.
Remove cmake content
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62045/new/
https://reviews.llvm.org/D62045
Files:
clang-tools-extra/clang-tidy/google/GlobalVariableDeclarationCheck.cpp
clang-tools-extra/test/clang-tidy/google-objc-global-variable-declaration.m
Index: clang-tools-extra/test/clang-tidy/google-objc-global-variable-declaration.m
===
--- clang-tools-extra/test/clang-tidy/google-objc-global-variable-declaration.m
+++ clang-tools-extra/test/clang-tidy/google-objc-global-variable-declaration.m
@@ -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";
@@ -15,7 +19,6 @@
static NSString* a = @"too simple";
// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: non-const global variable 'a' must have a name which starts with 'g[A-Z]' [google-objc-global-variable-declaration]
-// CHECK-FIXES: static NSString* a = @"too simple";
static NSString* noDef;
// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: non-const global variable 'noDef' must have a name which starts with 'g[A-Z]' [google-objc-global-variable-declaration]
@@ -25,12 +28,24 @@
// 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 +57,8 @@
@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
Index: clang-tools-extra/clang-tidy/google/GlobalVariableDeclarationCheck.cpp
===
--- clang-tools-extra/clang-tidy/google/GlobalVariableDeclarationCheck.cpp
+++ clang-tools-extra/clang-tidy/google/GlobalVariableDeclarationCheck.cpp
@@ -23,29 +23,35 @@
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
[PATCH] D62045: Revise the google-objc-global-variable-declaration check to match the style guide.
yaqiji updated this revision to Diff 201971.
yaqiji added a comment.
Removed check-fixes for those without fixithint.
Changed name of Y2Good to Y2Bad to match meaning.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62045/new/
https://reviews.llvm.org/D62045
Files:
clang-tools-extra/clang-tidy/google/CMakeLists.txt
clang-tools-extra/clang-tidy/google/GlobalVariableDeclarationCheck.cpp
clang-tools-extra/test/clang-tidy/google-objc-global-variable-declaration.m
Index: clang-tools-extra/test/clang-tidy/google-objc-global-variable-declaration.m
===
--- clang-tools-extra/test/clang-tidy/google-objc-global-variable-declaration.m
+++ clang-tools-extra/test/clang-tidy/google-objc-global-variable-declaration.m
@@ -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";
@@ -15,7 +19,6 @@
static NSString* a = @"too simple";
// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: non-const global variable 'a' must have a name which starts with 'g[A-Z]' [google-objc-global-variable-declaration]
-// CHECK-FIXES: static NSString* a = @"too simple";
static NSString* noDef;
// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: non-const global variable 'noDef' must have a name which starts with 'g[A-Z]' [google-objc-global-variable-declaration]
@@ -25,12 +28,24 @@
// 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 +57,8 @@
@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
Index: clang-tools-extra/clang-tidy/google/GlobalVariableDeclarationCheck.cpp
===
--- clang-tools-extra/clang-tidy/google/GlobalVariableDeclarationCheck.cpp
+++ clang-tools-extra/clang-tidy/google/GlobalVariableDeclarationCheck.cpp
@@ -23,29 +23,35 @@
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
[PATCH] D62045: Revise the google-objc-global-variable-declaration check to match the style guide.
yaqiji updated this revision to Diff 201969.
yaqiji added a comment.
Added check-fix
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62045/new/
https://reviews.llvm.org/D62045
Files:
clang-tools-extra/clang-tidy/google/GlobalVariableDeclarationCheck.cpp
clang-tools-extra/test/clang-tidy/google-objc-global-variable-declaration.m
Index: clang-tools-extra/test/clang-tidy/google-objc-global-variable-declaration.m
===
--- clang-tools-extra/test/clang-tidy/google-objc-global-variable-declaration.m
+++ clang-tools-extra/test/clang-tidy/google-objc-global-variable-declaration.m
@@ -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,13 +29,25 @@
// 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";
+
static NSString* const kGood = @"hello";
static NSString* const XYGood = @"hello";
+static NSString* const X1Good = @"hello";
static NSString* gMyIntGood = 0;
+extern NSString* Y2Good;
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: non-const global variable 'Y2Good' must have a name which starts with 'g[A-Z]' [google-objc-global-variable-declaration]
+// CHECK-FIXES: extern NSString* gY2Good;
extern NSString* const GTLServiceErrorDomain;
@@ -42,8 +58,8 @@
@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
Index: clang-tools-extra/clang-tidy/google/GlobalVariableDeclarationCheck.cpp
===
--- clang-tools-extra/clang-tidy/google/GlobalVariableDeclarationCheck.cpp
+++ clang-tools-extra/clang-tidy/google/GlobalVariableDeclarationCheck.cpp
@@ -23,29 +23,35 @@
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
[PATCH] D62045: Revise the google-objc-global-variable-declaration check to match the style guide.
yaqiji updated this revision to Diff 201968.
yaqiji added a comment.
Updated warning
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62045/new/
https://reviews.llvm.org/D62045
Files:
clang-tools-extra/clang-tidy/google/GlobalVariableDeclarationCheck.cpp
clang-tools-extra/test/clang-tidy/google-objc-global-variable-declaration.m
Index: clang-tools-extra/test/clang-tidy/google-objc-global-variable-declaration.m
===
--- clang-tools-extra/test/clang-tidy/google-objc-global-variable-declaration.m
+++ clang-tools-extra/test/clang-tidy/google-objc-global-variable-declaration.m
@@ -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,13 +29,24 @@
// 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";
+
static NSString* const kGood = @"hello";
static NSString* const XYGood = @"hello";
+static NSString* const X1Good = @"hello";
static NSString* gMyIntGood = 0;
+extern NSString* Y2Good;
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: non-const global variable 'Y2Good' must have a name which starts with 'g[A-Z]' [google-objc-global-variable-declaration]
extern NSString* const GTLServiceErrorDomain;
@@ -42,8 +57,8 @@
@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
Index: clang-tools-extra/clang-tidy/google/GlobalVariableDeclarationCheck.cpp
===
--- clang-tools-extra/clang-tidy/google/GlobalVariableDeclarationCheck.cpp
+++ clang-tools-extra/clang-tidy/google/GlobalVariableDeclarationCheck.cpp
@@ -23,29 +23,35 @@
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
+//
[PATCH] D62045: Revise the google-objc-global-variable-declaration check to match the style guide.
stephanemoore reopened this revision. stephanemoore added inline comments. This revision is now accepted and ready to land. Comment at: test/clang-tidy/google-objc-global-variable-declaration.m:48 static NSString* gMyIntGood = 0; +extern NSString* Y2Good; Whoops, we forgot the warning that should be generated for this case Test failure(s): http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/49124 http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/49124/steps/test/logs/stdio I had to rollback the commit. Can you update the patch to resolve the test failure(s)? Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62045/new/ https://reviews.llvm.org/D62045 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D62045: Revise the google-objc-global-variable-declaration check to match the style guide.
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE361907: Revise the google-objc-global-variable-declaration
check to match the style… (authored by stephanemoore, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D62045?vs=201352=201806#toc
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62045/new/
https://reviews.llvm.org/D62045
Files:
clang-tidy/google/GlobalVariableDeclarationCheck.cpp
test/clang-tidy/google-objc-global-variable-declaration.m
Index: clang-tidy/google/GlobalVariableDeclarationCheck.cpp
===
--- clang-tidy/google/GlobalVariableDeclarationCheck.cpp
+++ clang-tidy/google/GlobalVariableDeclarationCheck.cpp
@@ -23,29 +23,35 @@
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 @@
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);
}
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
@@ -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,13 +29,23 @@
// 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
[PATCH] D62045: Revise the google-objc-global-variable-declaration check to match the style guide.
yaqiji added a comment. Yes please, thank you :D Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62045/new/ https://reviews.llvm.org/D62045 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D62045: Revise the google-objc-global-variable-declaration check to match the style guide.
stephanemoore added a comment. It looks like all concerns have been addressed. Do you need me to land this commit for you? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62045/new/ https://reviews.llvm.org/D62045 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D62045: Revise the google-objc-global-variable-declaration check to match the style guide.
yaqiji added a comment. Thank you so much for reviewing the code and providing great feedbacks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62045/new/ https://reviews.llvm.org/D62045 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D62045: Revise the google-objc-global-variable-declaration check to match the style guide.
stephanemoore accepted this revision. stephanemoore added a comment. Looks good! Thanks for being patient with me! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62045/new/ https://reviews.llvm.org/D62045 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D62045: Revise the google-objc-global-variable-declaration check to match the style guide.
yaqiji updated this revision to Diff 201352.
yaqiji marked 3 inline comments as done.
yaqiji added a comment.
Added fix message, and change global to nonstatic.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62045/new/
https://reviews.llvm.org/D62045
Files:
clang-tools-extra/clang-tidy/google/GlobalVariableDeclarationCheck.cpp
clang-tools-extra/test/clang-tidy/google-objc-global-variable-declaration.m
Index: clang-tools-extra/test/clang-tidy/google-objc-global-variable-declaration.m
===
--- clang-tools-extra/test/clang-tidy/google-objc-global-variable-declaration.m
+++ clang-tools-extra/test/clang-tidy/google-objc-global-variable-declaration.m
@@ -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,13 +29,23 @@
// 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";
+
static NSString* const kGood = @"hello";
static NSString* const XYGood = @"hello";
+static NSString* const X1Good = @"hello";
static NSString* gMyIntGood = 0;
+extern NSString* Y2Good;
extern NSString* const GTLServiceErrorDomain;
@@ -42,8 +56,8 @@
@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
Index: clang-tools-extra/clang-tidy/google/GlobalVariableDeclarationCheck.cpp
===
--- clang-tools-extra/clang-tidy/google/GlobalVariableDeclarationCheck.cpp
+++ clang-tools-extra/clang-tidy/google/GlobalVariableDeclarationCheck.cpp
@@ -23,29 +23,35 @@
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.
[PATCH] D62045: Revise the google-objc-global-variable-declaration check to match the style guide.
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
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D62045: Revise the google-objc-global-variable-declaration check to match the style guide.
yaqiji updated this revision to Diff 200637.
yaqiji edited the summary of this revision.
yaqiji added a comment.
Removed other mdodified file.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62045/new/
https://reviews.llvm.org/D62045
Files:
clang-tools-extra/clang-tidy/google/GlobalVariableDeclarationCheck.cpp
clang-tools-extra/test/clang-tidy/google-objc-global-variable-declaration.m
Index: clang-tools-extra/test/clang-tidy/google-objc-global-variable-declaration.m
===
--- clang-tools-extra/test/clang-tidy/google-objc-global-variable-declaration.m
+++ clang-tools-extra/test/clang-tidy/google-objc-global-variable-declaration.m
@@ -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,13 +29,21 @@
// 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]
+
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]
+
static NSString* const kGood = @"hello";
static NSString* const XYGood = @"hello";
+static NSString* const X1Good = @"hello";
static NSString* gMyIntGood = 0;
+extern NSString* Y2Good;
extern NSString* const GTLServiceErrorDomain;
@@ -42,8 +54,8 @@
@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
Index: clang-tools-extra/clang-tidy/google/GlobalVariableDeclarationCheck.cpp
===
--- clang-tools-extra/clang-tidy/google/GlobalVariableDeclarationCheck.cpp
+++ clang-tools-extra/clang-tidy/google/GlobalVariableDeclarationCheck.cpp
@@ -23,29 +23,35 @@
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->hasExternalStorage()) {
+// No fix available if it is a extern global 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() +
