[PATCH] D56424: [clang-tidy] Add check for underscores in googletest names.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

Thanks, mostly good to me. just a few nits.



================
Comment at: clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.cpp:72
+    TestName.consume_front(kDisabledTestPrefix);
+    if (TestName.find('_') != std::string::npos) {
+      Check->diag(TestNameToken->getLocation(),
----------------
nit: using `Test.contains`


================
Comment at: clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.h:10
+
+#ifndef LLVM_TOOLS_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANG_TIDY_READABILITY_UNDERSCORE_IN_GOOGLETEST_TEST_MACRO_H_
+#define LLVM_TOOLS_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANG_TIDY_READABILITY_UNDERSCORE_IN_GOOGLETEST_TEST_MACRO_H_
----------------
nit: the header guard should be `LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_AVOIDUNDERSCOREINGOOGLETESTNAMECHECK_H`


================
Comment at: docs/clang-tidy/checks/google-readability-avoid-underscore-in-googletest-name.rst:3
+
+google-readability-avoid-underscore-in-googletest-name
+======================================================
----------------
nit: also mention the new check in `/docs/ReleaseNotes.rst`


================
Comment at: docs/clang-tidy/checks/google-readability-avoid-underscore-in-googletest-name.rst:7
+Checks whether there are underscores in googletest test and test case names in
+test macros, not including the ``FRIEND_TEST`` macro.
+
----------------
maybe list all test macros that the check detects?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56424