Positional argument checks (#262)

* some tweaks with optional * remove set_results function that was bypassing some of the result processing in some cases of config files. * add positional Validator example and tests add CLI::Number validator. * add positional Validator example and tests add CLI::Number validator. * do some reformatting for style checks and remove auto in test lambda.

Positional argument checks (#262)
* some tweaks with optional * remove set_results function that was bypassing some of the result processing in some cases of config files. * add positional Validator example and tests add CLI::Number validator. * add positional Validator example and tests add CLI::Number validator. * do some reformatting for style checks and remove auto in test lambda.
Philip Top · Henry Schreiner
1 parent a1c18e05
Showing 10 changed files with 166 additions and 31 deletions
README.md
examples/CMakeLists.txt
examples/positional_validation.cpp
include/CLI/App.hpp
include/CLI/Option.hpp
include/CLI/Validators.hpp
tests/AppTest.cpp
tests/CreationTest.cpp
tests/HelpersTest.cpp
tests/OptionalTest.cpp
@@ -342,8 +342,9 @@ CLI11 has several Validators built-in that perform some common checks
 -   `CLI::ExistingPath`: Requires that the path (file or directory) exists.
 -   `CLI::NonexistentPath`: Requires that the path does not exist.
 -   `CLI::Range(min,max)`: Requires that the option be between min and max (make sure to use floating point if needed). Min defaults to 0.
--   `CLI::Bounded(min,max)`: 🚧 Modify the input such that it is always between min and max (make sure to use floating point if needed). Min defaults to 0.  Will produce an Error if conversion is not possible.
--   `CLI::PositiveNumber`: 🚧 Requires the number be greater or equal to 0.
+-   `CLI::Bounded(min,max)`: 🚧 Modify the input such that it is always between min and max (make sure to use floating point if needed). Min defaults to 0.  Will produce an error if conversion is not possible.
+-   `CLI::PositiveNumber`: 🚧 Requires the number be greater or equal to 0
+-   `CLI::Number`: 🚧 Requires the input be a number.
 -   `CLI::ValidIPV4`: 🚧 Requires that the option be a valid IPv4 string e.g. `'255.255.255.255'`, `'10.1.1.7'`.
 These Validators can be used by simply passing the name into the `check` or `transform` methods on an option
@@ -467,6 +468,7 @@ There are several options that are supported on the main app and subcommands and
 -   `.disable()`: 🚧 Specify that the subcommand is disabled, if given with a bool value it will enable or disable the subcommand or option group.
 -   `.disabled_by_default()`:🚧 Specify that at the start of parsing the subcommand/option_group should be disabled. This is useful for allowing some Subcommands to trigger others.
 -   `.enabled_by_default()`: 🚧 Specify that at the start of each parse the subcommand/option_group should be enabled.  This is useful for allowing some Subcommands to disable others.
+-   `.validate_positionals()`:🚧 Specify that positionals should pass validation before matching.  Validation is specified through `transform`, `check`, and `each` for an option.  If an argument fails validation it is not an error and matching proceeds to the next available positional or extra arguments.  
 -   `.excludes(option_or_subcommand)`: 🚧 If given an option pointer or pointer to another subcommand, these subcommands cannot be given together.  In the case of options, if the option is passed the subcommand cannot be used and will generate an error.  
 -   `.require_option()`: 🚧 Require 1 or more options or option groups be used.
 -   `.require_option(N)`:  🚧 Require `N` options or option groups, if `N>0`, or up to `N` if `N<0`. `N=0` resets to the default to 0 or more.
@@ -109,6 +109,22 @@ add_test(NAME positional_arity_fail COMMAND positional_arity 1 one two)
 set_property(TEST positional_arity_fail PROPERTY PASS_REGULAR_EXPRESSION
     "Could not convert")
+    add_cli_exe(positional_validation positional_validation.cpp)
+add_test(NAME positional_validation1 COMMAND positional_validation one )
+set_property(TEST positional_validation1 PROPERTY PASS_REGULAR_EXPRESSION
+    "File 1 = one")
+add_test(NAME positional_validation2 COMMAND positional_validation one 1 2 two )
+set_property(TEST positional_validation2 PROPERTY PASS_REGULAR_EXPRESSION
+    "File 1 = one"
+    "File 2 = two")
+add_test(NAME positional_validation3 COMMAND positional_validation 1 2 one)
+set_property(TEST positional_validation3 PROPERTY PASS_REGULAR_EXPRESSION
+    "File 1 = one")
+add_test(NAME positional_validation4 COMMAND positional_validation 1 one two 2)
+set_property(TEST positional_validation4 PROPERTY PASS_REGULAR_EXPRESSION
+     "File 1 = one"
+    "File 2 = two")
+
 add_cli_exe(shapes shapes.cpp)
 add_test(NAME shapes_all COMMAND shapes circle 4.4 circle 10.7 rectangle 4 4 circle 2.3 triangle 4.5 ++ rectangle 2.1 ++ circle 234.675)
 set_property(TEST shapes_all PROPERTY PASS_REGULAR_EXPRESSION
+#include "CLI/CLI.hpp"
+
+int main(int argc, char **argv) {
+
+    CLI::App app("test for positional validation");
+
+    int num1 = -1, num2 = -1;
+    app.add_option("num1", num1, "first number")->check(CLI::Number);
+    app.add_option("num2", num2, "second number")->check(CLI::Number);
+    std::string file1, file2;
+    app.add_option("file1", file1, "first file")->required();
+    app.add_option("file2", file2, "second file");
+    app.validate_positionals();
+
+    CLI11_PARSE(app, argc, argv);
+
+    if(num1 != -1)
+        std::cout << "Num1 = " << num1 << '\n';
+
+    if(num2 != -1)
+        std::cout << "Num2 = " << num2 << '\n';
+
+    std::cout << "File 1 = " << file1 << '\n';
+    if(!file2.empty()) {
+        std::cout << "File 2 = " << file2 << '\n';
+    }
+
+    return 0;
+}
@@ -187,7 +187,8 @@ class App {
     bool disabled_by_default_{false};
     /// If set to true the subcommand will be reenabled at the start of each parse
     bool enabled_by_default_{false};
-
+    /// If set to true positional options are validated before assigning INHERITABLE
+    bool validate_positionals_{false};
     /// A pointer to the parent if this is a subcommand
     App *parent_{nullptr};
@@ -250,6 +251,7 @@ class App {
             ignore_case_ = parent_->ignore_case_;
             ignore_underscore_ = parent_->ignore_underscore_;
             fallthrough_ = parent_->fallthrough_;
+            validate_positionals_ = parent_->validate_positionals_;
             allow_windows_style_options_ = parent_->allow_windows_style_options_;
             group_ = parent_->group_;
             footer_ = parent_->footer_;
@@ -334,6 +336,12 @@ class App {
         return this;
     }
+    /// Set the subcommand to validate positional arguments before assigning
+    App *validate_positionals(bool validate = true) {
+        validate_positionals_ = validate;
+        return this;
+    }
+
     /// Remove the error when extras are left over on the command line.
     /// Will also call App::allow_extras().
     App *allow_config_extras(bool allow = true) {
@@ -489,18 +497,19 @@ class App {
         return add_option(option_name, CLI::callback_t(), option_description, false);
     }
-    /// Add option for non-vectors with a default print
+    /// Add option for non-vectors with a default print, allow template to specify conversion type
     template <typename T,
-              enable_if_t<!is_vector<T>::value && !std::is_const<T>::value, detail::enabler> = detail::dummy>
+              typename XC = T,
+              enable_if_t<!is_vector<XC>::value && !std::is_const<XC>::value, detail::enabler> = detail::dummy>
     Option *add_option(std::string option_name,
                        T &variable, ///< The variable to set
                        std::string option_description,
                        bool defaulted) {
-
-        CLI::callback_t fun = [&variable](CLI::results_t res) { return detail::lexical_cast(res[0], variable); };
+        static_assert(std::is_constructible<T, XC>::value, "assign type must be assignable from conversion type");
+        CLI::callback_t fun = [&variable](CLI::results_t res) { return detail::lexical_cast<XC>(res[0], variable); };
         Option *opt = add_option(option_name, fun, option_description, defaulted);
-        opt->type_name(detail::type_name<T>());
+        opt->type_name(detail::type_name<XC>());
         if(defaulted) {
             std::stringstream out;
             out << variable;
@@ -1654,6 +1663,8 @@ class App {
     /// Get the status of disabled by default
     bool get_enabled_by_default() const { return enabled_by_default_; }
+    /// Get the status of validating positionals
+    bool get_validate_positionals() const { return validate_positionals_; }
     /// Get the status of allow extras
     bool get_allow_config_extras() const { return allow_config_extras_; }
@@ -2192,7 +2203,7 @@ class App {
                 op->add_result(res);
             } else {
-                op->set_results(item.inputs);
+                op->add_result(item.inputs);
                 op->run_callback();
             }
         }
@@ -2274,7 +2285,13 @@ class App {
             // Eat options, one by one, until done
             if(opt->get_positional() &&
                (static_cast<int>(opt->count()) < opt->get_items_expected() || opt->get_items_expected() < 0)) {
-
+                if(validate_positionals_) {
+                    std::string pos = positional;
+                    pos = opt->_validate(pos);
+                    if(!pos.empty()) {
+                        continue;
+                    }
+                }
                 opt->add_result(positional);
                 parse_order_.push_back(opt.get());
                 args.pop_back();
@@ -666,19 +666,11 @@ class Option : public OptionBase&lt;Option&gt; {
         // Run the validators (can change the string)
         if(!validators_.empty()) {
-            for(std::string &result : results_)
-                for(const auto &vali : validators_) {
-                    std::string err_msg;
-
-                    try {
-                        err_msg = vali(result);
-                    } catch(const ValidationError &err) {
-                        throw ValidationError(get_name(), err.what());
-                    }
-
-                    if(!err_msg.empty())
-                        throw ValidationError(get_name(), err_msg);
-                }
+            for(std::string &result : results_) {
+                auto err_msg = _validate(result);
+                if(!err_msg.empty())
+                    throw ValidationError(get_name(), err_msg);
+            }
         }
         if(!(callback_)) {
             return;
@@ -842,13 +834,6 @@ class Option : public OptionBase&lt;Option&gt; {
         return this;
     }
-    /// Set the results vector all at once
-    Option *set_results(std::vector<std::string> result_vector) {
-        results_ = std::move(result_vector);
-        callback_run_ = false;
-        return this;
-    }
-
     /// Get a copy of the results
     std::vector<std::string> results() const { return results_; }
@@ -963,6 +948,21 @@ class Option : public OptionBase&lt;Option&gt; {
     }
   private:
+    // run through the validators
+    std::string _validate(std::string &result) {
+        std::string err_msg;
+        for(const auto &vali : validators_) {
+            try {
+                err_msg = vali(result);
+            } catch(const ValidationError &err) {
+                err_msg = err.what();
+            }
+            if(!err_msg.empty())
+                break;
+        }
+        return err_msg;
+    }
+
     int _add_result(std::string &&result) {
         int result_count = 0;
         if(delimiter_ == '\0') {
@@ -321,6 +321,20 @@ class PositiveNumber : public Validator {
     }
 };
+/// Validate the argument is a number and greater than or equal to 0
+class Number : public Validator {
+  public:
+    Number() : Validator("NUMBER") {
+        func_ = [](std::string &number_str) {
+            double number;
+            if(!detail::lexical_cast(number_str, number)) {
+                return "Failed parsing as a number " + number_str;
+            }
+            return std::string();
+        };
+    }
+};
+
 } // namespace detail
 // Static is not needed here, because global const implies static.
@@ -343,6 +357,9 @@ const detail::IPV4Validator ValidIPV4;
 /// Check for a positive number
 const detail::PositiveNumber PositiveNumber;
+/// Check for a number
+const detail::Number Number;
+
 /// Produce a range (factory). Min and max are inclusive.
 class Range : public Validator {
   public:
@@ -962,6 +962,27 @@ TEST_F(TApp, PositionalAtEnd) {
     EXPECT_THROW(run(), CLI::ExtrasError);
 }
+// Tests positionals at end
+TEST_F(TApp, PositionalValidation) {
+    std::string options;
+    std::string foo;
+
+    app.add_option("bar", options)->check(CLI::Number);
+    app.add_option("foo", foo);
+    app.validate_positionals();
+    args = {"1", "param1"};
+    run();
+
+    EXPECT_EQ(options, "1");
+    EXPECT_EQ(foo, "param1");
+
+    args = {"param1", "1"};
+    run();
+
+    EXPECT_EQ(options, "1");
+    EXPECT_EQ(foo, "param1");
+}
+
 TEST_F(TApp, PositionalNoSpaceLong) {
     std::vector<std::string> options;
     std::string foo, bar;
@@ -467,7 +467,7 @@ TEST_F(TApp, GetNameCheck) {
 }
 TEST_F(TApp, SubcommandDefaults) {
-    // allow_extras, prefix_command, ignore_case, fallthrough, group, min/max subcommand
+    // allow_extras, prefix_command, ignore_case, fallthrough, group, min/max subcommand, validate_positionals
     // Initial defaults
     EXPECT_FALSE(app.get_allow_extras());
@@ -481,6 +481,8 @@ TEST_F(TApp, SubcommandDefaults) {
     EXPECT_FALSE(app.get_allow_windows_style_options());
 #endif
     EXPECT_FALSE(app.get_fallthrough());
+    EXPECT_FALSE(app.get_validate_positionals());
+
     EXPECT_EQ(app.get_footer(), "");
     EXPECT_EQ(app.get_group(), "Subcommands");
     EXPECT_EQ(app.get_require_subcommand_min(), 0u);
@@ -498,6 +500,7 @@ TEST_F(TApp, SubcommandDefaults) {
 #endif
     app.fallthrough();
+    app.validate_positionals();
     app.footer("footy");
     app.group("Stuff");
     app.require_subcommand(2, 3);
@@ -516,6 +519,7 @@ TEST_F(TApp, SubcommandDefaults) {
     EXPECT_TRUE(app2->get_allow_windows_style_options());
 #endif
     EXPECT_TRUE(app2->get_fallthrough());
+    EXPECT_TRUE(app2->get_validate_positionals());
     EXPECT_EQ(app2->get_footer(), "footy");
     EXPECT_EQ(app2->get_group(), "Stuff");
     EXPECT_EQ(app2->get_require_subcommand_min(), 0u);
@@ -239,6 +239,21 @@ TEST(Validators, PositiveValidator) {
     EXPECT_FALSE(CLI::PositiveNumber(num).empty());
 }
+TEST(Validators, NumberValidator) {
+    std::string num = "1.1.1.1";
+    EXPECT_FALSE(CLI::Number(num).empty());
+    num = "1.7";
+    EXPECT_TRUE(CLI::Number(num).empty());
+    num = "10000";
+    EXPECT_TRUE(CLI::Number(num).empty());
+    num = "-0.000";
+    EXPECT_TRUE(CLI::Number(num).empty());
+    num = "+1.55";
+    EXPECT_TRUE(CLI::Number(num).empty());
+    num = "a";
+    EXPECT_FALSE(CLI::Number(num).empty());
+}
+
 TEST(Validators, CombinedAndRange) {
     auto crange = CLI::Range(0, 12) & CLI::Range(4, 16);
     EXPECT_TRUE(crange("4").empty());
@@ -62,6 +62,20 @@ TEST_F(TApp, BoostOptionalTest) {
     EXPECT_EQ(*opt, 3);
 }
+TEST_F(TApp, BoostOptionalVector) {
+    boost::optional<std::vector<int>> opt;
+    app.add_option_function<std::vector<int>>("-v,--vec", [&opt](const std::vector<int> &v) { opt = v; }, "some vector")
+        ->expected(3);
+    run();
+    EXPECT_FALSE(opt);
+
+    args = {"-v", "1", "4", "5"};
+    run();
+    EXPECT_TRUE(opt);
+    std::vector<int> expV{1, 4, 5};
+    EXPECT_EQ(*opt, expV);
+}
+
 #endif
 #if !CLI11_OPTIONAL