Index: clang-tidy/ClangTidy.h =================================================================== --- clang-tidy/ClangTidy.h +++ clang-tidy/ClangTidy.h @@ -16,6 +16,8 @@ #include "clang/Basic/Diagnostic.h" #include "clang/Basic/SourceManager.h" #include "clang/Tooling/Refactoring.h" +#include "llvm/ADT/StringExtras.h" +#include "llvm/Support/raw_ostream.h" #include #include @@ -49,6 +51,7 @@ /// useful/necessary. class ClangTidyCheck : public ast_matchers::MatchFinder::MatchCallback { public: + ClangTidyCheck() : Context(nullptr) {} virtual ~ClangTidyCheck() {} /// \brief Overwrite this to register \c PPCallbacks with \c Compiler. @@ -76,17 +79,91 @@ virtual void check(const ast_matchers::MatchFinder::MatchResult &Result) {} /// \brief The infrastructure sets the context to \p Ctx with this function. - void setContext(ClangTidyContext *Ctx) { Context = Ctx; } + /// + /// This function can only be called once. + void setContext(ClangTidyContext *Ctx); /// \brief Add a diagnostic with the check's name. DiagnosticBuilder diag(SourceLocation Loc, StringRef Description, DiagnosticIDs::Level Level = DiagnosticIDs::Warning); + /// \brief Should store all options supported by this check with their + /// current values or default values for options that haven't been overridden. + /// + /// The check should use \c storeOption() to store each option it supports + /// whether it has the default value or has been overridden. + virtual void storeOptions(ClangTidyOptions::OptionMap &Options) {} + /// \brief Sets the check name. Intended to be used by the clang-tidy /// framework. Can be called only once. void setName(StringRef Name); +protected: + /// \brief Implement this method to read check options. + /// + /// Options can be accessed via getOption() method below or directly using + /// Context->getOptions().CheckOptions. + virtual void readOptions() {} + + /// \brief Read a named option from the \c Context. + /// + /// Reads the option named CheckName + "." + \p LocalName + /// from the current options in the \c Context to \p Result if the + /// corresponding key is present. Otherwise leaves the \p Result unchanged. + /// + /// \returns \c true, if the value has been found and read to \p Result. + bool getOption(StringRef LocalName, std::string &Result) const { + assert(Context != nullptr); + const auto &Options = Context->getOptions().CheckOptions; + const auto &Iter = Options.find(getQualifiedOptionName(LocalName)); + if (Iter != Options.end()) { + Result = Iter->second; + return true; + } + return false; + } + + /// \brief Read a named option from the \c Context. + /// + /// Reads the option named CheckName + "." + \p LocalName + /// from the current options in the \c Context to \p Result if the + /// corresponding key is present. Otherwise leaves the \p Result unchanged. + /// + /// \returns \c true, if the value has been found, successfully converted + /// to the target type \c T and stored to \p Result. + template bool getOption(StringRef LocalName, T &Result) const { + std::string Value; + if (!getOption(LocalName, Value)) + return false; + return StringRef(Value).getAsInteger(10, Result); + } + + /// \brief Stores an option named \p LocalName with string value \p Value to + /// \p Options. + /// + /// The function calls \c getQualifiedOptionName(LocalName) to get the global + /// option name by prepending the check name. + void storeOption(ClangTidyOptions::OptionMap &Options, StringRef LocalName, + StringRef Value) const { + Options[getQualifiedOptionName(LocalName)] = Value; + } + + /// \brief Stores an option named \p LocalName with \c int64_t value \p Value + /// to \p Options. + /// + /// The function calls \c getQualifiedOptionName(LocalName) to get the global + /// option name by prepending the check name. + void storeOption(ClangTidyOptions::OptionMap &Options, StringRef LocalName, + int64_t Value) const { + storeOption(Options, LocalName, llvm::itostr(Value)); + } + private: + std::string getQualifiedOptionName(StringRef OptionName) const { + assert(!CheckName.empty()); + return CheckName + "." + OptionName.str(); + } + void run(const ast_matchers::MatchFinder::MatchResult &Result) override; ClangTidyContext *Context; std::string CheckName; @@ -103,10 +180,13 @@ CreateASTConsumer(clang::CompilerInstance &Compiler, StringRef File); /// \brief Get the list of enabled checks. - std::vector getCheckNames(GlobList &Filter); + std::vector getCheckNames(); + + /// \brief Get the union of options from all checks. + ClangTidyOptions::OptionMap getCheckOptions(); private: - typedef std::vector > CheckersList; + typedef std::vector> CheckersList; CheckersList getCheckersControlList(GlobList &Filter); ClangTidyContext &Context; @@ -117,6 +197,14 @@ /// filters are applied. std::vector getCheckNames(const ClangTidyOptions &Options); +/// \brief Returns the effective check-specific options. +/// +/// The method configures ClangTidy with the specified \p Options and collects +/// effective options from all created checks. The returned set of options +/// includes default check-specific options for all keys not overridden by \p +/// Options. +ClangTidyOptions::OptionMap getCheckOptions(const ClangTidyOptions &Options); + /// \brief Run a set of clang-tidy checks on a set of files. ClangTidyStats runClangTidy(std::unique_ptr OptionsProvider, Index: clang-tidy/ClangTidy.cpp =================================================================== --- clang-tidy/ClangTidy.cpp +++ clang-tidy/ClangTidy.cpp @@ -251,9 +251,9 @@ std::move(Consumers), std::move(Finder), std::move(Checks)); } -std::vector -ClangTidyASTConsumerFactory::getCheckNames(GlobList &Filter) { +std::vector ClangTidyASTConsumerFactory::getCheckNames() { std::vector CheckNames; + GlobList &Filter = Context.getChecksFilter(); for (const auto &CheckFactory : *CheckFactories) { if (Filter.contains(CheckFactory.first)) CheckNames.push_back(CheckFactory.first); @@ -266,6 +266,18 @@ return CheckNames; } +ClangTidyOptions::OptionMap ClangTidyASTConsumerFactory::getCheckOptions() { + ClangTidyOptions::OptionMap Options; + std::vector> Checks; + GlobList &Filter = Context.getChecksFilter(); + CheckFactories->createChecks(Filter, Checks); + for (const auto &Check : Checks) { + Check->setContext(&Context); + Check->storeOptions(Options); + } + return Options; +} + ClangTidyASTConsumerFactory::CheckersList ClangTidyASTConsumerFactory::getCheckersControlList(GlobList &Filter) { CheckersList List; @@ -312,12 +324,25 @@ CheckName = Name.str(); } +void ClangTidyCheck::setContext(ClangTidyContext *Ctx) { + assert(Context == nullptr); + Context = Ctx; + readOptions(); +} + std::vector getCheckNames(const ClangTidyOptions &Options) { clang::tidy::ClangTidyContext Context( llvm::make_unique(ClangTidyGlobalOptions(), Options)); ClangTidyASTConsumerFactory Factory(Context); - return Factory.getCheckNames(Context.getChecksFilter()); + return Factory.getCheckNames(); +} +ClangTidyOptions::OptionMap getCheckOptions(const ClangTidyOptions &Options) { + clang::tidy::ClangTidyContext Context( + llvm::make_unique(ClangTidyGlobalOptions(), + Options)); + ClangTidyASTConsumerFactory Factory(Context); + return Factory.getCheckOptions(); } ClangTidyStats Index: clang-tidy/ClangTidyOptions.h =================================================================== --- clang-tidy/ClangTidyOptions.h +++ clang-tidy/ClangTidyOptions.h @@ -14,6 +14,7 @@ #include "llvm/ADT/StringMap.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/ErrorOr.h" +#include #include #include #include @@ -70,6 +71,12 @@ /// \brief Turns on temporary destructor-based analysis. llvm::Optional AnalyzeTemporaryDtors; + + typedef std::pair StringPair; + typedef std::map OptionMap; + + /// \brief Key-value mapping used to store check-specific options. + OptionMap CheckOptions; }; /// \brief Abstract interface for retrieving various ClangTidy options. Index: clang-tidy/ClangTidyOptions.cpp =================================================================== --- clang-tidy/ClangTidyOptions.cpp +++ clang-tidy/ClangTidyOptions.cpp @@ -25,6 +25,7 @@ LLVM_YAML_IS_FLOW_SEQUENCE_VECTOR(FileFilter) LLVM_YAML_IS_FLOW_SEQUENCE_VECTOR(FileFilter::LineRange) +LLVM_YAML_IS_SEQUENCE_VECTOR(ClangTidyOptions::StringPair); namespace llvm { namespace yaml { @@ -57,11 +58,34 @@ } }; +template <> struct MappingTraits { + static void mapping(IO &IO, ClangTidyOptions::StringPair &KeyValue) { + IO.mapRequired("key", KeyValue.first); + IO.mapRequired("value", KeyValue.second); + } +}; + +struct NOptionMap { + NOptionMap(IO &) {} + NOptionMap(IO &, const ClangTidyOptions::OptionMap &OptionMap) + : Options(OptionMap.begin(), OptionMap.end()) {} + ClangTidyOptions::OptionMap denormalize(IO &) { + ClangTidyOptions::OptionMap Map; + for (const auto &KeyValue : Options) + Map[KeyValue.first] = KeyValue.second; + return Map; + } + std::vector Options; +}; + template <> struct MappingTraits { static void mapping(IO &IO, ClangTidyOptions &Options) { + MappingNormalization NOpts( + IO, Options.CheckOptions); IO.mapOptional("Checks", Options.Checks); IO.mapOptional("HeaderFilterRegex", Options.HeaderFilterRegex); IO.mapOptional("AnalyzeTemporaryDtors", Options.AnalyzeTemporaryDtors); + IO.mapOptional("CheckOptions", NOpts->Options); } }; @@ -85,6 +109,10 @@ Result.HeaderFilterRegex = Other.HeaderFilterRegex; if (Other.AnalyzeTemporaryDtors) Result.AnalyzeTemporaryDtors = Other.AnalyzeTemporaryDtors; + + for (const auto &KeyValue : Other.CheckOptions) + Result.CheckOptions[KeyValue.first] = KeyValue.second; + return Result; } @@ -169,6 +197,10 @@ llvm::MemoryBuffer::getFile(ConfigFile.c_str()); if (std::error_code EC = Text.getError()) return EC; + // Skip empty files, e.g. files opened for writing via shell output + // redirection. + if ((*Text)->getBuffer().empty()) + return make_error_code(llvm::errc::no_such_file_or_directory); if (std::error_code EC = parseConfiguration((*Text)->getBuffer(), Options)) return EC; return Options.mergeWith(OverrideOptions); Index: clang-tidy/llvm/NamespaceCommentCheck.h =================================================================== --- clang-tidy/llvm/NamespaceCommentCheck.h +++ clang-tidy/llvm/NamespaceCommentCheck.h @@ -26,8 +26,12 @@ void check(const ast_matchers::MatchFinder::MatchResult &Result) override; private: + void storeOptions(ClangTidyOptions::OptionMap &Options) override; + void readOptions() override; + llvm::Regex NamespaceCommentPattern; - const unsigned ShortNamespaceLines; + unsigned ShortNamespaceLines; + unsigned SpacesBeforeComments; }; } // namespace tidy Index: clang-tidy/llvm/NamespaceCommentCheck.cpp =================================================================== --- clang-tidy/llvm/NamespaceCommentCheck.cpp +++ clang-tidy/llvm/NamespaceCommentCheck.cpp @@ -11,9 +11,7 @@ #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchers.h" #include "clang/Lex/Lexer.h" - - -#include "llvm/Support/raw_ostream.h" +#include "llvm/ADT/StringExtras.h" using namespace clang::ast_matchers; @@ -24,7 +22,17 @@ : NamespaceCommentPattern("^/[/*] *(end (of )?)? *(anonymous|unnamed)? *" "namespace( +([a-zA-Z0-9_]+))? *(\\*/)?$", llvm::Regex::IgnoreCase), - ShortNamespaceLines(1) {} + ShortNamespaceLines(1u), SpacesBeforeComments(1u) {} + +void NamespaceCommentCheck::storeOptions(ClangTidyOptions::OptionMap &Options) { + storeOption(Options, "ShortNamespaceLines", ShortNamespaceLines); + storeOption(Options, "SpacesBeforeComments", SpacesBeforeComments); +} + +void NamespaceCommentCheck::readOptions() { + getOption("ShortNamespaceLines", ShortNamespaceLines); + getOption("SpacesBeforeComments", SpacesBeforeComments); +} void NamespaceCommentCheck::registerMatchers(MatchFinder *Finder) { Finder->addMatcher(namespaceDecl().bind("namespace"), this); @@ -36,10 +44,12 @@ Sources.getFileID(Loc1) == Sources.getFileID(Loc2); } -std::string getNamespaceComment(const NamespaceDecl *ND, bool InsertLineBreak) { +std::string getNamespaceComment(const NamespaceDecl *ND, bool InsertLineBreak, + unsigned SpacesBeforeComments) { std::string Fix = "// namespace"; if (!ND->isAnonymousNamespace()) - Fix.append(" ").append(ND->getNameAsString()); + Fix.append(std::string(SpacesBeforeComments, ' ')) + .append(ND->getNameAsString()); if (InsertLineBreak) Fix.append("\n"); return Fix; @@ -97,7 +107,8 @@ diag(Loc, "namespace closing comment refers to a wrong namespace '%0'") << NamespaceNameInComment << FixItHint::CreateReplacement( - OldCommentRange, getNamespaceComment(ND, NeedLineBreak)); + OldCommentRange, + getNamespaceComment(ND, NeedLineBreak, SpacesBeforeComments)); return; } @@ -110,7 +121,8 @@ diag(ND->getLocation(), "namespace not terminated with a closing comment") << FixItHint::CreateInsertion( - AfterRBrace, " " + getNamespaceComment(ND, NeedLineBreak)); + AfterRBrace, + " " + getNamespaceComment(ND, NeedLineBreak, SpacesBeforeComments)); } } // namespace tidy Index: clang-tidy/tool/ClangTidyMain.cpp =================================================================== --- clang-tidy/tool/ClangTidyMain.cpp +++ clang-tidy/tool/ClangTidyMain.cpp @@ -173,6 +173,7 @@ } if (DumpConfig) { + EffectiveOptions.CheckOptions = getCheckOptions(EffectiveOptions); llvm::outs() << configurationAsText(ClangTidyOptions::getDefaults() .mergeWith(EffectiveOptions)) << "\n"; Index: unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp =================================================================== --- unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp +++ unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp @@ -23,9 +23,8 @@ std::vector Errors; runCheckOnCode("int a;", &Errors); EXPECT_EQ(2ul, Errors.size()); - // FIXME: Remove " []" once the check name is removed from the message text. - EXPECT_EQ("type specifier []", Errors[0].Message.Message); - EXPECT_EQ("variable []", Errors[1].Message.Message); + EXPECT_EQ("type specifier", Errors[0].Message.Message); + EXPECT_EQ("variable", Errors[1].Message.Message); } TEST(GlobList, Empty) { Index: unittests/clang-tidy/ClangTidyTest.h =================================================================== --- unittests/clang-tidy/ClangTidyTest.h +++ unittests/clang-tidy/ClangTidyTest.h @@ -50,6 +50,7 @@ ClangTidyContext Context(llvm::make_unique( ClangTidyGlobalOptions(), Options)); ClangTidyDiagnosticConsumer DiagConsumer(Context); + Check.setName("test-check"); Check.setContext(&Context); std::vector ArgCXX11(1, "-std=c++11"); ArgCXX11.insert(ArgCXX11.end(), ExtraArgs.begin(), ExtraArgs.end());