diff --git a/clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp b/clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp index c175f2b5928a..cf187c25a273 100644 --- a/clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp +++ b/clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp @@ -109,8 +109,7 @@ public: EndIfs[Ifndefs[MacroEntry.first.getIdentifierInfo()].first]; // If the macro Name is not equal to what we can compute, correct it in - // the - // #ifndef and #define. + // the #ifndef and #define. StringRef CurHeaderGuard = MacroEntry.first.getIdentifierInfo()->getName(); std::string NewGuard = checkHeaderGuardDefinition( @@ -119,6 +118,22 @@ public: // Now look at the #endif. We want a comment with the header guard. Fix it // at the slightest deviation. checkEndifComment(FileName, EndIf, NewGuard); + + // Bundle all fix-its into one warning. The message depends on whether we + // changed the header guard or not. + if (!FixIts.empty()) { + if (CurHeaderGuard != NewGuard) { + auto D = Check->diag(Ifndef, + "header guard does not follow preferred style"); + for (const FixItHint Fix : FixIts) + D.AddFixItHint(Fix); + } else { + auto D = Check->diag(EndIf, "#endif for a header guard should " + "reference the guard macro in a comment"); + for (const FixItHint Fix : FixIts) + D.AddFixItHint(Fix); + } + } } // Emit warnings for headers that are missing guards. @@ -129,6 +144,7 @@ public: Files.clear(); Ifndefs.clear(); EndIfs.clear(); + FixIts.clear(); } bool wouldFixEndifComment(StringRef FileName, SourceLocation EndIf, @@ -170,15 +186,14 @@ public: if (Ifndef.isValid() && CurHeaderGuard != CPPVar && (CurHeaderGuard != CPPVarUnder || wouldFixEndifComment(FileName, EndIf, CurHeaderGuard))) { - Check->diag(Ifndef, "header guard does not follow preferred style") - << FixItHint::CreateReplacement( - CharSourceRange::getTokenRange( - Ifndef, Ifndef.getLocWithOffset(CurHeaderGuard.size())), - CPPVar) - << FixItHint::CreateReplacement( - CharSourceRange::getTokenRange( - Define, Define.getLocWithOffset(CurHeaderGuard.size())), - CPPVar); + FixIts.push_back(FixItHint::CreateReplacement( + CharSourceRange::getTokenRange( + Ifndef, Ifndef.getLocWithOffset(CurHeaderGuard.size())), + CPPVar)); + FixIts.push_back(FixItHint::CreateReplacement( + CharSourceRange::getTokenRange( + Define, Define.getLocWithOffset(CurHeaderGuard.size())), + CPPVar)); return CPPVar; } return CurHeaderGuard; @@ -191,12 +206,10 @@ public: size_t EndIfLen; if (wouldFixEndifComment(FileName, EndIf, HeaderGuard, &EndIfLen)) { std::string Correct = "endif // " + HeaderGuard.str(); - Check->diag(EndIf, "#endif for a header guard should reference the " - "guard macro in a comment") - << FixItHint::CreateReplacement( - CharSourceRange::getCharRange(EndIf, - EndIf.getLocWithOffset(EndIfLen)), - Correct); + FixIts.push_back(FixItHint::CreateReplacement( + CharSourceRange::getCharRange(EndIf, + EndIf.getLocWithOffset(EndIfLen)), + Correct)); } } @@ -257,6 +270,7 @@ private: std::map> Ifndefs; std::map EndIfs; + std::vector FixIts; Preprocessor *PP; HeaderGuardCheck *Check; diff --git a/clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp b/clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp index 67b3d67de98b..dc7db4c95b74 100644 --- a/clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp +++ b/clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp @@ -88,9 +88,12 @@ TEST(NamespaceCommentCheckTest, FixWrongComments) { // FIXME: It seems this might be incompatible to dos path. Investigating. #if !defined(_WIN32) -static std::string runHeaderGuardCheck(StringRef Code, const Twine &Filename) { - return test::runCheckOnCode( - Code, /*Errors=*/nullptr, Filename, std::string("-xc++-header")); +static std::string runHeaderGuardCheck(StringRef Code, const Twine &Filename, + unsigned NumWarnings = 1) { + std::vector Errors; + std::string Result = test::runCheckOnCode( + Code, &Errors, Filename, std::string("-xc++-header")); + return Errors.size() == NumWarnings ? Result : "invalid error count"; } namespace { @@ -102,9 +105,12 @@ struct WithEndifComment : public LLVMHeaderGuardCheck { } // namespace static std::string runHeaderGuardCheckWithEndif(StringRef Code, - const Twine &Filename) { - return test::runCheckOnCode( - Code, /*Errors=*/nullptr, Filename, std::string("-xc++-header")); + const Twine &Filename, + unsigned NumWarnings = 1) { + std::vector Errors; + std::string Result = test::runCheckOnCode( + Code, &Errors, Filename, std::string("-xc++-header")); + return Errors.size() == NumWarnings ? Result : "invalid error count"; } TEST(LLVMHeaderGuardCheckTest, FixHeaderGuards) { @@ -116,7 +122,7 @@ TEST(LLVMHeaderGuardCheckTest, FixHeaderGuards) { 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")); + "include/llvm/ADT/foo.h", 0)); 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")); @@ -157,14 +163,14 @@ 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")); + "include/llvm/ADT/foo.h", 0)); 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")); + "include/llvm/ADT/foo.h", 0)); EXPECT_EQ( "#ifndef LLVM_ADT_FOO_H\n#define LLVM_ADT_FOO_H\n#endif // " @@ -178,14 +184,14 @@ TEST(LLVMHeaderGuardCheckTest, FixHeaderGuards) { 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")); + "include/llvm/ADT/foo.h", 0)); EXPECT_EQ("#ifndef LLVM_ADT_FOO_H\n#define LLVM_ADT_FOO_H\n#endif /* " "LLVM_ADT_FOO_H\\ \n FOO */", runHeaderGuardCheckWithEndif( "#ifndef LLVM_ADT_FOO_H\n#define LLVM_ADT_FOO_H\n#endif /* " "LLVM_ADT_FOO_H\\ \n FOO */", - "include/llvm/ADT/foo.h")); + "include/llvm/ADT/foo.h", 0)); } #endif