Merge pull request #40 from pavel-belikov/subparsers-validation

Subparsers validation

4 files changed, 302 insertions, 53 deletions

diff --git a/Doxyfile b/Doxyfile
index faf7fe3..74c187b 100644
--- a/Doxyfile
+++ b/Doxyfile
@@ -833,7 +833,7 @@ EXCLUDE_PATTERNS       = catch.hpp
 # Note that the wildcards are matched against the file with absolute path, so to
 # exclude all test directories use the pattern */test/*
 
-EXCLUDE_SYMBOLS        =
+EXCLUDE_SYMBOLS        = args::Command::RaiiSubparser args::SubparserError
 
 # The EXAMPLE_PATH tag can be used to specify one or more files or directories
 # that contain example code fragments that are included (see the \include
diff --git a/README.md b/README.md
index f3660a1..c89804a 100644
--- a/README.md
+++ b/README.md
@@ -59,6 +59,9 @@ It:
 * Allows you to create subparsers somewhat like argparse, through the use of
   kick-out arguments (check the gitlike.cxx example program for a simple sample
   of this)
+* Allow one value flag to take a specific number of values (like `--foo first
+  second`, where --foo slurps both arguments).
+* Allow you to have value flags only optionally accept values
 
 # What does it not do?
 
@@ -66,14 +69,9 @@ There are tons of things this library does not do!
 
 ## It will not ever:
 
-* Allow one value flag to take a specific number of values (like `--foo first
-  second`, where --foo slurps both arguments).  You can instead split that with
-  a flag list (`--foo first --foo second`) or a custom type extraction (
-  `--foo first,second`)
 * Allow you to intermix multiple different prefix types (eg. `++foo` and
   `--foo` in the same parser), though shortopt and longopt prefixes can be
   different.
-* Allow you to have value flags only optionally accept values
 * Allow you to make flags sensitive to order (like gnu find), or make them
   sensitive to relative ordering with positionals.  The only orderings that are
   order-sensitive are:
@@ -152,7 +150,7 @@ groups and spit out messages accordingly.
 
 Yes.  tests.cxx in the git repository has a set of standard tests (which are
 still relatively small in number, but I would welcome some expansion here), and
