Index: clang-tidy/ClangTidy.h =================================================================== --- clang-tidy/ClangTidy.h +++ clang-tidy/ClangTidy.h @@ -17,7 +17,9 @@ #include "clang/Basic/SourceManager.h" #include "clang/Tooling/Refactoring.h" #include "llvm/ADT/StringExtras.h" +#include "llvm/Support/Errc.h" #include "llvm/Support/raw_ostream.h" +#include "llvm/Support/YAMLTraits.h" #include #include #include @@ -46,23 +48,33 @@ /// /// 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; + /// \c llvm::None. + llvm::Optional get(StringRef LocalName) const; - /// \brief Read a named option from the \c Context and parse it as an integral - /// type \c T. + /// \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 the type + /// \c T. + /// + /// Reads the option with the check-local name \p LocalName from the + /// \c CheckOptions, parses the value as the type \c T using + /// \c llvm::yaml::ScalarTraits and stores it to \p Value. On success + /// returns an empty \c StringRef. If the corresponding key is not present, + /// doesn't change the \p Value and returns an empty \c StringRef. If a + /// parsing error occurs, returns a non-empty \c StringRef describing the + /// error. 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, + llvm::StringRef>::type + parseOption(StringRef LocalName, T& Value) const { + if (llvm::Optional String = get(LocalName)) + return llvm::yaml::ScalarTraits::input(*String, nullptr, Value); + return llvm::StringRef(); } /// \brief Stores an option with the check-local name \p LocalName with string Index: clang-tidy/ClangTidy.cpp =================================================================== --- clang-tidy/ClangTidy.cpp +++ clang-tidy/ClangTidy.cpp @@ -319,6 +319,13 @@ const ClangTidyOptions::OptionMap &CheckOptions) : NamePrefix(CheckName.str() + "."), CheckOptions(CheckOptions) {} +llvm::Optional OptionsView::get(StringRef LocalName) const { + const auto &Iter = CheckOptions.find(NamePrefix + LocalName.str()); + if (Iter != CheckOptions.end()) + return Iter->second; + return llvm::None; +} + std::string OptionsView::get(StringRef LocalName, std::string Default) const { const auto &Iter = CheckOptions.find(NamePrefix + LocalName.str()); if (Iter != CheckOptions.end()) Index: clang-tidy/readability/BracesAroundStatementsCheck.h =================================================================== --- clang-tidy/readability/BracesAroundStatementsCheck.h +++ clang-tidy/readability/BracesAroundStatementsCheck.h @@ -51,7 +51,7 @@ const ASTContext *Context); private: - const unsigned ShortStatementLines; + unsigned ShortStatementLines; }; } // namespace readability Index: clang-tidy/readability/BracesAroundStatementsCheck.cpp =================================================================== --- clang-tidy/readability/BracesAroundStatementsCheck.cpp +++ clang-tidy/readability/BracesAroundStatementsCheck.cpp @@ -115,7 +115,12 @@ StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), // Always add braces by default. - ShortStatementLines(Options.get("ShortStatementLines", 0U)) {} + ShortStatementLines(0U) { + StringRef Error = + Options.parseOption("ShortStatementLines", ShortStatementLines); + if (!Error.empty()) + llvm::errs() << "Invalid value for ShortStatementLines: " << Error << "\n"; +} void BracesAroundStatementsCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { Index: clang-tidy/readability/FunctionSize.h =================================================================== --- clang-tidy/readability/FunctionSize.h +++ clang-tidy/readability/FunctionSize.h @@ -34,9 +34,9 @@ unsigned Branches; }; - const unsigned LineThreshold; - const unsigned StatementThreshold; - const unsigned BranchThreshold; + unsigned LineThreshold; + unsigned StatementThreshold; + unsigned BranchThreshold; llvm::DenseMap FunctionInfos; }; Index: clang-tidy/readability/FunctionSize.cpp =================================================================== --- clang-tidy/readability/FunctionSize.cpp +++ clang-tidy/readability/FunctionSize.cpp @@ -17,10 +17,18 @@ namespace readability { FunctionSizeCheck::FunctionSizeCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context), - LineThreshold(Options.get("LineThreshold", -1U)), - StatementThreshold(Options.get("StatementThreshold", 800U)), - BranchThreshold(Options.get("BranchThreshold", -1U)) {} + : ClangTidyCheck(Name, Context), LineThreshold(-1U), + StatementThreshold(800U), BranchThreshold(-1U) { + StringRef Error = Options.parseOption("LineThreshold", LineThreshold); + if (!Error.empty()) + llvm::errs() << "Invalid value for LineThreshold: " << Error << "\n"; + Error = Options.parseOption("StatementThreshold", StatementThreshold); + if (!Error.empty()) + llvm::errs() << "Invalid value for StatementThreshold: " << Error << "\n"; + Error = Options.parseOption("BranchThreshold", BranchThreshold); + if (!Error.empty()) + llvm::errs() << "Invalid value for BranchThreshold: " << Error << "\n"; +} void FunctionSizeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { Options.store(Opts, "LineThreshold", LineThreshold); Index: clang-tidy/readability/NamespaceCommentCheck.h =================================================================== --- clang-tidy/readability/NamespaceCommentCheck.h +++ clang-tidy/readability/NamespaceCommentCheck.h @@ -31,8 +31,8 @@ void storeOptions(ClangTidyOptions::OptionMap &Options) override; llvm::Regex NamespaceCommentPattern; - const unsigned ShortNamespaceLines; - const unsigned SpacesBeforeComments; + unsigned ShortNamespaceLines; + unsigned SpacesBeforeComments; }; } // namespace readability Index: clang-tidy/readability/NamespaceCommentCheck.cpp =================================================================== --- clang-tidy/readability/NamespaceCommentCheck.cpp +++ clang-tidy/readability/NamespaceCommentCheck.cpp @@ -25,8 +25,15 @@ NamespaceCommentPattern("^/[/*] *(end (of )?)? *(anonymous|unnamed)? *" "namespace( +([a-zA-Z0-9_]+))?\\.? *(\\*/)?$", llvm::Regex::IgnoreCase), - ShortNamespaceLines(Options.get("ShortNamespaceLines", 1u)), - SpacesBeforeComments(Options.get("SpacesBeforeComments", 1u)) {} + ShortNamespaceLines(1u), SpacesBeforeComments(1u) { + StringRef Error = + Options.parseOption("ShortNamespaceLines", ShortNamespaceLines); + if (!Error.empty()) + llvm::errs() << "Invalid value for ShortNamespaceLines: " << Error << "\n"; + Error = Options.parseOption("SpacesBeforeComments", SpacesBeforeComments); + if (!Error.empty()) + llvm::errs() << "Invalid value for SpacesBeforeComments: " << Error << "\n"; +} void NamespaceCommentCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { Options.store(Opts, "ShortNamespaceLines", ShortNamespaceLines); 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" @@ -67,6 +68,61 @@ 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.BooleanTrue\n" + " value: true\n" + " - key: test-check.BooleanFalse\n" + " value: false\n" + ); + EXPECT_TRUE(!!Options); + OptionsView Opt("test-check", Options->CheckOptions); + EXPECT_EQ("qqq", Opt.get("NonExistent", "qqq")); + EXPECT_EQ(" qwe !@#* ", Opt.get("String", "")); + + EXPECT_FALSE(Opt.get("NonExistent").hasValue()); + EXPECT_EQ(" qwe !@#* ", *Opt.get("String")); + + int i = 123; + EXPECT_TRUE(Opt.parseOption("NonExistent", i).empty()); + EXPECT_EQ(123, i); + EXPECT_TRUE(Opt.parseOption("IntegerPositive", i).empty()); + EXPECT_EQ(12345, i); + EXPECT_TRUE(Opt.parseOption("IntegerNegative", i).empty()); + EXPECT_EQ(-67890, i); + EXPECT_FALSE(Opt.parseOption("BooleanFalse", i).empty()); + EXPECT_FALSE(Opt.parseOption("BooleanTrue", i).empty()); + EXPECT_FALSE(Opt.parseOption("String", i).empty()); + + unsigned u = 123U; + EXPECT_TRUE(Opt.parseOption("NonExistent", u).empty()); + EXPECT_EQ(123U, u); + EXPECT_TRUE(Opt.parseOption("IntegerPositive", u).empty()); + EXPECT_EQ(12345U, u); + EXPECT_FALSE(Opt.parseOption("BooleanFalse", u).empty()); + EXPECT_FALSE(Opt.parseOption("BooleanTrue", u).empty()); + EXPECT_FALSE(Opt.parseOption("IntegerNegative", u).empty()); + EXPECT_FALSE(Opt.parseOption("String", u).empty()); + + bool b = false; + EXPECT_TRUE(Opt.parseOption("NonExistent", b).empty()); + EXPECT_FALSE(b); + EXPECT_TRUE(Opt.parseOption("BooleanTrue", b).empty()); + EXPECT_TRUE(b); + EXPECT_TRUE(Opt.parseOption("BooleanFalse", b).empty()); + EXPECT_FALSE(b); + EXPECT_FALSE(Opt.parseOption("IntegerNegative", b).empty()); + EXPECT_FALSE(Opt.parseOption("IntegerPositive", b).empty()); + EXPECT_FALSE(Opt.parseOption("String", b).empty()); +} + } // namespace test } // namespace tidy } // namespace clang