Fix for the command-line workflow (proper tests for this will be later).
http://reviews.llvm.org/D5602
Files:
clang-tidy/ClangTidy.cpp
clang-tidy/ClangTidy.h
clang-tidy/ClangTidyOptions.cpp
clang-tidy/ClangTidyOptions.h
clang-tidy/google/GoogleTidyModule.cpp
clang-tidy/readability/BracesAroundStatementsCheck.cpp
clang-tidy/readability/FunctionSize.cpp
clang-tidy/readability/NamespaceCommentCheck.cpp
clang-tidy/tool/ClangTidyMain.cpp
unittests/clang-tidy/ClangTidyOptionsTest.cpp
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<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
@@ -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 <memory>
#include <type_traits>
#include <vector>
@@ -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 T>
- typename std::enable_if<std::is_integral<T>::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<llvm::yaml::has_ScalarTraits<T>::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/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<ClangTidyOptions::StringPair> {
- static void mapping(IO &IO, ClangTidyOptions::StringPair &KeyValue) {
+template <> struct MappingTraits<ClangTidyOptions::OptionItem> {
+ 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<const ClangTidyOptions *>(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<ClangTidyOptions::StringPair> Options;
+ std::vector<ClangTidyOptions::OptionItem> Options;
};
template <> struct MappingTraits<ClangTidyOptions> {
@@ -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<ClangTidyOptions> 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<ClangTidyOptions> parseConfiguration(StringRef Config) {
+llvm::ErrorOr<ClangTidyOptions>
+parseConfiguration(StringRef Config, const ClangTidyOptions &Defaults) {
llvm::yaml::Input Input(Config);
- ClangTidyOptions Options;
+ Input.setContext(const_cast<ClangTidyOptions *>(&Defaults));
+ ClangTidyOptions Options = Defaults;
Input >> Options;
if (Input.error())
return Input.error();
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 <functional>
#include <map>
#include <string>
#include <system_error>
@@ -74,8 +76,41 @@
/// comments in the relevant check.
llvm::Optional<std::string> User;
- typedef std::pair<std::string, std::string> StringPair;
- typedef std::map<std::string, std::string> OptionMap;
+ struct Option {
+ template <typename T>
+ typename std::enable_if<llvm::yaml::has_ScalarTraits<T>::value, T>::type
+ get(const T &Default) const {
+ T Result = Default;
+ llvm::yaml::ScalarTraits<T>::input(Value, nullptr, Result);
+ return Result;
+ }
+
+ template <typename T>
+ typename std::enable_if<llvm::yaml::has_ScalarTraits<T>::value, void>::type
+ store(const T &TypedValue) {
+ llvm::raw_string_ostream OS(Value);
+ llvm::yaml::ScalarTraits<T>::output(TypedValue, nullptr, OS);
+ }
+
+ std::string Value;
+ std::function<llvm::StringRef(llvm::StringRef)> Validate;
+ };
+
+ template <typename T>
+ static typename std::enable_if<llvm::yaml::has_ScalarTraits<T>::value,
+ Option>::type
+ TypedOption(const T &Default) {
+ Option Result;
+ Result.store(Default);
+ Result.Validate = [](llvm::StringRef Input) {
+ T Dummy;
+ return llvm::yaml::ScalarTraits<T>::input(Input, nullptr, Dummy);
+ };
+ return Result;
+ }
+
+ typedef std::map<std::string, Option> OptionMap;
+ typedef std::pair<std::string, Option> 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<ClangTidyOptions> parseConfiguration(llvm::StringRef Config);
+llvm::ErrorOr<ClangTidyOptions>
+parseConfiguration(llvm::StringRef Config, const ClangTidyOptions &Defaults);
/// \brief Serializes configuration to a YAML-encoded string.
std::string configurationAsText(const ClangTidyOptions &Options);
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
@@ -151,7 +151,7 @@
return nullptr;
}
- ClangTidyOptions DefaultOptions;
+ ClangTidyOptions DefaultOptions = ClangTidyOptions::getDefaults();
DefaultOptions.Checks = DefaultChecks;
DefaultOptions.HeaderFilterRegex = HeaderFilter;
DefaultOptions.AnalyzeTemporaryDtors = AnalyzeTemporaryDtors;
@@ -170,12 +170,9 @@
if (!Config.empty()) {
if (llvm::ErrorOr<ClangTidyOptions> ParsedConfig =
- parseConfiguration(Config)) {
+ parseConfiguration(Config, DefaultOptions)) {
return llvm::make_unique<DefaultOptionsProvider>(
- 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,14 +60,66 @@
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);
EXPECT_TRUE(*Options->AnalyzeTemporaryDtors);
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.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<ClangTidyOptions> Options =
+ parseConfiguration("CheckOptions:\n"
+ " - key: test-check.BooleanInvalid\n"
+ " value: asdf\n",
+ Defaults);
+ EXPECT_FALSE(!!Options);
+}
+
} // namespace test
} // namespace tidy
} // namespace clang
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