-thanks to GitLab's CI, these tests run with every single push:
+thanks to Travis CI and AppVeyor, these tests run with every single push:
 
 ```shell
 % make runtests
diff --git a/args.hxx b/args.hxx
index 4eff869..6ec4ab0 100644
--- a/args.hxx
+++ b/args.hxx
@@ -160,7 +160,8 @@ namespace args
         Required,
         Map,
         Extra,
-        Help
+        Help,
+        Subparser,
     };
 #else
     /** Base error class
@@ -234,6 +235,15 @@ namespace args
             Help(const std::string &flag) : Error(flag) {}
             virtual ~Help() {};
     };
+
+    /** (INTERNAL) An exception that emulates coroutine-like control flow for subparsers.
+     */
+    class SubparserError : public Error
+    {
+        public:
+            SubparserError() : Error("") {}
+            virtual ~SubparserError() {};
+    };
 #endif
 
     /** A simple unified option type for unified initializer lists for the Matcher class.
@@ -394,6 +404,8 @@ namespace args
             }
     };
 
+    /** Attributes for flags.
+     */
     enum class Options
     {
         /** Default options.
@@ -527,7 +539,7 @@ namespace args
             const std::string help;
 #ifdef ARGS_NOEXCEPT
             /// Only for ARGS_NOEXCEPT
-            Error error;
+            mutable Error error;
 #endif
 
         public:
@@ -544,7 +556,7 @@ namespace args
                 return matched;
             }
 
-            virtual void Validate(const std::string &, const std::string &)
+            virtual void Validate(const std::string &, const std::string &) const
             {
             }
 
@@ -664,6 +676,10 @@ namespace args
             }
     };
 
+    /** A number of arguments which can be consumed by an option.
+     *
+     * Represents a closed interval [min, max].
+     */
     struct Nargs
     {
         const size_t min;
@@ -674,7 +690,7 @@ namespace args
 #ifndef ARGS_NOEXCEPT
             if (max < min)
             {
-                throw std::invalid_argument("Nargs: max > min");
+                throw UsageError("Nargs: max > min");
             }
 #endif
         }
@@ -718,7 +734,7 @@ namespace args
                 return nullptr;
             }
 
-            virtual void Validate(const std::string &shortPrefix, const std::string &longPrefix) override
+            virtual void Validate(const std::string &shortPrefix, const std::string &longPrefix) const override
             {
                 if (!Matched() && (GetOptions() & Options::Required) != Options::None)
                 {
@@ -758,6 +774,20 @@ namespace args
                 return true;
             }
 
+#ifdef ARGS_NOEXCEPT
+            /// Only for ARGS_NOEXCEPT
+            virtual Error GetError() const override
+            {
+                const auto nargs = NumberOfArguments();
+                if (nargs.min > nargs.max)
+                {
+                    return Error::Usage;
+                }
+
+                return error;
+            }
+#endif
+
             /** Defines how many values can be consumed by this option.
              *
              * \return closed interval [min, max]
@@ -847,7 +877,7 @@ namespace args
                 return { "[" + Name() + ']' };
             }
 
-            virtual void Validate(const std::string &, const std::string &) override
+            virtual void Validate(const std::string &, const std::string &) const override
             {
                 if ((GetOptions() & Options::Required) != Options::None && !Matched())
                 {
@@ -962,7 +992,7 @@ namespace args
                 return nullptr;
             }
 
-            virtual void Validate(const std::string &shortPrefix, const std::string &longPrefix) override
+            virtual void Validate(const std::string &shortPrefix, const std::string &longPrefix) const override
             {
                 for (Base *child: Children())
                 {
@@ -1136,6 +1166,8 @@ namespace args
 
     };
 
+    /** Class for using global options in ArgumentParser.
+     */
     class GlobalOptions : public Group
     {
         public:
@@ -1145,6 +1177,23 @@ namespace args
             }
     };
 
+    /** Utility class for building subparsers with coroutines/callbacks.
+     *
+     * Brief example:
+     * \code
+     * Command command(argumentParser, "command", "my command", [](args::Subparser &s)
+     * {
+     *      // your command flags/positionals
+     *      s.Parse(); //required
+     *      //your command code
+     * });
+     * \endcode
+     *
+     * For ARGS_NOEXCEPT mode don't forget to check `s.GetError()` after `s.Parse()`
+     * and return if it isn't equals to args::Error::None.
+     *
+     * \sa Command
+     */
     class Subparser : public Group
     {
         private:
@@ -1175,19 +1224,31 @@ namespace args
                 return command;
             }
 
+            /** (INTERNAL) Determines whether Parse was called or not.
+             */
             bool IsParsed() const
             {
                 return isParsed;
             }
 
+            /** Continue parsing arguments for new command.
+             */
             void Parse();
 
+            /** Returns a vector of kicked out arguments.
+             *
+             * \sa Base::KickOut
+             */
             const std::vector<std::string> &KickedOut() const noexcept
             {
                 return kicked;
             }
     };
 
+    /** Main class for building subparsers.
+     *
+     * /sa Subparser
+     */
     class Command : public Group
     {
         private:
@@ -1200,13 +1261,17 @@ namespace args
             std::string proglinePostfix;
 
             std::function<void(Subparser&)> parserCoroutine;
-            bool commandIsRequired = false;
+            bool commandIsRequired = true;
             Command *selectedCommand = nullptr;
 
             mutable std::vector<std::tuple<std::string, std::string, unsigned>> subparserDescription;
             mutable std::vector<std::string> subparserProgramLine;
             mutable bool subparserHasFlag = false;
             mutable bool subparserHasPositional = false;
+            mutable bool subparserHasCommand = false;
+#ifdef ARGS_NOEXCEPT
+            mutable Error subparserError = Error::None;
+#endif
             mutable Subparser *subparser = nullptr;
 
         protected:
@@ -1299,40 +1364,34 @@ namespace args
             void Epilog(const std::string &epilog_)
             { this->epilog = epilog_; }
 
-            const std::function<void(Subparser&)> &GetCoroutine() const
-            {
-                return parserCoroutine;
-            }
-
+            /** The name of command
+             */
             const std::string &Name() const
-            {
-                return name;
-            }
+            { return name; }
 
+            /** The description of command
+             */
             const std::string &Help() const
-            {
-                return help;
-            }
+            { return help; }
+
+            /** If value is true, parser will fail if no command was parsed.
+             *
+             * Default: true.
+             */
+            void RequireCommand(bool value)
+            { commandIsRequired = value; }
 
             virtual bool IsGroup() const override
-            {
-                return false;
-            }
+            { return false; }
 
             virtual bool Matched() const noexcept override
-            {
-                return Base::Matched();
-            }
+            { return Base::Matched(); }
 
             operator bool() const noexcept
-            {
-                return Matched();
-            }
+            { return Matched(); }
 
             void Match() noexcept
-            {
-                matched = true;
-            }
+            { matched = true; }
 
             void SelectCommand(Command *c) noexcept
             {
@@ -1344,11 +1403,6 @@ namespace args
                 }
             }
 
-            void RequireCommand(bool value)
-            {
-                commandIsRequired = value;
-            }
-
             virtual FlagBase *Match(const EitherFlag &flag) override
             {
                 if (selectedCommand != nullptr)
@@ -1431,7 +1485,7 @@ namespace args
                 auto res = Group::GetProgramLine(params);
                 res.insert(res.end(), subparserProgramLine.begin(), subparserProgramLine.end());
 
-                if (!params.proglineCommand.empty() && Group::HasCommand())
+                if (!params.proglineCommand.empty() && (Group::HasCommand() || subparserHasCommand))
                 {
                     res.insert(res.begin(), commandIsRequired ? params.proglineCommand : "[" + params.proglineCommand + "]");
                 }
@@ -1493,7 +1547,7 @@ namespace args
                     {
                         parserCoroutine(coro.Parser());
                     }
-                    catch (args::Help)
+                    catch (args::SubparserError)
                     {
                     }
 #else
@@ -1577,8 +1631,13 @@ namespace args
                 return descriptions;
             }
 
-            virtual void Validate(const std::string &shortprefix, const std::string &longprefix) override
+            virtual void Validate(const std::string &shortprefix, const std::string &longprefix) const override
             {
+                if (!Matched())
+                {
+                    return;
+                }
+
                 for (Base *child: Children())
                 {
                     if (child->IsGroup() && !child->Matched())
@@ -1594,6 +1653,22 @@ namespace args
 
                     child->Validate(shortprefix, longprefix);
                 }
+
+                if (subparser != nullptr)
+                {
+                    subparser->Validate(shortprefix, longprefix);
+                }
+
+                if (selectedCommand == nullptr && commandIsRequired && (Group::HasCommand() || subparserHasCommand))
+                {
+#ifdef ARGS_NOEXCEPT
+                    error = Error::Validation;
+#else
+                    std::ostringstream problem;
+                    problem << "Command is required";
+                    throw ValidationError(problem.str());
+#endif
+                }
             }
 
             virtual void Reset() noexcept override
@@ -1604,7 +1679,34 @@ namespace args
                 subparserDescription.clear();
                 subparserHasFlag = false;
                 subparserHasPositional = false;
+                subparserHasCommand = false;
+#ifdef ARGS_NOEXCEPT
+                subparserError = Error::None;
+#endif
+            }
+
+#ifdef ARGS_NOEXCEPT
+            /// Only for ARGS_NOEXCEPT
+            virtual Error GetError() const override
+            {
+                if (!Matched())
+                {
+                    return Error::None;
+                }
+
+                if (error != Error::None)
+                {
+                    return error;
+                }
+
+                if (subparserError != Error::None)
+                {
+                    return subparserError;
+                }
+
+                return Group::GetError();
             }
+#endif
     };
 
     /** The main user facing command line argument parser class
@@ -1877,10 +1979,22 @@ namespace args
                             RaiiSubparser coro(*this, std::vector<std::string>(it, end));
                             coroutine(coro.Parser());
 #ifdef ARGS_NOEXCEPT
-                            if (GetError() != Error::None)
+                            error = GetError();
+                            if (error != Error::None)
+                            {
+                                return end;
+                            }
+
+                            if (!coro.Parser().IsParsed())
                             {
+                                error = Error::Usage;
                                 return end;
                             }
+#else
+                            if (!coro.Parser().IsParsed())
+                            {
+                                throw UsageError("Subparser::Parse was not called");
+                            }
 #endif
 
                             break;
@@ -2011,10 +2125,10 @@ namespace args
 
             /** Change allowed option separation.
              *
-             * \param allowJoinedShortValue Allow a short flag that accepts an argument to be passed its argument immediately next to it (ie. in the same argv field)
-             * \param allowJoinedLongValue Allow a long flag that accepts an argument to be passed its argument separated by the longseparator (ie. in the same argv field)
-             * \param allowSeparateShortValue Allow a short flag that accepts an argument to be passed its argument separated by whitespace (ie. in the next argv field)
-             * \param allowSeparateLongValue Allow a long flag that accepts an argument to be passed its argument separated by whitespace (ie. in the next argv field)
+             * \param allowJoinedShortValue_ Allow a short flag that accepts an argument to be passed its argument immediately next to it (ie. in the same argv field)
+             * \param allowJoinedLongValue_ Allow a long flag that accepts an argument to be passed its argument separated by the longseparator (ie. in the same argv field)
+             * \param allowSeparateShortValue_ Allow a short flag that accepts an argument to be passed its argument separated by whitespace (ie. in the next argv field)
+             * \param allowSeparateLongValue_ Allow a long flag that accepts an argument to be passed its argument separated by whitespace (ie. in the next argv field)
              */
             void SetArgumentSeparations(
                 const bool allowJoinedShortValue_,
@@ -2159,6 +2273,13 @@ namespace args
             {
                 // Reset all Matched statuses and errors
                 Reset();
+#ifdef ARGS_NOEXCEPT
+                error = GetError();
+                if (error != Error::None)
+                {
+                    return end;
+                }
+#endif
                 return Parse(begin, end);
             }
 
@@ -2204,22 +2325,29 @@ namespace args
     void Subparser::Parse()
     {
         isParsed = true;
+        Reset();
         command.subparserDescription = GetDescription(helpParams, 0);
         command.subparserHasFlag = HasFlag();
         command.subparserHasPositional = HasPositional();
+        command.subparserHasCommand = HasCommand();
         command.subparserProgramLine = GetProgramLine(helpParams);
         if (parser == nullptr)
         {
 #ifndef ARGS_NOEXCEPT
-            throw args::Help("");
+            throw args::SubparserError();
 #else
-            error = Error::Help;
+            error = Error::Subparser;
             return;
 #endif
         }
 
         auto it = parser->Parse(args.begin(), args.end());
+        command.Validate(parser->ShortPrefix(), parser->LongPrefix());
         kicked.assign(it, args.end());
+
+#ifdef ARGS_NOEXCEPT
+        command.subparserError = GetError();
+#endif
     }
 
     inline std::ostream &operator<<(std::ostream &os, const ArgumentParser &parser)
diff --git a/test.cxx b/test.cxx
index 135d1ee..ba4d5f8 100644
--- a/test.cxx
+++ b/test.cxx
@@ -635,8 +635,11 @@ TEST_CASE("Nargs work as expected", "[args]")
     args::NargsValueFlag<int> a(parser, "", "", {'a'}, 2);
     args::NargsValueFlag<int> b(parser, "", "", {'b'}, {2, 3});
     args::NargsValueFlag<std::string> c(parser, "", "", {'c'}, {0, 2});
+    args::NargsValueFlag<int> d(parser, "", "", {'d'}, {1, 3});
     args::Flag f(parser, "", "", {'f'});
 
+    REQUIRE_THROWS_AS(args::Nargs(3, 2), args::UsageError);
+
     REQUIRE_NOTHROW(parser.ParseArgs(std::vector<std::string>{"-a", "1", "2"}));
     REQUIRE((args::get(a) == std::vector<int>{1, 2}));
 
@@ -676,6 +679,9 @@ TEST_CASE("Nargs work as expected", "[args]")
     REQUIRE_NOTHROW(parser.ParseArgs(std::vector<std::string>{"-cf"}));
     REQUIRE((args::get(c) == std::vector<std::string>{"f"}));
     REQUIRE(args::get(f) == false);
+
+    REQUIRE_THROWS_AS(parser.ParseArgs(std::vector<std::string>{"-d"}), args::ParseError);
+    REQUIRE_THROWS_AS(parser.ParseArgs(std::vector<std::string>{"-b"}), args::ParseError);
 }
 
 TEST_CASE("Simple commands work as expected", "[args]")
@@ -764,6 +770,7 @@ TEST_CASE("Subparser help works as expected", "[args]")
     });
 
     p.Prog("git");
+    p.RequireCommand(false);
 
     std::ostringstream s;
 
@@ -857,6 +864,51 @@ TEST_CASE("Subparser help works as expected", "[args]")
 
 }
 
+TEST_CASE("Subparser validation works as expected", "[args]")
+{
+    args::ArgumentParser p("parser");
+    args::Command a(p, "a", "command a", [](args::Subparser &s)
+    {
+        args::ValueFlag<std::string> f(s, "", "", {'f'}, args::Options::Required);
+        s.Parse();
+    });
+
+    args::Command b(p, "b", "command b");
+    args::ValueFlag<std::string> f(b, "", "", {'f'}, args::Options::Required);
+
+    args::Command c(p, "c", "command c", [](args::Subparser&){});
+
+    REQUIRE_THROWS_AS(p.ParseArgs(std::vector<std::string>{}), args::ValidationError);
+    REQUIRE_THROWS_AS(p.ParseArgs(std::vector<std::string>{"a"}), args::RequiredError);
+    REQUIRE_NOTHROW(p.ParseArgs(std::vector<std::string>{"a", "-f", "F"}));
+    REQUIRE_THROWS_AS(p.ParseArgs(std::vector<std::string>{"b"}), args::RequiredError);
+    REQUIRE_NOTHROW(p.ParseArgs(std::vector<std::string>{"b", "-f", "F"}));
+
+    p.RequireCommand(false);
+    REQUIRE_NOTHROW(p.ParseArgs(std::vector<std::string>{}));
+
+    REQUIRE_THROWS_AS(p.ParseArgs(std::vector<std::string>{"c"}), args::UsageError);
+
+    REQUIRE_THROWS_AS(p.ParseArgs(std::vector<std::string>{"unknown-command"}), args::ParseError);
+}
+
+TEST_CASE("Global options work as expected", "[args]")
+{
+    args::Group globals;
+    args::Flag f(globals, "f", "f", {'f'});
+
+    args::ArgumentParser p("parser");
+    args::GlobalOptions g(p, globals);
+    args::Command a(p, "a", "command a");
+    args::Command b(p, "b", "command b");
+
+    p.RequireCommand(false);
+
+    REQUIRE_NOTHROW(p.ParseArgs(std::vector<std::string>{"-f"}));
+    REQUIRE_NOTHROW(p.ParseArgs(std::vector<std::string>{"a", "-f"}));
+    REQUIRE_NOTHROW(p.ParseArgs(std::vector<std::string>{"b", "-f"}));
+}
+
 #undef ARGS_HXX
 #define ARGS_TESTNAMESPACE
 #define ARGS_NOEXCEPT
@@ -920,3 +972,74 @@ TEST_CASE("Noexcept mode works as expected", "[args]")
     parser.ParseArgs(std::vector<std::string>{"--mf", "yellow"});
     REQUIRE(parser.GetError() == argstest::Error::None);
 }
+
+TEST_CASE("Required flags work as expected in noexcept mode", "[args]")
+{
+    argstest::ArgumentParser parser1("Test command");
+    argstest::ValueFlag<int> foo(parser1, "foo", "foo", {'f', "foo"}, argstest::Options::Required);
+    argstest::ValueFlag<int> bar(parser1, "bar", "bar", {'b', "bar"});
+
+    parser1.ParseArgs(std::vector<std::string>{"-f", "42"});
+    REQUIRE(foo.Get() == 42);
+    REQUIRE(parser1.GetError() == argstest::Error::None);
+
+    parser1.ParseArgs(std::vector<std::string>{"-b4"});
+    REQUIRE(parser1.GetError() == argstest::Error::Required);
+
+    argstest::ArgumentParser parser2("Test command");
+    argstest::Positional<int> pos1(parser2, "a", "a");
+    parser2.ParseArgs(std::vector<std::string>{});
+    REQUIRE(parser2.GetError() == argstest::Error::None);
+
+    argstest::ArgumentParser parser3("Test command");
+    argstest::Positional<int> pos2(parser3, "a", "a", argstest::Options::Required);
+    parser3.ParseArgs(std::vector<std::string>{});
+    REQUIRE(parser3.GetError() == argstest::Error::Required);
+}
+
+TEST_CASE("Subparser validation works as expected in noexcept mode", "[args]")
+{
+    argstest::ArgumentParser p("parser");
+    argstest::Command a(p, "a", "command a", [](argstest::Subparser &s)
+    {
+        argstest::ValueFlag<std::string> f(s, "", "", {'f'}, argstest::Options::Required);
+        s.Parse();
+    });
+
+    argstest::Command b(p, "b", "command b");
+    argstest::ValueFlag<std::string> f(b, "", "", {'f'}, argstest::Options::Required);
+
+    argstest::Command c(p, "c", "command c", [](argstest::Subparser&){});
+
+    p.ParseArgs(std::vector<std::string>{});
+    REQUIRE(p.GetError() == argstest::Error::Validation);
+
+    p.ParseArgs(std::vector<std::string>{"a"});
+    REQUIRE((size_t)p.GetError() == (size_t)argstest::Error::Required);
+
+    p.ParseArgs(std::vector<std::string>{"a", "-f", "F"});
+    REQUIRE(p.GetError() == argstest::Error::None);
+
+    p.ParseArgs(std::vector<std::string>{"b"});
+    REQUIRE(p.GetError() == argstest::Error::Required);
+
+    p.ParseArgs(std::vector<std::string>{"b", "-f", "F"});
+    REQUIRE(p.GetError() == argstest::Error::None);
+
+    p.RequireCommand(false);
+    p.ParseArgs(std::vector<std::string>{});
+    REQUIRE(p.GetError() == argstest::Error::None);
+
+    p.ParseArgs(std::vector<std::string>{"c"});
+    REQUIRE(p.GetError() == argstest::Error::Usage);
+}
+
+TEST_CASE("Nargs work as expected in noexcept mode", "[args]")
+{
+    argstest::ArgumentParser parser("Test command");
+    argstest::NargsValueFlag<int> a(parser, "", "", {'a'}, {3, 2});
+
+    parser.ParseArgs(std::vector<std::string>{"-a", "1", "2"});
+    REQUIRE(parser.GetError() == argstest::Error::Usage);
+}
+