KyleFromKitware created this revision. KyleFromKitware added a reviewer: clang-tools-extra. KyleFromKitware created this object with edit policy "Only User: KyleFromKitware (Kyle Edwards)". KyleFromKitware added a project: clang-tools-extra. Herald added subscribers: carlosgalvezp, xazax.hun. Herald added a reviewer: njames93. Herald added a project: All. KyleFromKitware requested review of this revision. Herald added a subscriber: cfe-commits.
Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D142121 Files: clang-tools-extra/clang-tidy/utils/CMakeLists.txt clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp clang-tools-extra/clang-tidy/utils/HeaderGuard.h clang-tools-extra/clang-tidy/utils/HeaderGuardBase.cpp clang-tools-extra/clang-tidy/utils/HeaderGuardBase.h
Index: clang-tools-extra/clang-tidy/utils/HeaderGuardBase.h =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/utils/HeaderGuardBase.h @@ -0,0 +1,93 @@ +//===--- HeaderGuard.h - clang-tidy -----------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_HEADERGUARDBASE_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_HEADERGUARDBASE_H + +#include "../ClangTidyCheck.h" +#include "../utils/FileExtensionsUtils.h" + +#include <tuple> + +namespace clang { +class MacroInfo; + +namespace tidy { +namespace utils { + +/// Base class for header guard detection. +/// The check supports these options: +/// - `HeaderFileExtensions`: a semicolon-separated list of filename +/// extensions of header files (The filename extension should not contain +/// "." prefix). ";h;hh;hpp;hxx" by default. +/// +/// For extension-less header files, using an empty string or leaving an +/// empty string between ";" if there are other filename extensions. +class HeaderGuardBase : public ClangTidyCheck { +public: + HeaderGuardBase(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + RawStringHeaderFileExtensions(Options.getLocalOrGlobal( + "HeaderFileExtensions", utils::defaultHeaderFileExtensions())) { + utils::parseFileExtensions(RawStringHeaderFileExtensions, + HeaderFileExtensions, + utils::defaultFileExtensionDelimiters()); + } + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, + Preprocessor *ModuleExpanderPP) override; + + /// Returns ``true`` if the check should suggest fixing a header file if it + /// has an existing header guard. + virtual bool shouldFixIfHeaderGuard(StringRef Filename); + + /// Returns ``true`` if the check should suggest fixing a header file if it + /// does not have an existing header guard. + virtual bool shouldFixIfNoHeaderGuard(StringRef Filename); + + /// @brief Called when a header guard is detected. + /// @param PP Preprocessor. + /// @param FileName Name of the file. + /// @param FE FileEntry. + /// @param IfndefHash Location of '#' in '#ifndef'. + /// @param Ifndef Location of 'ifndef' in '#ifndef'. + /// @param IfndefToken Location of macro token in '#ifndef'. + /// @param DefineHash Location of '#' in '#define'. + /// @param Define Location of macro token in '#define'. + /// @param EndIfHash Location of '#' in '#endif'. + /// @param EndIf Location of 'endif' in '#endif'. + virtual void onHeaderGuard(Preprocessor *PP, StringRef FileName, + const FileEntry *FE, SourceLocation IfndefHash, + SourceLocation Ifndef, SourceLocation IfndefToken, + SourceLocation DefineHash, const Token &Define, + SourceLocation EndIfHash, + SourceLocation EndIf) = 0; + + /// @brief Called when a header with no header guard is detected. + /// @param PP Preprocessor. + /// @param FileName Name of the file. + /// @param FE FileEntry. + /// @param StartLoc Location of the start of the file. + /// @param Macros List of macros in this file. Contains location of '#', macro + /// token, and macro info. + virtual void onGuardlessHeader( + Preprocessor *PP, StringRef FileName, const FileEntry *FE, + SourceLocation StartLoc, + const std::vector<std::tuple<SourceLocation, Token, const MacroInfo *>> + &Macros) = 0; + +protected: + std::string RawStringHeaderFileExtensions; + utils::FileExtensionsSet HeaderFileExtensions; +}; + +} // namespace utils +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_HEADERGUARDBASE_H Index: clang-tools-extra/clang-tidy/utils/HeaderGuardBase.cpp =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/utils/HeaderGuardBase.cpp @@ -0,0 +1,167 @@ +//===--- HeaderGuard.cpp - clang-tidy -------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "HeaderGuardBase.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" + +#include <tuple> + +namespace clang::tidy::utils { + +/// canonicalize a path by removing ./ and ../ components. +static std::string cleanPath(StringRef Path) { + SmallString<256> Result = Path; + llvm::sys::path::remove_dots(Result, true); + return std::string(Result.str()); +} + +namespace { +class HeaderGuardBasePPCallbacks : public PPCallbacks { +public: + HeaderGuardBasePPCallbacks(Preprocessor *PP, HeaderGuardBase *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 HashLoc, SourceLocation Loc, + const Token &MacroNameTok, const MacroDefinition &MD) override { + if (MD) + return; + + // Record #ifndefs that succeeded. We also need the Location of the Name. + Ifndefs[MacroNameTok.getIdentifierInfo()] = + std::make_tuple(HashLoc, Loc, MacroNameTok.getLocation()); + } + + void MacroDefined(SourceLocation HashLoc, 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(HashLoc, MacroNameTok, MD->getMacroInfo()); + } + + void Endif(SourceLocation HashLoc, SourceLocation Loc, + SourceLocation IfLoc) override { + // Record all #endif and the corresponding #ifs (including #ifndefs). + EndIfs[IfLoc] = std::make_pair(HashLoc, 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 = std::get<2>(MacroEntry); + + // 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 + // preceded 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->shouldFixIfHeaderGuard(FileName)) + continue; + + // Look up Locations for this guard. + std::tuple<SourceLocation, SourceLocation, SourceLocation> Ifndef = + Ifndefs[std::get<1>(MacroEntry).getIdentifierInfo()]; + std::pair<SourceLocation, SourceLocation> EndIf = + EndIfs[std::get<1>(Ifndef)]; + + Check->onHeaderGuard(PP, FileName, FE, std::get<0>(Ifndef), + std::get<1>(Ifndef), std::get<2>(Ifndef), + std::get<0>(MacroEntry), std::get<1>(MacroEntry), + EndIf.first, EndIf.second); + } + + // Emit warnings for headers that are missing guards. + checkGuardlessHeaders(); + clearAllState(); + } + + /// 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->shouldFixIfNoHeaderGuard(FileName)) + continue; + + SourceManager &SM = PP->getSourceManager(); + FileID FID = SM.translateFile(FE.getValue()); + SourceLocation StartLoc = SM.getLocForStartOfFile(FID); + if (StartLoc.isInvalid()) + continue; + + Check->onGuardlessHeader(PP, FileName, FE.getValue(), StartLoc, Macros); + } + } + +private: + void clearAllState() { + Macros.clear(); + Files.clear(); + Ifndefs.clear(); + EndIfs.clear(); + } + + std::vector<std::tuple<SourceLocation, Token, const MacroInfo *>> Macros; + llvm::StringMap<const FileEntry *> Files; + std::map<const IdentifierInfo *, + std::tuple<SourceLocation, SourceLocation, SourceLocation>> + Ifndefs; + std::map<SourceLocation, std::pair<SourceLocation, SourceLocation>> EndIfs; + + Preprocessor *PP; + HeaderGuardBase *Check; +}; +} // namespace + +void HeaderGuardBase::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "HeaderFileExtensions", RawStringHeaderFileExtensions); +} + +void HeaderGuardBase::registerPPCallbacks(const SourceManager &SM, + Preprocessor *PP, + Preprocessor *ModuleExpanderPP) { + PP->addPPCallbacks(std::make_unique<HeaderGuardBasePPCallbacks>(PP, this)); +} + +bool HeaderGuardBase::shouldFixIfHeaderGuard(StringRef FileName) { + return true; +} + +bool HeaderGuardBase::shouldFixIfNoHeaderGuard(StringRef FileName) { + return utils::isFileExtension(FileName, HeaderFileExtensions); +} +} // namespace clang::tidy::utils Index: clang-tools-extra/clang-tidy/utils/HeaderGuard.h =================================================================== --- clang-tools-extra/clang-tidy/utils/HeaderGuard.h +++ clang-tools-extra/clang-tidy/utils/HeaderGuard.h @@ -9,8 +9,7 @@ #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_HEADERGUARD_H #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_HEADERGUARD_H -#include "../ClangTidyCheck.h" -#include "../utils/FileExtensionsUtils.h" +#include "HeaderGuardBase.h" namespace clang { namespace tidy { @@ -24,19 +23,10 @@ /// /// For extension-less header files, using an empty string or leaving an /// empty string between ";" if there are other filename extensions. -class HeaderGuardCheck : public ClangTidyCheck { +class HeaderGuardCheck : public HeaderGuardBase { public: HeaderGuardCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context), - RawStringHeaderFileExtensions(Options.getLocalOrGlobal( - "HeaderFileExtensions", utils::defaultHeaderFileExtensions())) { - utils::parseFileExtensions(RawStringHeaderFileExtensions, - HeaderFileExtensions, - utils::defaultFileExtensionDelimiters()); - } - void storeOptions(ClangTidyOptions::OptionMap &Opts) override; - void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, - Preprocessor *ModuleExpanderPP) override; + : HeaderGuardBase(Name, Context) {} /// Ensure that the provided header guard is a non-reserved identifier. std::string sanitizeHeaderGuard(StringRef Guard); @@ -45,12 +35,6 @@ /// on the ``#endif`` of the header guard. It will use the same name as /// returned by ``HeaderGuardCheck::getHeaderGuard``. virtual bool shouldSuggestEndifComment(StringRef Filename); - /// Returns ``true`` if the check should suggest changing an existing header - /// guard to the string returned by ``HeaderGuardCheck::getHeaderGuard``. - virtual bool shouldFixHeaderGuard(StringRef Filename); - /// Returns ``true`` if the check should add a header guard to the file - /// if it has none. - virtual bool shouldSuggestToAddHeaderGuard(StringRef Filename); /// Returns a replacement for the ``#endif`` line with a comment mentioning /// \p HeaderGuard. The replacement should start with ``endif``. virtual std::string formatEndIf(StringRef HeaderGuard); @@ -58,9 +42,29 @@ virtual std::string getHeaderGuard(StringRef Filename, StringRef OldGuard = StringRef()) = 0; + void onHeaderGuard(Preprocessor *PP, StringRef FileName, const FileEntry *FE, + SourceLocation IfndefHash, SourceLocation Ifndef, + SourceLocation IfndefToken, SourceLocation DefineHash, + const Token &Define, SourceLocation EndIfHash, + SourceLocation EndIf) override; + void onGuardlessHeader( + Preprocessor *PP, StringRef FileName, const FileEntry *FE, + SourceLocation StartLoc, + const std::vector<std::tuple<SourceLocation, Token, const MacroInfo *>> + &Macros) override; + private: - std::string RawStringHeaderFileExtensions; - utils::FileExtensionsSet HeaderFileExtensions; + bool wouldFixEndifComment(Preprocessor *PP, StringRef FileName, + SourceLocation EndIf, StringRef HeaderGuard, + size_t *EndIfLenPtr = nullptr); + std::string + checkHeaderGuardDefinition(Preprocessor *PP, SourceLocation Ifndef, + SourceLocation Define, SourceLocation EndIf, + StringRef FileName, StringRef CurHeaderGuard, + std::vector<FixItHint> &FixIts); + void checkEndifComment(Preprocessor *PP, StringRef FileName, + SourceLocation EndIf, StringRef HeaderGuard, + std::vector<FixItHint> &FixIts); }; } // namespace utils Index: clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp =================================================================== --- clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp +++ clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp @@ -15,268 +15,146 @@ namespace clang::tidy::utils { -/// canonicalize a path by removing ./ and ../ components. -static std::string cleanPath(StringRef Path) { - SmallString<256> Result = Path; - llvm::sys::path::remove_dots(Result, true); - return std::string(Result.str()); -} - -namespace { -class HeaderGuardPPCallbacks : public PPCallbacks { -public: - 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 HashLoc, SourceLocation Loc, - const Token &MacroNameTok, const MacroDefinition &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(SourceLocation HashLoc, 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->getMacroInfo()); - } - - void Endif(SourceLocation HashLoc, 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; - - // 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 - // preceded 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::vector<FixItHint> FixIts; - std::string NewGuard = checkHeaderGuardDefinition( - Ifndef, Define, EndIf, FileName, CurHeaderGuard, FixIts); - - // Now look at the #endif. We want a comment with the header guard. Fix it - // at the slightest deviation. - checkEndifComment(FileName, EndIf, NewGuard, FixIts); - - // 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) { - Check->diag(Ifndef, "header guard does not follow preferred style") - << FixIts; - } else { - Check->diag(EndIf, "#endif for a header guard should reference the " - "guard macro in a comment") - << FixIts; - } - } +void HeaderGuardCheck::onHeaderGuard( + Preprocessor *PP, StringRef FileName, const FileEntry *FE, + SourceLocation IfndefHash, SourceLocation Ifndef, + SourceLocation IfndefToken, SourceLocation DefineHash, const Token &Define, + SourceLocation EndIfHash, SourceLocation EndIf) { + // If the macro Name is not equal to what we can compute, correct it in + // the #ifndef and #define. + StringRef CurHeaderGuard = Define.getIdentifierInfo()->getName(); + std::vector<FixItHint> FixIts; + std::string NewGuard = + checkHeaderGuardDefinition(PP, IfndefToken, Define.getLocation(), EndIf, + FileName, CurHeaderGuard, FixIts); + + // Now look at the #endif. We want a comment with the header guard. Fix it + // at the slightest deviation. + checkEndifComment(PP, FileName, EndIf, NewGuard, FixIts); + + // 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) { + diag(Ifndef, "header guard does not follow preferred style") << FixIts; + } else { + diag(EndIf, "#endif for a header guard should reference the " + "guard macro in a comment") + << FixIts; } - - // Emit warnings for headers that are missing guards. - checkGuardlessHeaders(); - clearAllState(); } +} - bool wouldFixEndifComment(StringRef FileName, SourceLocation EndIf, - StringRef HeaderGuard, - size_t *EndIfLenPtr = nullptr) { - 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(' '); - if (FindEscapedNewline != StringRef::npos && - EndIfStr[FindEscapedNewline] == '\\') - return false; - - bool IsLineComment = - EndIfStr.consume_front("//") || - (EndIfStr.consume_front("/*") && EndIfStr.consume_back("*/")); - if (!IsLineComment) - return Check->shouldSuggestEndifComment(FileName); - - return EndIfStr.trim() != HeaderGuard; - } +bool HeaderGuardCheck::wouldFixEndifComment(Preprocessor *PP, + StringRef FileName, + SourceLocation EndIf, + StringRef HeaderGuard, + size_t *EndIfLenPtr) { + 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(' '); + if (FindEscapedNewline != StringRef::npos && + EndIfStr[FindEscapedNewline] == '\\') + return false; + + bool IsLineComment = + EndIfStr.consume_front("//") || + (EndIfStr.consume_front("/*") && EndIfStr.consume_back("*/")); + if (!IsLineComment) + return shouldSuggestEndifComment(FileName); + + return EndIfStr.trim() != HeaderGuard; +} /// 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, - SourceLocation EndIf, - StringRef FileName, - StringRef CurHeaderGuard, - std::vector<FixItHint> &FixIts) { - std::string CPPVar = Check->getHeaderGuard(FileName, CurHeaderGuard); - CPPVar = Check->sanitizeHeaderGuard(CPPVar); - std::string CPPVarUnder = CPPVar + '_'; - - // Allow a trailing underscore if and only if we don't have to change the - // endif comment too. - if (Ifndef.isValid() && CurHeaderGuard != CPPVar && - (CurHeaderGuard != CPPVarUnder || - wouldFixEndifComment(FileName, EndIf, CurHeaderGuard))) { - 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 std::string(CurHeaderGuard); +std::string HeaderGuardCheck::checkHeaderGuardDefinition( + Preprocessor *PP, SourceLocation Ifndef, SourceLocation Define, + SourceLocation EndIf, StringRef FileName, StringRef CurHeaderGuard, + std::vector<FixItHint> &FixIts) { + std::string CPPVar = getHeaderGuard(FileName, CurHeaderGuard); + CPPVar = sanitizeHeaderGuard(CPPVar); + std::string CPPVarUnder = CPPVar + '_'; + + // Allow a trailing underscore if and only if we don't have to change the + // endif comment too. + if (Ifndef.isValid() && CurHeaderGuard != CPPVar && + (CurHeaderGuard != CPPVarUnder || + wouldFixEndifComment(PP, FileName, EndIf, CurHeaderGuard))) { + 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 std::string(CurHeaderGuard); +} /// Checks the comment after the #endif of a header guard and fixes it /// if it doesn't match \c HeaderGuard. - void checkEndifComment(StringRef FileName, SourceLocation EndIf, - StringRef HeaderGuard, - std::vector<FixItHint> &FixIts) { - size_t EndIfLen; - if (wouldFixEndifComment(FileName, EndIf, HeaderGuard, &EndIfLen)) { - FixIts.push_back(FixItHint::CreateReplacement( - CharSourceRange::getCharRange(EndIf, - EndIf.getLocWithOffset(EndIfLen)), - Check->formatEndIf(HeaderGuard))); - } +void HeaderGuardCheck::checkEndifComment(Preprocessor *PP, StringRef FileName, + SourceLocation EndIf, + StringRef HeaderGuard, + std::vector<FixItHint> &FixIts) { + size_t EndIfLen; + if (wouldFixEndifComment(PP, FileName, EndIf, HeaderGuard, &EndIfLen)) { + FixIts.push_back(FixItHint::CreateReplacement( + CharSourceRange::getCharRange(EndIf, EndIf.getLocWithOffset(EndIfLen)), + formatEndIf(HeaderGuard))); } +} - /// 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); - CPPVar = Check->sanitizeHeaderGuard(CPPVar); - std::string CPPVarUnder = CPPVar + '_'; // Allow a trailing underscore. - // 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, "code/includes outside of area guarded by " - "header guard; consider moving it"); - 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), - Check->shouldSuggestEndifComment(FileName) - ? "\n#" + Check->formatEndIf(CPPVar) + "\n" - : "\n#endif\n"); +void HeaderGuardCheck::onGuardlessHeader( + Preprocessor *PP, StringRef FileName, const FileEntry *FE, + SourceLocation StartLoc, + const std::vector<std::tuple<SourceLocation, Token, const MacroInfo *>> + &Macros) { + SourceManager &SM = PP->getSourceManager(); + std::string CPPVar = getHeaderGuard(FileName); + CPPVar = sanitizeHeaderGuard(CPPVar); + std::string CPPVarUnder = CPPVar + '_'; // Allow a trailing underscore. + // 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 = std::get<1>(MacroEntry).getIdentifierInfo()->getName(); + SourceLocation DefineLoc = std::get<1>(MacroEntry).getLocation(); + if ((Name == CPPVar || Name == CPPVarUnder) && + SM.isWrittenInSameFile(StartLoc, DefineLoc)) { + diag(DefineLoc, "code/includes outside of area guarded by " + "header guard; consider moving it"); + SeenMacro = true; + break; } } -private: - void clearAllState() { - Macros.clear(); - Files.clear(); - Ifndefs.clear(); - EndIfs.clear(); - } - - std::vector<std::pair<Token, const MacroInfo *>> Macros; - llvm::StringMap<const FileEntry *> Files; - std::map<const IdentifierInfo *, std::pair<SourceLocation, SourceLocation>> - Ifndefs; - std::map<SourceLocation, SourceLocation> EndIfs; - - Preprocessor *PP; - HeaderGuardCheck *Check; -}; -} // namespace + if (SeenMacro) + return; -void HeaderGuardCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { - Options.store(Opts, "HeaderFileExtensions", RawStringHeaderFileExtensions); -} - -void HeaderGuardCheck::registerPPCallbacks(const SourceManager &SM, - Preprocessor *PP, - Preprocessor *ModuleExpanderPP) { - PP->addPPCallbacks(std::make_unique<HeaderGuardPPCallbacks>(PP, this)); + diag(StartLoc, "header is missing header guard") + << FixItHint::CreateInsertion( + StartLoc, "#ifndef " + CPPVar + "\n#define " + CPPVar + "\n\n") + << FixItHint::CreateInsertion(SM.getLocForEndOfFile(SM.translateFile(FE)), + shouldSuggestEndifComment(FileName) + ? "\n#" + formatEndIf(CPPVar) + "\n" + : "\n#endif\n"); } std::string HeaderGuardCheck::sanitizeHeaderGuard(StringRef Guard) { @@ -288,12 +166,6 @@ return utils::isFileExtension(FileName, HeaderFileExtensions); } -bool HeaderGuardCheck::shouldFixHeaderGuard(StringRef FileName) { return true; } - -bool HeaderGuardCheck::shouldSuggestToAddHeaderGuard(StringRef FileName) { - return utils::isFileExtension(FileName, HeaderFileExtensions); -} - std::string HeaderGuardCheck::formatEndIf(StringRef HeaderGuard) { return "endif // " + HeaderGuard.str(); } Index: clang-tools-extra/clang-tidy/utils/CMakeLists.txt =================================================================== --- clang-tools-extra/clang-tidy/utils/CMakeLists.txt +++ clang-tools-extra/clang-tidy/utils/CMakeLists.txt @@ -12,6 +12,7 @@ FileExtensionsUtils.cpp FixItHintUtils.cpp HeaderGuard.cpp + HeaderGuardBase.cpp IncludeInserter.cpp IncludeSorter.cpp LexerUtils.cpp
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
