From 17ddce2fb2e76d538ec449c395ae9ae0a36ca215 Mon Sep 17 00:00:00 2001
From: Philip Top <phlptp@gmail.com>
Date: Tue, 30 Jul 2019 06:46:10 -0700
Subject: [PATCH] Add classification type traits (#286)

This cleans up the type checking a bit and makes it more readable, along with some other cleanup.

* start work on trying to clean up the type traits for which lexical cast overload to use

* fix readme issue and make the condition tests a little clearer

* add a check for out of range errors on boolean conversions

* Fix capitalization and some comments on option functions

* fix a few code analysis warnings for VS2019
---
 README.md                 |   2 +-
 include/CLI/App.hpp       |   4 +-
 include/CLI/Option.hpp    |  14 +--
 include/CLI/TypeTools.hpp | 215 ++++++++++++++++++++++++++------------
 tests/AppTest.cpp         |   9 ++
 tests/OptionGroupTest.cpp |   2 +-
 6 files changed, 171 insertions(+), 75 deletions(-)

diff --git a/README.md b/README.md
index 2a96e559..22d15069 100644
--- a/README.md
+++ b/README.md
@@ -604,7 +604,7 @@ The subcommand method
 .add_option_group(name,description)
 ```
 
-Will create an option group, and return a pointer to it.  An option group allows creation of a collection of options, similar to the groups function on options, but with additional controls and requirements.  They allow specific sets of options to be composed and controlled as a collective.  For an example see [range test](./tests/ranges.cpp).  Option groups are a specialization of an App so all [functions](#subcommand-options) that work with an App or subcommand also work on option groups.  Options can be created as part of an option group using the add functions just like a subcommand, or previously created options can be added through
+Will create an option group, and return a pointer to it.  An option group allows creation of a collection of options, similar to the groups function on options, but with additional controls and requirements.  They allow specific sets of options to be composed and controlled as a collective.  For an example see [range example](https://github.com/CLIUtils/CLI11/blob/master/examples/ranges.cpp).  Option groups are a specialization of an App so all [functions](#subcommand-options) that work with an App or subcommand also work on option groups.  Options can be created as part of an option group using the add functions just like a subcommand, or previously created options can be added through
 
 ```cpp
 ogroup->add_option(option_pointer);
diff --git a/include/CLI/App.hpp b/include/CLI/App.hpp
index c1e06b34..1e19607b 100644
--- a/include/CLI/App.hpp
+++ b/include/CLI/App.hpp
@@ -1292,7 +1292,7 @@ class App {
         }
 
         std::vector<std::string> args;
-        args.reserve(static_cast<size_t>(argc - 1));
+        args.reserve(static_cast<size_t>(argc) - 1);
         for(int i = argc - 1; i > 0; i--)
             args.emplace_back(argv[i]);
         parse(std::move(args));
@@ -2317,7 +2317,7 @@ class App {
 
             // LCOV_EXCL_START
         default:
-            HorribleError("unrecognized classifier (you should not see this!)");
+            throw HorribleError("unrecognized classifier (you should not see this!)");
             // LCOV_EXCL_END
         }
         return retval;
diff --git a/include/CLI/Option.hpp b/include/CLI/Option.hpp
index 3d90ab87..024e2fb7 100644
--- a/include/CLI/Option.hpp
+++ b/include/CLI/Option.hpp
@@ -527,7 +527,7 @@ class Option : public OptionBase<Option> {
         return this;
     }
 
-    /// disable flag overrides
+    /// Disable flag overrides values, e.g. --flag=<value> is not allowed
     Option *disable_flag_override(bool value = true) {
         disable_flag_override_ = value;
         return this;
@@ -564,7 +564,7 @@ class Option : public OptionBase<Option> {
     /// Get the short names
     const std::vector<std::string> get_snames() const { return snames_; }
 
-    /// get the flag names with specified default values
+    /// Get the flag names with specified default values
     const std::vector<std::string> get_fnames() const { return fnames_; }
 
     /// The number of times the option expects to be included
@@ -790,6 +790,7 @@ class Option : public OptionBase<Option> {
         return (detail::find_member(name, fnames_, ignore_case_, ignore_underscore_) >= 0);
     }
 
+    /// Get the value that goes for a flag, nominally gets the default value but allows for overrides if not disabled
     std::string get_flag_value(std::string name, std::string input_value) const {
         static const std::string trueString{"true"};
         static const std::string falseString{"false"};
@@ -855,7 +856,7 @@ class Option : public OptionBase<Option> {
     /// Get a copy of the results
     std::vector<std::string> results() const { return results_; }
 
-    /// get the results as a particular type
+    /// Get the results as a specified type
     template <typename T,
               enable_if_t<!is_vector<T>::value && !std::is_const<T>::value, detail::enabler> = detail::dummy>
     void results(T &output) const {
@@ -884,7 +885,7 @@ class Option : public OptionBase<Option> {
             throw ConversionError(get_name(), results_);
         }
     }
-    /// get the results as a vector of a particular type
+    /// Get the results as a vector of the specified type
     template <typename T> void results(std::vector<T> &output) const {
         output.clear();
         bool retval = true;
@@ -899,7 +900,7 @@ class Option : public OptionBase<Option> {
         }
     }
 
-    /// return the results as a particular type
+    /// Return the results as the specified type
     template <typename T> T as() const {
         T output;
         results(output);
@@ -980,7 +981,7 @@ class Option : public OptionBase<Option> {
     }
 
   private:
-    // run through the validators
+    // Run a result through the validators
     std::string _validate(std::string &result) {
         std::string err_msg;
         for(const auto &vali : validators_) {
@@ -995,6 +996,7 @@ class Option : public OptionBase<Option> {
         return err_msg;
     }
 
+    /// Add a single result to the result set, taking into account delimiters
     int _add_result(std::string &&result) {
         int result_count = 0;
         if(delimiter_ == '\0') {
diff --git a/include/CLI/TypeTools.hpp b/include/CLI/TypeTools.hpp
index b8cba544..f51a7d8a 100644
--- a/include/CLI/TypeTools.hpp
+++ b/include/CLI/TypeTools.hpp
@@ -228,6 +228,118 @@ std::string checked_to_string(T &&) {
     return std::string{};
 }
 
+// Enumeration of the different supported categorizations of objects
+enum objCategory : int {
+    integral_value = 2,
+    unsigned_integral = 4,
+    enumeration = 6,
+    boolean_value = 8,
+    floating_point = 10,
+    number_constructible = 12,
+    double_constructible = 14,
+    integer_constructible = 16,
+    vector_value = 30,
+    // string assignable or greater used in a condition so anything string like must come last
+    string_assignable = 50,
+    string_constructible = 60,
+    other = 200,
+
+};
+
+/// some type that is not otherwise recognized
+template <typename T, typename Enable = void> struct classify_object { static constexpr objCategory value{other}; };
+
+/// Set of overloads to classify an object according to type
+template <typename T>
+struct classify_object<T,
+                       typename std::enable_if<std::is_integral<T>::value && std::is_signed<T>::value &&
+                                               !is_bool<T>::value && !std::is_enum<T>::value>::type> {
+    static constexpr objCategory value{integral_value};
+};
+
+/// Unsigned integers
+template <typename T>
+struct classify_object<
+    T,
+    typename std::enable_if<std::is_integral<T>::value && std::is_unsigned<T>::value && !is_bool<T>::value>::type> {
+    static constexpr objCategory value{unsigned_integral};
+};
+
+/// Boolean values
+template <typename T> struct classify_object<T, typename std::enable_if<is_bool<T>::value>::type> {
+    static constexpr objCategory value{boolean_value};
+};
+
+/// Floats
+template <typename T> struct classify_object<T, typename std::enable_if<std::is_floating_point<T>::value>::type> {
+    static constexpr objCategory value{floating_point};
+};
+
+/// String and similar direct assignment
+template <typename T>
+struct classify_object<
+    T,
+    typename std::enable_if<!std::is_floating_point<T>::value && !std::is_integral<T>::value &&
+                            std::is_assignable<T &, std::string>::value && !is_vector<T>::value>::type> {
+    static constexpr objCategory value{string_assignable};
+};
+
+/// String and similar constructible and copy assignment
+template <typename T>
+struct classify_object<
+    T,
+    typename std::enable_if<!std::is_floating_point<T>::value && !std::is_integral<T>::value &&
+                            !std::is_assignable<T &, std::string>::value &&
+                            std::is_constructible<T, std::string>::value && !is_vector<T>::value>::type> {
+    static constexpr objCategory value{string_constructible};
+};
+
+/// Enumerations
+template <typename T> struct classify_object<T, typename std::enable_if<std::is_enum<T>::value>::type> {
+    static constexpr objCategory value{enumeration};
+};
+
+/// Handy helper to contain a bunch of checks that rule out many common types (integers, string like, floating point,
+/// vectors, and enumerations
+template <typename T> struct uncommon_type {
+    using type = typename std::conditional<!std::is_floating_point<T>::value && !std::is_integral<T>::value &&
+                                               !std::is_assignable<T &, std::string>::value &&
+                                               !std::is_constructible<T, std::string>::value && !is_vector<T>::value &&
+                                               !std::is_enum<T>::value,
+                                           std::true_type,
+                                           std::false_type>::type;
+    static const bool value = type::value;
+};
+
+/// Assignable from double or int
+template <typename T>
+struct classify_object<T,
+                       typename std::enable_if<uncommon_type<T>::value && is_direct_constructible<T, double>::value &&
+                                               is_direct_constructible<T, int>::value>::type> {
+    static constexpr objCategory value{number_constructible};
+};
+
+/// Assignable from int
+template <typename T>
+struct classify_object<T,
+                       typename std::enable_if<uncommon_type<T>::value && !is_direct_constructible<T, double>::value &&
+                                               is_direct_constructible<T, int>::value>::type> {
+    static const objCategory value{integer_constructible};
+};
+
+/// Assignable from double
+template <typename T>
+struct classify_object<T,
+                       typename std::enable_if<uncommon_type<T>::value && is_direct_constructible<T, double>::value &&
+                                               !is_direct_constructible<T, int>::value>::type> {
+    static const objCategory value{double_constructible};
+};
+
+/// vector type
+template <typename T> struct classify_object<T, typename std::enable_if<is_vector<T>::value>::type> {
+    static const objCategory value{vector_value};
+};
+
 // Type name print
 
 /// Was going to be based on
@@ -235,38 +347,45 @@ std::string checked_to_string(T &&) {
 /// But this is cleaner and works better in this case
 
 template <typename T,
-          enable_if_t<std::is_integral<T>::value && std::is_signed<T>::value, detail::enabler> = detail::dummy>
+          enable_if_t<classify_object<T>::value == integral_value || classify_object<T>::value == integer_constructible,
+                      detail::enabler> = detail::dummy>
 constexpr const char *type_name() {
     return "INT";
 }
 
-template <typename T,
-          enable_if_t<std::is_integral<T>::value && std::is_unsigned<T>::value, detail::enabler> = detail::dummy>
+template <typename T, enable_if_t<classify_object<T>::value == unsigned_integral, detail::enabler> = detail::dummy>
 constexpr const char *type_name() {
     return "UINT";
 }
 
-template <typename T, enable_if_t<std::is_floating_point<T>::value, detail::enabler> = detail::dummy>
+template <
+    typename T,
+    enable_if_t<classify_object<T>::value == floating_point || classify_object<T>::value == number_constructible ||
+                    classify_object<T>::value == double_constructible,
+                detail::enabler> = detail::dummy>
 constexpr const char *type_name() {
     return "FLOAT";
 }
 
 /// This one should not be used, since vector types print the internal type
-template <typename T, enable_if_t<is_vector<T>::value, detail::enabler> = detail::dummy>
+template <typename T, enable_if_t<classify_object<T>::value == vector_value, detail::enabler> = detail::dummy>
 constexpr const char *type_name() {
     return "VECTOR";
 }
 /// Print name for enumeration types
-template <typename T, enable_if_t<std::is_enum<T>::value, detail::enabler> = detail::dummy>
+template <typename T, enable_if_t<classify_object<T>::value == enumeration, detail::enabler> = detail::dummy>
 constexpr const char *type_name() {
     return "ENUM";
 }
 
+/// Print name for enumeration types
+template <typename T, enable_if_t<classify_object<T>::value == boolean_value, detail::enabler> = detail::dummy>
+constexpr const char *type_name() {
+    return "BOOLEAN";
+}
+
 /// Print for all other types
-template <typename T,
-          enable_if_t<!std::is_floating_point<T>::value && !std::is_integral<T>::value && !is_vector<T>::value &&
-                          !std::is_enum<T>::value,
-                      detail::enabler> = detail::dummy>
+template <typename T, enable_if_t<classify_object<T>::value >= string_assignable, detail::enabler> = detail::dummy>
 constexpr const char *type_name() {
     return "TEXT";
 }
@@ -286,6 +405,9 @@ inline int64_t to_flag_value(std::string val) {
     val = detail::to_lower(val);
     int64_t ret;
     if(val.size() == 1) {
+        if(val[0] >= '1' && val[0] <= '9') {
+            return (static_cast<int64_t>(val[0]) - '0');
+        }
         switch(val[0]) {
         case '0':
         case 'f':
@@ -293,22 +415,11 @@ inline int64_t to_flag_value(std::string val) {
         case '-':
             ret = -1;
             break;
-        case '1':
         case 't':
         case 'y':
         case '+':
             ret = 1;
             break;
-        case '2':
-        case '3':
-        case '4':
-        case '5':
-        case '6':
-        case '7':
-        case '8':
-        case '9':
-            ret = val[0] - '0';
-            break;
         default:
             throw std::invalid_argument("unrecognized character");
         }
@@ -325,10 +436,7 @@ inline int64_t to_flag_value(std::string val) {
 }
 
 /// Signed integers
-template <
-    typename T,
-    enable_if_t<std::is_integral<T>::value && std::is_signed<T>::value && !is_bool<T>::value && !std::is_enum<T>::value,
-                detail::enabler> = detail::dummy>
+template <typename T, enable_if_t<classify_object<T>::value == integral_value, detail::enabler> = detail::dummy>
 bool lexical_cast(const std::string &input, T &output) {
     try {
         size_t n = 0;
@@ -343,9 +451,7 @@ bool lexical_cast(const std::string &input, T &output) {
 }
 
 /// Unsigned integers
-template <typename T,
-          enable_if_t<std::is_integral<T>::value && std::is_unsigned<T>::value && !is_bool<T>::value, detail::enabler> =
-              detail::dummy>
+template <typename T, enable_if_t<classify_object<T>::value == unsigned_integral, detail::enabler> = detail::dummy>
 bool lexical_cast(const std::string &input, T &output) {
     if(!input.empty() && input.front() == '-')
         return false; // std::stoull happily converts negative values to junk without any errors.
@@ -363,7 +469,7 @@ bool lexical_cast(const std::string &input, T &output) {
 }
 
 /// Boolean values
-template <typename T, enable_if_t<is_bool<T>::value, detail::enabler> = detail::dummy>
+template <typename T, enable_if_t<classify_object<T>::value == boolean_value, detail::enabler> = detail::dummy>
 bool lexical_cast(const std::string &input, T &output) {
     try {
         auto out = to_flag_value(input);
@@ -371,11 +477,16 @@ bool lexical_cast(const std::string &input, T &output) {
         return true;
     } catch(const std::invalid_argument &) {
         return false;
+    } catch(const std::out_of_range &) {
+        // if the number is out of the range of a 64 bit value then it is still a number and for this purpose is still
+        // valid all we care about the sign
+        output = (input[0] != '-');
+        return true;
     }
 }
 
 /// Floats
-template <typename T, enable_if_t<std::is_floating_point<T>::value, detail::enabler> = detail::dummy>
+template <typename T, enable_if_t<classify_object<T>::value == floating_point, detail::enabler> = detail::dummy>
 bool lexical_cast(const std::string &input, T &output) {
     try {
         size_t n = 0;
@@ -389,27 +500,21 @@ bool lexical_cast(const std::string &input, T &output) {
 }
 
 /// String and similar direct assignment
-template <typename T,
-          enable_if_t<!std::is_floating_point<T>::value && !std::is_integral<T>::value &&
-                          std::is_assignable<T &, std::string>::value,
-                      detail::enabler> = detail::dummy>
+template <typename T, enable_if_t<classify_object<T>::value == string_assignable, detail::enabler> = detail::dummy>
 bool lexical_cast(const std::string &input, T &output) {
     output = input;
     return true;
 }
 
 /// String and similar constructible and copy assignment
-template <typename T,
-          enable_if_t<!std::is_floating_point<T>::value && !std::is_integral<T>::value &&
-                          !std::is_assignable<T &, std::string>::value && std::is_constructible<T, std::string>::value,
-                      detail::enabler> = detail::dummy>
+template <typename T, enable_if_t<classify_object<T>::value == string_constructible, detail::enabler> = detail::dummy>
 bool lexical_cast(const std::string &input, T &output) {
     output = T(input);
     return true;
 }
 
 /// Enumerations
-template <typename T, enable_if_t<std::is_enum<T>::value, detail::enabler> = detail::dummy>
+template <typename T, enable_if_t<classify_object<T>::value == enumeration, detail::enabler> = detail::dummy>
 bool lexical_cast(const std::string &input, T &output) {
     typename std::underlying_type<T>::type val;
     bool retval = detail::lexical_cast(input, val);
@@ -421,12 +526,7 @@ bool lexical_cast(const std::string &input, T &output) {
 }
 
 /// Assignable from double or int
-template <typename T,
-          enable_if_t<!std::is_floating_point<T>::value && !std::is_integral<T>::value &&
-                          !std::is_assignable<T &, std::string>::value &&
-                          !std::is_constructible<T, std::string>::value && !std::is_enum<T>::value &&
-                          is_direct_constructible<T, double>::value && is_direct_constructible<T, int>::value,
-                      detail::enabler> = detail::dummy>
+template <typename T, enable_if_t<classify_object<T>::value == number_constructible, detail::enabler> = detail::dummy>
 bool lexical_cast(const std::string &input, T &output) {
     int val;
     if(lexical_cast(input, val)) {
@@ -442,13 +542,8 @@ bool lexical_cast(const std::string &input, T &output) {
     return from_stream(input, output);
 }
 
-/// Assignable from int64
-template <typename T,
-          enable_if_t<!std::is_floating_point<T>::value && !std::is_integral<T>::value &&
-                          !std::is_assignable<T &, std::string>::value &&
-                          !std::is_constructible<T, std::string>::value && !std::is_enum<T>::value &&
-                          !is_direct_constructible<T, double>::value && is_direct_constructible<T, int>::value,
-                      detail::enabler> = detail::dummy>
+/// Assignable from int
+template <typename T, enable_if_t<classify_object<T>::value == integer_constructible, detail::enabler> = detail::dummy>
 bool lexical_cast(const std::string &input, T &output) {
     int val;
     if(lexical_cast(input, val)) {
@@ -459,12 +554,7 @@ bool lexical_cast(const std::string &input, T &output) {
 }
 
 /// Assignable from double
-template <typename T,
-          enable_if_t<!std::is_floating_point<T>::value && !std::is_integral<T>::value &&
-                          !std::is_assignable<T &, std::string>::value &&
-                          !std::is_constructible<T, std::string>::value && !std::is_enum<T>::value &&
-                          is_direct_constructible<T, double>::value && !is_direct_constructible<T, int>::value,
-                      detail::enabler> = detail::dummy>
+template <typename T, enable_if_t<classify_object<T>::value == double_constructible, detail::enabler> = detail::dummy>
 bool lexical_cast(const std::string &input, T &output) {
     double val;
     if(lexical_cast(input, val)) {
@@ -475,15 +565,10 @@ bool lexical_cast(const std::string &input, T &output) {
 }
 
 /// Non-string parsable by a stream
-template <typename T,
-          enable_if_t<!std::is_floating_point<T>::value && !std::is_integral<T>::value &&
-                          !std::is_assignable<T &, std::string>::value &&
-                          !std::is_constructible<T, std::string>::value && !std::is_enum<T>::value &&
-                          !is_direct_constructible<T, double>::value && !is_direct_constructible<T, int>::value,
-                      detail::enabler> = detail::dummy>
+template <typename T, enable_if_t<classify_object<T>::value == other, detail::enabler> = detail::dummy>
 bool lexical_cast(const std::string &input, T &output) {
     static_assert(is_istreamable<T>::value,
-                  "option object type must have a lexical cast overload or streaming input operator(>>) defined if it "
+                  "option object type must have a lexical cast overload or streaming input operator(>>) defined, if it "
                   "is convertible from another type use the add_option<T, XC>(...) with XC being the known type");
     return from_stream(input, output);
 }
diff --git a/tests/AppTest.cpp b/tests/AppTest.cpp
index 4742dfe1..4a829dff 100644
--- a/tests/AppTest.cpp
+++ b/tests/AppTest.cpp
@@ -647,6 +647,15 @@ TEST_F(TApp, BoolOption) {
     args = {"-b", "-7"};
     run();
     EXPECT_FALSE(bflag);
+
+    // cause an out of bounds error internally
+    args = {"-b", "751615654161688126132138844896646748852"};
+    run();
+    EXPECT_TRUE(bflag);
+
+    args = {"-b", "-751615654161688126132138844896646748852"};
+    run();
+    EXPECT_FALSE(bflag);
 }
 
 TEST_F(TApp, ShortOpts) {
diff --git a/tests/OptionGroupTest.cpp b/tests/OptionGroupTest.cpp
index e6de7014..2620e1b9 100644
--- a/tests/OptionGroupTest.cpp
+++ b/tests/OptionGroupTest.cpp
@@ -622,7 +622,7 @@ TEST_F(ManyGroups, Moving) {
 }
 
 struct ManyGroupsPreTrigger : public ManyGroups {
-    size_t triggerMain, trigger1{87u}, trigger2{34u}, trigger3{27u};
+    size_t triggerMain{0u}, trigger1{87u}, trigger2{34u}, trigger3{27u};
     ManyGroupsPreTrigger() {
         remove_required();
         app.preparse_callback([this](size_t count) { triggerMain = count; });