[MISC] Refactor parse()

include/sharg/parser.hpp

@@ -359,7 +359,6 @@ class parser
      * \throws sharg::too_many_arguments if the command line call contained more arguments than expected.
      * \throws sharg::too_few_arguments if the command line call contained less arguments than expected.
      * \throws sharg::validation_error if the argument was not excepted by the provided validator.
-     * \throws sharg::user_input_error if a subparser was configured at construction but a subcommand is missing.
      *
      * \details
      *
@@ -419,63 +418,22 @@ class parser
         if (parse_was_called)
             throw design_error("The function parse() must only be called once!");
 
-        // Before creating the detail::version_checker, we have to make sure that
-        // malicious code cannot be injected through the app name.
-        if (!std::regex_match(info.app_name, app_name_regex))
-        {
-            throw design_error{("The application name must only contain alpha-numeric characters or '_' and '-' "
-                                "(regex: \"^[a-zA-Z0-9_-]+$\").")};
-        }
-
-        for (auto & sub : this->subcommands)
-        {
-            if (!std::regex_match(sub, app_name_regex))
-            {
-                throw design_error{"The subcommand name must only contain alpha-numeric characters or '_' and '-' "
-                                   "(regex: \"^[a-zA-Z0-9_-]+$\")."};
-            }
-        }
+        parse_was_called = true;
 
-        detail::version_checker app_version{info.app_name, info.version, info.url};
+        // User input sanitization must happen before version check!
+        verify_app_and_subcommand_names();
 
         init();
 
-        if (std::holds_alternative<detail::format_parse>(format) && !subcommands.empty() && sub_parser == nullptr)
-        {
-            assert(!subcommands.empty());
-            std::string subcommands_str{"["};
-            for (std::string const & command : subcommands)
-                subcommands_str += command + ", ";
-            subcommands_str.replace(subcommands_str.size() - 2, 2, "]"); // replace last ", " by "]"
-
-            throw too_few_arguments{"You misspelled the subcommand! Please specify which sub-program "
-                                    "you want to use: one of "
-                                    + subcommands_str
-                                    + ". Use -h/--help for more "
-                                      "information."};
-        }
-
-        if (app_version.decide_if_check_is_performed(version_check_dev_decision, version_check_user_decision))
-        {
-            // must be done before calling parse on the format because this might std::exit
-            std::promise<bool> app_version_prom;
-            version_check_future = app_version_prom.get_future();
-            app_version(std::move(app_version_prom));
-        }
-
+        // Apply all defered operations to the parser, e.g., `add_option`, `add_flag`, `add_positional_option`.
         for (auto & operation : operations)
             operation();
 
-        std::visit(
-            [this]<typename T>(T & f)
-            {
-                if constexpr (std::same_as<T, detail::format_tdl>)
-                    f.parse(info, executable_name);
-                else
-                    f.parse(info);
-            },
-            format);
-        parse_was_called = true;
+        // The version check, which might exit the program, must be called before calling parse on the format.
+        run_version_check();
+
+        // Parse the command line arguments.
+        parse_format();
 
         // Exit after parsing any special format.
         if (!std::holds_alternative<detail::format_parse>(format))
@@ -1059,6 +1017,92 @@ class parser
         if (parse_was_called)
             throw design_error{detail::to_string(function_name.data(), " may only be used before calling parse().")};
     }
+
+    /*!\brief Verifies that the app and subcommand names are correctly formatted.
+     * \throws sharg::design_error if the app name is not correctly formatted.
+     * \throws sharg::design_error if the subcommand names are not correctly formatted.
+     * \details
+     * The app name must only contain alphanumeric characters, '_', or '-'.
+     * The subcommand names must only contain alphanumeric characters, '_', or '-'.
+     */
+    inline void verify_app_and_subcommand_names() const
+    {
+        // Before creating the detail::version_checker, we have to make sure that
+        // malicious code cannot be injected through the app name.
+        if (!std::regex_match(info.app_name, app_name_regex))
+        {
+            throw design_error{("The application name must only contain alpha-numeric characters or '_' and '-' "
+                                "(regex: \"^[a-zA-Z0-9_-]+$\").")};
+        }
+
+        for (auto & sub : this->subcommands)
+        {
+            if (!std::regex_match(sub, app_name_regex))
+            {
+                throw design_error{"The subcommand name must only contain alpha-numeric characters or '_' and '-' "
+                                   "(regex: \"^[a-zA-Z0-9_-]+$\")."};
+            }
+        }
+    }
+
+    /*!\brief Runs the version check if the user has not disabled it.
+     * \details
+     * If the user has not disabled the version check, the function will start a detached thread that will call the
+     * sharg::detail::version_checker and print a message if a new version is available.
+     */
+    inline void run_version_check()
+    {
+        detail::version_checker app_version{info.app_name, info.version, info.url};
+
+        if (app_version.decide_if_check_is_performed(version_check_dev_decision, version_check_user_decision))
+        {
+            // must be done before calling parse on the format because this might std::exit
+            std::promise<bool> app_version_prom;
+            version_check_future = app_version_prom.get_future();
+            app_version(std::move(app_version_prom));
+        }
+    }
+
+    /*!\brief Parses the command line arguments according to the format.
+     * \throws sharg::option_declared_multiple_times if an option that is not a list was declared multiple times.
+     * \throws sharg::user_input_error if an incorrect argument is given as (positional) option value.
+     * \throws sharg::required_option_missing if the user did not provide a required option.
+     * \throws sharg::too_many_arguments if the command line call contained more arguments than expected.
+     * \throws sharg::too_few_arguments if the command line call contained less arguments than expected.
+     * \throws sharg::validation_error if the argument was not excepted by the provided validator.
+     * \details
+     * This function calls the parse function of the format member variable.
+     */
+    inline void parse_format()
+    {
+        auto format_parse_fn = [this]<typename format_t>(format_t & f)
+        {
+            if constexpr (std::same_as<format_t, detail::format_tdl>)
+                f.parse(info, executable_name);
+            else
+                f.parse(info);
+        };
+
+        try
+        {
+            std::visit(std::move(format_parse_fn), format);
+        }
+        catch (sharg::too_many_arguments const & exception)
+        {
+            if (subcommands.empty() || sub_parser)
+                throw;
+
+            std::string message = exception.what();
+            message += "\nIf you intended to execute a subcommand, it is possible that you have misspelled it.\n"
+                       "Available subcommands are: [";
+
+            for (std::string const & command : subcommands)
+                message += command + ", ";
+            message.replace(message.size() - 2, 2, "].\n"); // replace last ", " by "].\n"
+
+            throw too_many_arguments{message};
+        }
+    }
 };
 
 } // namespace sharg

