Let the check decide what to do with invalid option values.
http://reviews.llvm.org/D5602
Files:
clang-tidy/ClangTidy.cpp
clang-tidy/ClangTidy.h
clang-tidy/misc/FunctionSize.cpp
clang-tidy/readability/NamespaceCommentCheck.cpp
unittests/clang-tidy/ClangTidyOptionsTest.cpp
Index: clang-tidy/ClangTidy.cpp
===================================================================
--- clang-tidy/ClangTidy.cpp
+++ clang-tidy/ClangTidy.cpp
@@ -319,7 +319,7 @@
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;
@@ -331,11 +331,6 @@
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<std::string> getCheckNames(const ClangTidyOptions &Options) {
clang::tidy::ClangTidyContext Context(
llvm::make_unique<DefaultOptionsProvider>(ClangTidyGlobalOptions(),
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 <memory>
#include <type_traits>
#include <vector>
@@ -47,33 +49,46 @@
/// 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 and parse it as an integral
- /// type \c T.
+ /// \brief Read a named option from the \c Context and parse it as any
+ /// type \c T that has ScalarTraits<>.
///
/// Reads the option with the check-local name \p LocalName from the
/// \c CheckOptions. If the corresponding key is not present, returns
/// \p Default.
template <typename T>
- typename std::enable_if<std::is_integral<T>::value, T>::type
+ typename std::enable_if<llvm::yaml::has_ScalarTraits<T>::value,
+ llvm::ErrorOr<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;
+ std::string String = get(LocalName, "");
+ if (String.empty())
+ return Default;
+
+ T Value;
+ StringRef Result =
+ llvm::yaml::ScalarTraits<T>::input(String, nullptr, Value);
+ if (!Result.empty())
+ return llvm::make_error_code(llvm::errc::invalid_argument);
+ return Value;
}
/// \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;
+ /// \brief Stores an option with the check-local name \p LocalName of any
+ /// type that has ScalarTraits<>.
+ template <typename T>
+ typename std::enable_if<llvm::yaml::has_ScalarTraits<T>::value, void>::type
+ store(ClangTidyOptions::OptionMap &Options, StringRef LocalName,
+ const T &Value) {
+ std::string Storage;
+ llvm::raw_string_ostream Buffer(Storage);
+ llvm::yaml::ScalarTraits<T>::output(Value, nullptr, Buffer);
+ store(Options, LocalName, Buffer.str());
+ }
private:
std::string NamePrefix;
Index: clang-tidy/misc/FunctionSize.cpp
===================================================================
--- clang-tidy/misc/FunctionSize.cpp
+++ clang-tidy/misc/FunctionSize.cpp
@@ -17,9 +17,9 @@
FunctionSizeCheck::FunctionSizeCheck(StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
- LineThreshold(Options.get("LineThreshold", -1U)),
- StatementThreshold(Options.get("StatementThreshold", 800U)),
- BranchThreshold(Options.get("BranchThreshold", -1U)) {}
+ LineThreshold(*Options.get("LineThreshold", -1U)),
+ StatementThreshold(*Options.get("StatementThreshold", 800U)),
+ BranchThreshold(*Options.get("BranchThreshold", -1U)) {}
void FunctionSizeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "LineThreshold", LineThreshold);
Index: clang-tidy/readability/NamespaceCommentCheck.cpp
===================================================================
--- clang-tidy/readability/NamespaceCommentCheck.cpp
+++ clang-tidy/readability/NamespaceCommentCheck.cpp
@@ -25,9 +25,9 @@
NamespaceCommentPattern("^/[/*] *(end (of )?)? *(anonymous|unnamed)? *"
"namespace( +([a-zA-Z0-9_]+))? *(\\*/)?$",
llvm::Regex::IgnoreCase),
- ShortNamespaceLines(Options.get("ShortNamespaceLines", 1u)),
- SpacesBeforeComments(Options.get("SpacesBeforeComments",
- Name.startswith("google") ? 2u : 1u)) {}
+ ShortNamespaceLines(*Options.get("ShortNamespaceLines", 1u)),
+ SpacesBeforeComments(*Options.get("SpacesBeforeComments",
+ Name.startswith("google") ? 2u : 1u)) {}
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"
@@ -54,7 +55,7 @@
EXPECT_EQ(1000u, Options.LineFilter[2].LineRanges[0].second);
}
-TEST(ParseConfiguration, ValidConfiguration) {
+TEST(ParseConfiguration, BaseConfiguration) {
llvm::ErrorOr<ClangTidyOptions> Options =
parseConfiguration("Checks: \"-*,misc-*\"\n"
"HeaderFilterRegex: \".*\"\n"
@@ -67,6 +68,36 @@
EXPECT_EQ("some.user", *Options->User);
}
+TEST(ParseConfiguration, CheckOptions) {
+ llvm::ErrorOr<ClangTidyOptions> 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"
+ " - key: test-check.BooleanInvalid\n"
+ " value: asdf\n"
+ );
+ EXPECT_TRUE(!!Options);
+ OptionsView Opt("test-check", Options->CheckOptions);
+ EXPECT_EQ("qqq", Opt.get("NonExistent", "qqq"));
+ EXPECT_EQ(-123, *Opt.get("NonExistent", -123));
+ EXPECT_EQ(123U, *Opt.get("NonExistent", 123U));
+ EXPECT_EQ(" qwe !@#* ", Opt.get("String", ""));
+ EXPECT_EQ(12345, *Opt.get("IntegerPositive", 0));
+ EXPECT_EQ(12345U, *Opt.get("IntegerPositive", 0U));
+ EXPECT_EQ(-67890, *Opt.get("IntegerNegative", 0));
+ EXPECT_TRUE(*Opt.get("BooleanTrue", false));
+ EXPECT_FALSE(*Opt.get("BooleanFalse", true));
+ EXPECT_FALSE(Opt.get("BooleanInvalid", true));
+}
+
} // namespace test
} // namespace tidy
} // namespace clang
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
