[clang-tidy] Added support for validating configuration options

[njames93]

Summary

Adds support for ClangTidyCheck::OptionsView to deteremine:

• If an option is found in the configuration.
• If an integer option read from configuration is parsable to an integer.
• Parse and Serialize enum configuration options directly using a mapping from llvm::StringRef to EnumType.
• If an integer or enum option isn't parseable but there is a default value it will issue a warning to stderr that the config value hasn't been used.
• If an enum option isn't parsable it can provide a hint if the value was a typo.

[njames93]

This pretty much nulls out (clang-tidy) Warn on invalid "case" configurations for readability-identifier-naming I have moved that into a follow up patch that can be submitted now if you'd like.

[njames93]

• Fix assertions failing

[njames93]

• Small refactor

[aaron.ballman]

Thank you for working on this, I think it's going to be a very useful interface!

clang-tools-extra/clang-tidy/ClangTidyCheck.cpp

25

I think the diagnostic text should probably not start with a capital letter. Also, the name of the classes are confusing in that they say error but the diagnostic is a warning. When I hear "error", the class name makes me think this would stop compilation and give a nonzero result code from the program.

165–170

Elide braces

179–181

Preference to add the braces to this case because the surrounding ones do as well.

clang-tools-extra/clang-tidy/ClangTidyCheck.h

41

Make the constructor explicit? (May want to consider the same for the other ctors, but this one seems more dangerous.)

204

Don't use auto as the type is not spelled out.

208

Default.str() instead?

226–230

Same here as above.

287

Elide braces

[njames93]

• Address nits

[aaron.ballman]

clang-tools-extra/clang-tidy/ClangTidyCheck.cpp

25

Sorry, I was unclear with what I was looking for.

It looks to me like the behavior of OptionError is to act more like a warning than an error in that a message is printed and execution of the tool continues, correct? If so, then I'd prefer to rename it to OptionWarning (unless you want to generalize it to be either a warning or an error with a return code, then OptionDiagnostic would be good), and all the subclasses to be FooWarning. Or did I misunderstand the design?

Also, I like having the warning: in the message (though we could bikeshed if we want to make it look less like a check warning by doing something like warning [clang-tidy command line]:), but maybe that's being added by OptionsView::logErrToStdErr()?

[njames93]

clang-tools-extra/clang-tidy/ClangTidyCheck.cpp

25

OptionError is just there for convenience to avoid having to retype the log and convertToErrorCode function. Arguably, the MissingOptionError isn't really an error, but the UnparseableEnumOptionError and UnparseableIntegerOptionError should be classed as errors.

From what it seems with other errors, the message function shouldn't contain a warning: or error: prefix, instead that now gets added in OptionsView::logErrToStdErr. The output for enumerations looks like

warning: invalid configuration value 'camelback' for option 'readability-identifier-naming.FunctionCase'; did you mean 'camelBack'?

I do think down the line this should be actually handled as a ClangTidyDiagnostic however that is a much larger change to implement given how diagnostics are currently handled.

[njames93]

• Extended support for boolean types

[njames93]

I added support for bools. The previous integer parsing behaviour is still there, however now it also responds to true or false.
This won't parse True|TRUE|False|FALSE etc as I wanted it to be in line with .clang-format configuration files for handling of bool.

[aaron.ballman]

LGTM with a few minor nits.

clang-tools-extra/clang-tidy/ClangTidyCheck.cpp

117

Elide braces

144

Elide braces

[nemanjai]

A recent commit has taken down a whole bunch of bots. The build error messages all seem to point to code in this patch. If this is indeed the cause, please revert.

[njames93]

In D77085#1967807, @nemanjai wrote:

A recent commit has taken down a whole bunch of bots. The build error messages all seem to point to code in this patch. If this is indeed the cause, please revert.

I was aware and hopefully this fixes the issue https://github.com/llvm/llvm-project/commit/0361798dbeb6ead0a79ab7985f02da347fce988e

