[clang-tools-extra] r295715 - [clang-tidy] Reword the "code outside header guard" warning.
Benjamin Kramer via cfe-commits
cfe-commits at lists.llvm.org
Tue Feb 21 03:25:46 PST 2017
Author: d0k
Date: Tue Feb 21 05:25:45 2017
New Revision: 295715
URL: http://llvm.org/viewvc/llvm-project?rev=295715&view=rev
Log:
[clang-tidy] Reword the "code outside header guard" warning.
The check doesn't really know if the code it is warning about came before
or after the header guard, so phrase it more neutral instead of complaining
about code before the header guard. The location for the warning is still
not optimal, but I don't think fixing that is worth the effort, the
preprocessor doesn't give us a better location.
Differential Revision: https://reviews.llvm.org/D30191
Modified:
clang-tools-extra/trunk/clang-tidy/utils/HeaderGuard.cpp
clang-tools-extra/trunk/unittests/clang-tidy/LLVMModuleTest.cpp
Modified: clang-tools-extra/trunk/clang-tidy/utils/HeaderGuard.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/utils/HeaderGuard.cpp?rev=295715&r1=295714&r2=295715&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/utils/HeaderGuard.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/utils/HeaderGuard.cpp Tue Feb 21 05:25:45 2017
@@ -223,9 +223,10 @@ public:
std::string CPPVar = Check->getHeaderGuard(FileName);
std::string CPPVarUnder = CPPVar + '_'; // Allow a trailing underscore.
- // If there is a header guard macro but it's not in the topmost position
- // emit a plain warning without fix-its. This often happens when the guard
- // macro is preceeded by includes.
+ // If there's a macro with a name that follows the header guard convention
+ // but was not recognized by the preprocessor as a header guard there must
+ // be code outside of the guarded area. Emit a plain warning without
+ // fix-its.
// FIXME: Can we move it into the right spot?
bool SeenMacro = false;
for (const auto &MacroEntry : Macros) {
@@ -233,9 +234,8 @@ public:
SourceLocation DefineLoc = MacroEntry.first.getLocation();
if ((Name == CPPVar || Name == CPPVarUnder) &&
SM.isWrittenInSameFile(StartLoc, DefineLoc)) {
- Check->diag(
- DefineLoc,
- "Header guard after code/includes. Consider moving it up.");
+ Check->diag(DefineLoc, "code/includes outside of area guarded by "
+ "header guard; consider moving it");
SeenMacro = true;
break;
}
Modified: clang-tools-extra/trunk/unittests/clang-tidy/LLVMModuleTest.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-tidy/LLVMModuleTest.cpp?rev=295715&r1=295714&r2=295715&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clang-tidy/LLVMModuleTest.cpp (original)
+++ clang-tools-extra/trunk/unittests/clang-tidy/LLVMModuleTest.cpp Tue Feb 21 05:25:45 2017
@@ -12,11 +12,16 @@ namespace test {
// FIXME: It seems this might be incompatible to dos path. Investigating.
#if !defined(_WIN32)
static std::string runHeaderGuardCheck(StringRef Code, const Twine &Filename,
- unsigned ExpectedWarnings) {
+ Optional<StringRef> ExpectedWarning) {
std::vector<ClangTidyError> Errors;
std::string Result = test::runCheckOnCode<LLVMHeaderGuardCheck>(
Code, &Errors, Filename, std::string("-xc++-header"));
- return Errors.size() == ExpectedWarnings ? Result : "invalid error count";
+ if (Errors.size() != (size_t)ExpectedWarning.hasValue())
+ return "invalid error count";
+ if (ExpectedWarning && *ExpectedWarning != Errors.back().Message.Message)
+ return "expected: '" + ExpectedWarning->str() + "', saw: '" +
+ Errors.back().Message.Message + "'";
+ return Result;
}
namespace {
@@ -27,24 +32,30 @@ struct WithEndifComment : public LLVMHea
};
} // namespace
-static std::string runHeaderGuardCheckWithEndif(StringRef Code,
- const Twine &Filename,
- unsigned ExpectedWarnings) {
+static std::string
+runHeaderGuardCheckWithEndif(StringRef Code, const Twine &Filename,
+ Optional<StringRef> ExpectedWarning) {
std::vector<ClangTidyError> Errors;
std::string Result = test::runCheckOnCode<WithEndifComment>(
Code, &Errors, Filename, std::string("-xc++-header"));
- return Errors.size() == ExpectedWarnings ? Result : "invalid error count";
+ if (Errors.size() != (size_t)ExpectedWarning.hasValue())
+ return "invalid error count";
+ if (ExpectedWarning && *ExpectedWarning != Errors.back().Message.Message)
+ return "expected: '" + ExpectedWarning->str() + "', saw: '" +
+ Errors.back().Message.Message + "'";
+ return Result;
}
TEST(LLVMHeaderGuardCheckTest, FixHeaderGuards) {
EXPECT_EQ("#ifndef LLVM_ADT_FOO_H\n"
"#define LLVM_ADT_FOO_H\n"
"#endif\n",
- runHeaderGuardCheck("#ifndef FOO\n"
- "#define FOO\n"
- "#endif\n",
- "include/llvm/ADT/foo.h",
- /*ExpectedWarnings=*/1));
+ runHeaderGuardCheck(
+ "#ifndef FOO\n"
+ "#define FOO\n"
+ "#endif\n",
+ "include/llvm/ADT/foo.h",
+ StringRef("header guard does not follow preferred style")));
// Allow trailing underscores.
EXPECT_EQ("#ifndef LLVM_ADT_FOO_H_\n"
@@ -53,8 +64,7 @@ TEST(LLVMHeaderGuardCheckTest, FixHeader
runHeaderGuardCheck("#ifndef LLVM_ADT_FOO_H_\n"
"#define LLVM_ADT_FOO_H_\n"
"#endif\n",
- "include/llvm/ADT/foo.h",
- /*ExpectedWarnings=*/0));
+ "include/llvm/ADT/foo.h", None));
EXPECT_EQ("#ifndef LLVM_CLANG_C_BAR_H\n"
"#define LLVM_CLANG_C_BAR_H\n"
@@ -62,7 +72,7 @@ TEST(LLVMHeaderGuardCheckTest, FixHeader
"\n"
"#endif\n",
runHeaderGuardCheck("", "./include/clang-c/bar.h",
- /*ExpectedWarnings=*/1));
+ StringRef("header is missing header guard")));
EXPECT_EQ("#ifndef LLVM_CLANG_LIB_CODEGEN_C_H\n"
"#define LLVM_CLANG_LIB_CODEGEN_C_H\n"
@@ -70,7 +80,7 @@ TEST(LLVMHeaderGuardCheckTest, FixHeader
"\n"
"#endif\n",
runHeaderGuardCheck("", "tools/clang/lib/CodeGen/c.h",
- /*ExpectedWarnings=*/1));
+ StringRef("header is missing header guard")));
EXPECT_EQ("#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_X_H\n"
"#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_X_H\n"
@@ -78,17 +88,33 @@ TEST(LLVMHeaderGuardCheckTest, FixHeader
"\n"
"#endif\n",
runHeaderGuardCheck("", "tools/clang/tools/extra/clang-tidy/x.h",
- /*ExpectedWarnings=*/1));
+ StringRef("header is missing header guard")));
- EXPECT_EQ("int foo;\n"
- "#ifndef LLVM_CLANG_BAR_H\n"
- "#define LLVM_CLANG_BAR_H\n"
- "#endif\n",
- runHeaderGuardCheck("int foo;\n"
- "#ifndef LLVM_CLANG_BAR_H\n"
- "#define LLVM_CLANG_BAR_H\n"
- "#endif\n",
- "include/clang/bar.h", /*ExpectedWarnings=*/1));
+ EXPECT_EQ(
+ "int foo;\n"
+ "#ifndef LLVM_CLANG_BAR_H\n"
+ "#define LLVM_CLANG_BAR_H\n"
+ "#endif\n",
+ runHeaderGuardCheck("int foo;\n"
+ "#ifndef LLVM_CLANG_BAR_H\n"
+ "#define LLVM_CLANG_BAR_H\n"
+ "#endif\n",
+ "include/clang/bar.h",
+ StringRef("code/includes outside of area guarded by "
+ "header guard; consider moving it")));
+
+ EXPECT_EQ(
+ "#ifndef LLVM_CLANG_BAR_H\n"
+ "#define LLVM_CLANG_BAR_H\n"
+ "#endif\n"
+ "int foo;\n",
+ runHeaderGuardCheck("#ifndef LLVM_CLANG_BAR_H\n"
+ "#define LLVM_CLANG_BAR_H\n"
+ "#endif\n"
+ "int foo;\n",
+ "include/clang/bar.h",
+ StringRef("code/includes outside of area guarded by "
+ "header guard; consider moving it")));
EXPECT_EQ("#ifndef LLVM_CLANG_BAR_H\n"
"#define LLVM_CLANG_BAR_H\n"
@@ -103,35 +129,40 @@ TEST(LLVMHeaderGuardCheckTest, FixHeader
"#ifndef FOOLOLO\n"
"#define FOOLOLO\n"
"#endif\n",
- "include/clang/bar.h", /*ExpectedWarnings=*/1));
+ "include/clang/bar.h",
+ StringRef("header is missing header guard")));
// Fix incorrect #endif comments even if we shouldn't add new ones.
EXPECT_EQ("#ifndef LLVM_ADT_FOO_H\n"
"#define LLVM_ADT_FOO_H\n"
"#endif // LLVM_ADT_FOO_H\n",
- runHeaderGuardCheck("#ifndef FOO\n"
- "#define FOO\n"
- "#endif // FOO\n",
- "include/llvm/ADT/foo.h",
- /*ExpectedWarnings=*/1));
+ runHeaderGuardCheck(
+ "#ifndef FOO\n"
+ "#define FOO\n"
+ "#endif // FOO\n",
+ "include/llvm/ADT/foo.h",
+ StringRef("header guard does not follow preferred style")));
EXPECT_EQ("#ifndef LLVM_ADT_FOO_H\n"
"#define LLVM_ADT_FOO_H\n"
"#endif // LLVM_ADT_FOO_H\n",
- runHeaderGuardCheckWithEndif("#ifndef FOO\n"
- "#define FOO\n"
- "#endif\n",
- "include/llvm/ADT/foo.h",
- /*ExpectedWarnings=*/1));
+ runHeaderGuardCheckWithEndif(
+ "#ifndef FOO\n"
+ "#define FOO\n"
+ "#endif\n",
+ "include/llvm/ADT/foo.h",
+ StringRef("header guard does not follow preferred style")));
EXPECT_EQ("#ifndef LLVM_ADT_FOO_H\n"
"#define LLVM_ADT_FOO_H\n"
"#endif // LLVM_ADT_FOO_H\n",
- runHeaderGuardCheckWithEndif("#ifndef LLVM_ADT_FOO_H\n"
- "#define LLVM_ADT_FOO_H\n"
- "#endif // LLVM_H\n",
- "include/llvm/ADT/foo.h",
- /*ExpectedWarnings=*/1));
+ runHeaderGuardCheckWithEndif(
+ "#ifndef LLVM_ADT_FOO_H\n"
+ "#define LLVM_ADT_FOO_H\n"
+ "#endif // LLVM_H\n",
+ "include/llvm/ADT/foo.h",
+ StringRef("#endif for a header guard should reference the "
+ "guard macro in a comment")));
EXPECT_EQ("#ifndef LLVM_ADT_FOO_H\n"
"#define LLVM_ADT_FOO_H\n"
@@ -139,8 +170,7 @@ TEST(LLVMHeaderGuardCheckTest, FixHeader
runHeaderGuardCheckWithEndif("#ifndef LLVM_ADT_FOO_H\n"
"#define LLVM_ADT_FOO_H\n"
"#endif /* LLVM_ADT_FOO_H */\n",
- "include/llvm/ADT/foo.h",
- /*ExpectedWarnings=*/0));
+ "include/llvm/ADT/foo.h", None));
EXPECT_EQ("#ifndef LLVM_ADT_FOO_H_\n"
"#define LLVM_ADT_FOO_H_\n"
@@ -148,28 +178,29 @@ TEST(LLVMHeaderGuardCheckTest, FixHeader
runHeaderGuardCheckWithEndif("#ifndef LLVM_ADT_FOO_H_\n"
"#define LLVM_ADT_FOO_H_\n"
"#endif // LLVM_ADT_FOO_H_\n",
- "include/llvm/ADT/foo.h",
- /*ExpectedWarnings=*/0));
+ "include/llvm/ADT/foo.h", None));
EXPECT_EQ("#ifndef LLVM_ADT_FOO_H\n"
"#define LLVM_ADT_FOO_H\n"
"#endif // LLVM_ADT_FOO_H\n",
- runHeaderGuardCheckWithEndif("#ifndef LLVM_ADT_FOO_H_\n"
- "#define LLVM_ADT_FOO_H_\n"
- "#endif // LLVM\n",
- "include/llvm/ADT/foo.h",
- /*ExpectedWarnings=*/1));
+ runHeaderGuardCheckWithEndif(
+ "#ifndef LLVM_ADT_FOO_H_\n"
+ "#define LLVM_ADT_FOO_H_\n"
+ "#endif // LLVM\n",
+ "include/llvm/ADT/foo.h",
+ StringRef("header guard does not follow preferred style")));
EXPECT_EQ("#ifndef LLVM_ADT_FOO_H\n"
"#define LLVM_ADT_FOO_H\n"
"#endif \\ \n"
"// LLVM_ADT_FOO_H\n",
- runHeaderGuardCheckWithEndif("#ifndef LLVM_ADT_FOO_H\n"
- "#define LLVM_ADT_FOO_H\n"
- "#endif \\ \n"
- "// LLVM_ADT_FOO_H\n",
- "include/llvm/ADT/foo.h",
- /*ExpectedWarnings=*/1));
+ runHeaderGuardCheckWithEndif(
+ "#ifndef LLVM_ADT_FOO_H\n"
+ "#define LLVM_ADT_FOO_H\n"
+ "#endif \\ \n"
+ "// LLVM_ADT_FOO_H\n",
+ "include/llvm/ADT/foo.h",
+ StringRef("backslash and newline separated by space")));
EXPECT_EQ("#ifndef LLVM_ADT_FOO_H\n"
"#define LLVM_ADT_FOO_H\n"
@@ -179,8 +210,7 @@ TEST(LLVMHeaderGuardCheckTest, FixHeader
"#define LLVM_ADT_FOO_H\n"
"#endif /* LLVM_ADT_FOO_H\\ \n"
" FOO */",
- "include/llvm/ADT/foo.h",
- /*ExpectedWarnings=*/0));
+ "include/llvm/ADT/foo.h", None));
}
#endif
More information about the cfe-commits
mailing list