diff options
| -rw-r--r-- | clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp | 12 | ||||
| -rw-r--r-- | clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp | 148 |
2 files changed, 95 insertions, 65 deletions
diff --git a/clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp b/clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp index acd63d3b0ad..f89362d88cd 100644 --- a/clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp +++ b/clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp @@ -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; } diff --git a/clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp b/clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp index 1d7fb24098a..8ab207d9c53 100644 --- a/clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp +++ b/clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp @@ -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 LLVMHeaderGuardCheck { }; } // 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, FixHeaderGuards) { 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, FixHeaderGuards) { "\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, FixHeaderGuards) { "\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, FixHeaderGuards) { "\n" "#endif\n", runHeaderGuardCheck("", "tools/clang/tools/extra/clang-tidy/x.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", /*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", + 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, FixHeaderGuards) { "#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, FixHeaderGuards) { 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, FixHeaderGuards) { 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, FixHeaderGuards) { "#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 |
