Index: clang-tidy/CMakeLists.txt =================================================================== --- clang-tidy/CMakeLists.txt +++ clang-tidy/CMakeLists.txt @@ -27,3 +27,4 @@ add_subdirectory(llvm) add_subdirectory(google) add_subdirectory(misc) +add_subdirectory(utils) Index: clang-tidy/llvm/CMakeLists.txt =================================================================== --- clang-tidy/llvm/CMakeLists.txt +++ clang-tidy/llvm/CMakeLists.txt @@ -1,6 +1,7 @@ set(LLVM_LINK_COMPONENTS support) add_clang_library(clangTidyLLVMModule + HeaderGuardCheck.cpp IncludeOrderCheck.cpp LLVMTidyModule.cpp NamespaceCommentCheck.cpp @@ -12,4 +13,5 @@ clangBasic clangLex clangTidy + clangTidyUtils ) Index: clang-tidy/llvm/HeaderGuardCheck.h =================================================================== --- /dev/null +++ clang-tidy/llvm/HeaderGuardCheck.h @@ -0,0 +1,30 @@ +//===--- HeaderGuardCheck.h - clang-tidy ------------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_HEADER_GUARD_CHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_HEADER_GUARD_CHECK_H + +#include "../utils/HeaderGuard.h" + +namespace clang { +namespace tidy { + +/// Finds and fixes header guards that do not adhere to LLVM style. +class LLVMHeaderGuardCheck : public HeaderGuardCheck { +public: + bool shouldSuggestEndifComment(StringRef Filename) override { return false; } + bool shouldFixHeaderGuard(StringRef Filename) override; + std::string getHeaderGuard(StringRef Filename, + StringRef OldGuard = StringRef()) override; +}; + +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_HEADER_GUARD_CHECK_H Index: clang-tidy/llvm/HeaderGuardCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/llvm/HeaderGuardCheck.cpp @@ -0,0 +1,36 @@ +//===--- HeaderGuardCheck.cpp - clang-tidy --------------------------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "HeaderGuardCheck.h" + +namespace clang { +namespace tidy { + +bool LLVMHeaderGuardCheck::shouldFixHeaderGuard(StringRef Filename) { + return Filename.find("include/") != StringRef::npos; +} + +std::string LLVMHeaderGuardCheck::getHeaderGuard(StringRef Filename, + StringRef OldGuard) { + size_t PostInclude = Filename.find("include/") + std::strlen("include/"); + std::string Guard = Filename.substr(PostInclude); + + std::replace(Guard.begin(), Guard.end(), '/', '_'); + std::replace(Guard.begin(), Guard.end(), '.', '_'); + std::replace(Guard.begin(), Guard.end(), '-', '_'); + + // The prevalent style in clang is LLVM_CLANG_FOO_BAR_H + if (StringRef(Guard).startswith("clang")) + Guard = "LLVM_" + Guard; + + return StringRef(Guard).upper(); +} + +} // namespace tidy +} // namespace clang Index: clang-tidy/llvm/LLVMTidyModule.cpp =================================================================== --- clang-tidy/llvm/LLVMTidyModule.cpp +++ clang-tidy/llvm/LLVMTidyModule.cpp @@ -10,6 +10,7 @@ #include "../ClangTidy.h" #include "../ClangTidyModule.h" #include "../ClangTidyModuleRegistry.h" +#include "HeaderGuardCheck.h" #include "IncludeOrderCheck.h" #include "NamespaceCommentCheck.h" #include "TwineLocalCheck.h" @@ -21,6 +22,8 @@ public: void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { CheckFactories.addCheckFactory( + "llvm-header-guard", new ClangTidyCheckFactory()); + CheckFactories.addCheckFactory( "llvm-include-order", new ClangTidyCheckFactory()); CheckFactories.addCheckFactory( "llvm-namespace-comment", Index: clang-tidy/utils/CMakeLists.txt =================================================================== --- /dev/null +++ clang-tidy/utils/CMakeLists.txt @@ -0,0 +1,12 @@ +set(LLVM_LINK_COMPONENTS support) + +add_clang_library(clangTidyUtils + HeaderGuard.cpp + + LINK_LIBS + clangAST + clangASTMatchers + clangBasic + clangLex + clangTidy + ) Index: clang-tidy/utils/HeaderGuard.h =================================================================== --- /dev/null +++ clang-tidy/utils/HeaderGuard.h @@ -0,0 +1,42 @@ +//===--- HeaderGuard.h - clang-tidy -----------------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_HEADER_GUARD_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_HEADER_GUARD_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { + +/// \brief Finds and fixes header guards. +class HeaderGuardCheck : public ClangTidyCheck { +public: + void registerPPCallbacks(CompilerInstance &Compiler) override; + + /// \brief Returns true if the checker should suggest inserting a trailing + /// comment on the #endif of the header guard. It will use the same name as + /// returned by getHeaderGuard. + virtual bool shouldSuggestEndifComment(StringRef Filename); + /// \brief Returns true if the checker should suggest changing an existing + /// header guard to the string returned by getHeaderGuard. + virtual bool shouldFixHeaderGuard(StringRef Filename); + /// \brief Returns true if the checker should add a header guard to the file + /// if + /// it has none. + virtual bool shouldSuggestToAddHeaderGuard(StringRef Filename); + /// \brief Get the canonical header guard for a file. + virtual std::string getHeaderGuard(StringRef Filename, + StringRef OldGuard = StringRef()) = 0; +}; + +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_HEADER_GUARD_H Index: clang-tidy/utils/HeaderGuard.cpp =================================================================== --- /dev/null +++ clang-tidy/utils/HeaderGuard.cpp @@ -0,0 +1,248 @@ +//===--- HeaderGuard.cpp - clang-tidy -------------------------------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "HeaderGuard.h" +#include "clang/Frontend/CompilerInstance.h" +#include "clang/Lex/PPCallbacks.h" +#include "clang/Lex/Preprocessor.h" +#include "clang/Tooling/Tooling.h" +#include "llvm/Support/Path.h" + +namespace clang { +namespace tidy { + +/// \brief canonicalize a path by removing ./ and ../ components. +// FIXME: Consider moving this to llvm::sys::path. +static std::string cleanPath(StringRef Path) { + SmallString<256> NewPath; + for (auto I = llvm::sys::path::begin(Path), E = llvm::sys::path::end(Path); + I != E; ++I) { + if (*I == ".") + continue; + if (*I == "..") { + // Drop the last component. + NewPath.resize(llvm::sys::path::parent_path(NewPath).size()); + } else { + if (!NewPath.empty()) + NewPath += '/'; + NewPath += *I; + } + } + return NewPath.str(); +} + +namespace { +class HeaderGuardPPCallbacks : public PPCallbacks { +public: + explicit HeaderGuardPPCallbacks(Preprocessor *PP, HeaderGuardCheck *Check) + : PP(PP), Check(Check) {} + + void FileChanged(SourceLocation Loc, FileChangeReason Reason, + SrcMgr::CharacteristicKind FileType, + FileID PrevFID) override { + // Record all files we enter. We'll need them to diagnose headers without + // guards. + SourceManager &SM = PP->getSourceManager(); + if (Reason == EnterFile && FileType == SrcMgr::C_User) { + if (const FileEntry *FE = SM.getFileEntryForID(SM.getFileID(Loc))) { + std::string FileName = cleanPath(FE->getName()); + Files[FileName] = FE; + } + } + } + + void Ifndef(SourceLocation Loc, const Token &MacroNameTok, + const MacroDirective *MD) override { + if (MD) + return; + + // Record #ifndefs that succeeded. We also need the Location of the Name. + Ifndefs[MacroNameTok.getIdentifierInfo()] = + std::make_pair(Loc, MacroNameTok.getLocation()); + } + + void MacroDefined(const Token &MacroNameTok, + const MacroDirective *MD) override { + // Record all defined macros. We store the whole token to get info on the + // name later. + Macros.emplace_back(MacroNameTok, MD); + } + + void Endif(SourceLocation Loc, SourceLocation IfLoc) override { + // Record all #endif and the corresponding #ifs (including #ifndefs). + EndIfs[IfLoc] = Loc; + } + + void EndOfMainFile() override { + // Now that we have all this information from the preprocessor, use it! + SourceManager &SM = PP->getSourceManager(); + + for (const auto &MacroEntry : Macros) { + const MacroInfo *MI = MacroEntry.second->getMacroInfo(); + + // We use clang's header guard detection. This has the advantage of also + // emitting a warning for cases where a pseudo header guard is found but + // preceeded by something blocking the header guard optimization. + if (!MI->isUsedForHeaderGuard()) + continue; + + const FileEntry *FE = + SM.getFileEntryForID(SM.getFileID(MI->getDefinitionLoc())); + std::string FileName = cleanPath(FE->getName()); + Files.erase(FileName); + + // See if we should check and fix this header guard. + if (!Check->shouldFixHeaderGuard(FileName)) + continue; + + // Look up Locations for this guard. + SourceLocation Ifndef = + Ifndefs[MacroEntry.first.getIdentifierInfo()].second; + SourceLocation Define = MacroEntry.first.getLocation(); + SourceLocation EndIf = + 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. + StringRef CurHeaderGuard = + MacroEntry.first.getIdentifierInfo()->getName(); + std::string NewGuard = + checkHeaderGuardDefinition(Ifndef, Define, FileName, CurHeaderGuard); + + // Now look at the #endif. We want a comment with the header guard. Fix it + // at the slightest deviation. + if (Check->shouldSuggestEndifComment(FileName)) + checkEndifComment(EndIf, NewGuard); + } + + // Emit warnings for headers that are missing guards. + checkGuardlessHeaders(); + } + + /// \brief Look for header guards that don't match the preferred style. Emit + /// fix-its and return the suggested header guard (or the original if no + /// change was made. + std::string checkHeaderGuardDefinition(SourceLocation Ifndef, + SourceLocation Define, + StringRef FileName, + StringRef CurHeaderGuard) { + std::string CPPVar = Check->getHeaderGuard(FileName, CurHeaderGuard); + std::string CPPVarUnder = CPPVar + '_'; // Allow a trailing underscore. + if (Ifndef.isValid() && CurHeaderGuard != CPPVar && + CurHeaderGuard != CPPVarUnder) { + 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); + return CPPVar; + } + return CurHeaderGuard; + } + + /// \brief Checks the comment after the #endif of a header guard and fixes it + /// if it doesn't match \c HeaderGuard. + void checkEndifComment(SourceLocation EndIf, StringRef HeaderGuard) { + const char *EndIfData = PP->getSourceManager().getCharacterData(EndIf); + size_t EndIfLen = std::strcspn(EndIfData, "\r\n"); + + StringRef EndIfStr(EndIfData, EndIfLen); + if (EndIf.isValid() && !EndIfStr.endswith("// " + HeaderGuard.str())) { + 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); + } + } + + /// \brief Looks for files that were visited but didn't have a header guard. + /// emits a warning with fixits suggesting adding one. + void checkGuardlessHeaders() { + // Look for header files that didn't have a header guard. Emit a warning and + // fix-its to add the guard. + // TODO: Insert the guard after top comments. + for (const auto &FE : Files) { + StringRef FileName = FE.getKey(); + if (!Check->shouldSuggestToAddHeaderGuard(FileName)) + continue; + + SourceManager &SM = PP->getSourceManager(); + FileID FID = SM.translateFile(FE.getValue()); + SourceLocation StartLoc = SM.getLocForStartOfFile(FID); + if (StartLoc.isInvalid()) + continue; + + 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. + // 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."); + SeenMacro = true; + break; + } + } + + if (SeenMacro) + continue; + + Check->diag(StartLoc, "header is missing header guard") + << FixItHint::CreateInsertion( + StartLoc, "#ifndef " + CPPVar + "\n#define " + CPPVar + "\n\n") + << FixItHint::CreateInsertion(SM.getLocForEndOfFile(FID), + "\n#endif // " + CPPVar + "\n"); + } + } + +private: + std::vector> Macros; + llvm::StringMap Files; + std::map> + Ifndefs; + std::map EndIfs; + + Preprocessor *PP; + HeaderGuardCheck *Check; +}; +} // namespace + +void HeaderGuardCheck::registerPPCallbacks(CompilerInstance &Compiler) { + Compiler.getPreprocessor().addPPCallbacks( + new HeaderGuardPPCallbacks(&Compiler.getPreprocessor(), this)); +} + +bool HeaderGuardCheck::shouldSuggestEndifComment(StringRef FileName) { + return FileName.endswith(".h"); +} + +bool HeaderGuardCheck::shouldFixHeaderGuard(StringRef FileName) { return true; } + +bool HeaderGuardCheck::shouldSuggestToAddHeaderGuard(StringRef FileName) { + return FileName.endswith(".h"); +} + +} // namespace tidy +} // namespace clang Index: unittests/clang-tidy/LLVMModuleTest.cpp =================================================================== --- unittests/clang-tidy/LLVMModuleTest.cpp +++ unittests/clang-tidy/LLVMModuleTest.cpp @@ -1,4 +1,5 @@ #include "ClangTidyTest.h" +#include "llvm/HeaderGuardCheck.h" #include "llvm/IncludeOrderCheck.h" #include "llvm/NamespaceCommentCheck.h" #include "gtest/gtest.h" @@ -85,6 +86,43 @@ "} // namespace asdf")); } +static std::string runHeaderGuardCheck(StringRef Code, const Twine &Filename) { + return test::runCheckOnCode( + Code, /*Errors=*/nullptr, Filename, std::string("-xc++-header")); +} + +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")); + + // 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")); + + EXPECT_EQ("#ifndef LLVM_CLANG_C_BAR_H\n#define LLVM_CLANG_C_BAR_H\n#endif\n", + runHeaderGuardCheck("", "./include/clang-c/bar.h")); + + // No change expected here. + EXPECT_EQ("#ifndef FOO\n#define FOO\n#endif\n", + runHeaderGuardCheck("#ifndef FOO\n#define FOO\n#endif\n", + "lib/internal/Bincludes.h")); + + 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")); + + EXPECT_EQ("#ifndef LLVM_CLANG_BAR_H\n#define LLVM_CLANG_BAR_H\n" + "int foo;\n#ifndef FOOLOLO\n#define FOOLOLO\n#endif\n#endif\n", + runHeaderGuardCheck( + "int foo;\n#ifndef FOOLOLO\n#define FOOLOLO\n#endif\n", + "include/clang/bar.h")); +} + } // namespace test } // namespace tidy } // namespace clang