test/unit/parser/format_parse_test.cpp

@@ -680,17 +680,43 @@ TEST_F(format_parse_test, subcommand_parser_success)
 
 TEST_F(format_parse_test, subcommand_parser_error)
 {
+    auto parser = get_parser();
+
+    constexpr std::string_view expected_message =
+        "Too many arguments provided. Please see -h/--help for more information.\n"
+        "If you intended to execute a subcommand, it is possible that you have misspelled it.\n"
+        "Available subcommands are: [sub1].\n";
+
+    auto check_error = [&parser, &expected_message](char const * file, int line)
+    {
+        ::testing::ScopedTrace trace(file, line, "check_error");
+        try
+        {
+            parser.parse();
+            FAIL();
+        }
+        catch (sharg::too_many_arguments const & exception)
+        {
+            std::string_view const actual_message{exception.what()};
+            EXPECT_EQ(expected_message, actual_message);
+        }
+        catch (...)
+        {
+            FAIL();
+        }
+    };
+
     // incorrect sub command regardless of following arguments, https://github.com/seqan/seqan3/issues/2172
-    auto top_level_parser = get_subcommand_parser({"subiddysub", "-f"}, {"sub1"});
-    EXPECT_THROW(top_level_parser.parse(), sharg::parser_error);
+    parser = get_subcommand_parser({"subiddysub", "-f"}, {"sub1"});
+    check_error(__FILE__, __LINE__);
 
     // incorrect sub command with no other arguments
-    top_level_parser = get_subcommand_parser({"subiddysub"}, {"sub1"});
-    EXPECT_THROW(top_level_parser.parse(), sharg::parser_error);
+    parser = get_subcommand_parser({"subiddysub"}, {"sub1"});
+    check_error(__FILE__, __LINE__);
 
     // incorrect sub command with trailing special option, https://github.com/seqan/sharg-parser/issues/171
-    top_level_parser = get_subcommand_parser({"subiddysub", "-h"}, {"sub1"});
-    EXPECT_THROW(top_level_parser.parse(), sharg::parser_error);
+    parser = get_subcommand_parser({"subiddysub", "-h"}, {"sub1"});
+    check_error(__FILE__, __LINE__);
 }
 
 TEST_F(format_parse_test, issue1544)

test/unit/parser/parser_design_error_test.cpp

@@ -263,25 +263,27 @@ TEST_F(design_error_test, subcommand_parser_error)
     EXPECT_THROW(parser.parse(), sharg::design_error);
 
     // no positional options are allowed
-    parser = get_subcommand_parser({"foo"}, {"foo"});
+    parser = get_subcommand_parser({"-f", "foo"}, {"foo"});
 
     EXPECT_THROW(parser.add_positional_option(flag_value, sharg::config{}), sharg::design_error);
     EXPECT_NO_THROW(parser.add_option(flag_value, sharg::config{.short_id = 'o'}));
     EXPECT_NO_THROW(parser.add_flag(flag_value, sharg::config{.short_id = 'f'}));
-    EXPECT_EQ(flag_value, false);
-
     EXPECT_THROW(parser.get_sub_parser(), sharg::design_error);
+    EXPECT_EQ(flag_value, false);
     EXPECT_NO_THROW(parser.parse()); // Prints nothing, but sets sub_parser.
     EXPECT_NO_THROW(parser.get_sub_parser());
+    EXPECT_EQ(flag_value, true);
+
+    flag_value = false;
 
     // options are allowed
     parser = get_subcommand_parser({"-o", "true"}, {"foo"});
 
     EXPECT_THROW(parser.add_positional_option(flag_value, sharg::config{}), sharg::design_error);
     EXPECT_NO_THROW(parser.add_option(flag_value, sharg::config{.short_id = 'o'}));
-    EXPECT_NO_THROW(parser.add_flag(flag_value, sharg::config{.short_id = 'f'}));
     EXPECT_EQ(flag_value, false);
-    EXPECT_THROW(parser.parse(), sharg::user_input_error); // Todo: Subcommand is required?
+    EXPECT_NO_THROW(parser.parse());
+    EXPECT_EQ(flag_value, true);
 }
 
 TEST_F(design_error_test, not_allowed_after_parse)