[clang-tools-extra] 82289aa - [clang-tidy] Remove OptionError
Nathan James via cfe-commits
cfe-commits at lists.llvm.org
Mon Mar 1 09:55:23 PST 2021
Author: Nathan James
Date: 2021-03-01T17:55:17Z
New Revision: 82289aa6c88ad9840369db294cc66ed829e8c435
URL: https://github.com/llvm/llvm-project/commit/82289aa6c88ad9840369db294cc66ed829e8c435
DIFF: https://github.com/llvm/llvm-project/commit/82289aa6c88ad9840369db294cc66ed829e8c435.diff
LOG: [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
Added:
Modified:
clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
clang-tools-extra/clang-tidy/ClangTidyCheck.h
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/ClangTidyCheck.cpp b/clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
index e1bea430a89a..46b69ed538cb 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
@@ -15,30 +15,6 @@
namespace clang {
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)
: CheckName(CheckName), Context(Context),
Options(CheckName, Context->getOptions().CheckOptions, Context) {
@@ -75,12 +51,12 @@ ClangTidyCheck::OptionsView::OptionsView(
: NamePrefix(CheckName.str() + "."), CheckOptions(CheckOptions),
Context(Context) {}
-llvm::Expected<std::string>
+llvm::Optional<std::string>
ClangTidyCheck::OptionsView::get(StringRef LocalName) const {
const auto &Iter = CheckOptions.find(NamePrefix + LocalName.str());
if (Iter != CheckOptions.end())
return Iter->getValue().Value;
- return llvm::make_error<MissingOptionError>((NamePrefix + LocalName).str());
+ return None;
}
static ClangTidyOptions::OptionMap::const_iterator
@@ -97,16 +73,16 @@ findPriorityOption(const ClangTidyOptions::OptionMap &Options, StringRef NamePre
return IterGlobal;
}
-llvm::Expected<std::string>
+llvm::Optional<std::string>
ClangTidyCheck::OptionsView::getLocalOrGlobal(StringRef LocalName) const {
auto Iter = findPriorityOption(CheckOptions, NamePrefix, LocalName);
if (Iter != CheckOptions.end())
return Iter->getValue().Value;
- return llvm::make_error<MissingOptionError>((NamePrefix + LocalName).str());
+ return None;
}
-static llvm::Expected<bool> getAsBool(StringRef Value,
- const llvm::Twine &LookupName) {
+static Optional<bool> getAsBool(StringRef Value,
+ const llvm::Twine &LookupName) {
if (llvm::Optional<bool> Parsed = llvm::yaml::parseBool(Value))
return *Parsed;
@@ -115,46 +91,30 @@ static llvm::Expected<bool> getAsBool(StringRef Value,
long long Number;
if (!Value.getAsInteger(10, Number))
return Number != 0;
- return llvm::make_error<UnparseableIntegerOptionError>(LookupName.str(),
- Value.str(), true);
+ return None;
}
template <>
-llvm::Expected<bool>
+llvm::Optional<bool>
ClangTidyCheck::OptionsView::get<bool>(StringRef LocalName) const {
- llvm::Expected<std::string> ValueOr = get(LocalName);
- if (ValueOr)
- return getAsBool(*ValueOr, NamePrefix + LocalName);
- return ValueOr.takeError();
-}
-
-template <>
-bool ClangTidyCheck::OptionsView::get<bool>(StringRef LocalName,
- bool Default) const {
- llvm::Expected<bool> ValueOr = get<bool>(LocalName);
- if (ValueOr)
- return *ValueOr;
- reportOptionParsingError(ValueOr.takeError());
- return Default;
+ if (llvm::Optional<std::string> ValueOr = get(LocalName)) {
+ if (auto Result = getAsBool(*ValueOr, NamePrefix + LocalName))
+ return Result;
+ diagnoseBadBooleanOption(NamePrefix + LocalName, *ValueOr);
+ }
+ return None;
}
template <>
-llvm::Expected<bool>
+llvm::Optional<bool>
ClangTidyCheck::OptionsView::getLocalOrGlobal<bool>(StringRef LocalName) const {
auto Iter = findPriorityOption(CheckOptions, NamePrefix, LocalName);
- if (Iter != CheckOptions.end())
- return getAsBool(Iter->getValue().Value, Iter->getKey());
- return llvm::make_error<MissingOptionError>((NamePrefix + LocalName).str());
-}
-
-template <>
-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;
+ if (Iter != CheckOptions.end()) {
+ if (auto Result = getAsBool(Iter->getValue().Value, Iter->getKey()))
+ return Result;
+ diagnoseBadBooleanOption(Iter->getKey(), Iter->getValue().Value);
+ }
+ return None;
}
void ClangTidyCheck::OptionsView::store(ClangTidyOptions::OptionMap &Options,
@@ -176,14 +136,14 @@ void ClangTidyCheck::OptionsView::store<bool>(
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,
bool IgnoreCase) const {
auto Iter = CheckGlobal
? findPriorityOption(CheckOptions, NamePrefix, LocalName)
: CheckOptions.find((NamePrefix + LocalName).str());
if (Iter == CheckOptions.end())
- return llvm::make_error<MissingOptionError>((NamePrefix + LocalName).str());
+ return None;
StringRef Value = Iter->getValue().Value;
StringRef Closest;
@@ -206,39 +166,54 @@ llvm::Expected<int64_t> ClangTidyCheck::OptionsView::getEnumInt(
}
}
if (EditDistance < 3)
- return llvm::make_error<UnparseableEnumOptionError>(
- Iter->getKey().str(), Iter->getValue().Value, Closest.str());
- return llvm::make_error<UnparseableEnumOptionError>(Iter->getKey().str(),
- Iter->getValue().Value);
+ diagnoseBadEnumOption(Iter->getKey().str(), Iter->getValue().Value,
+ Closest);
+ else
+ diagnoseBadEnumOption(Iter->getKey().str(), Iter->getValue().Value);
+ return None;
}
-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)));
+static constexpr llvm::StringLiteral ConfigWarning(
+ "invalid configuration value '%0' for option '%1'%select{|; expected a "
+ "bool|; expected an integer|; did you mean '%3'?}2");
+
+void ClangTidyCheck::OptionsView::diagnoseBadBooleanOption(
+ const Twine &Lookup, StringRef Unparsed) const {
+ SmallString<64> Buffer;
+ Context->configurationDiag(ConfigWarning)
+ << Unparsed << Lookup.toStringRef(Buffer) << 1;
}
-template <>
-Optional<std::string> ClangTidyCheck::OptionsView::getOptional<std::string>(
- StringRef LocalName) const {
- if (auto ValueOr = get(LocalName))
- return *ValueOr;
- else
- consumeError(ValueOr.takeError());
- return llvm::None;
+void ClangTidyCheck::OptionsView::diagnoseBadIntegerOption(
+ const Twine &Lookup, StringRef Unparsed) const {
+ SmallString<64> Buffer;
+ Context->configurationDiag(ConfigWarning)
+ << Unparsed << Lookup.toStringRef(Buffer) << 2;
}
-template <>
-Optional<std::string>
-ClangTidyCheck::OptionsView::getOptionalLocalOrGlobal<std::string>(
- StringRef LocalName) const {
- if (auto ValueOr = getLocalOrGlobal(LocalName))
- return *ValueOr;
+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
- consumeError(ValueOr.takeError());
- return llvm::None;
+ Diag << 3 << Suggestion;
}
+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 clang
diff --git a/clang-tools-extra/clang-tidy/ClangTidyCheck.h b/clang-tools-extra/clang-tidy/ClangTidyCheck.h
index 9fa0d6328640..20e9b8e47e6f 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyCheck.h
+++ b/clang-tools-extra/clang-tidy/ClangTidyCheck.h
@@ -14,7 +14,6 @@
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/Basic/Diagnostic.h"
#include "llvm/ADT/Optional.h"
-#include "llvm/Support/Error.h"
#include <type_traits>
#include <utility>
#include <vector>
@@ -33,65 +32,6 @@ template <class T> struct OptionEnumMapping {
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.
///
/// To implement a ``ClangTidyCheck``, write a subclass and override some of the
@@ -198,6 +138,13 @@ class ClangTidyCheck : public ast_matchers::MatchFinder::MatchCallback {
/// Methods of this class prepend ``CheckName + "."`` to translate check-local
/// option names to global option names.
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:
/// Initializes the instance using \p CheckName + "." as a prefix.
OptionsView(StringRef CheckName,
@@ -207,30 +154,24 @@ class ClangTidyCheck : public ast_matchers::MatchFinder::MatchCallback {
/// 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<std::string> get(StringRef LocalName) const;
+ /// ``CheckOptions``. If the corresponding key is not present, return
+ /// ``None``.
+ llvm::Optional<std::string> 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 {
- if (llvm::Expected<std::string> Val = get(LocalName))
- return *Val;
- else
- llvm::consumeError(Val.takeError());
- return Default.str();
- }
+ std::string get(StringRef LocalName, StringRef Default) 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 a ``MissingOptionError``.
- llvm::Expected<std::string> getLocalOrGlobal(StringRef LocalName) const;
+ /// present either, return ``None``.
+ llvm::Optional<std::string> getLocalOrGlobal(StringRef LocalName) const;
/// Read a named option from the ``Context``.
///
@@ -238,48 +179,42 @@ class ClangTidyCheck : public ast_matchers::MatchFinder::MatchCallback {
/// 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 \p Default.
- 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();
- }
+ std::string getLocalOrGlobal(StringRef LocalName, StringRef Default) const;
/// 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
- /// a ``MissingOptionError``. If the corresponding key can't be parsed as
- /// a ``T``, return an ``UnparseableIntegerOptionError``.
+ /// ``CheckOptions``. If the corresponding key is not present, return
+ /// ``None``.
+ ///
+ /// If the corresponding key can't be parsed as a ``T``, emit a
+ /// diagnostic and return ``None``.
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 {
- if (llvm::Expected<std::string> Value = get(LocalName)) {
+ if (llvm::Optional<std::string> Value = get(LocalName)) {
T Result{};
if (!StringRef(*Value).getAsInteger(10, Result))
return Result;
- return llvm::make_error<UnparseableIntegerOptionError>(
- (NamePrefix + LocalName).str(), *Value);
- } else
- return std::move(Value.takeError());
+ diagnoseBadIntegerOption(NamePrefix + LocalName, *Value);
+ }
+ return None;
}
/// 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.
+ /// ``CheckOptions``. If the corresponding key is not present, return
+ /// \p Default.
+ ///
+ /// If the corresponding key can't be parsed as a ``T``, emit a
+ /// diagnostic and return \p Default.
template <typename T>
std::enable_if_t<std::is_integral<T>::value, T> get(StringRef LocalName,
T Default) const {
- if (llvm::Expected<T> ValueOr = get<T>(LocalName))
- return *ValueOr;
- else
- reportOptionParsingError(ValueOr.takeError());
- return Default;
+ return get<T>(LocalName).getValueOr(Default);
}
/// Read a named option from the ``Context`` and parse it as an
@@ -288,27 +223,27 @@ class ClangTidyCheck : public ast_matchers::MatchFinder::MatchCallback {
/// 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``.
+ /// present either, return ``None``.
+ ///
+ /// If the corresponding key can't be parsed as a ``T``, emit a
+ /// diagnostic and return ``None``.
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 {
- llvm::Expected<std::string> ValueOr = get(LocalName);
+ llvm::Optional<std::string> ValueOr = get(LocalName);
bool IsGlobal = false;
if (!ValueOr) {
IsGlobal = true;
- llvm::consumeError(ValueOr.takeError());
ValueOr = getLocalOrGlobal(LocalName);
if (!ValueOr)
- return std::move(ValueOr.takeError());
+ return None;
}
T Result{};
if (!StringRef(*ValueOr).getAsInteger(10, Result))
return Result;
- return llvm::make_error<UnparseableIntegerOptionError>(
- (IsGlobal ? LocalName.str() : (NamePrefix + LocalName).str()),
- *ValueOr);
+ diagnoseBadIntegerOption(
+ IsGlobal ? Twine(LocalName) : NamePrefix + LocalName, *ValueOr);
+ return None;
}
/// Read a named option from the ``Context`` and parse it as an
@@ -317,54 +252,53 @@ class ClangTidyCheck : public ast_matchers::MatchFinder::MatchCallback {
/// 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.
+ /// 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>
std::enable_if_t<std::is_integral<T>::value, T>
getLocalOrGlobal(StringRef LocalName, T Default) const {
- if (llvm::Expected<T> ValueOr = getLocalOrGlobal<T>(LocalName))
- return *ValueOr;
- else
- reportOptionParsingError(ValueOr.takeError());
- return Default;
+ return getLocalOrGlobal<T>(LocalName).getValueOr(Default);
}
/// Read a named option from the ``Context`` and parse it as an
/// enum type ``T``.
///
/// 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``.
+ /// ``CheckOptions``. If the corresponding key is not present, return
+ /// ``None``.
+ ///
+ /// 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
/// supply the mapping required to convert between ``T`` and a string.
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 {
- if (llvm::Expected<int64_t> ValueOr =
+ if (llvm::Optional<int64_t> ValueOr =
getEnumInt(LocalName, typeEraseMapping<T>(), false, IgnoreCase))
return static_cast<T>(*ValueOr);
- else
- return std::move(ValueOr.takeError());
+ return None;
}
/// Read a named option from the ``Context`` and parse it as an
/// enum 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.
+ /// ``CheckOptions``. If the corresponding key is not present, 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
/// supply the mapping required to convert between ``T`` and a string.
template <typename T>
std::enable_if_t<std::is_enum<T>::value, T>
get(StringRef LocalName, T Default, bool IgnoreCase = false) const {
- if (auto ValueOr = get<T>(LocalName, IgnoreCase))
- return *ValueOr;
- else
- reportOptionParsingError(ValueOr.takeError());
- return Default;
+ return get<T>(LocalName, IgnoreCase).getValueOr(Default);
}
/// Read a named option from the ``Context`` and parse it as an
@@ -373,19 +307,20 @@ class ClangTidyCheck : public ast_matchers::MatchFinder::MatchCallback {
/// 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``.
+ /// present either, returns ``None``.
+ ///
+ /// 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
/// supply the mapping required to convert between ``T`` and a string.
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 {
- if (llvm::Expected<int64_t> ValueOr =
+ if (llvm::Optional<int64_t> ValueOr =
getEnumInt(LocalName, typeEraseMapping<T>(), true, IgnoreCase))
return static_cast<T>(*ValueOr);
- else
- return std::move(ValueOr.takeError());
+ return None;
}
/// Read a named option from the ``Context`` and parse it as an
@@ -394,7 +329,10 @@ class ClangTidyCheck : public ast_matchers::MatchFinder::MatchCallback {
/// 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.
+ /// 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
/// supply the mapping required to convert between ``T`` and a string.
@@ -402,36 +340,7 @@ class ClangTidyCheck : public ast_matchers::MatchFinder::MatchCallback {
std::enable_if_t<std::is_enum<T>::value, T>
getLocalOrGlobal(StringRef LocalName, T Default,
bool IgnoreCase = false) const {
- if (auto ValueOr = getLocalOrGlobal<T>(LocalName, IgnoreCase))
- 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;
+ return getLocalOrGlobal<T>(LocalName, IgnoreCase).getValueOr(Default);
}
/// Stores an option with the check-local name \p LocalName with
@@ -470,7 +379,7 @@ class ClangTidyCheck : public ast_matchers::MatchFinder::MatchCallback {
private:
using NameAndValue = std::pair<int64_t, StringRef>;
- llvm::Expected<int64_t> getEnumInt(StringRef LocalName,
+ llvm::Optional<int64_t> getEnumInt(StringRef LocalName,
ArrayRef<NameAndValue> Mapping,
bool CheckGlobal, bool IgnoreCase) const;
@@ -491,8 +400,6 @@ class ClangTidyCheck : public ast_matchers::MatchFinder::MatchCallback {
void storeInt(ClangTidyOptions::OptionMap &Options, StringRef LocalName,
int64_t Value) const;
- /// Emits a diagnostic if \p Err is not a MissingOptionError.
- void reportOptionParsingError(llvm::Error &&Err) const;
std::string NamePrefix;
const ClangTidyOptions::OptionMap &CheckOptions;
@@ -516,44 +423,27 @@ class ClangTidyCheck : public ast_matchers::MatchFinder::MatchCallback {
/// 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
-/// ``CheckOptions``. If the corresponding key is not present, returns
-/// a ``MissingOptionError``. If the corresponding key can't be parsed as
-/// a bool, return an ``UnparseableIntegerOptionError``.
+/// ``CheckOptions``. If the corresponding key is not present, return
+/// ``None``.
+///
+/// If the corresponding key can't be parsed as a bool, emit a
+/// diagnostic and return ``None``.
template <>
-llvm::Expected<bool>
+llvm::Optional<bool>
ClangTidyCheck::OptionsView::get<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 the
-/// ``CheckOptions``. If the corresponding key is not present or it can't be
-/// parsed as a bool, returns \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.
+/// ``CheckOptions``. If the corresponding key is not present, return
+/// \p Default.
///
-/// 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 bool, return an
-/// ``UnparseableIntegerOptionError``.
+/// If the corresponding key can't be parsed as a bool, emit a
+/// diagnostic and return \p Default.
template <>
-llvm::Expected<bool>
+llvm::Optional<bool>
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
/// bool value \p Value to \p Options.
template <>
@@ -561,18 +451,6 @@ void ClangTidyCheck::OptionsView::store<bool>(
ClangTidyOptions::OptionMap &Options, StringRef LocalName,
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 clang
diff --git a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
index fccff502e183..09b3cc25678c 100644
--- a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -157,7 +157,7 @@ getFileStyleFromOptions(const ClangTidyCheck::OptionsView &Options) {
StyleString.pop_back();
StyleString.pop_back();
auto CaseOptional =
- Options.getOptional<IdentifierNamingCheck::CaseType>(StyleString);
+ Options.get<IdentifierNamingCheck::CaseType>(StyleString);
if (CaseOptional || !Prefix.empty() || !Postfix.empty() ||
!IgnoredRegexpStr.empty())
diff --git a/clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp b/clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
index 6447f34661e9..c4fb8bd7b756 100644
--- a/clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
+++ b/clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
@@ -190,6 +190,7 @@ MATCHER_P(DiagRange, P, "") { return arg.Range && *arg.Range == P; }
using ::testing::AllOf;
using ::testing::ElementsAre;
+using ::testing::UnorderedElementsAre;
TEST(ParseConfiguration, CollectDiags) {
DiagCollecter Collector;
@@ -243,7 +244,6 @@ class TestCheck : public ClangTidyCheck {
return Options.getLocalOrGlobal<IntType>(std::forward<Args>(Arguments)...);
}
};
-} // namespace
#define CHECK_VAL(Value, Expected) \
do { \
@@ -252,17 +252,22 @@ class TestCheck : public ClangTidyCheck {
EXPECT_EQ(*Item, Expected); \
} while (false)
-#define CHECK_ERROR(Value, ErrorType, ExpectedMessage) \
- do { \
- auto Item = Value; \
- ASSERT_FALSE(Item); \
- ASSERT_TRUE(Item.errorIsA<ErrorType>()); \
- ASSERT_FALSE(llvm::handleErrors( \
- Item.takeError(), [&](const ErrorType &Err) -> llvm::Error { \
- EXPECT_EQ(Err.message(), ExpectedMessage); \
- return llvm::Error::success(); \
- })); \
- } while (false)
+MATCHER_P(ToolDiagMessage, M, "") { return arg.Message.Message == M; }
+MATCHER_P(ToolDiagLevel, L, "") { return arg.DiagLevel == L; }
+
+} // namespace
+
+} // namespace test
+
+static constexpr auto Warning = tooling::Diagnostic::Warning;
+static constexpr auto Error = tooling::Diagnostic::Error;
+
+static void PrintTo(const ClangTidyError &Err, ::std::ostream *OS) {
+ *OS << (Err.DiagLevel == Error ? "error: " : "warning: ")
+ << Err.Message.Message;
+}
+
+namespace test {
TEST(CheckOptionsValidation, MissingOptions) {
ClangTidyOptions Options;
@@ -273,21 +278,21 @@ TEST(CheckOptionsValidation, MissingOptions) {
&DiagConsumer, false);
Context.setDiagnosticsEngine(&DE);
TestCheck TestCheck(&Context);
- CHECK_ERROR(TestCheck.getLocal("Opt"), MissingOptionError,
- "option not found 'test.Opt'");
+ EXPECT_FALSE(TestCheck.getLocal("Opt").hasValue());
EXPECT_EQ(TestCheck.getLocal("Opt", "Unknown"), "Unknown");
+ // Missing options aren't errors.
+ EXPECT_TRUE(DiagConsumer.take().empty());
}
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["test.IntExpected"] = "1";
+ CheckOptions["test.IntInvalid1"] = "1WithMore";
+ CheckOptions["test.IntInvalid2"] = "NoInt";
+ CheckOptions["GlobalIntExpected"] = "1";
CheckOptions["GlobalIntInvalid"] = "NoInt";
+ CheckOptions["test.DefaultedIntInvalid"] = "NoInt";
CheckOptions["test.BoolITrueValue"] = "1";
CheckOptions["test.BoolIFalseValue"] = "0";
CheckOptions["test.BoolTrueValue"] = "true";
@@ -304,22 +309,12 @@ TEST(CheckOptionsValidation, ValidIntOptions) {
Context.setDiagnosticsEngine(&DE);
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"),
- "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");
+ CHECK_VAL(TestCheck.getIntLocal("IntExpected"), 1);
+ CHECK_VAL(TestCheck.getIntGlobal("GlobalIntExpected"), 1);
+ EXPECT_FALSE(TestCheck.getIntLocal("IntInvalid1").hasValue());
+ EXPECT_FALSE(TestCheck.getIntLocal("IntInvalid2").hasValue());
+ EXPECT_FALSE(TestCheck.getIntGlobal("GlobalIntInvalid").hasValue());
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>("BoolIFalseValue"), false);
@@ -327,11 +322,31 @@ TEST(CheckOptionsValidation, ValidIntOptions) {
CHECK_VAL(TestCheck.getIntLocal<bool>("BoolFalseValue"), false);
CHECK_VAL(TestCheck.getIntLocal<bool>("BoolTrueShort"), true);
CHECK_VAL(TestCheck.getIntLocal<bool>("BoolFalseShort"), false);
- CHECK_ERROR_INT(TestCheck.getIntLocal<bool>("BoolUnparseable"),
- "invalid configuration value 'Nothing' for option "
- "'test.BoolUnparseable'; expected a bool");
-
-#undef CHECK_ERROR_INT
+ EXPECT_FALSE(TestCheck.getIntLocal<bool>("BoolUnparseable").hasValue());
+
+ 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) {
@@ -350,11 +365,12 @@ TEST(ValidConfiguration, ValidEnumOptions) {
ClangTidyContext Context(std::make_unique<DefaultOptionsProvider>(
ClangTidyGlobalOptions(), Options));
+ ClangTidyDiagnosticConsumer DiagConsumer(Context);
+ DiagnosticsEngine DE(new DiagnosticIDs(), new DiagnosticOptions,
+ &DiagConsumer, false);
+ Context.setDiagnosticsEngine(&DE);
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.getIntGlobal<Colours>("GlobalValid"), Colours::Violet);
@@ -364,30 +380,42 @@ TEST(ValidConfiguration, ValidEnumOptions) {
CHECK_VAL(TestCheck.getIntGlobal<Colours>("GlobalValidWrongCase",
/*IgnoreCase*/ true),
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_ERROR
} // namespace test
} // namespace tidy
More information about the cfe-commits
mailing list