JonasToth added inline comments.
================
Comment at: clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.cpp:23
+
+constexpr char kDisabledTestPrefix[] = "DISABLED_";
+
----------------
Please use `llvm::StringLiteral`
================
Comment at: clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.cpp:45
+ const MacroArgs *Args) override {
+ auto IdentifierInfo = MacroNameToken.getIdentifierInfo();
+ if (!IdentifierInfo) {
----------------
please dont use `auto` here, subjectiv as `IdentifierInfo` is the type, but new
contributors might not know that, so i would prefer not-auto here.
================
Comment at: clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.cpp:49
+ }
+ const StringRef MacroName = IdentifierInfo->getName();
+ if (!isGoogletestTestMacro(MacroName)) {
----------------
no `const` in this case, as its a value and those are not const'ed in LLVM.
================
Comment at: clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.cpp:50
+ const StringRef MacroName = IdentifierInfo->getName();
+ if (!isGoogletestTestMacro(MacroName)) {
+ return;
----------------
You can ellide the braces and merge these two ifs.
================
Comment at: clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.cpp:58
+ const Token *TestNameToken = Args->getUnexpArgument(1);
+ if (!TestCaseNameToken || !TestNameToken) {
+ return;
----------------
you can ellide the braces
================
Comment at: clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.cpp:62
+ std::string TestCaseName = PP->getSpelling(*TestCaseNameToken);
+ if (TestCaseName.find('_') != std::string::npos) {
+ Check->diag(TestCaseNameToken->getLocation(),
----------------
Maybe the duplicated `diag` call can be merged, you can store the diagnostic
with `auto Diag = Check->diag(...)` and pass in the right location and
arguments.
================
Comment at: clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.cpp:64
+ Check->diag(TestCaseNameToken->getLocation(),
+ "avoid using \"_\" in test case name \"%0\" according to "
+ "Googletest FAQ")
----------------
Duplicated message text, you can store it in a local
variable/StringRef/StringLiteral,
ellide braces
================
Comment at: clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.cpp:69
+
+ std::string TestName_maybe_disabled = PP->getSpelling(*TestNameToken);
+ StringRef TestName = TestName_maybe_disabled;
----------------
Please use camel-case.
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56424/new/
https://reviews.llvm.org/D56424
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