[nemanjai]

In D77085#1967864, @njames93 wrote:

In D77085#1967807, @nemanjai wrote:

A recent commit has taken down a whole bunch of bots. The build error messages all seem to point to code in this patch. If this is indeed the cause, please revert.

I was aware and hopefully this fixes the issue https://github.com/llvm/llvm-project/commit/0361798dbeb6ead0a79ab7985f02da347fce988e

Awesome, thanks. Certainly fixes the compile time failures in my local build. There is still a link-time failure (undefined reference) with my shared libraries build:

tools/clang/tools/extra/clang-tidy/tool/CMakeFiles/obj.clangTidyMain.dir/ClangTidyMain.cpp.o: In function `clang::ast_matchers::internal::matcher_isAllowedToContainClauseKind0Matcher::matches(clang::OMPExecutableDirective const&, clang::ast_matchers::internal::ASTMatchFinder*, clang::ast_matchers::internal::BoundNodesTreeBuilder*) const':
ClangTidyMain.cpp:(.text._ZNK5clang12ast_matchers8internal44matcher_isAllowedToContainClauseKind0Matcher7matchesERKNS_22OMPExecutableDirectiveEPNS1_14ASTMatchFinderEPNS1_21BoundNodesTreeBuilderE[_ZNK5clang12ast_matchers8internal44matcher_isAllowedToContainClauseKind0Matcher7matchesERKNS_22OMPExecutableDirectiveEPNS1_14ASTMatchFinderEPNS1_21BoundNodesTreeBuilderE]+0x50): undefined reference to `llvm::omp::isAllowedClauseForDirective(llvm::omp::Directive, llvm::omp::Clause, unsigned int)'
collect2: error: ld returned 1 exit status

But that may be unrelated to this patch.

[njames93]

In D77085#1967883, @nemanjai wrote:

Awesome, thanks. Certainly fixes the compile time failures in my local build. There is still a link-time failure (undefined reference) with my shared libraries build:

tools/clang/tools/extra/clang-tidy/tool/CMakeFiles/obj.clangTidyMain.dir/ClangTidyMain.cpp.o: In function `clang::ast_matchers::internal::matcher_isAllowedToContainClauseKind0Matcher::matches(clang::OMPExecutableDirective const&, clang::ast_matchers::internal::ASTMatchFinder*, clang::ast_matchers::internal::BoundNodesTreeBuilder*) const':
ClangTidyMain.cpp:(.text._ZNK5clang12ast_matchers8internal44matcher_isAllowedToContainClauseKind0Matcher7matchesERKNS_22OMPExecutableDirectiveEPNS1_14ASTMatchFinderEPNS1_21BoundNodesTreeBuilderE[_ZNK5clang12ast_matchers8internal44matcher_isAllowedToContainClauseKind0Matcher7matchesERKNS_22OMPExecutableDirectiveEPNS1_14ASTMatchFinderEPNS1_21BoundNodesTreeBuilderE]+0x50): undefined reference to `llvm::omp::isAllowedClauseForDirective(llvm::omp::Directive, llvm::omp::Clause, unsigned int)'
collect2: error: ld returned 1 exit status

But that may be unrelated to this patch.

That's unrelated to this patch, something about ASTMatchers and specifically OpenMP @jdoerfert has changed a few things there, may be worth hitting him up to see.

