Adding check for invalid positionals

Henry Fredrick Schreiner
1 parent af79da5d
Showing 3 changed files with 37 additions and 1 deletions
include/CLI/App.hpp
include/CLI/Error.hpp
tests/CreationTest.cpp
@@ -198,7 +198,7 @@ public:
     /// 
     /// For example,
     /// 
-    ///     std::string filename
+    ///     std::string filename;
     ///     program.add_option("filename", filename, "description of filename");
     ///
     Option* add_option(
@@ -474,6 +474,7 @@ public:
     /// The real work is done here. Expects a reversed vector.
     /// Changes the vector to the remaining options.
     std::vector<std::string>& parse(std::vector<std::string> &args) {
+        _validate();
         _parse(args);
         run_callback();
         return args;
@@ -678,6 +679,18 @@ public:
  
 protected:
  
+    /// Check the options to make sure there are no conficts.
+    ///
+    /// Currenly checks to see if mutiple positionals exist with -1 args
+    void _validate() const {
+        auto count = std::count_if(std::begin(options_), std::end(options_),
+                [](const Option_p& opt){return opt->get_expected() == -1 && opt->get_positional();});
+        if(count > 1)
+            throw InvalidError(name_ + ": Too many positional arguments with unlimited expected args");
+        for(const App_p& app : subcommands_)
+            app->_validate();
+    }
+
  
     /// Return missing from the master
     missing_t* missing() {
@@ -100,6 +100,11 @@ struct ExtrasError : public ParseError {
     ExtrasError(std::string name) : ParseError("ExtrasError", name, 6) {}
 };
  
+/// Thrown when validation fails before parsing
+struct InvalidError : public ParseError {
+    InvalidError(std::string name) : ParseError("InvalidError", name, 15) {}
+};
+
 /// This is just a safety check to verify selection and parsing match
 struct HorribleError : public ParseError {
     HorribleError(std::string name) : ParseError("HorribleError", "(You should never see this error) " + name, 7) {}
@@ -35,6 +35,24 @@ TEST_F(TApp, AddingExistingWithCaseAfter2) {
     EXPECT_THROW(cat->ignore_case(), CLI::OptionAlreadyAdded);
 }
  
+TEST_F(TApp, AddingMultipleInfPositionals) {
+    std::vector<std::string> one, two;
+    app.add_option("one", one);
+    app.add_option("two", two);
+
+    EXPECT_THROW(run(), CLI::InvalidError);
+}
+
+
+TEST_F(TApp, AddingMultipleInfPositionalsSubcom) {
+    std::vector<std::string> one, two;
+    CLI::App* below = app.add_subcommand("below");
+    below->add_option("one", one);
+    below->add_option("two", two);
+
+    EXPECT_THROW(run(), CLI::InvalidError);
+}
+
 TEST_F(TApp, MultipleSubcomMatching) {
     app.add_subcommand("first");
     app.add_subcommand("second");