Index: clang-tidy/utils/HeaderGuard.h =================================================================== --- clang-tidy/utils/HeaderGuard.h +++ clang-tidy/utils/HeaderGuard.h @@ -32,6 +32,9 @@ /// \brief Returns true if the checker should add a header guard to the file /// if it has none. virtual bool shouldSuggestToAddHeaderGuard(StringRef Filename); + /// \brief Returns a replacement for endif line with a comment mentioning + /// \p HeaderGuard. The replacement should start with "endif". + virtual std::string formatEndIf(StringRef HeaderGuard); /// \brief Get the canonical header guard for a file. virtual std::string getHeaderGuard(StringRef Filename, StringRef OldGuard = StringRef()) = 0; Index: clang-tidy/utils/HeaderGuard.cpp =================================================================== --- clang-tidy/utils/HeaderGuard.cpp +++ clang-tidy/utils/HeaderGuard.cpp @@ -150,15 +150,15 @@ bool wouldFixEndifComment(StringRef FileName, SourceLocation EndIf, StringRef HeaderGuard, size_t *EndIfLenPtr = nullptr) { - if (!Check->shouldSuggestEndifComment(FileName)) + if (!EndIf.isValid()) return false; - const char *EndIfData = PP->getSourceManager().getCharacterData(EndIf); size_t EndIfLen = std::strcspn(EndIfData, "\r\n"); if (EndIfLenPtr) *EndIfLenPtr = EndIfLen; StringRef EndIfStr(EndIfData, EndIfLen); + EndIfStr = EndIfStr.substr(EndIfStr.find_first_not_of("#endif \t")); // Give up if there's an escaped newline. size_t FindEscapedNewline = EndIfStr.find_last_not_of(' '); @@ -166,8 +166,13 @@ EndIfStr[FindEscapedNewline] == '\\') return false; - return (EndIf.isValid() && !EndIfStr.endswith("// " + HeaderGuard.str()) && - !EndIfStr.endswith("/* " + HeaderGuard.str() + " */")); + if (!Check->shouldSuggestEndifComment(FileName) && + !(EndIfStr.startswith("//") || + (EndIfStr.startswith("/*") && EndIfStr.endswith("*/")))) + return false; + + return (EndIfStr != "// " + HeaderGuard.str()) && + (EndIfStr != "/* " + HeaderGuard.str() + " */"); } /// \brief Look for header guards that don't match the preferred style. Emit @@ -207,11 +212,10 @@ std::vector &FixIts) { size_t EndIfLen; if (wouldFixEndifComment(FileName, EndIf, HeaderGuard, &EndIfLen)) { - std::string Correct = "endif // " + HeaderGuard.str(); FixIts.push_back(FixItHint::CreateReplacement( CharSourceRange::getCharRange(EndIf, EndIf.getLocWithOffset(EndIfLen)), - Correct)); + Check->formatEndIf(HeaderGuard))); } } @@ -261,7 +265,7 @@ << FixItHint::CreateInsertion( SM.getLocForEndOfFile(FID), Check->shouldSuggestEndifComment(FileName) - ? "\n#endif // " + CPPVar + "\n" + ? "\n#" + Check->formatEndIf(CPPVar) + "\n" : "\n#endif\n"); } } @@ -294,5 +298,9 @@ return FileName.endswith(".h"); } +std::string HeaderGuardCheck::formatEndIf(StringRef HeaderGuard) { + return "endif // " + HeaderGuard.str(); +} + } // namespace tidy } // namespace clang Index: unittests/clang-tidy/LLVMModuleTest.cpp =================================================================== --- unittests/clang-tidy/LLVMModuleTest.cpp +++ unittests/clang-tidy/LLVMModuleTest.cpp @@ -103,9 +103,19 @@ "#endif\n", "include/clang/bar.h", /*ExpectedWarnings=*/1)); + // 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", + "#endif // LLVM_ADT_FOO_H\n", + runHeaderGuardCheck("#ifndef FOO\n" + "#define FOO\n" + "#endif // FOO\n", + "include/llvm/ADT/foo.h", + /*ExpectedWarnings=*/1)); + + 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", @@ -114,7 +124,7 @@ EXPECT_EQ("#ifndef LLVM_ADT_FOO_H\n" "#define LLVM_ADT_FOO_H\n" - "#endif // 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", @@ -141,7 +151,7 @@ EXPECT_EQ("#ifndef LLVM_ADT_FOO_H\n" "#define LLVM_ADT_FOO_H\n" - "#endif // 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",