[clang-tidy][NFC] Added convienence methods for getting optional options

[njames93]

Summary

These methods abstract away Error handling when trying to read options that can't be parsed by logging the error automatically and returning None.

[njames93]

Fix wrong patch diff

[gribozavr2]

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

224

Is this specialization defined only because parsing a string option can never fail? I'd let this special case behavior fall out of the primary template if possible.

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

485

"logIfOptionParsingError" to express the conditionality of behavior?

[njames93]

Rename logOptionParsingError

[njames93]

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

224

It's because the call to get that returns a std::string isn't a template.
So in the actual template definition of getOptional, trying to call get<T> when T is a std::string results in a compile time error.
Consuming the error makes sense knowing the it will always be a MissingOptionError

[njames93]

Revert to using warning for logging parsing errors

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

public:
     /// ``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 {
       if (llvm::Expected<T> ValueOr = get<T>(LocalName))
         return *ValueOr;
       else
-        logErrToStdErr(ValueOr.takeError());
+        logOptionParsingError(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
public:
     /// 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_integral<T>::value, T>
     getLocalOrGlobal(StringRef LocalName, T Default) const {
       if (llvm::Expected<T> ValueOr = getLocalOrGlobal<T>(LocalName))
         return *ValueOr;
       else
-        logErrToStdErr(ValueOr.takeError());
+        logOptionParsingError(ValueOr.takeError());
       return 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
public:
     /// \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
-        logErrToStdErr(ValueOr.takeError());
+        logOptionParsingError(ValueOr.takeError());
       return 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 local or
     /// global ``CheckOptions``. Gets local option first. If local is not
public:
     /// supply the mapping required to convert between ``T`` and a string.
     template <typename T>
     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
-        logErrToStdErr(ValueOr.takeError());
+        logOptionParsingError(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
+        logOptionParsingError(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
+        logOptionParsingError(ValueOr.takeError());
+      return llvm::None;
+    }
+
     /// 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
     /// integer value \p Value to \p Options.
     template <typename T>
typeEraseMapping() const {
                             MappedItem.second);
       }
       return Result;
     }
 
     void storeInt(ClangTidyOptions::OptionMap &Options, StringRef LocalName,
                   int64_t Value) const;
 
-    static void logErrToStdErr(llvm::Error &&Err);
+    /// Logs an Error to stderr if a \p Err is not a MissingOptionError.
+    static void logOptionParsingError(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; }

 
 /// Stores an option with the check-local name \p LocalName with
 /// bool value \p Value to \p Options.
 template <>
 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
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANGTIDYCHECK_H

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/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
 
 namespace clang {
 namespace tidy {
 
 char MissingOptionError::ID;
 char UnparseableEnumOptionError::ID;
 char UnparseableIntegerOptionError::ID;

 }
 
 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());
+  logOptionParsingError(ValueOr.takeError());
   return Default;
 }
 
 template <>
 llvm::Expected<bool>
 ClangTidyCheck::OptionsView::getLocalOrGlobal<bool>(StringRef LocalName) const {
   auto Iter = findPriorityOption(CheckOptions, NamePrefix, LocalName);
   if (Iter != CheckOptions.end())
     return getAsBool(Iter->second.Value, Iter->first);
   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;
-  logErrToStdErr(ValueOr.takeError());
+  logOptionParsingError(ValueOr.takeError());
   return Default;
 }
 
 void ClangTidyCheck::OptionsView::store(ClangTidyOptions::OptionMap &Options,
                                         StringRef LocalName,
                                         StringRef Value) const {
   Options[NamePrefix + LocalName.str()] = Value;
 }
llvm::Expected<int64_t> ClangTidyCheck::OptionsView::getEnumInt(
   }
   if (EditDistance < 3)
     return llvm::make_error<UnparseableEnumOptionError>(
         Iter->first, Iter->second.Value, std::string(Closest));
   return llvm::make_error<UnparseableEnumOptionError>(Iter->first,
                                                       Iter->second.Value);
 }
 
-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: ");
+void ClangTidyCheck::OptionsView::logOptionParsingError(llvm::Error &&Err) {
+  auto RemainingErrors = llvm::handleErrors(
+      std::move(Err), [](const MissingOptionError &) -> llvm::Error {
+      });
+  if (RemainingErrors)
+    llvm::logAllUnhandledErrors(std::move(RemainingErrors),
+                                llvm::WithColor::error());
 }
+
+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;
+}
+
+template <>
+Optional<std::string>
+ClangTidyCheck::OptionsView::getOptionalLocalOrGlobal<std::string>(
+    StringRef LocalName) const {
+  if (auto ValueOr = getLocalOrGlobal(LocalName))
+    return *ValueOr;
+  else
+    consumeError(ValueOr.takeError());
+  return llvm::None;
+}
+
 } // namespace tidy
 } // namespace clang