clang-tools-extra/clang-tidy/ClangTidyCheck.h

 //===--- ClangTidyCheck.h - clang-tidy --------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANGTIDYCHECK_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANGTIDYCHECK_H
 
 #include "ClangTidyDiagnosticConsumer.h"
 #include "ClangTidyOptions.h"
 #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 <memory>
 #include <type_traits>
 #include <vector>
 
 namespace clang {
 
 class CompilerInstance;
 
 namespace tidy {
 
+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
 /// base class's methods. E.g. to implement a check that validates namespace
 /// declarations, override ``registerMatchers``:
 ///
 /// ~~~{.cpp}
 /// void registerMatchers(ast_matchers::MatchFinder *Finder) override {
public:
     /// Initializes the instance using \p CheckName + "." as a prefix.
     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<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;
+    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``.
+    ///
+    /// 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;
 
     /// 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 (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
     /// 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 <typename T>
+    std::enable_if_t<std::is_integral<T>::value, llvm::Expected<T>>
+    get(StringRef LocalName) const {
+      if (llvm::Expected<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());
+    }
+
+    /// 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 <typename T>
     std::enable_if_t<std::is_integral<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);
+      if (llvm::Expected<T> ValueOr = get<T>(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 <typename T>
+    std::enable_if_t<std::is_integral<T>::value, llvm::Expected<T>>
+    getLocalOrGlobal(StringRef LocalName) const {
+      llvm::Expected<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());
+      }
+      T Result{};
+      if (!StringRef(*ValueOr).getAsInteger(10, Result))
         return Result;
+      return llvm::make_error<UnparseableIntegerOptionError>(
+          (IsGlobal ? LocalName.str() : (NamePrefix + LocalName).str()),
+          *ValueOr);
     }
 
     /// 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 Default.
+    /// present either or it can't be parsed as a ``T``, returns \p Default.
     template <typename T>
     std::enable_if_t<std::is_integral<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<T> ValueOr = getLocalOrGlobal<T>(LocalName))
