Index: clang-tidy/ClangTidy.h =================================================================== --- clang-tidy/ClangTidy.h +++ clang-tidy/ClangTidy.h @@ -18,6 +18,7 @@ #include "clang/Tooling/Refactoring.h" #include "llvm/ADT/StringExtras.h" #include "llvm/Support/raw_ostream.h" +#include "llvm/Support/YAMLTraits.h" #include #include #include @@ -42,38 +43,26 @@ OptionsView(StringRef CheckName, const ClangTidyOptions::OptionMap &CheckOptions); - /// \brief Read a named option from the \c Context. - /// - /// 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; - - /// \brief Read a named option from the \c Context and parse it as an integral - /// type \c T. - /// - /// 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, StringRef Default) const { + const auto &Iter = CheckOptions.find(NamePrefix + LocalName.str()); + if (Iter != CheckOptions.end()) + return Iter->second.Value; + return Default; + } + template - typename std::enable_if::value, T>::type - get(StringRef LocalName, T Default) const { - std::string Value = get(LocalName, ""); - T Result = Default; - if (!Value.empty()) - StringRef(Value).getAsInteger(10, Result); - return Result; + typename std::enable_if::value, T>::type + get(StringRef LocalName, const T &Default) const { + const auto &Iter = CheckOptions.find(NamePrefix + LocalName.str()); + if (Iter != CheckOptions.end()) + return Iter->second.get(Default); + return Default; } - /// \brief Stores an option with the check-local name \p LocalName with string - /// value \p Value to \p Options. - void store(ClangTidyOptions::OptionMap &Options, StringRef LocalName, - StringRef Value) const; - - /// \brief Stores an option with the check-local name \p LocalName with - /// \c int64_t value \p Value to \p Options. - void store(ClangTidyOptions::OptionMap &Options, StringRef LocalName, - int64_t Value) const; + ClangTidyOptions::Option &get(ClangTidyOptions::OptionMap &Options, + StringRef LocalName) const { + return Options[NamePrefix + LocalName.str()]; + } private: std::string NamePrefix; Index: clang-tidy/ClangTidy.cpp =================================================================== --- clang-tidy/ClangTidy.cpp +++ clang-tidy/ClangTidy.cpp @@ -319,23 +319,6 @@ const ClangTidyOptions::OptionMap &CheckOptions) : NamePrefix(CheckName.str() + "."), CheckOptions(CheckOptions) {} -std::string OptionsView::get(StringRef LocalName, std::string Default) const { - const auto &Iter = CheckOptions.find(NamePrefix + 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; -} - -void OptionsView::store(ClangTidyOptions::OptionMap &Options, - StringRef LocalName, int64_t Value) const { - store(Options, LocalName, llvm::itostr(Value)); -} - std::vector getCheckNames(const ClangTidyOptions &Options) { clang::tidy::ClangTidyContext Context( llvm::make_unique(ClangTidyGlobalOptions(), Index: clang-tidy/ClangTidyOptions.h =================================================================== --- clang-tidy/ClangTidyOptions.h +++ clang-tidy/ClangTidyOptions.h @@ -14,6 +14,8 @@ #include "llvm/ADT/StringMap.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/ErrorOr.h" +#include "llvm/Support/YAMLTraits.h" +#include #include #include #include @@ -74,8 +76,41 @@ /// comments in the relevant check. llvm::Optional User; - typedef std::pair StringPair; - typedef std::map OptionMap; + struct Option { + template + typename std::enable_if::value, T>::type + get(const T &Default) const { + T Result = Default; + llvm::yaml::ScalarTraits::input(Value, nullptr, Result); + return Result; + } + + template + typename std::enable_if::value, void>::type + store(const T &TypedValue) { + llvm::raw_string_ostream OS(Value); + llvm::yaml::ScalarTraits::output(TypedValue, nullptr, OS); + } + + std::string Value; + std::function Validate; + }; + + template + static typename std::enable_if::value, + Option>::type + TypedOption(const T &Default) { + Option Result; + Result.store(Default); + Result.Validate = [](llvm::StringRef Input) { + T Dummy; + return llvm::yaml::ScalarTraits::input(Input, nullptr, Dummy); + }; + return Result; + } + + typedef std::map OptionMap; + typedef std::pair OptionItem; /// \brief Key-value mapping used to store check-specific options. OptionMap CheckOptions; @@ -148,7 +183,8 @@ /// \brief Parses configuration from JSON and returns \c ClangTidyOptions or an /// error. -llvm::ErrorOr parseConfiguration(llvm::StringRef Config); +llvm::ErrorOr +parseConfiguration(llvm::StringRef Config, const ClangTidyOptions &Defaults); /// \brief Serializes configuration to a YAML-encoded string. std::string configurationAsText(const ClangTidyOptions &Options); Index: clang-tidy/ClangTidyOptions.cpp =================================================================== --- clang-tidy/ClangTidyOptions.cpp +++ clang-tidy/ClangTidyOptions.cpp @@ -26,7 +26,7 @@ LLVM_YAML_IS_FLOW_SEQUENCE_VECTOR(FileFilter) LLVM_YAML_IS_FLOW_SEQUENCE_VECTOR(FileFilter::LineRange) -LLVM_YAML_IS_SEQUENCE_VECTOR(ClangTidyOptions::StringPair) +LLVM_YAML_IS_SEQUENCE_VECTOR(ClangTidyOptions::OptionItem) namespace llvm { namespace yaml { @@ -59,10 +59,19 @@ } }; -template <> struct MappingTraits { - static void mapping(IO &IO, ClangTidyOptions::StringPair &KeyValue) { +template <> struct MappingTraits { + static void mapping(IO &IO, ClangTidyOptions::OptionItem &KeyValue) { IO.mapRequired("key", KeyValue.first); - IO.mapRequired("value", KeyValue.second); + IO.mapRequired("value", KeyValue.second.Value); + if (auto Defaults = + reinterpret_cast(IO.getContext())) { + auto Iter = Defaults->CheckOptions.find(KeyValue.first); + if (Iter != Defaults->CheckOptions.end() && Iter->second.Validate) { + StringRef Error = Iter->second.Validate(KeyValue.second.Value); + if (!Error.empty()) + IO.setError(Error); + } + } } }; @@ -76,7 +85,7 @@ Map[KeyValue.first] = KeyValue.second; return Map; } - std::vector Options; + std::vector Options; }; template <> struct MappingTraits { @@ -217,14 +226,13 @@ // redirection. if ((*Text)->getBuffer().empty()) return make_error_code(llvm::errc::no_such_file_or_directory); + ClangTidyOptions Defaults = DefaultOptionsProvider::getOptions(Directory); + // Only use checks from the config file. + Defaults.Checks = None; llvm::ErrorOr ParsedOptions = - parseConfiguration((*Text)->getBuffer()); - if (ParsedOptions) { - ClangTidyOptions Defaults = DefaultOptionsProvider::getOptions(Directory); - // Only use checks from the config file. - Defaults.Checks = None; - return Defaults.mergeWith(*ParsedOptions).mergeWith(OverrideOptions); - } + parseConfiguration((*Text)->getBuffer(), Defaults); + if (ParsedOptions) + return ParsedOptions->mergeWith(OverrideOptions); return ParsedOptions.getError(); } @@ -236,9 +244,11 @@ return Input.error(); } -llvm::ErrorOr parseConfiguration(StringRef Config) { +llvm::ErrorOr +parseConfiguration(StringRef Config, const ClangTidyOptions &Defaults) { llvm::yaml::Input Input(Config); - ClangTidyOptions Options; + Input.setContext(const_cast(&Defaults)); + ClangTidyOptions Options = Defaults; Input >> Options; if (Input.error()) return Input.error(); Index: clang-tidy/google/GoogleTidyModule.cpp =================================================================== --- clang-tidy/google/GoogleTidyModule.cpp +++ clang-tidy/google/GoogleTidyModule.cpp @@ -64,9 +64,11 @@ ClangTidyOptions Options; auto &Opts = Options.CheckOptions; Opts["google-readability-braces-around-statements.ShortStatementLines"] = - "1"; - Opts["google-readability-namespace-comments.ShortNamespaceLines"] = "1"; - Opts["google-readability-namespace-comments.SpacesBeforeComments"] = "2"; + ClangTidyOptions::TypedOption(1U); + Opts["google-readability-namespace-comments.ShortNamespaceLines"] = + ClangTidyOptions::TypedOption(1U); + Opts["google-readability-namespace-comments.SpacesBeforeComments"] = + ClangTidyOptions::TypedOption(2U); return Options; } }; Index: clang-tidy/readability/BracesAroundStatementsCheck.cpp =================================================================== --- clang-tidy/readability/BracesAroundStatementsCheck.cpp +++ clang-tidy/readability/BracesAroundStatementsCheck.cpp @@ -119,7 +119,7 @@ void BracesAroundStatementsCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { - Options.store(Opts, "ShortStatementLines", ShortStatementLines); + Options.get(Opts, "ShortStatementLines").store(ShortStatementLines); } void BracesAroundStatementsCheck::registerMatchers(MatchFinder *Finder) { Index: clang-tidy/readability/FunctionSize.cpp =================================================================== --- clang-tidy/readability/FunctionSize.cpp +++ clang-tidy/readability/FunctionSize.cpp @@ -23,9 +23,9 @@ BranchThreshold(Options.get("BranchThreshold", -1U)) {} void FunctionSizeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { - Options.store(Opts, "LineThreshold", LineThreshold); - Options.store(Opts, "StatementThreshold", StatementThreshold); - Options.store(Opts, "BranchThreshold", BranchThreshold); + Options.get(Opts, "LineThreshold").store(LineThreshold); + Options.get(Opts, "StatementThreshold").store(StatementThreshold); + Options.get(Opts, "BranchThreshold").store(BranchThreshold); } void FunctionSizeCheck::registerMatchers(MatchFinder *Finder) { Index: clang-tidy/readability/NamespaceCommentCheck.cpp =================================================================== --- clang-tidy/readability/NamespaceCommentCheck.cpp +++ clang-tidy/readability/NamespaceCommentCheck.cpp @@ -29,8 +29,8 @@ SpacesBeforeComments(Options.get("SpacesBeforeComments", 1u)) {} void NamespaceCommentCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { - Options.store(Opts, "ShortNamespaceLines", ShortNamespaceLines); - Options.store(Opts, "SpacesBeforeComments", SpacesBeforeComments); + Options.get(Opts, "ShortNamespaceLines").store(ShortNamespaceLines); + Options.get(Opts, "SpacesBeforeComments").store(SpacesBeforeComments); } void NamespaceCommentCheck::registerMatchers(MatchFinder *Finder) { Index: clang-tidy/tool/ClangTidyMain.cpp =================================================================== --- clang-tidy/tool/ClangTidyMain.cpp +++ clang-tidy/tool/ClangTidyMain.cpp @@ -169,13 +169,11 @@ OverrideOptions.AnalyzeTemporaryDtors = AnalyzeTemporaryDtors; if (!Config.empty()) { - if (llvm::ErrorOr ParsedConfig = - parseConfiguration(Config)) { + if (llvm::ErrorOr ParsedConfig = parseConfiguration( + Config, + ClangTidyOptions::getDefaults().mergeWith(DefaultOptions))) { return llvm::make_unique( - GlobalOptions, ClangTidyOptions::getDefaults() - .mergeWith(DefaultOptions) - .mergeWith(*ParsedConfig) - .mergeWith(OverrideOptions)); + GlobalOptions, ParsedConfig->mergeWith(OverrideOptions)); } else { llvm::errs() << "Error: invalid configuration specified.\n" << ParsedConfig.getError().message() << "\n"; Index: unittests/clang-tidy/ClangTidyOptionsTest.cpp =================================================================== --- unittests/clang-tidy/ClangTidyOptionsTest.cpp +++ unittests/clang-tidy/ClangTidyOptionsTest.cpp @@ -1,3 +1,4 @@ +#include "ClangTidy.h" #include "ClangTidyOptions.h" #include "gtest/gtest.h" @@ -59,7 +60,8 @@ parseConfiguration("Checks: \"-*,misc-*\"\n" "HeaderFilterRegex: \".*\"\n" "AnalyzeTemporaryDtors: true\n" - "User: some.user"); + "User: some.user", + ClangTidyOptions()); EXPECT_TRUE(!!Options); EXPECT_EQ("-*,misc-*", *Options->Checks); EXPECT_EQ(".*", *Options->HeaderFilterRegex); @@ -67,6 +69,57 @@ EXPECT_EQ("some.user", *Options->User); } +TEST(ParseConfiguration, CheckOptions) { + llvm::ErrorOr Options = + parseConfiguration("CheckOptions:\n" + " - key: test-check.String\n" + " value: ' qwe !@#* '\n" + " - key: test-check.IntegerPositive\n" + " value: 12345\n" + " - key: test-check.IntegerNegative\n" + " value: -67890\n" + " - key: test-check.IntegerInvalid\n" + " value: asdf\n" + " - key: test-check.BooleanTrue\n" + " value: true\n" + " - key: test-check.BooleanFalse\n" + " value: false\n" + " - key: test-check.BooleanInvalid\n" + " value: asdf\n", + ClangTidyOptions()); + EXPECT_TRUE(!!Options); + OptionsView Opt("test-check", Options->CheckOptions); + EXPECT_EQ("qqq", Opt.get("NonExistent", "qqq")); + EXPECT_EQ(" qwe !@#* ", Opt.get("String", "")); + + EXPECT_EQ(-123, Opt.get("NonExistent", -123)); + EXPECT_EQ(123, Opt.get("IntegerInvalid", 123)); + EXPECT_EQ(12345, Opt.get("IntegerPositive", 0)); + EXPECT_EQ(-67890, Opt.get("IntegerNegative", 0)); + + EXPECT_EQ(123U, Opt.get("NonExistent", 123U)); + EXPECT_EQ(123U, Opt.get("IntegerNegative", 123U)); + EXPECT_EQ(12345U, Opt.get("IntegerPositive", 0U)); + + EXPECT_TRUE(Opt.get("NonExistent", true)); + EXPECT_FALSE(Opt.get("NonExistent", false)); + EXPECT_TRUE(Opt.get("BooleanTrue", false)); + EXPECT_FALSE(Opt.get("BooleanFalse", true)); + EXPECT_TRUE(Opt.get("BooleanInvalid", true)); +} + +TEST(ParseConfiguration, CheckOptionsValidation) { + ClangTidyOptions Defaults; + Defaults.CheckOptions["test-check.BooleanInvalid"] = + ClangTidyOptions::TypedOption(true); + llvm::ErrorOr Options = + parseConfiguration("CheckOptions:\n" + " - key: test-check.BooleanInvalid\n" + " value: asdf\n", + Defaults); + EXPECT_FALSE(!!Options); +} + } // namespace test } // namespace tidy } // namespace clang