diff --git a/clang-tools-extra/clang-tidy/ClangTidyCheck.h b/clang-tools-extra/clang-tidy/ClangTidyCheck.h --- a/clang-tools-extra/clang-tidy/ClangTidyCheck.h +++ b/clang-tools-extra/clang-tidy/ClangTidyCheck.h @@ -14,7 +14,9 @@ #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/Basic/Diagnostic.h" #include "clang/Basic/SourceManager.h" +#include "llvm/ADT/Optional.h" #include "llvm/ADT/StringExtras.h" +#include "llvm/Support/Error.h" #include #include #include @@ -25,6 +27,58 @@ namespace tidy { +template class OptionError : public llvm::ErrorInfo { + std::error_code convertToErrorCode() const override { + return llvm::inconvertibleErrorCode(); + } + +public: + void log(raw_ostream &OS) const override { OS << this->message(); } +}; + +class MissingOptionError : public OptionError { +public: + MissingOptionError(std::string OptionName) : OptionName(OptionName) {} + + std::string message() const override; + static char ID; +private: + std::string OptionName; +}; + +class UnparseableEnumOptionError + : public OptionError { +public: + UnparseableEnumOptionError(std::string LookupName, std::string LookupValue) + : LookupName(LookupName), LookupValue(LookupValue) {} + UnparseableEnumOptionError(std::string LookupName, std::string LookupValue, + std::string SuggestedValue) + : LookupName(LookupName), LookupValue(LookupValue), + SuggestedValue(SuggestedValue) {} + + std::string message() const override; + static char ID; + +private: + std::string LookupName; + std::string LookupValue; + llvm::Optional SuggestedValue; +}; + +class UnparseableIntegerOptionError + : public OptionError { +public: + UnparseableIntegerOptionError(std::string LookupName, std::string LookupValue) + : LookupName(LookupName), LookupValue(LookupValue) {} + + std::string message() const override; + static char ID; + +private: + std::string LookupName; + std::string LookupValue; +}; + /// Base class for all clang-tidy checks. /// /// To implement a ``ClangTidyCheck``, write a subclass and override some of the @@ -127,35 +181,112 @@ OptionsView(StringRef CheckName, const ClangTidyOptions::OptionMap &CheckOptions); + /// Read a named option from the ``Context``. + /// + /// Reads the option with the check-local name \p LocalName from the + /// ``CheckOptions``. If the corresponding key is not present, returns + /// a ``MissingOptionError``. + llvm::Expected get(StringRef LocalName) const; + /// Read a named option from the ``Context``. /// /// Reads the option with the check-local name \p LocalName from the /// ``CheckOptions``. If the corresponding key is not present, returns /// \p Default. - std::string get(StringRef LocalName, StringRef Default) const; + std::string get(StringRef LocalName, StringRef Default) const { + if (auto Val = get(LocalName)) + return *Val; + else + llvm::consumeError(Val.takeError()); + return std::string(Default); + } + + /// Read a named option from the ``Context``. + /// + /// Reads the option with the check-local name \p LocalName from local or + /// global ``CheckOptions``. Gets local option first. If local is not + /// present, falls back to get global option. If global option is not + /// present either, returns a ``MissingOptionError``. + llvm::Expected getLocalOrGlobal(StringRef LocalName) const; /// Read a named option from the ``Context``. /// /// Reads the option with the check-local name \p LocalName from local or /// global ``CheckOptions``. Gets local option first. If local is not /// present, falls back to get global option. If global option is not - /// present either, returns Default. - std::string getLocalOrGlobal(StringRef LocalName, StringRef Default) const; + /// present either, returns \p Default. + std::string getLocalOrGlobal(StringRef LocalName, StringRef Default) const { + if (auto Val = getLocalOrGlobal(LocalName)) + return *Val; + else + llvm::consumeError(Val.takeError()); + return std::string(Default); + } /// Read a named option from the ``Context`` and parse it as an /// integral type ``T``. /// /// Reads the option with the check-local name \p LocalName from the /// ``CheckOptions``. If the corresponding key is not present, returns - /// \p Default. + /// a ``MissingOptionError``. If the corresponding key can't be parsed as + /// a ``T``, return an ``UnparseableIntegerOptionError``. + template + std::enable_if_t::value, llvm::Expected> + get(StringRef LocalName) const { + if (llvm::Expected Value = get(LocalName)) { + T Result{}; + if (!StringRef(*Value).getAsInteger(10, Result)) + return Result; + return llvm::make_error( + (NamePrefix + LocalName).str(), *Value); + } else + return std::move(Value.takeError()); + } + + /// Read a named option from the ``Context`` and parse it as an + /// integral type ``T``. + /// + /// Reads the option with the check-local name \p LocalName from the + /// ``CheckOptions``. If the corresponding key is not present or it can't be + /// parsed as a ``T``, returns \p Default. template std::enable_if_t::value, T> get(StringRef LocalName, T Default) const { - std::string Value = get(LocalName, ""); - T Result = Default; - if (!Value.empty()) - StringRef(Value).getAsInteger(10, Result); - return Result; + if (llvm::Expected ValueOr = get(LocalName)) + return *ValueOr; + else + logErrToStdErr(ValueOr.takeError()); + return Default; + } + + /// Read a named option from the ``Context`` and parse it as an + /// integral type ``T``. + /// + /// Reads the option with the check-local name \p LocalName from local or + /// global ``CheckOptions``. Gets local option first. If local is not + /// present, falls back to get global option. If global option is not + /// present either, returns a ``MissingOptionError``. If the corresponding + /// key can't be parsed as a ``T``, return an + /// ``UnparseableIntegerOptionError``. + template + std::enable_if_t::value, llvm::Expected> + getLocalOrGlobal(StringRef LocalName) const { + llvm::Expected ValueOr = get(LocalName); + bool IsGlobal = false; + if (!ValueOr) { + IsGlobal = true; + llvm::consumeError(ValueOr.takeError()); + ValueOr = getLocalOrGlobal(LocalName); + if (!ValueOr) { + return std::move(ValueOr.takeError()); + } + } + T Result{}; + if (!StringRef(*ValueOr).getAsInteger(10, Result)) + return Result; + return llvm::make_error( + (IsGlobal ? LocalName.str() : (NamePrefix + LocalName).str()), + *ValueOr); } /// Read a named option from the ``Context`` and parse it as an @@ -164,15 +295,93 @@ /// Reads the option with the check-local name \p LocalName from local or /// global ``CheckOptions``. Gets local option first. If local is not /// present, falls back to get global option. If global option is not - /// present either, returns Default. + /// present either or it can't be parsed as a ``T``, returns \p Default. template std::enable_if_t::value, T> getLocalOrGlobal(StringRef LocalName, T Default) const { - std::string Value = getLocalOrGlobal(LocalName, ""); - T Result = Default; - if (!Value.empty()) - StringRef(Value).getAsInteger(10, Result); - return Result; + if (llvm::Expected ValueOr = getLocalOrGlobal(LocalName)) + return *ValueOr; + else + logErrToStdErr(ValueOr.takeError()); + return Default; + } + + /// Read a named option from the ``Context`` and parse it as an + /// enum type ``T`` using the \p Mapping provided. If \p IgnoreCase is set, + /// it will search the mapping ignoring the case. + /// + /// Reads the option with the check-local name \p LocalName from the + /// ``CheckOptions``. If the corresponding key is not present, returns a + /// ``MissingOptionError``. If the key can't be parsed as a ``T`` returns a + /// ``UnparseableEnumOptionError``. + template + std::enable_if_t::value, llvm::Expected> + get(StringRef LocalName, ArrayRef> Mapping, + bool IgnoreCase = false) { + if (llvm::Expected ValueOr = getEnumInt( + LocalName, typeEraseMapping(Mapping), false, IgnoreCase)) + return static_cast(*ValueOr); + else + return std::move(ValueOr.takeError()); + } + + /// Read a named option from the ``Context`` and parse it as an + /// enum type ``T`` using the \p Mapping provided. If \p IgnoreCase is set, + /// it will search the mapping ignoring the case. + /// + /// Reads the option with the check-local name \p LocalName from the + /// ``CheckOptions``. If the corresponding key is not present or it can't be + /// parsed as a ``T``, returns \p Default. + template + std::enable_if_t::value, T> + get(StringRef LocalName, ArrayRef> Mapping, + T Default, bool IgnoreCase = false) { + if (auto ValueOr = get(LocalName, Mapping, IgnoreCase)) + return *ValueOr; + else + logErrToStdErr(ValueOr.takeError()); + return Default; + } + + /// Read a named option from the ``Context`` and parse it as an + /// enum type ``T`` using the \p Mapping provided. If \p IgnoreCase is set, + /// it will search the mapping ignoring the case. + /// + /// Reads the option with the check-local name \p LocalName from local or + /// global ``CheckOptions``. Gets local option first. If local is not + /// present, falls back to get global option. If global option is not + /// present either, returns a ``MissingOptionError``. If the key can't be + /// parsed as a ``T`` returns a ``UnparseableEnumOptionError``. + template + std::enable_if_t::value, llvm::Expected> + getLocalOrGlobal(StringRef LocalName, + ArrayRef> Mapping, + bool IgnoreCase = false) { + if (llvm::Expected ValueOr = getEnumInt( + LocalName, typeEraseMapping(Mapping), true, IgnoreCase)) + return static_cast(*ValueOr); + else + return std::move(ValueOr.takeError()); + } + + /// Read a named option from the ``Context`` and parse it as an + /// enum type ``T`` using the \p Mapping provided. If \p IgnoreCase is set, + /// it will search the mapping ignoring the case. + /// + /// Reads the option with the check-local name \p LocalName from local or + /// global ``CheckOptions``. Gets local option first. If local is not + /// present, falls back to get global option. If global option is not + /// present either or it can't be parsed as a ``T``, returns \p Default. + template + std::enable_if_t::value, T> + getLocalOrGlobal(StringRef LocalName, + ArrayRef> Mapping, T Default, + bool IgnoreCase = false) { + if (auto ValueOr = getLocalOrGlobal(LocalName, Mapping, IgnoreCase)) + return *ValueOr; + else + logErrToStdErr(ValueOr.takeError()); + return Default; } /// Stores an option with the check-local name \p LocalName with @@ -185,7 +394,41 @@ void store(ClangTidyOptions::OptionMap &Options, StringRef LocalName, int64_t Value) const; + /// Stores an option with the check-local name \p LocalName as the string + /// representation of the Enum \p Value using the \p Mapping to \p Options. + template + std::enable_if_t::value> + store(ClangTidyOptions::OptionMap &Options, StringRef LocalName, T Value, + ArrayRef> Mapping) { + auto Iter = llvm::find_if( + Mapping, [&](const std::pair &NameAndEnum) { + return NameAndEnum.second == Value; + }); + assert(Iter != Mapping.end() && "Unknown Case Value"); + store(Options, LocalName, Iter->first); + } + private: + using NameAndValue = std::pair; + + llvm::Expected getEnumInt(StringRef LocalName, + ArrayRef Mapping, + bool CheckGlobal, bool IgnoreCase); + + template + std::enable_if_t::value, std::vector> + typeEraseMapping(ArrayRef> Mapping) { + std::vector Result; + Result.reserve(Mapping.size()); + for (auto &MappedItem : Mapping) { + Result.emplace_back(MappedItem.first, + static_cast(MappedItem.second)); + } + return Result; + } + + static void logErrToStdErr(llvm::Error &&Err); + std::string NamePrefix; const ClangTidyOptions::OptionMap &CheckOptions; }; diff --git a/clang-tools-extra/clang-tidy/ClangTidyCheck.cpp b/clang-tools-extra/clang-tidy/ClangTidyCheck.cpp --- a/clang-tools-extra/clang-tidy/ClangTidyCheck.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidyCheck.cpp @@ -7,10 +7,45 @@ //===----------------------------------------------------------------------===// #include "ClangTidyCheck.h" +#include "llvm/ADT/SmallString.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/Support/Error.h" +#include "llvm/Support/raw_ostream.h" namespace clang { namespace tidy { +char MissingOptionError::ID; +char UnparseableEnumOptionError::ID; +char UnparseableIntegerOptionError::ID; + +std::string MissingOptionError::message() const { + llvm::SmallString<128> Buffer; + llvm::raw_svector_ostream Output(Buffer); + Output << "warning: Option not found '" << OptionName << "'\n"; + return std::string(Buffer); +} + +std::string UnparseableEnumOptionError::message() const { + llvm::SmallString<128> Buffer; + llvm::raw_svector_ostream Output(Buffer); + Output << "warning: Invalid configuration value '" << LookupValue + << "' for option '" << LookupName; + if (SuggestedValue) + Output << "'; did you mean '" << *SuggestedValue << "'?\n"; + else + Output << "'\n"; + return std::string(Buffer); +} + +std::string UnparseableIntegerOptionError::message() const { + llvm::SmallString<128> Buffer; + llvm::raw_svector_ostream Output(Buffer); + Output << "warning: Invalid configuration value '" << LookupValue + << "' for option '" << LookupName << "'; expected an integer value\n"; + return std::string(Buffer); +} + ClangTidyCheck::ClangTidyCheck(StringRef CheckName, ClangTidyContext *Context) : CheckName(CheckName), Context(Context), Options(CheckName, Context->getOptions().CheckOptions) { @@ -34,17 +69,16 @@ const ClangTidyOptions::OptionMap &CheckOptions) : NamePrefix(CheckName.str() + "."), CheckOptions(CheckOptions) {} -std::string ClangTidyCheck::OptionsView::get(StringRef LocalName, - StringRef Default) const { +llvm::Expected +ClangTidyCheck::OptionsView::get(StringRef LocalName) const { const auto &Iter = CheckOptions.find(NamePrefix + LocalName.str()); if (Iter != CheckOptions.end()) return Iter->second; - return std::string(Default); + return llvm::make_error((NamePrefix + LocalName).str()); } -std::string -ClangTidyCheck::OptionsView::getLocalOrGlobal(StringRef LocalName, - StringRef Default) const { +llvm::Expected +ClangTidyCheck::OptionsView::getLocalOrGlobal(StringRef LocalName) const { auto Iter = CheckOptions.find(NamePrefix + LocalName.str()); if (Iter != CheckOptions.end()) return Iter->second; @@ -52,7 +86,7 @@ Iter = CheckOptions.find(LocalName.str()); if (Iter != CheckOptions.end()) return Iter->second; - return std::string(Default); + return llvm::make_error((NamePrefix + LocalName).str()); } void ClangTidyCheck::OptionsView::store(ClangTidyOptions::OptionMap &Options, @@ -67,5 +101,51 @@ store(Options, LocalName, llvm::itostr(Value)); } +llvm::Expected ClangTidyCheck::OptionsView::getEnumInt( + StringRef LocalName, ArrayRef> Mapping, + bool CheckGlobal, bool IgnoreCase) { + auto Iter = CheckOptions.find((NamePrefix + LocalName).str()); + if (CheckGlobal && Iter == CheckOptions.end()) { + Iter = CheckOptions.find(LocalName.str()); + } + if (Iter == CheckOptions.end()) { + return llvm::make_error((NamePrefix + LocalName).str()); + } + + StringRef Value = Iter->second; + StringRef Closest; + unsigned EditDistance = -1; + for (const auto &NameAndEnum : Mapping) { + if (IgnoreCase) { + if (Value.equals_lower(NameAndEnum.first)) + return NameAndEnum.second; + } else if (Value.equals(NameAndEnum.first)) + return NameAndEnum.second; + else if (Value.equals_lower(NameAndEnum.first)) { + Closest = NameAndEnum.first; + EditDistance = 0; + continue; + } + unsigned Distance = Value.edit_distance(NameAndEnum.first); + if (Distance < EditDistance) { + EditDistance = Distance; + Closest = NameAndEnum.first; + } + } + if (EditDistance < 3) + return llvm::make_error( + Iter->first, Iter->second, std::string(Closest)); + return llvm::make_error(Iter->first, + Iter->second); +} + +void ClangTidyCheck::OptionsView::logErrToStdErr(llvm::Error &&Err) { + llvm::logAllUnhandledErrors( + llvm::handleErrors(std::move(Err), + [](const MissingOptionError &) -> llvm::Error { + return llvm::Error::success(); + }), + llvm::errs()); +} } // namespace tidy } // namespace clang diff --git a/clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp b/clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp --- a/clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp +++ b/clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp @@ -1,6 +1,8 @@ #include "ClangTidyOptions.h" -#include "gtest/gtest.h" +#include "ClangTidyCheck.h" +#include "ClangTidyDiagnosticConsumer.h" #include "llvm/ADT/StringExtras.h" +#include "gtest/gtest.h" namespace clang { namespace tidy { @@ -97,6 +99,155 @@ llvm::join(Options.ExtraArgsBefore->begin(), Options.ExtraArgsBefore->end(), ",")); } + +class TestCheck : public ClangTidyCheck { +public: + TestCheck(ClangTidyContext *Context) : ClangTidyCheck("test", Context) {} + + template auto getLocal(Args &&... Arguments) { + return Options.get(std::forward(Arguments)...); + } + + template auto getGlobal(Args &&... Arguments) { + return Options.getLocalOrGlobal(std::forward(Arguments)...); + } + + template + auto getIntLocal(Args &&... Arguments) { + return Options.get(std::forward(Arguments)...); + } + + template + auto getIntGlobal(Args &&... Arguments) { + return Options.getLocalOrGlobal(std::forward(Arguments)...); + } +}; + +#define CHECK_VAL(Value, Expected) \ + do { \ + auto Item = Value; \ + ASSERT_TRUE(!!Item); \ + EXPECT_EQ(*Item, Expected); \ + } while (false) + +#define CHECK_ERROR(Value, ErrorType, ExpectedMessage) \ + do { \ + auto Item = Value; \ + ASSERT_FALSE(Item); \ + ASSERT_TRUE(Item.errorIsA()); \ + ASSERT_FALSE(llvm::handleErrors( \ + Item.takeError(), [&](const ErrorType &Err) -> llvm::Error { \ + EXPECT_EQ(Err.message(), ExpectedMessage); \ + return llvm::Error::success(); \ + })); \ + } while (false) + +TEST(CheckOptionsValidation, MissingOptions) { + ClangTidyOptions Options; + ClangTidyContext Context(std::make_unique( + ClangTidyGlobalOptions(), Options)); + TestCheck TestCheck(&Context); + CHECK_ERROR(TestCheck.getLocal("Opt"), MissingOptionError, + "warning: Option not found 'test.Opt'\n"); + EXPECT_EQ(TestCheck.getLocal("Opt", "Unknown"), "Unknown"); +} + +TEST(CheckOptionsValidation, ValidIntOptions) { + ClangTidyOptions Options; + auto &CheckOptions = Options.CheckOptions; + CheckOptions["test.IntExpected1"] = "1"; + CheckOptions["test.IntExpected2"] = "1WithMore"; + CheckOptions["test.IntExpected3"] = "NoInt"; + CheckOptions["GlobalIntExpected1"] = "1"; + CheckOptions["GlobalIntExpected2"] = "NoInt"; + CheckOptions["test.DefaultedIntInvalid"] = "NoInt"; + CheckOptions["GlobalIntInvalid"] = "NoInt"; + + ClangTidyContext Context(std::make_unique( + ClangTidyGlobalOptions(), Options)); + TestCheck TestCheck(&Context); + +#define CHECK_ERROR_INT(Name, Expected) \ + CHECK_ERROR(Name, UnparseableIntegerOptionError, Expected) + + CHECK_VAL(TestCheck.getIntLocal("IntExpected1"), 1); + CHECK_VAL(TestCheck.getIntGlobal("GlobalIntExpected1"), 1); + CHECK_ERROR_INT(TestCheck.getIntLocal("IntExpected2"), + "warning: Invalid configuration value '1WithMore' for option " + "'test.IntExpected2'; expected an integer value\n"); + CHECK_ERROR_INT(TestCheck.getIntLocal("IntExpected3"), + "warning: Invalid configuration value 'NoInt' for option " + "'test.IntExpected3'; expected an integer value\n"); + CHECK_ERROR_INT(TestCheck.getIntGlobal("GlobalIntExpected2"), + "warning: Invalid configuration value 'NoInt' for option " + "'GlobalIntExpected2'; expected an integer value\n"); + ASSERT_EQ(TestCheck.getIntLocal("DefaultedIntInvalid", 1), 1); + ASSERT_EQ(TestCheck.getIntGlobal("GlobalIntInvalid", 1), 1); + +#undef CHECK_ERROR_INT +} + +TEST(ValidConfiguration, ValidEnumOptions) { + + enum class Colours { Red, Orange, Yellow, Green, Blue, Indigo, Violet }; + static constexpr std::pair Mapping[] = { + {"Red", Colours::Red}, {"Orange", Colours::Orange}, + {"Yellow", Colours::Yellow}, {"Green", Colours::Green}, + {"Blue", Colours::Blue}, {"Indigo", Colours::Indigo}, + {"Violet", Colours::Violet}}; + static const auto Map = makeArrayRef(Mapping); + + ClangTidyOptions Options; + auto &CheckOptions = Options.CheckOptions; + + CheckOptions["test.Valid"] = "Red"; + CheckOptions["test.Invalid"] = "Scarlet"; + CheckOptions["test.ValidWrongCase"] = "rED"; + CheckOptions["test.NearMiss"] = "Oragne"; + CheckOptions["GlobalValid"] = "Violet"; + CheckOptions["GlobalInvalid"] = "Purple"; + CheckOptions["GlobalValidWrongCase"] = "vIOLET"; + CheckOptions["GlobalNearMiss"] = "Yelow"; + + ClangTidyContext Context(std::make_unique( + ClangTidyGlobalOptions(), Options)); + TestCheck TestCheck(&Context); + +#define CHECK_ERROR_ENUM(Name, Expected) \ + CHECK_ERROR(Name, UnparseableEnumOptionError, Expected) + + CHECK_VAL(TestCheck.getLocal("Valid", Map), Colours::Red); + CHECK_VAL(TestCheck.getGlobal("GlobalValid", Map), Colours::Violet); + CHECK_VAL(TestCheck.getLocal("ValidWrongCase", Map, /*IgnoreCase*/ true), + Colours::Red); + CHECK_VAL( + TestCheck.getGlobal("GlobalValidWrongCase", Map, /*IgnoreCase*/ true), + Colours::Violet); + CHECK_ERROR_ENUM(TestCheck.getLocal("Invalid", Map), + "warning: Invalid configuration value " + "'Scarlet' for option 'test.Invalid'\n"); + CHECK_ERROR_ENUM(TestCheck.getLocal("ValidWrongCase", Map), + "warning: Invalid configuration value 'rED' for option " + "'test.ValidWrongCase'; did you mean 'Red'?\n"); + CHECK_ERROR_ENUM(TestCheck.getLocal("NearMiss", Map), + "warning: Invalid configuration value 'Oragne' for option " + "'test.NearMiss'; did you mean 'Orange'?\n"); + CHECK_ERROR_ENUM(TestCheck.getGlobal("GlobalInvalid", Map), + "warning: Invalid configuration value " + "'Purple' for option 'GlobalInvalid'\n"); + CHECK_ERROR_ENUM(TestCheck.getGlobal("GlobalValidWrongCase", Map), + "warning: Invalid configuration value 'vIOLET' for option " + "'GlobalValidWrongCase'; did you mean 'Violet'?\n"); + CHECK_ERROR_ENUM(TestCheck.getGlobal("GlobalNearMiss", Map), + "warning: Invalid configuration value 'Yelow' for option " + "'GlobalNearMiss'; did you mean 'Yellow'?\n"); + +#undef CHECK_ERROR_ENUM +} + +#undef CHECK_VAL +#undef CHECK_ERROR + } // namespace test } // namespace tidy } // namespace clang