Index: clang-tidy/ClangTidy.h =================================================================== --- clang-tidy/ClangTidy.h +++ clang-tidy/ClangTidy.h @@ -47,7 +47,15 @@ /// Reads the option with the check-local name \p LocalName from the /// \c CheckOptions. If the corresponding key is not present, returns /// \p Default. - std::string get(StringRef LocalName, std::string Default) const; + std::string get(StringRef LocalName, StringRef Default) const; + + /// \brief Read a named option from the \c Context. + /// + /// Reads the option with the check-local name \p LocalName from local or + /// global \c CheckOptions. Gets local option first. If local is not + /// present, falls back to get global option. If global option is not present + /// either, returns Default. + std::string getLocalOrGlobal(StringRef LocalName, StringRef Default) const; /// \brief Read a named option from the \c Context and parse it as an integral /// type \c T. Index: clang-tidy/ClangTidy.cpp =================================================================== --- clang-tidy/ClangTidy.cpp +++ clang-tidy/ClangTidy.cpp @@ -344,13 +344,25 @@ const ClangTidyOptions::OptionMap &CheckOptions) : NamePrefix(CheckName.str() + "."), CheckOptions(CheckOptions) {} -std::string OptionsView::get(StringRef LocalName, std::string Default) const { +std::string OptionsView::get(StringRef LocalName, StringRef Default) const { const auto &Iter = CheckOptions.find(NamePrefix + LocalName.str()); if (Iter != CheckOptions.end()) return Iter->second; return Default; } +std::string OptionsView::getLocalOrGlobal(StringRef LocalName, + StringRef Default) const { + auto Iter = CheckOptions.find(NamePrefix + LocalName.str()); + if (Iter != CheckOptions.end()) + return Iter->second; + // Fallback to global setting, if present. + Iter = CheckOptions.find(LocalName.str()); + if (Iter != CheckOptions.end()) + return Iter->second; + return Default; +} + void OptionsView::store(ClangTidyOptions::OptionMap &Options, StringRef LocalName, StringRef Value) const { Options[NamePrefix + LocalName.str()] = Value; Index: clang-tidy/google/GlobalNamesInHeadersCheck.h =================================================================== --- clang-tidy/google/GlobalNamesInHeadersCheck.h +++ clang-tidy/google/GlobalNamesInHeadersCheck.h @@ -11,20 +11,32 @@ #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_GLOBALNAMESINHEADERSCHECK_H #include "../ClangTidy.h" +#include "../utils/HeaderFileExtensionsUtils.h" namespace clang { namespace tidy { namespace google { namespace readability { -// Flag global namespace pollution in header files. -// Right now it only triggers on using declarations and directives. +/// Flag global namespace pollution in header files. +/// Right now it only triggers on using declarations and directives. +/// +/// The check supports these options: +/// - `HeaderFileExtensions`: a comma-separated list of filename extensions +/// of header files (The filename extensions should not contain "." prefix). +/// "h" by default. +/// For extension-less header files, using an empty string or leaving an +/// empty string between "," if there are other filename extensions. class GlobalNamesInHeadersCheck : public ClangTidyCheck { public: - GlobalNamesInHeadersCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} + GlobalNamesInHeadersCheck(StringRef Name, ClangTidyContext *Context); + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + const std::string RawStringHeaderFileExtensions; + utils::HeaderFileExtensionsSet HeaderFileExtensions; }; } // namespace readability Index: clang-tidy/google/GlobalNamesInHeadersCheck.cpp =================================================================== --- clang-tidy/google/GlobalNamesInHeadersCheck.cpp +++ clang-tidy/google/GlobalNamesInHeadersCheck.cpp @@ -20,6 +20,24 @@ namespace google { namespace readability { +GlobalNamesInHeadersCheck::GlobalNamesInHeadersCheck(StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + RawStringHeaderFileExtensions( + Options.getLocalOrGlobal("HeaderFileExtensions", "h")) { + if (!utils::parseHeaderFileExtensions(RawStringHeaderFileExtensions, + HeaderFileExtensions, + ',')) { + llvm::errs() << "Invalid header file extension: " + << RawStringHeaderFileExtensions << "\n"; + } +} + +void GlobalNamesInHeadersCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "HeaderFileExtensions", RawStringHeaderFileExtensions); +} + void GlobalNamesInHeadersCheck::registerMatchers(ast_matchers::MatchFinder *Finder) { Finder->addMatcher( @@ -38,10 +56,8 @@ if (Result.SourceManager->isInMainFile( Result.SourceManager->getExpansionLoc(D->getLocStart()))) { // unless that file is a header. - StringRef Filename = Result.SourceManager->getFilename( - Result.SourceManager->getSpellingLoc(D->getLocStart())); - - if (!Filename.endswith(".h")) + if (!utils::isSpellingLocInHeaderFile( + D->getLocStart(), *Result.SourceManager, HeaderFileExtensions)) return; } Index: clang-tidy/google/UnnamedNamespaceInHeaderCheck.h =================================================================== --- clang-tidy/google/UnnamedNamespaceInHeaderCheck.h +++ clang-tidy/google/UnnamedNamespaceInHeaderCheck.h @@ -11,6 +11,7 @@ #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_UNNAMEDNAMESPACEINHEADERCHECK_H #include "../ClangTidy.h" +#include "../utils/HeaderFileExtensionsUtils.h" namespace clang { namespace tidy { @@ -19,15 +20,26 @@ /// Finds anonymous namespaces in headers. /// +/// The check supports these options: +/// - `HeaderFileExtensions`: a comma-separated list of filename extensions of +/// header files (The filename extensions 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. +/// /// http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Namespaces#Namespaces /// /// Corresponding cpplint.py check name: 'build/namespaces'. class UnnamedNamespaceInHeaderCheck : public ClangTidyCheck { public: - UnnamedNamespaceInHeaderCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} + UnnamedNamespaceInHeaderCheck(StringRef Name, ClangTidyContext *Context); + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + const std::string RawStringHeaderFileExtensions; + utils::HeaderFileExtensionsSet HeaderFileExtensions; }; } // namespace build Index: clang-tidy/google/UnnamedNamespaceInHeaderCheck.cpp =================================================================== --- clang-tidy/google/UnnamedNamespaceInHeaderCheck.cpp +++ clang-tidy/google/UnnamedNamespaceInHeaderCheck.cpp @@ -19,6 +19,24 @@ namespace google { namespace build { +UnnamedNamespaceInHeaderCheck::UnnamedNamespaceInHeaderCheck( + StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + RawStringHeaderFileExtensions( + Options.getLocalOrGlobal("HeaderFileExtensions", "h,hh,hpp,hxx")) { + if (!utils::parseHeaderFileExtensions(RawStringHeaderFileExtensions, + HeaderFileExtensions, + ',')) { + llvm::errs() << "Invalid header file extension: " + << RawStringHeaderFileExtensions << "\n"; + } +} + +void UnnamedNamespaceInHeaderCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "HeaderFileExtensions", RawStringHeaderFileExtensions); +} + void UnnamedNamespaceInHeaderCheck::registerMatchers( ast_matchers::MatchFinder *Finder) { // Only register the matchers for C++; the functionality currently does not @@ -30,17 +48,13 @@ void UnnamedNamespaceInHeaderCheck::check(const MatchFinder::MatchResult &Result) { - SourceManager *SM = Result.SourceManager; const auto *N = Result.Nodes.getNodeAs("anonymousNamespace"); SourceLocation Loc = N->getLocStart(); if (!Loc.isValid()) return; - // Look if we're inside a header, check for common suffixes only. - // TODO: Allow configuring the set of file extensions. - StringRef FileName = SM->getPresumedLoc(Loc).getFilename(); - if (FileName.endswith(".h") || FileName.endswith(".hh") || - FileName.endswith(".hpp") || FileName.endswith(".hxx")) + if (utils::isPresumedLocInHeaderFile(Loc, *Result.SourceManager, + HeaderFileExtensions)) diag(Loc, "do not use unnamed namespaces in header files"); } Index: clang-tidy/misc/DefinitionsInHeadersCheck.h =================================================================== --- clang-tidy/misc/DefinitionsInHeadersCheck.h +++ clang-tidy/misc/DefinitionsInHeadersCheck.h @@ -11,20 +11,26 @@ #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_DEFINITIONS_IN_HEADERS_H #include "../ClangTidy.h" +#include "../utils/HeaderFileExtensionsUtils.h" namespace clang { namespace tidy { namespace misc { -// Finds non-extern non-inline function and variable definitions in header -// files, which can lead to potential ODR violations. -// -// There is one option: -// - `UseHeaderFileExtension`: Whether to use file extension (h, hh, hpp, hxx) -// to distinguish header files. True by default. -// -// For the user-facing documentation see: -// http://clang.llvm.org/extra/clang-tidy/checks/misc-definitions-in-headers.html +/// Finds non-extern non-inline function and variable definitions in header +/// files, which can lead to potential ODR violations. +/// +/// The check supports these options: +/// - `UseHeaderFileExtension`: Whether to use file extension to distinguish +/// header files. True by default. +/// - `HeaderFileExtensions`: a comma-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. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/misc-definitions-in-headers.html class DefinitionsInHeadersCheck : public ClangTidyCheck { public: DefinitionsInHeadersCheck(StringRef Name, ClangTidyContext *Context); @@ -34,6 +40,8 @@ private: const bool UseHeaderFileExtension; + const std::string RawStringHeaderFileExtensions; + utils::HeaderFileExtensionsSet HeaderFileExtensions; }; } // namespace misc Index: clang-tidy/misc/DefinitionsInHeadersCheck.cpp =================================================================== --- clang-tidy/misc/DefinitionsInHeadersCheck.cpp +++ clang-tidy/misc/DefinitionsInHeadersCheck.cpp @@ -19,38 +19,50 @@ namespace { -AST_MATCHER(NamedDecl, isHeaderFileExtension) { - SourceManager& SM = Finder->getASTContext().getSourceManager(); - SourceLocation ExpansionLoc = SM.getExpansionLoc(Node.getLocStart()); - StringRef Filename = SM.getFilename(ExpansionLoc); - return Filename.endswith(".h") || Filename.endswith(".hh") || - Filename.endswith(".hpp") || Filename.endswith(".hxx") || - llvm::sys::path::extension(Filename).empty(); +AST_MATCHER_P(NamedDecl, usesHeaderFileExtension, + utils::HeaderFileExtensionsSet, HeaderFileExtensions) { + return utils::isExpansionLocInHeaderFile( + Node.getLocStart(), Finder->getASTContext().getSourceManager(), + HeaderFileExtensions); } } // namespace -DefinitionsInHeadersCheck::DefinitionsInHeadersCheck( - StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context), - UseHeaderFileExtension(Options.get("UseHeaderFileExtension", true)) {} +DefinitionsInHeadersCheck::DefinitionsInHeadersCheck(StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + UseHeaderFileExtension(Options.get("UseHeaderFileExtension", true)), + RawStringHeaderFileExtensions( + Options.getLocalOrGlobal("HeaderFileExtensions", ",h,hh,hpp,hxx")) { + if (!utils::parseHeaderFileExtensions(RawStringHeaderFileExtensions, + HeaderFileExtensions, + ',')) { + // FIXME: Find a more suitable way to handle invalid configuration + // options. + llvm::errs() << "Invalid header file extension: " + << RawStringHeaderFileExtensions << "\n"; + } +} void DefinitionsInHeadersCheck::storeOptions( ClangTidyOptions::OptionMap &Opts) { Options.store(Opts, "UseHeaderFileExtension", UseHeaderFileExtension); + Options.store(Opts, "HeaderFileExtensions", RawStringHeaderFileExtensions); } void DefinitionsInHeadersCheck::registerMatchers(MatchFinder *Finder) { if (UseHeaderFileExtension) { Finder->addMatcher( namedDecl(anyOf(functionDecl(isDefinition()), varDecl(isDefinition())), - isHeaderFileExtension()).bind("name-decl"), + usesHeaderFileExtension(HeaderFileExtensions)) + .bind("name-decl"), this); } else { Finder->addMatcher( namedDecl(anyOf(functionDecl(isDefinition()), varDecl(isDefinition())), - anyOf(isHeaderFileExtension(), - unless(isExpansionInMainFile()))).bind("name-decl"), + anyOf(usesHeaderFileExtension(HeaderFileExtensions), + unless(isExpansionInMainFile()))) + .bind("name-decl"), this); } } Index: clang-tidy/utils/CMakeLists.txt =================================================================== --- clang-tidy/utils/CMakeLists.txt +++ clang-tidy/utils/CMakeLists.txt @@ -2,6 +2,7 @@ add_clang_library(clangTidyUtils HeaderGuard.cpp + HeaderFileExtensionsUtils.cpp IncludeInserter.cpp IncludeSorter.cpp LexerUtils.cpp Index: clang-tidy/utils/HeaderFileExtensionsUtils.h =================================================================== --- /dev/null +++ clang-tidy/utils/HeaderFileExtensionsUtils.h @@ -0,0 +1,51 @@ +//===--- HeaderFileExtensionsUtils.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_FILE_EXTENSIONS_UTILS_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_HEADER_FILE_EXTENSIONS_UTILS_H + +#include + +#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/SourceManager.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/ADT/SmallSet.h" +#include "llvm/Support/Path.h" + +namespace clang { +namespace tidy { +namespace utils { + +typedef llvm::SmallSet HeaderFileExtensionsSet; + +/// \brief Checks whether expansion location of Loc is in header file. +bool isExpansionLocInHeaderFile( + SourceLocation Loc, const SourceManager &SM, + const HeaderFileExtensionsSet &HeaderFileExtensions); + +/// \brief Checks whether presumed location of Loc is in header file. +bool isPresumedLocInHeaderFile( + SourceLocation Loc, SourceManager &SM, + const HeaderFileExtensionsSet &HeaderFileExtensions); + +/// \brief Checks whether spelling location of Loc is in header file. +bool isSpellingLocInHeaderFile( + SourceLocation Loc, SourceManager &SM, + const HeaderFileExtensionsSet &HeaderFileExtensions); + +/// \brief Parses header file extensions from a semicolon-separated list. +bool parseHeaderFileExtensions(StringRef AllHeaderFileExtensions, + HeaderFileExtensionsSet &HeaderFileExtensions, + char delimiter); + +} // namespace utils +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_HEADER_FILE_EXTENSIONS_UTILS_H Index: clang-tidy/utils/HeaderFileExtensionsUtils.cpp =================================================================== --- /dev/null +++ clang-tidy/utils/HeaderFileExtensionsUtils.cpp @@ -0,0 +1,65 @@ +//===--- HeaderFileExtensionsUtils.cpp - clang-tidy--------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "HeaderFileExtensionsUtils.h" +#include "clang/Basic/CharInfo.h" + +namespace clang { +namespace tidy { +namespace utils { + +bool isExpansionLocInHeaderFile( + SourceLocation Loc, const SourceManager &SM, + const HeaderFileExtensionsSet &HeaderFileExtensions) { + SourceLocation ExpansionLoc = SM.getExpansionLoc(Loc); + StringRef FileExtension = + llvm::sys::path::extension(SM.getFilename(ExpansionLoc)); + return HeaderFileExtensions.count(FileExtension.substr(1)) > 0; +} + +bool isPresumedLocInHeaderFile( + SourceLocation Loc, SourceManager &SM, + const HeaderFileExtensionsSet &HeaderFileExtensions) { + PresumedLoc PresumedLocation = SM.getPresumedLoc(Loc); + StringRef FileExtension = + llvm::sys::path::extension(PresumedLocation.getFilename()); + return HeaderFileExtensions.count(FileExtension.substr(1)) > 0; +} + +bool isSpellingLocInHeaderFile( + SourceLocation Loc, SourceManager &SM, + const HeaderFileExtensionsSet &HeaderFileExtensions) { + SourceLocation SpellingLoc = SM.getSpellingLoc(Loc); + StringRef FileExtension = + llvm::sys::path::extension(SM.getFilename(SpellingLoc)); + + return HeaderFileExtensions.count(FileExtension.substr(1)) > 0; +} + +bool parseHeaderFileExtensions(StringRef AllHeaderFileExtensions, + HeaderFileExtensionsSet &HeaderFileExtensions, + char delimiter) { + SmallVector Suffixes; + AllHeaderFileExtensions.split(Suffixes, delimiter); + HeaderFileExtensions.clear(); + for (StringRef Suffix : Suffixes) { + StringRef Extension = Suffix.trim(); + for (StringRef::const_iterator it = Extension.begin(); + it != Extension.end(); ++it) { + if (!isAlphanumeric(*it)) + return false; + } + HeaderFileExtensions.insert(Extension); + } + return true; +} + +} // namespace utils +} // namespace tidy +} // namespace clang