This revision was automatically updated to reflect the committed changes.
Closed by commit rL295715: [clang-tidy] Reword the "code outside header guard"
warning. (authored by d0k).
Changed prior to commit:
https://reviews.llvm.org/D30191?vs=89182&id=89184#toc
Repository:
rL LLVM
https://reviews.llvm.org/D30191
Files:
clang-tools-extra/trunk/clang-tidy/utils/HeaderGuard.cpp
clang-tools-extra/trunk/unittests/clang-tidy/LLVMModuleTest.cpp
Index: clang-tools-extra/trunk/clang-tidy/utils/HeaderGuard.cpp
===================================================================
--- clang-tools-extra/trunk/clang-tidy/utils/HeaderGuard.cpp
+++ clang-tools-extra/trunk/clang-tidy/utils/HeaderGuard.cpp
@@ -223,19 +223,19 @@
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) {
StringRef Name = MacroEntry.first.getIdentifierInfo()->getName();
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;
}
Index: clang-tools-extra/trunk/unittests/clang-tidy/LLVMModuleTest.cpp
===================================================================
--- clang-tools-extra/trunk/unittests/clang-tidy/LLVMModuleTest.cpp
+++ clang-tools-extra/trunk/unittests/clang-tidy/LLVMModuleTest.cpp
@@ -12,11 +12,16 @@
// 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,68 +32,89 @@
};
} // 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"
"#define LLVM_ADT_FOO_H_\n"
"#endif\n",
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"
"\n"
"\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"
"\n"
"\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"
"\n"
"\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,73 +129,78 @@
"#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"
"#endif /* LLVM_ADT_FOO_H */\n",
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_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 @@
"#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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
