[clang-tidy] Remove OptionError

The interface served a purpose, but since the ability to emit diagnostics when parsing configuration was added, its become mostly redundant. Emitting the diagnostic and removing the boilerplate is much cleaner.

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D97614
This commit is contained in:
Nathan James 2021-03-01 17:55:16 +00:00
parent e745f7c563
commit 82289aa6c8
No known key found for this signature in database
GPG Key ID: CC007AFCDA90AA5F
4 changed files with 240 additions and 359 deletions

View File

@ -15,30 +15,6 @@
namespace clang { namespace clang {
namespace tidy { namespace tidy {
char MissingOptionError::ID;
char UnparseableEnumOptionError::ID;
char UnparseableIntegerOptionError::ID;
std::string MissingOptionError::message() const {
llvm::SmallString<128> Buffer({"option not found '", OptionName, "'"});
return std::string(Buffer);
}
std::string UnparseableEnumOptionError::message() const {
llvm::SmallString<256> Buffer({"invalid configuration value '", LookupValue,
"' for option '", LookupName, "'"});
if (SuggestedValue)
Buffer.append({"; did you mean '", *SuggestedValue, "'?"});
return std::string(Buffer);
}
std::string UnparseableIntegerOptionError::message() const {
llvm::SmallString<256> Buffer({"invalid configuration value '", LookupValue,
"' for option '", LookupName, "'; expected ",
(IsBoolean ? "a bool" : "an integer value")});
return std::string(Buffer);
}
ClangTidyCheck::ClangTidyCheck(StringRef CheckName, ClangTidyContext *Context) ClangTidyCheck::ClangTidyCheck(StringRef CheckName, ClangTidyContext *Context)
: CheckName(CheckName), Context(Context), : CheckName(CheckName), Context(Context),
Options(CheckName, Context->getOptions().CheckOptions, Context) { Options(CheckName, Context->getOptions().CheckOptions, Context) {
@ -75,12 +51,12 @@ ClangTidyCheck::OptionsView::OptionsView(
: NamePrefix(CheckName.str() + "."), CheckOptions(CheckOptions), : NamePrefix(CheckName.str() + "."), CheckOptions(CheckOptions),
Context(Context) {} Context(Context) {}
llvm::Expected<std::string> llvm::Optional<std::string>
ClangTidyCheck::OptionsView::get(StringRef LocalName) const { ClangTidyCheck::OptionsView::get(StringRef LocalName) const {
const auto &Iter = CheckOptions.find(NamePrefix + LocalName.str()); const auto &Iter = CheckOptions.find(NamePrefix + LocalName.str());
if (Iter != CheckOptions.end()) if (Iter != CheckOptions.end())
return Iter->getValue().Value; return Iter->getValue().Value;
return llvm::make_error<MissingOptionError>((NamePrefix + LocalName).str()); return None;
} }
static ClangTidyOptions::OptionMap::const_iterator static ClangTidyOptions::OptionMap::const_iterator
@ -97,15 +73,15 @@ findPriorityOption(const ClangTidyOptions::OptionMap &Options, StringRef NamePre
return IterGlobal; return IterGlobal;
} }
llvm::Expected<std::string> llvm::Optional<std::string>
ClangTidyCheck::OptionsView::getLocalOrGlobal(StringRef LocalName) const { ClangTidyCheck::OptionsView::getLocalOrGlobal(StringRef LocalName) const {
auto Iter = findPriorityOption(CheckOptions, NamePrefix, LocalName); auto Iter = findPriorityOption(CheckOptions, NamePrefix, LocalName);
if (Iter != CheckOptions.end()) if (Iter != CheckOptions.end())
return Iter->getValue().Value; return Iter->getValue().Value;
return llvm::make_error<MissingOptionError>((NamePrefix + LocalName).str()); return None;
} }
static llvm::Expected<bool> getAsBool(StringRef Value, static Optional<bool> getAsBool(StringRef Value,
const llvm::Twine &LookupName) { const llvm::Twine &LookupName) {
if (llvm::Optional<bool> Parsed = llvm::yaml::parseBool(Value)) if (llvm::Optional<bool> Parsed = llvm::yaml::parseBool(Value))
@ -115,46 +91,30 @@ static llvm::Expected<bool> getAsBool(StringRef Value,
long long Number; long long Number;
if (!Value.getAsInteger(10, Number)) if (!Value.getAsInteger(10, Number))
return Number != 0; return Number != 0;
return llvm::make_error<UnparseableIntegerOptionError>(LookupName.str(), return None;
Value.str(), true);
} }
template <> template <>
llvm::Expected<bool> llvm::Optional<bool>
ClangTidyCheck::OptionsView::get<bool>(StringRef LocalName) const { ClangTidyCheck::OptionsView::get<bool>(StringRef LocalName) const {
llvm::Expected<std::string> ValueOr = get(LocalName); if (llvm::Optional<std::string> ValueOr = get(LocalName)) {
if (ValueOr) if (auto Result = getAsBool(*ValueOr, NamePrefix + LocalName))
return getAsBool(*ValueOr, NamePrefix + LocalName); return Result;
return ValueOr.takeError(); diagnoseBadBooleanOption(NamePrefix + LocalName, *ValueOr);
}
return None;
} }
template <> template <>
bool ClangTidyCheck::OptionsView::get<bool>(StringRef LocalName, llvm::Optional<bool>
bool Default) const {
llvm::Expected<bool> ValueOr = get<bool>(LocalName);
if (ValueOr)
return *ValueOr;
reportOptionParsingError(ValueOr.takeError());
return Default;
}
template <>
llvm::Expected<bool>
ClangTidyCheck::OptionsView::getLocalOrGlobal<bool>(StringRef LocalName) const { ClangTidyCheck::OptionsView::getLocalOrGlobal<bool>(StringRef LocalName) const {
auto Iter = findPriorityOption(CheckOptions, NamePrefix, LocalName); auto Iter = findPriorityOption(CheckOptions, NamePrefix, LocalName);
if (Iter != CheckOptions.end()) if (Iter != CheckOptions.end()) {
return getAsBool(Iter->getValue().Value, Iter->getKey()); if (auto Result = getAsBool(Iter->getValue().Value, Iter->getKey()))
return llvm::make_error<MissingOptionError>((NamePrefix + LocalName).str()); return Result;
} diagnoseBadBooleanOption(Iter->getKey(), Iter->getValue().Value);
}
template <> return None;
bool ClangTidyCheck::OptionsView::getLocalOrGlobal<bool>(StringRef LocalName,
bool Default) const {
llvm::Expected<bool> ValueOr = getLocalOrGlobal<bool>(LocalName);
if (ValueOr)
return *ValueOr;
reportOptionParsingError(ValueOr.takeError());
return Default;
} }
void ClangTidyCheck::OptionsView::store(ClangTidyOptions::OptionMap &Options, void ClangTidyCheck::OptionsView::store(ClangTidyOptions::OptionMap &Options,
@ -176,14 +136,14 @@ void ClangTidyCheck::OptionsView::store<bool>(
store(Options, LocalName, Value ? StringRef("true") : StringRef("false")); store(Options, LocalName, Value ? StringRef("true") : StringRef("false"));
} }
llvm::Expected<int64_t> ClangTidyCheck::OptionsView::getEnumInt( llvm::Optional<int64_t> ClangTidyCheck::OptionsView::getEnumInt(
StringRef LocalName, ArrayRef<NameAndValue> Mapping, bool CheckGlobal, StringRef LocalName, ArrayRef<NameAndValue> Mapping, bool CheckGlobal,
bool IgnoreCase) const { bool IgnoreCase) const {
auto Iter = CheckGlobal auto Iter = CheckGlobal
? findPriorityOption(CheckOptions, NamePrefix, LocalName) ? findPriorityOption(CheckOptions, NamePrefix, LocalName)
: CheckOptions.find((NamePrefix + LocalName).str()); : CheckOptions.find((NamePrefix + LocalName).str());
if (Iter == CheckOptions.end()) if (Iter == CheckOptions.end())
return llvm::make_error<MissingOptionError>((NamePrefix + LocalName).str()); return None;
StringRef Value = Iter->getValue().Value; StringRef Value = Iter->getValue().Value;
StringRef Closest; StringRef Closest;
@ -206,39 +166,54 @@ llvm::Expected<int64_t> ClangTidyCheck::OptionsView::getEnumInt(
} }
} }
if (EditDistance < 3) if (EditDistance < 3)
return llvm::make_error<UnparseableEnumOptionError>( diagnoseBadEnumOption(Iter->getKey().str(), Iter->getValue().Value,
Iter->getKey().str(), Iter->getValue().Value, Closest.str()); Closest);
return llvm::make_error<UnparseableEnumOptionError>(Iter->getKey().str(),
Iter->getValue().Value);
}
void ClangTidyCheck::OptionsView::reportOptionParsingError(
llvm::Error &&Err) const {
if (auto RemainingErrors =
llvm::handleErrors(std::move(Err), [](const MissingOptionError &) {}))
Context->configurationDiag(llvm::toString(std::move(RemainingErrors)));
}
template <>
Optional<std::string> ClangTidyCheck::OptionsView::getOptional<std::string>(
StringRef LocalName) const {
if (auto ValueOr = get(LocalName))
return *ValueOr;
else else
consumeError(ValueOr.takeError()); diagnoseBadEnumOption(Iter->getKey().str(), Iter->getValue().Value);
return llvm::None; return None;
} }
template <> static constexpr llvm::StringLiteral ConfigWarning(
Optional<std::string> "invalid configuration value '%0' for option '%1'%select{|; expected a "
ClangTidyCheck::OptionsView::getOptionalLocalOrGlobal<std::string>( "bool|; expected an integer|; did you mean '%3'?}2");
StringRef LocalName) const {
if (auto ValueOr = getLocalOrGlobal(LocalName)) void ClangTidyCheck::OptionsView::diagnoseBadBooleanOption(
return *ValueOr; const Twine &Lookup, StringRef Unparsed) const {
SmallString<64> Buffer;
Context->configurationDiag(ConfigWarning)
<< Unparsed << Lookup.toStringRef(Buffer) << 1;
}
void ClangTidyCheck::OptionsView::diagnoseBadIntegerOption(
const Twine &Lookup, StringRef Unparsed) const {
SmallString<64> Buffer;
Context->configurationDiag(ConfigWarning)
<< Unparsed << Lookup.toStringRef(Buffer) << 2;
}
void ClangTidyCheck::OptionsView::diagnoseBadEnumOption(
const Twine &Lookup, StringRef Unparsed, StringRef Suggestion) const {
SmallString<64> Buffer;
auto Diag = Context->configurationDiag(ConfigWarning)
<< Unparsed << Lookup.toStringRef(Buffer);
if (Suggestion.empty())
Diag << 0;
else else
consumeError(ValueOr.takeError()); Diag << 3 << Suggestion;
return llvm::None;
} }
std::string ClangTidyCheck::OptionsView::get(StringRef LocalName,
StringRef Default) const {
if (llvm::Optional<std::string> Val = get(LocalName))
return std::move(*Val);
return Default.str();
}
std::string
ClangTidyCheck::OptionsView::getLocalOrGlobal(StringRef LocalName,
StringRef Default) const {
if (llvm::Optional<std::string> Val = getLocalOrGlobal(LocalName))
return std::move(*Val);
return Default.str();
}
} // namespace tidy } // namespace tidy
} // namespace clang } // namespace clang

View File

@ -14,7 +14,6 @@
#include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/Basic/Diagnostic.h" #include "clang/Basic/Diagnostic.h"
#include "llvm/ADT/Optional.h" #include "llvm/ADT/Optional.h"
#include "llvm/Support/Error.h"
#include <type_traits> #include <type_traits>
#include <utility> #include <utility>
#include <vector> #include <vector>
@ -33,65 +32,6 @@ template <class T> struct OptionEnumMapping {
static ArrayRef<std::pair<T, StringRef>> getEnumMapping() = delete; static ArrayRef<std::pair<T, StringRef>> getEnumMapping() = delete;
}; };
template <typename T> class OptionError : public llvm::ErrorInfo<T> {
std::error_code convertToErrorCode() const override {
return llvm::inconvertibleErrorCode();
}
public:
void log(raw_ostream &OS) const override { OS << this->message(); }
};
class MissingOptionError : public OptionError<MissingOptionError> {
public:
explicit MissingOptionError(std::string OptionName)
: OptionName(OptionName) {}
std::string message() const override;
static char ID;
private:
const std::string OptionName;
};
class UnparseableEnumOptionError
: public OptionError<UnparseableEnumOptionError> {
public:
explicit UnparseableEnumOptionError(std::string LookupName,
std::string LookupValue)
: LookupName(LookupName), LookupValue(LookupValue) {}
explicit 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:
const std::string LookupName;
const std::string LookupValue;
const llvm::Optional<std::string> SuggestedValue;
};
class UnparseableIntegerOptionError
: public OptionError<UnparseableIntegerOptionError> {
public:
explicit UnparseableIntegerOptionError(std::string LookupName,
std::string LookupValue,
bool IsBoolean = false)
: LookupName(LookupName), LookupValue(LookupValue), IsBoolean(IsBoolean) {
}
std::string message() const override;
static char ID;
private:
const std::string LookupName;
const std::string LookupValue;
const bool IsBoolean;
};
/// Base class for all clang-tidy checks. /// Base class for all clang-tidy checks.
/// ///
/// To implement a ``ClangTidyCheck``, write a subclass and override some of the /// To implement a ``ClangTidyCheck``, write a subclass and override some of the
@ -198,6 +138,13 @@ public:
/// Methods of this class prepend ``CheckName + "."`` to translate check-local /// Methods of this class prepend ``CheckName + "."`` to translate check-local
/// option names to global option names. /// option names to global option names.
class OptionsView { class OptionsView {
void diagnoseBadIntegerOption(const Twine &Lookup,
StringRef Unparsed) const;
void diagnoseBadBooleanOption(const Twine &Lookup,
StringRef Unparsed) const;
void diagnoseBadEnumOption(const Twine &Lookup, StringRef Unparsed,
StringRef Suggestion = StringRef()) const;
public: public:
/// Initializes the instance using \p CheckName + "." as a prefix. /// Initializes the instance using \p CheckName + "." as a prefix.
OptionsView(StringRef CheckName, OptionsView(StringRef CheckName,
@ -207,30 +154,24 @@ public:
/// Read a named option from the ``Context``. /// Read a named option from the ``Context``.
/// ///
/// Reads the option with the check-local name \p LocalName from the /// Reads the option with the check-local name \p LocalName from the
/// ``CheckOptions``. If the corresponding key is not present, returns /// ``CheckOptions``. If the corresponding key is not present, return
/// a ``MissingOptionError``. /// ``None``.
llvm::Expected<std::string> get(StringRef LocalName) const; llvm::Optional<std::string> get(StringRef LocalName) const;
/// Read a named option from the ``Context``. /// Read a named option from the ``Context``.
/// ///
/// Reads the option with the check-local name \p LocalName from the /// Reads the option with the check-local name \p LocalName from the
/// ``CheckOptions``. If the corresponding key is not present, returns /// ``CheckOptions``. If the corresponding key is not present, returns
/// \p Default. /// \p Default.
std::string get(StringRef LocalName, StringRef Default) const { std::string get(StringRef LocalName, StringRef Default) const;
if (llvm::Expected<std::string> Val = get(LocalName))
return *Val;
else
llvm::consumeError(Val.takeError());
return Default.str();
}
/// Read a named option from the ``Context``. /// Read a named option from the ``Context``.
/// ///
/// Reads the option with the check-local name \p LocalName from local or /// Reads the option with the check-local name \p LocalName from local or
/// global ``CheckOptions``. Gets local option first. If local is not /// global ``CheckOptions``. Gets local option first. If local is not
/// present, falls back to get global option. If global option is not /// present, falls back to get global option. If global option is not
/// present either, returns a ``MissingOptionError``. /// present either, return ``None``.
llvm::Expected<std::string> getLocalOrGlobal(StringRef LocalName) const; llvm::Optional<std::string> getLocalOrGlobal(StringRef LocalName) const;
/// Read a named option from the ``Context``. /// Read a named option from the ``Context``.
/// ///
@ -238,48 +179,42 @@ public:
/// global ``CheckOptions``. Gets local option first. If local is not /// global ``CheckOptions``. Gets local option first. If local is not
/// present, falls back to get global option. If global option is not /// present, falls back to get global option. If global option is not
/// present either, returns \p Default. /// present either, returns \p Default.
std::string getLocalOrGlobal(StringRef LocalName, StringRef Default) const { std::string getLocalOrGlobal(StringRef LocalName, StringRef Default) const;
if (llvm::Expected<std::string> Val = getLocalOrGlobal(LocalName))
return *Val;
else
llvm::consumeError(Val.takeError());
return Default.str();
}
/// Read a named option from the ``Context`` and parse it as an /// Read a named option from the ``Context`` and parse it as an
/// integral type ``T``. /// integral type ``T``.
/// ///
/// Reads the option with the check-local name \p LocalName from the /// Reads the option with the check-local name \p LocalName from the
/// ``CheckOptions``. If the corresponding key is not present, returns /// ``CheckOptions``. If the corresponding key is not present, return
/// a ``MissingOptionError``. If the corresponding key can't be parsed as /// ``None``.
/// a ``T``, return an ``UnparseableIntegerOptionError``. ///
/// If the corresponding key can't be parsed as a ``T``, emit a
/// diagnostic and return ``None``.
template <typename T> template <typename T>
std::enable_if_t<std::is_integral<T>::value, llvm::Expected<T>> std::enable_if_t<std::is_integral<T>::value, llvm::Optional<T>>
get(StringRef LocalName) const { get(StringRef LocalName) const {
if (llvm::Expected<std::string> Value = get(LocalName)) { if (llvm::Optional<std::string> Value = get(LocalName)) {
T Result{}; T Result{};
if (!StringRef(*Value).getAsInteger(10, Result)) if (!StringRef(*Value).getAsInteger(10, Result))
return Result; return Result;
return llvm::make_error<UnparseableIntegerOptionError>( diagnoseBadIntegerOption(NamePrefix + LocalName, *Value);
(NamePrefix + LocalName).str(), *Value); }
} else return None;
return std::move(Value.takeError());
} }
/// Read a named option from the ``Context`` and parse it as an /// Read a named option from the ``Context`` and parse it as an
/// integral type ``T``. /// integral type ``T``.
/// ///
/// Reads the option with the check-local name \p LocalName from the /// 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 /// ``CheckOptions``. If the corresponding key is not present, return
/// parsed as a ``T``, returns \p Default. /// \p Default.
///
/// If the corresponding key can't be parsed as a ``T``, emit a
/// diagnostic and return \p Default.
template <typename T> template <typename T>
std::enable_if_t<std::is_integral<T>::value, T> get(StringRef LocalName, std::enable_if_t<std::is_integral<T>::value, T> get(StringRef LocalName,
T Default) const { T Default) const {
if (llvm::Expected<T> ValueOr = get<T>(LocalName)) return get<T>(LocalName).getValueOr(Default);
return *ValueOr;
else
reportOptionParsingError(ValueOr.takeError());
return Default;
} }
/// Read a named option from the ``Context`` and parse it as an /// Read a named option from the ``Context`` and parse it as an
@ -288,27 +223,27 @@ public:
/// Reads the option with the check-local name \p LocalName from local or /// Reads the option with the check-local name \p LocalName from local or
/// global ``CheckOptions``. Gets local option first. If local is not /// global ``CheckOptions``. Gets local option first. If local is not
/// present, falls back to get global option. If global option is not /// present, falls back to get global option. If global option is not
/// present either, returns a ``MissingOptionError``. If the corresponding /// present either, return ``None``.
/// key can't be parsed as a ``T``, return an ///
/// ``UnparseableIntegerOptionError``. /// If the corresponding key can't be parsed as a ``T``, emit a
/// diagnostic and return ``None``.
template <typename T> template <typename T>
std::enable_if_t<std::is_integral<T>::value, llvm::Expected<T>> std::enable_if_t<std::is_integral<T>::value, llvm::Optional<T>>
getLocalOrGlobal(StringRef LocalName) const { getLocalOrGlobal(StringRef LocalName) const {
llvm::Expected<std::string> ValueOr = get(LocalName); llvm::Optional<std::string> ValueOr = get(LocalName);
bool IsGlobal = false; bool IsGlobal = false;
if (!ValueOr) { if (!ValueOr) {
IsGlobal = true; IsGlobal = true;
llvm::consumeError(ValueOr.takeError());
ValueOr = getLocalOrGlobal(LocalName); ValueOr = getLocalOrGlobal(LocalName);
if (!ValueOr) if (!ValueOr)
return std::move(ValueOr.takeError()); return None;
} }
T Result{}; T Result{};
if (!StringRef(*ValueOr).getAsInteger(10, Result)) if (!StringRef(*ValueOr).getAsInteger(10, Result))
return Result; return Result;
return llvm::make_error<UnparseableIntegerOptionError>( diagnoseBadIntegerOption(
(IsGlobal ? LocalName.str() : (NamePrefix + LocalName).str()), IsGlobal ? Twine(LocalName) : NamePrefix + LocalName, *ValueOr);
*ValueOr); return None;
} }
/// Read a named option from the ``Context`` and parse it as an /// Read a named option from the ``Context`` and parse it as an
@ -317,54 +252,53 @@ public:
/// Reads the option with the check-local name \p LocalName from local or /// Reads the option with the check-local name \p LocalName from local or
/// global ``CheckOptions``. Gets local option first. If local is not /// global ``CheckOptions``. Gets local option first. If local is not
/// present, falls back to get global option. If global option 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. /// present either, return \p Default.
///
/// If the corresponding key can't be parsed as a ``T``, emit a
/// diagnostic and return \p Default.
template <typename T> template <typename T>
std::enable_if_t<std::is_integral<T>::value, T> std::enable_if_t<std::is_integral<T>::value, T>
getLocalOrGlobal(StringRef LocalName, T Default) const { getLocalOrGlobal(StringRef LocalName, T Default) const {
if (llvm::Expected<T> ValueOr = getLocalOrGlobal<T>(LocalName)) return getLocalOrGlobal<T>(LocalName).getValueOr(Default);
return *ValueOr;
else
reportOptionParsingError(ValueOr.takeError());
return Default;
} }
/// Read a named option from the ``Context`` and parse it as an /// Read a named option from the ``Context`` and parse it as an
/// enum type ``T``. /// enum type ``T``.
/// ///
/// Reads the option with the check-local name \p LocalName from the /// Reads the option with the check-local name \p LocalName from the
/// ``CheckOptions``. If the corresponding key is not present, returns a /// ``CheckOptions``. If the corresponding key is not present, return
/// ``MissingOptionError``. If the key can't be parsed as a ``T`` returns a /// ``None``.
/// ``UnparseableEnumOptionError``. ///
/// If the corresponding key can't be parsed as a ``T``, emit a
/// diagnostic and return ``None``.
/// ///
/// \ref clang::tidy::OptionEnumMapping must be specialized for ``T`` to /// \ref clang::tidy::OptionEnumMapping must be specialized for ``T`` to
/// supply the mapping required to convert between ``T`` and a string. /// supply the mapping required to convert between ``T`` and a string.
template <typename T> template <typename T>
std::enable_if_t<std::is_enum<T>::value, llvm::Expected<T>> std::enable_if_t<std::is_enum<T>::value, llvm::Optional<T>>
get(StringRef LocalName, bool IgnoreCase = false) const { get(StringRef LocalName, bool IgnoreCase = false) const {
if (llvm::Expected<int64_t> ValueOr = if (llvm::Optional<int64_t> ValueOr =
getEnumInt(LocalName, typeEraseMapping<T>(), false, IgnoreCase)) getEnumInt(LocalName, typeEraseMapping<T>(), false, IgnoreCase))
return static_cast<T>(*ValueOr); return static_cast<T>(*ValueOr);
else return None;
return std::move(ValueOr.takeError());
} }
/// Read a named option from the ``Context`` and parse it as an /// Read a named option from the ``Context`` and parse it as an
/// enum type ``T``. /// enum type ``T``.
/// ///
/// Reads the option with the check-local name \p LocalName from the /// 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 /// ``CheckOptions``. If the corresponding key is not present, return
/// parsed as a ``T``, returns \p Default. /// \p Default.
///
/// If the corresponding key can't be parsed as a ``T``, emit a
/// diagnostic and return \p Default.
/// ///
/// \ref clang::tidy::OptionEnumMapping must be specialized for ``T`` to /// \ref clang::tidy::OptionEnumMapping must be specialized for ``T`` to
/// supply the mapping required to convert between ``T`` and a string. /// supply the mapping required to convert between ``T`` and a string.
template <typename T> template <typename T>
std::enable_if_t<std::is_enum<T>::value, T> std::enable_if_t<std::is_enum<T>::value, T>
get(StringRef LocalName, T Default, bool IgnoreCase = false) const { get(StringRef LocalName, T Default, bool IgnoreCase = false) const {
if (auto ValueOr = get<T>(LocalName, IgnoreCase)) return get<T>(LocalName, IgnoreCase).getValueOr(Default);
return *ValueOr;
else
reportOptionParsingError(ValueOr.takeError());
return Default;
} }
/// Read a named option from the ``Context`` and parse it as an /// Read a named option from the ``Context`` and parse it as an
@ -373,19 +307,20 @@ public:
/// Reads the option with the check-local name \p LocalName from local or /// Reads the option with the check-local name \p LocalName from local or
/// global ``CheckOptions``. Gets local option first. If local is not /// global ``CheckOptions``. Gets local option first. If local is not
/// present, falls back to get global option. If global option 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 /// present either, returns ``None``.
/// parsed as a ``T`` returns a ``UnparseableEnumOptionError``. ///
/// If the corresponding key can't be parsed as a ``T``, emit a
/// diagnostic and return ``None``.
/// ///
/// \ref clang::tidy::OptionEnumMapping must be specialized for ``T`` to /// \ref clang::tidy::OptionEnumMapping must be specialized for ``T`` to
/// supply the mapping required to convert between ``T`` and a string. /// supply the mapping required to convert between ``T`` and a string.
template <typename T> template <typename T>
std::enable_if_t<std::is_enum<T>::value, llvm::Expected<T>> std::enable_if_t<std::is_enum<T>::value, llvm::Optional<T>>
getLocalOrGlobal(StringRef LocalName, bool IgnoreCase = false) const { getLocalOrGlobal(StringRef LocalName, bool IgnoreCase = false) const {
if (llvm::Expected<int64_t> ValueOr = if (llvm::Optional<int64_t> ValueOr =
getEnumInt(LocalName, typeEraseMapping<T>(), true, IgnoreCase)) getEnumInt(LocalName, typeEraseMapping<T>(), true, IgnoreCase))
return static_cast<T>(*ValueOr); return static_cast<T>(*ValueOr);
else return None;
return std::move(ValueOr.takeError());
} }
/// Read a named option from the ``Context`` and parse it as an /// Read a named option from the ``Context`` and parse it as an
@ -394,7 +329,10 @@ public:
/// Reads the option with the check-local name \p LocalName from local or /// Reads the option with the check-local name \p LocalName from local or
/// global ``CheckOptions``. Gets local option first. If local is not /// global ``CheckOptions``. Gets local option first. If local is not
/// present, falls back to get global option. If global option 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. /// present either return \p Default.
///
/// If the corresponding key can't be parsed as a ``T``, emit a
/// diagnostic and return \p Default.
/// ///
/// \ref clang::tidy::OptionEnumMapping must be specialized for ``T`` to /// \ref clang::tidy::OptionEnumMapping must be specialized for ``T`` to
/// supply the mapping required to convert between ``T`` and a string. /// supply the mapping required to convert between ``T`` and a string.
@ -402,36 +340,7 @@ public:
std::enable_if_t<std::is_enum<T>::value, T> std::enable_if_t<std::is_enum<T>::value, T>
getLocalOrGlobal(StringRef LocalName, T Default, getLocalOrGlobal(StringRef LocalName, T Default,
bool IgnoreCase = false) const { bool IgnoreCase = false) const {
if (auto ValueOr = getLocalOrGlobal<T>(LocalName, IgnoreCase)) return getLocalOrGlobal<T>(LocalName, IgnoreCase).getValueOr(Default);
return *ValueOr;
else
reportOptionParsingError(ValueOr.takeError());
return Default;
}
/// Returns the value for the option \p LocalName represented as a ``T``.
/// If the option is missing returns None, if the option can't be parsed
/// as a ``T``, log that to stderr and return None.
template <typename T = std::string>
llvm::Optional<T> getOptional(StringRef LocalName) const {
if (auto ValueOr = get<T>(LocalName))
return *ValueOr;
else
reportOptionParsingError(ValueOr.takeError());
return llvm::None;
}
/// Returns the value for the local or global option \p LocalName
/// represented as a ``T``.
/// If the option is missing returns None, if the
/// option can't be parsed as a ``T``, log that to stderr and return None.
template <typename T = std::string>
llvm::Optional<T> getOptionalLocalOrGlobal(StringRef LocalName) const {
if (auto ValueOr = getLocalOrGlobal<T>(LocalName))
return *ValueOr;
else
reportOptionParsingError(ValueOr.takeError());
return llvm::None;
} }
/// Stores an option with the check-local name \p LocalName with /// Stores an option with the check-local name \p LocalName with
@ -470,7 +379,7 @@ public:
private: private:
using NameAndValue = std::pair<int64_t, StringRef>; using NameAndValue = std::pair<int64_t, StringRef>;
llvm::Expected<int64_t> getEnumInt(StringRef LocalName, llvm::Optional<int64_t> getEnumInt(StringRef LocalName,
ArrayRef<NameAndValue> Mapping, ArrayRef<NameAndValue> Mapping,
bool CheckGlobal, bool IgnoreCase) const; bool CheckGlobal, bool IgnoreCase) const;
@ -491,8 +400,6 @@ public:
void storeInt(ClangTidyOptions::OptionMap &Options, StringRef LocalName, void storeInt(ClangTidyOptions::OptionMap &Options, StringRef LocalName,
int64_t Value) const; int64_t Value) const;
/// Emits a diagnostic if \p Err is not a MissingOptionError.
void reportOptionParsingError(llvm::Error &&Err) const;
std::string NamePrefix; std::string NamePrefix;
const ClangTidyOptions::OptionMap &CheckOptions; const ClangTidyOptions::OptionMap &CheckOptions;
@ -516,44 +423,27 @@ protected:
/// Read a named option from the ``Context`` and parse it as a bool. /// Read a named option from the ``Context`` and parse it as a bool.
/// ///
/// Reads the option with the check-local name \p LocalName from the /// Reads the option with the check-local name \p LocalName from the
/// ``CheckOptions``. If the corresponding key is not present, returns /// ``CheckOptions``. If the corresponding key is not present, return
/// a ``MissingOptionError``. If the corresponding key can't be parsed as /// ``None``.
/// a bool, return an ``UnparseableIntegerOptionError``. ///
/// If the corresponding key can't be parsed as a bool, emit a
/// diagnostic and return ``None``.
template <> template <>
llvm::Expected<bool> llvm::Optional<bool>
ClangTidyCheck::OptionsView::get<bool>(StringRef LocalName) const; ClangTidyCheck::OptionsView::get<bool>(StringRef LocalName) const;
/// Read a named option from the ``Context`` and parse it as a bool. /// Read a named option from the ``Context`` and parse it as a bool.
/// ///
/// Reads the option with the check-local name \p LocalName from the /// 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 /// ``CheckOptions``. If the corresponding key is not present, return
/// parsed as a bool, returns \p Default. /// \p Default.
template <>
bool ClangTidyCheck::OptionsView::get<bool>(StringRef LocalName,
bool Default) const;
/// Read a named option from the ``Context`` and parse it as a bool.
/// ///
/// Reads the option with the check-local name \p LocalName from local or /// If the corresponding key can't be parsed as a bool, emit a
/// global ``CheckOptions``. Gets local option first. If local is not /// diagnostic and return \p Default.
/// 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 bool, return an
/// ``UnparseableIntegerOptionError``.
template <> template <>
llvm::Expected<bool> llvm::Optional<bool>
ClangTidyCheck::OptionsView::getLocalOrGlobal<bool>(StringRef LocalName) const; ClangTidyCheck::OptionsView::getLocalOrGlobal<bool>(StringRef LocalName) const;
/// Read a named option from the ``Context`` and parse it as a bool.
///
/// 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 bool, returns \p Default.
template <>
bool ClangTidyCheck::OptionsView::getLocalOrGlobal<bool>(StringRef LocalName,
bool Default) const;
/// Stores an option with the check-local name \p LocalName with /// Stores an option with the check-local name \p LocalName with
/// bool value \p Value to \p Options. /// bool value \p Value to \p Options.
template <> template <>
@ -561,18 +451,6 @@ void ClangTidyCheck::OptionsView::store<bool>(
ClangTidyOptions::OptionMap &Options, StringRef LocalName, ClangTidyOptions::OptionMap &Options, StringRef LocalName,
bool Value) const; bool Value) const;
/// Returns the value for the option \p LocalName.
/// If the option is missing returns None.
template <>
Optional<std::string> ClangTidyCheck::OptionsView::getOptional<std::string>(
StringRef LocalName) const;
/// Returns the value for the local or global option \p LocalName.
/// If the option is missing returns None.
template <>
Optional<std::string>
ClangTidyCheck::OptionsView::getOptionalLocalOrGlobal<std::string>(
StringRef LocalName) const;
} // namespace tidy } // namespace tidy
} // namespace clang } // namespace clang

View File

@ -157,7 +157,7 @@ getFileStyleFromOptions(const ClangTidyCheck::OptionsView &Options) {
StyleString.pop_back(); StyleString.pop_back();
StyleString.pop_back(); StyleString.pop_back();
auto CaseOptional = auto CaseOptional =
Options.getOptional<IdentifierNamingCheck::CaseType>(StyleString); Options.get<IdentifierNamingCheck::CaseType>(StyleString);
if (CaseOptional || !Prefix.empty() || !Postfix.empty() || if (CaseOptional || !Prefix.empty() || !Postfix.empty() ||
!IgnoredRegexpStr.empty()) !IgnoredRegexpStr.empty())

View File

@ -190,6 +190,7 @@ MATCHER_P(DiagRange, P, "") { return arg.Range && *arg.Range == P; }
using ::testing::AllOf; using ::testing::AllOf;
using ::testing::ElementsAre; using ::testing::ElementsAre;
using ::testing::UnorderedElementsAre;
TEST(ParseConfiguration, CollectDiags) { TEST(ParseConfiguration, CollectDiags) {
DiagCollecter Collector; DiagCollecter Collector;
@ -243,7 +244,6 @@ public:
return Options.getLocalOrGlobal<IntType>(std::forward<Args>(Arguments)...); return Options.getLocalOrGlobal<IntType>(std::forward<Args>(Arguments)...);
} }
}; };
} // namespace
#define CHECK_VAL(Value, Expected) \ #define CHECK_VAL(Value, Expected) \
do { \ do { \
@ -252,17 +252,22 @@ public:
EXPECT_EQ(*Item, Expected); \ EXPECT_EQ(*Item, Expected); \
} while (false) } while (false)
#define CHECK_ERROR(Value, ErrorType, ExpectedMessage) \ MATCHER_P(ToolDiagMessage, M, "") { return arg.Message.Message == M; }
do { \ MATCHER_P(ToolDiagLevel, L, "") { return arg.DiagLevel == L; }
auto Item = Value; \
ASSERT_FALSE(Item); \ } // namespace
ASSERT_TRUE(Item.errorIsA<ErrorType>()); \
ASSERT_FALSE(llvm::handleErrors( \ } // namespace test
Item.takeError(), [&](const ErrorType &Err) -> llvm::Error { \
EXPECT_EQ(Err.message(), ExpectedMessage); \ static constexpr auto Warning = tooling::Diagnostic::Warning;
return llvm::Error::success(); \ static constexpr auto Error = tooling::Diagnostic::Error;
})); \
} while (false) static void PrintTo(const ClangTidyError &Err, ::std::ostream *OS) {
*OS << (Err.DiagLevel == Error ? "error: " : "warning: ")
<< Err.Message.Message;
}
namespace test {
TEST(CheckOptionsValidation, MissingOptions) { TEST(CheckOptionsValidation, MissingOptions) {
ClangTidyOptions Options; ClangTidyOptions Options;
@ -273,21 +278,21 @@ TEST(CheckOptionsValidation, MissingOptions) {
&DiagConsumer, false); &DiagConsumer, false);
Context.setDiagnosticsEngine(&DE); Context.setDiagnosticsEngine(&DE);
TestCheck TestCheck(&Context); TestCheck TestCheck(&Context);
CHECK_ERROR(TestCheck.getLocal("Opt"), MissingOptionError, EXPECT_FALSE(TestCheck.getLocal("Opt").hasValue());
"option not found 'test.Opt'");
EXPECT_EQ(TestCheck.getLocal("Opt", "Unknown"), "Unknown"); EXPECT_EQ(TestCheck.getLocal("Opt", "Unknown"), "Unknown");
// Missing options aren't errors.
EXPECT_TRUE(DiagConsumer.take().empty());
} }
TEST(CheckOptionsValidation, ValidIntOptions) { TEST(CheckOptionsValidation, ValidIntOptions) {
ClangTidyOptions Options; ClangTidyOptions Options;
auto &CheckOptions = Options.CheckOptions; auto &CheckOptions = Options.CheckOptions;
CheckOptions["test.IntExpected1"] = "1"; CheckOptions["test.IntExpected"] = "1";
CheckOptions["test.IntExpected2"] = "1WithMore"; CheckOptions["test.IntInvalid1"] = "1WithMore";
CheckOptions["test.IntExpected3"] = "NoInt"; CheckOptions["test.IntInvalid2"] = "NoInt";
CheckOptions["GlobalIntExpected1"] = "1"; CheckOptions["GlobalIntExpected"] = "1";
CheckOptions["GlobalIntExpected2"] = "NoInt";
CheckOptions["test.DefaultedIntInvalid"] = "NoInt";
CheckOptions["GlobalIntInvalid"] = "NoInt"; CheckOptions["GlobalIntInvalid"] = "NoInt";
CheckOptions["test.DefaultedIntInvalid"] = "NoInt";
CheckOptions["test.BoolITrueValue"] = "1"; CheckOptions["test.BoolITrueValue"] = "1";
CheckOptions["test.BoolIFalseValue"] = "0"; CheckOptions["test.BoolIFalseValue"] = "0";
CheckOptions["test.BoolTrueValue"] = "true"; CheckOptions["test.BoolTrueValue"] = "true";
@ -304,22 +309,12 @@ TEST(CheckOptionsValidation, ValidIntOptions) {
Context.setDiagnosticsEngine(&DE); Context.setDiagnosticsEngine(&DE);
TestCheck TestCheck(&Context); TestCheck TestCheck(&Context);
#define CHECK_ERROR_INT(Name, Expected) \ CHECK_VAL(TestCheck.getIntLocal("IntExpected"), 1);
CHECK_ERROR(Name, UnparseableIntegerOptionError, Expected) CHECK_VAL(TestCheck.getIntGlobal("GlobalIntExpected"), 1);
EXPECT_FALSE(TestCheck.getIntLocal("IntInvalid1").hasValue());
CHECK_VAL(TestCheck.getIntLocal("IntExpected1"), 1); EXPECT_FALSE(TestCheck.getIntLocal("IntInvalid2").hasValue());
CHECK_VAL(TestCheck.getIntGlobal("GlobalIntExpected1"), 1); EXPECT_FALSE(TestCheck.getIntGlobal("GlobalIntInvalid").hasValue());
CHECK_ERROR_INT(TestCheck.getIntLocal("IntExpected2"),
"invalid configuration value '1WithMore' for option "
"'test.IntExpected2'; expected an integer value");
CHECK_ERROR_INT(TestCheck.getIntLocal("IntExpected3"),
"invalid configuration value 'NoInt' for option "
"'test.IntExpected3'; expected an integer value");
CHECK_ERROR_INT(TestCheck.getIntGlobal("GlobalIntExpected2"),
"invalid configuration value 'NoInt' for option "
"'GlobalIntExpected2'; expected an integer value");
ASSERT_EQ(TestCheck.getIntLocal("DefaultedIntInvalid", 1), 1); ASSERT_EQ(TestCheck.getIntLocal("DefaultedIntInvalid", 1), 1);
ASSERT_EQ(TestCheck.getIntGlobal("GlobalIntInvalid", 1), 1);
CHECK_VAL(TestCheck.getIntLocal<bool>("BoolITrueValue"), true); CHECK_VAL(TestCheck.getIntLocal<bool>("BoolITrueValue"), true);
CHECK_VAL(TestCheck.getIntLocal<bool>("BoolIFalseValue"), false); CHECK_VAL(TestCheck.getIntLocal<bool>("BoolIFalseValue"), false);
@ -327,11 +322,31 @@ TEST(CheckOptionsValidation, ValidIntOptions) {
CHECK_VAL(TestCheck.getIntLocal<bool>("BoolFalseValue"), false); CHECK_VAL(TestCheck.getIntLocal<bool>("BoolFalseValue"), false);
CHECK_VAL(TestCheck.getIntLocal<bool>("BoolTrueShort"), true); CHECK_VAL(TestCheck.getIntLocal<bool>("BoolTrueShort"), true);
CHECK_VAL(TestCheck.getIntLocal<bool>("BoolFalseShort"), false); CHECK_VAL(TestCheck.getIntLocal<bool>("BoolFalseShort"), false);
CHECK_ERROR_INT(TestCheck.getIntLocal<bool>("BoolUnparseable"), EXPECT_FALSE(TestCheck.getIntLocal<bool>("BoolUnparseable").hasValue());
"invalid configuration value 'Nothing' for option "
"'test.BoolUnparseable'; expected a bool");
#undef CHECK_ERROR_INT EXPECT_THAT(
DiagConsumer.take(),
UnorderedElementsAre(
AllOf(ToolDiagMessage(
"invalid configuration value '1WithMore' for option "
"'test.IntInvalid1'; expected an integer"),
ToolDiagLevel(Warning)),
AllOf(
ToolDiagMessage("invalid configuration value 'NoInt' for option "
"'test.IntInvalid2'; expected an integer"),
ToolDiagLevel(Warning)),
AllOf(
ToolDiagMessage("invalid configuration value 'NoInt' for option "
"'GlobalIntInvalid'; expected an integer"),
ToolDiagLevel(Warning)),
AllOf(ToolDiagMessage(
"invalid configuration value 'NoInt' for option "
"'test.DefaultedIntInvalid'; expected an integer"),
ToolDiagLevel(Warning)),
AllOf(ToolDiagMessage(
"invalid configuration value 'Nothing' for option "
"'test.BoolUnparseable'; expected a bool"),
ToolDiagLevel(Warning))));
} }
TEST(ValidConfiguration, ValidEnumOptions) { TEST(ValidConfiguration, ValidEnumOptions) {
@ -350,11 +365,12 @@ TEST(ValidConfiguration, ValidEnumOptions) {
ClangTidyContext Context(std::make_unique<DefaultOptionsProvider>( ClangTidyContext Context(std::make_unique<DefaultOptionsProvider>(
ClangTidyGlobalOptions(), Options)); ClangTidyGlobalOptions(), Options));
ClangTidyDiagnosticConsumer DiagConsumer(Context);
DiagnosticsEngine DE(new DiagnosticIDs(), new DiagnosticOptions,
&DiagConsumer, false);
Context.setDiagnosticsEngine(&DE);
TestCheck TestCheck(&Context); TestCheck TestCheck(&Context);
#define CHECK_ERROR_ENUM(Name, Expected) \
CHECK_ERROR(Name, UnparseableEnumOptionError, Expected)
CHECK_VAL(TestCheck.getIntLocal<Colours>("Valid"), Colours::Red); CHECK_VAL(TestCheck.getIntLocal<Colours>("Valid"), Colours::Red);
CHECK_VAL(TestCheck.getIntGlobal<Colours>("GlobalValid"), Colours::Violet); CHECK_VAL(TestCheck.getIntGlobal<Colours>("GlobalValid"), Colours::Violet);
@ -364,30 +380,42 @@ TEST(ValidConfiguration, ValidEnumOptions) {
CHECK_VAL(TestCheck.getIntGlobal<Colours>("GlobalValidWrongCase", CHECK_VAL(TestCheck.getIntGlobal<Colours>("GlobalValidWrongCase",
/*IgnoreCase*/ true), /*IgnoreCase*/ true),
Colours::Violet); Colours::Violet);
CHECK_ERROR_ENUM(TestCheck.getIntLocal<Colours>("Invalid"),
"invalid configuration value "
"'Scarlet' for option 'test.Invalid'");
CHECK_ERROR_ENUM(TestCheck.getIntLocal<Colours>("ValidWrongCase"),
"invalid configuration value 'rED' for option "
"'test.ValidWrongCase'; did you mean 'Red'?");
CHECK_ERROR_ENUM(TestCheck.getIntLocal<Colours>("NearMiss"),
"invalid configuration value 'Oragne' for option "
"'test.NearMiss'; did you mean 'Orange'?");
CHECK_ERROR_ENUM(TestCheck.getIntGlobal<Colours>("GlobalInvalid"),
"invalid configuration value "
"'Purple' for option 'GlobalInvalid'");
CHECK_ERROR_ENUM(TestCheck.getIntGlobal<Colours>("GlobalValidWrongCase"),
"invalid configuration value 'vIOLET' for option "
"'GlobalValidWrongCase'; did you mean 'Violet'?");
CHECK_ERROR_ENUM(TestCheck.getIntGlobal<Colours>("GlobalNearMiss"),
"invalid configuration value 'Yelow' for option "
"'GlobalNearMiss'; did you mean 'Yellow'?");
#undef CHECK_ERROR_ENUM EXPECT_FALSE(TestCheck.getIntLocal<Colours>("ValidWrongCase").hasValue());
EXPECT_FALSE(TestCheck.getIntLocal<Colours>("NearMiss").hasValue());
EXPECT_FALSE(TestCheck.getIntGlobal<Colours>("GlobalInvalid").hasValue());
EXPECT_FALSE(
TestCheck.getIntGlobal<Colours>("GlobalValidWrongCase").hasValue());
EXPECT_FALSE(TestCheck.getIntGlobal<Colours>("GlobalNearMiss").hasValue());
EXPECT_FALSE(TestCheck.getIntLocal<Colours>("Invalid").hasValue());
EXPECT_THAT(
DiagConsumer.take(),
UnorderedElementsAre(
AllOf(ToolDiagMessage("invalid configuration value "
"'Scarlet' for option 'test.Invalid'"),
ToolDiagLevel(Warning)),
AllOf(ToolDiagMessage("invalid configuration value 'rED' for option "
"'test.ValidWrongCase'; did you mean 'Red'?"),
ToolDiagLevel(Warning)),
AllOf(
ToolDiagMessage("invalid configuration value 'Oragne' for option "
"'test.NearMiss'; did you mean 'Orange'?"),
ToolDiagLevel(Warning)),
AllOf(ToolDiagMessage("invalid configuration value "
"'Purple' for option 'GlobalInvalid'"),
ToolDiagLevel(Warning)),
AllOf(
ToolDiagMessage("invalid configuration value 'vIOLET' for option "
"'GlobalValidWrongCase'; did you mean 'Violet'?"),
ToolDiagLevel(Warning)),
AllOf(
ToolDiagMessage("invalid configuration value 'Yelow' for option "
"'GlobalNearMiss'; did you mean 'Yellow'?"),
ToolDiagLevel(Warning))));
} }
#undef CHECK_VAL #undef CHECK_VAL
#undef CHECK_ERROR
} // namespace test } // namespace test
} // namespace tidy } // namespace tidy