+        return *ValueOr;
+      else
+        logErrToStdErr(ValueOr.takeError());
+      return Default;
+    }
+
+    /// 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``.
+    template <> llvm::Expected<bool> 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 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
+    /// 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``.
+    template <>
+    llvm::Expected<bool> 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 getLocalOrGlobal<bool>(StringRef LocalName, bool Default) const;
+
+    /// 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 <typename T>
+    std::enable_if_t<std::is_enum<T>::value, llvm::Expected<T>>
+    get(StringRef LocalName, ArrayRef<std::pair<StringRef, T>> Mapping,
+        bool IgnoreCase = false) {
+      if (llvm::Expected<int64_t> ValueOr = getEnumInt(
+              LocalName, typeEraseMapping(Mapping), false, IgnoreCase))
+        return static_cast<T>(*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 <typename T>
+    std::enable_if_t<std::is_enum<T>::value, T>
+    get(StringRef LocalName, ArrayRef<std::pair<StringRef, T>> 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 <typename T>
+    std::enable_if_t<std::is_enum<T>::value, llvm::Expected<T>>
+    getLocalOrGlobal(StringRef LocalName,
+                     ArrayRef<std::pair<StringRef, T>> Mapping,
+                     bool IgnoreCase = false) {
+      if (llvm::Expected<int64_t> ValueOr = getEnumInt(
+              LocalName, typeEraseMapping(Mapping), true, IgnoreCase))
+        return static_cast<T>(*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 <typename T>
+    std::enable_if_t<std::is_enum<T>::value, T>
+    getLocalOrGlobal(StringRef LocalName,
+                     ArrayRef<std::pair<StringRef, T>> 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
     /// string value \p Value to \p Options.
     void store(ClangTidyOptions::OptionMap &Options, StringRef LocalName,
                StringRef Value) const;
 
     /// Stores an option with the check-local name \p LocalName with
     /// ``int64_t`` value \p Value to \p Options.
     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 <typename T>
+    std::enable_if_t<std::is_enum<T>::value>
+    store(ClangTidyOptions::OptionMap &Options, StringRef LocalName, T Value,
+          ArrayRef<std::pair<StringRef, T>> Mapping) {
+      auto Iter = llvm::find_if(
+          Mapping, [&](const std::pair<StringRef, T> &NameAndEnum) {
+            return NameAndEnum.second == Value;
+          });
+      assert(Iter != Mapping.end() && "Unknown Case Value");
+      store(Options, LocalName, Iter->first);
+    }
+
   private:
+    using NameAndValue = std::pair<StringRef, int64_t>;
+
+    llvm::Expected<int64_t> getEnumInt(StringRef LocalName,
+                                       ArrayRef<NameAndValue> Mapping,
+                                       bool CheckGlobal, bool IgnoreCase);
+
+    template <typename T>
+    std::enable_if_t<std::is_enum<T>::value, std::vector<NameAndValue>>
+    typeEraseMapping(ArrayRef<std::pair<StringRef, T>> Mapping) {
+      std::vector<NameAndValue> Result;
+      Result.reserve(Mapping.size());
+      for (auto &MappedItem : Mapping) {
+        Result.emplace_back(MappedItem.first,
+                            static_cast<int64_t>(MappedItem.second));
+      }
+      return Result;
+    }
+
+    static void logErrToStdErr(llvm::Error &&Err);
+
     std::string NamePrefix;
     const ClangTidyOptions::OptionMap &CheckOptions;
   };
 
 private:
   void run(const ast_matchers::MatchFinder::MatchResult &Result) override;
   StringRef getID() const override { return CheckName; }
   std::string CheckName;

clang-tools-extra/clang-tidy/ClangTidyCheck.cpp

 //===--- ClangTidyCheck.cpp - clang-tidy ------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #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 << "option not found '" << OptionName << '\'';
+  return std::string(Buffer);
+}
+
+std::string UnparseableEnumOptionError::message() const {
+  llvm::SmallString<128> Buffer;
+  llvm::raw_svector_ostream Output(Buffer);
+  Output << "invalid configuration value '" << LookupValue << "' for option '"
+         << LookupName << '\'';
+  if (SuggestedValue)
+    Output << "; did you mean '" << *SuggestedValue << "'?";
+  return std::string(Buffer);
+}
+
+std::string UnparseableIntegerOptionError::message() const {
+  llvm::SmallString<128> Buffer;
+  llvm::raw_svector_ostream Output(Buffer);
+  Output << "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) {
   assert(Context != nullptr);
   assert(!CheckName.empty());
 }
 
 DiagnosticBuilder ClangTidyCheck::diag(SourceLocation Loc, StringRef Message,
                                        DiagnosticIDs::Level Level) {
   return Context->diag(CheckName, Loc, Message, Level);
 }
 
 void ClangTidyCheck::run(const ast_matchers::MatchFinder::MatchResult &Result) {
   // For historical reasons, checks don't implement the MatchFinder run()
   // callback directly. We keep the run()/check() distinction to avoid interface
   // churn, and to allow us to add cross-cutting logic in the future.
   check(Result);
 }
 
 ClangTidyCheck::OptionsView::OptionsView(StringRef CheckName,
                          const ClangTidyOptions::OptionMap &CheckOptions)
     : NamePrefix(CheckName.str() + "."), CheckOptions(CheckOptions) {}
 
-std::string ClangTidyCheck::OptionsView::get(StringRef LocalName,
-                                             StringRef Default) const {
+llvm::Expected<std::string>
+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<MissingOptionError>((NamePrefix + LocalName).str());
 }
 
-std::string
-ClangTidyCheck::OptionsView::getLocalOrGlobal(StringRef LocalName,
-                                              StringRef Default) const {
+llvm::Expected<std::string>
+ClangTidyCheck::OptionsView::getLocalOrGlobal(StringRef LocalName) const {
   auto Iter = CheckOptions.find(NamePrefix + LocalName.str());
   if (Iter != CheckOptions.end())
     return Iter->second;
   // Fallback to global setting, if present.
   Iter = CheckOptions.find(LocalName.str());
   if (Iter != CheckOptions.end())
     return Iter->second;
-  return std::string(Default);
+  return llvm::make_error<MissingOptionError>((NamePrefix + LocalName).str());
+}
+
+static llvm::Expected<bool> getAsBool(StringRef Value,
+                                      const llvm::Twine &LookupName) {
+  if (Value == "true")
+    return true;
+  if (Value == "false")
+    return false;
+  bool Result;
+  if (!Value.getAsInteger(10, Result))
+    return Result;
+  return llvm::make_error<UnparseableIntegerOptionError>(LookupName.str(),
+                                                         Value.str(), true);
+}
+
+template <>
+llvm::Expected<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;
+  logErrToStdErr(ValueOr.takeError());
+  return Default;
+}
+
+template <>
+llvm::Expected<bool>
+ClangTidyCheck::OptionsView::getLocalOrGlobal<bool>(StringRef LocalName) const {
+  llvm::Expected<std::string> ValueOr = get(LocalName);
+  bool IsGlobal = false;
+  if (!ValueOr) {
+    llvm::consumeError(ValueOr.takeError());
+    ValueOr = getLocalOrGlobal(LocalName);
+    IsGlobal = true;
+  }
+  if (!ValueOr)
+    return ValueOr.takeError();
+  return getAsBool(*ValueOr, IsGlobal ? llvm::Twine(LocalName)
+                                      : (NamePrefix + LocalName));
+}
+
+template <>
+bool ClangTidyCheck::OptionsView::getLocalOrGlobal<bool>(StringRef LocalName,
+                                                         bool Default) const {
+  llvm::Expected<bool> ValueOr = getLocalOrGlobal<bool>(LocalName);
+  if (ValueOr)
+    return *ValueOr;
+  logErrToStdErr(ValueOr.takeError());
+  return Default;
 }
 
 void ClangTidyCheck::OptionsView::store(ClangTidyOptions::OptionMap &Options,
                                         StringRef LocalName,
                                         StringRef Value) const {
   Options[NamePrefix + LocalName.str()] = std::string(Value);
 }
 
 void ClangTidyCheck::OptionsView::store(ClangTidyOptions::OptionMap &Options,
                                         StringRef LocalName,
                                         int64_t Value) const {
   store(Options, LocalName, llvm::itostr(Value));
 }
 
+llvm::Expected<int64_t> ClangTidyCheck::OptionsView::getEnumInt(
+    StringRef LocalName, ArrayRef<std::pair<StringRef, int64_t>> 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<MissingOptionError>((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<UnparseableEnumOptionError>(
+        Iter->first, Iter->second, std::string(Closest));
+  return llvm::make_error<UnparseableEnumOptionError>(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(), "warning: ");
+}
 } // namespace tidy
 } // namespace clang

clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp

 #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 {
 namespace test {
 
 TEST(ParseLineFilter, EmptyFilter) {
   ClangTidyGlobalOptions Options;
   EXPECT_FALSE(parseLineFilter("", Options));
TEST(ParseConfiguration, MergeConfigurations) {
   ASSERT_TRUE(Options.ExtraArgs.hasValue());
   EXPECT_EQ("arg1,arg2,arg3,arg4", llvm::join(Options.ExtraArgs->begin(),
                                               Options.ExtraArgs->end(), ","));
   ASSERT_TRUE(Options.ExtraArgsBefore.hasValue());
   EXPECT_EQ("arg-before1,arg-before2,arg-before3,arg-before4",
             llvm::join(Options.ExtraArgsBefore->begin(),
                        Options.ExtraArgsBefore->end(), ","));
 }
+
+class TestCheck : public ClangTidyCheck {
+public:
+  TestCheck(ClangTidyContext *Context) : ClangTidyCheck("test", Context) {}
+
+  template <typename... Args> auto getLocal(Args &&... Arguments) {
+    return Options.get(std::forward<Args>(Arguments)...);
+  }
+
+  template <typename... Args> auto getGlobal(Args &&... Arguments) {
+    return Options.getLocalOrGlobal(std::forward<Args>(Arguments)...);
+  }
+
+  template <typename IntType = int, typename... Args>
+  auto getIntLocal(Args &&... Arguments) {
+    return Options.get<IntType>(std::forward<Args>(Arguments)...);
+  }
+
+  template <typename IntType = int, typename... Args>
+  auto getIntGlobal(Args &&... Arguments) {
+    return Options.getLocalOrGlobal<IntType>(std::forward<Args>(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<ErrorType>());                                   \
+    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<DefaultOptionsProvider>(
+      ClangTidyGlobalOptions(), Options));
+  TestCheck TestCheck(&Context);
+  CHECK_ERROR(TestCheck.getLocal("Opt"), MissingOptionError,
+              "option not found 'test.Opt'");
+  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";
+  CheckOptions["test.BoolITrueValue"] = "1";
+  CheckOptions["test.BoolIFalseValue"] = "0";
+  CheckOptions["test.BoolTrueValue"] = "true";
+  CheckOptions["test.BoolFalseValue"] = "false";
+  CheckOptions["test.BoolUnparseable"] = "Nothing";
+  CheckOptions["test.BoolCaseMismatch"] = "True";
+
+  ClangTidyContext Context(std::make_unique<DefaultOptionsProvider>(
+      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"),
+                  "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.getIntGlobal("GlobalIntInvalid", 1), 1);
+
+  CHECK_VAL(TestCheck.getIntLocal<bool>("BoolITrueValue"), true);
+  CHECK_VAL(TestCheck.getIntLocal<bool>("BoolIFalseValue"), false);
+  CHECK_VAL(TestCheck.getIntLocal<bool>("BoolTrueValue"), true);
+  CHECK_VAL(TestCheck.getIntLocal<bool>("BoolFalseValue"), false);
+  CHECK_ERROR_INT(TestCheck.getIntLocal<bool>("BoolUnparseable"),
+                  "invalid configuration value 'Nothing' for option "
+                  "'test.BoolUnparseable'; expected a bool");
+  CHECK_ERROR_INT(TestCheck.getIntLocal<bool>("BoolCaseMismatch"),
+                  "invalid configuration value 'True' for option "
+                  "'test.BoolCaseMismatch'; expected a bool");
+
+#undef CHECK_ERROR_INT
+}
+
+TEST(ValidConfiguration, ValidEnumOptions) {
+
+  enum class Colours { Red, Orange, Yellow, Green, Blue, Indigo, Violet };
+  static constexpr std::pair<StringRef, Colours> 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<DefaultOptionsProvider>(
+      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),
+                   "invalid configuration value "
+                   "'Scarlet' for option 'test.Invalid'");
+  CHECK_ERROR_ENUM(TestCheck.getLocal("ValidWrongCase", Map),
+                   "invalid configuration value 'rED' for option "
+                   "'test.ValidWrongCase'; did you mean 'Red'?");
+  CHECK_ERROR_ENUM(TestCheck.getLocal("NearMiss", Map),
+                   "invalid configuration value 'Oragne' for option "
+                   "'test.NearMiss'; did you mean 'Orange'?");
+  CHECK_ERROR_ENUM(TestCheck.getGlobal("GlobalInvalid", Map),
+                   "invalid configuration value "
+                   "'Purple' for option 'GlobalInvalid'");
+  CHECK_ERROR_ENUM(TestCheck.getGlobal("GlobalValidWrongCase", Map),
+                   "invalid configuration value 'vIOLET' for option "
+                   "'GlobalValidWrongCase'; did you mean 'Violet'?");
+  CHECK_ERROR_ENUM(TestCheck.getGlobal("GlobalNearMiss", Map),
+                   "invalid configuration value 'Yelow' for option "
+                   "'GlobalNearMiss'; did you mean 'Yellow'?");
+
+#undef CHECK_ERROR_ENUM
+}
+
+#undef CHECK_VAL
+#undef CHECK_ERROR
+
 } // namespace test
 } // namespace tidy
 } // namespace clang