[FIX] Prohibit long-option value pairs wihtout a separation.

For example, given the long option name "--opt", then "--optValue" was
possible. This caused ambiguities if one long option name is the prefix
of another. We now only allow "--opt Value" and "--opt=value".

Author: smehringer

CHANGELOG.md

@@ -75,6 +75,15 @@ Note that 3.1.0 will be the first API stable release and interfaces in this rele
 
 ## Notable Bug-fixes
 
+### Argument Parser
+
+* Long option identifiers and their value must be separated by a space or equal sign `=`.
+  Handling this restriction resolves the ambiguity if one long option identifier is the prefix of 
+  another ([\#1792](https://github.com/seqan/seqan3/pull/1792)).
+
+  Valid short id value pairs: `-iValue`, `-i=Value`, `-i Value`
+  Valid long id value pairs: `--id=Value`, `--id Value` (prohibited now: `--idValue`)
+
 ### I/O
 
 * The `seqan3::field::cigar` was added to the default fields for reading and writing alignment files

include/seqan3/argument_parser/detail/format_parse.hpp

@@ -202,16 +202,25 @@ class format_parse : public format_base
     }
 
     /*!\brief Finds the position of a short/long identifier in format_parse::argv.
-     *
+     * \tparam id_type The identifier type; must be either of type `char` if it denotes a short identifier or
+     *                 std::string if it denotes a long identifier.
      * \param[in] begin_it The iterator where to start the search of the identifier.
      *                     Note that the end iterator is kept as a member variable.
      * \param[in] id       The identifier to search for (must not contain dashes).
      * \returns An iterator pointing to the first occurrence of the identifier in
      *          the list pointed to by begin_t. If the list does not contain the
      *          identifier `id`, the member variable `end_of_options_it` is returned.
      *
-     * Note: The `id` is compared to the prefix of each value in the list, such
-     *       that "-idValue" arguments are correctly identified.
+     * \details
+     *
+     * **Valid short-id value pairs are: `-iValue`, `-i=Value`, or `-i Value`**
+     * If the `id` passed to this function is of type `char`, it is assumed to be a short identifier.
+     * The `id` is found by comparing the prefix of every argument in argv to the `id` prepended with a single `-`.
+     *
+     * **Valid long id value pairs are: `--id=Value`, `--id Value`**.
+     * If the `id` passed to this function is of type `std::string`, it is assumed to be a long identifier.
+     * The `id` is found by comparing every argument in argv to `id` prepended with two dashes (`--`)
+     * or a prefix of such followed by the equal sign `=`.
      */
     template <typename id_type>
     std::vector<std::string>::iterator find_option_id(std::vector<std::string>::iterator const begin_it, id_type const & id)
@@ -220,13 +229,22 @@ class format_parse : public format_base
             return end_of_options_it;
 
         return (std::find_if(begin_it, end_of_options_it,
-            [&] (const std::string & v)
+            [&] (std::string const & current_arg)
             {
-                size_t id_size{(prepend_dash(id)).size()};
-                if (v.size() < id_size)
-                    return false; // cannot be the correct identifier
+                std::string full_id = prepend_dash(id);
 
-                return v.substr(0, id_size) == prepend_dash(id); // check if prefix of v is the same
+                if constexpr (std::same_as<id_type, char>) // short id
+                {
+                    // check if current_arg starts with "-o", i.e. it correctly identifies all short notations:
+                    // "-ovalue", "-o=value", and "-o value".
+                    return current_arg.substr(0, full_id.size()) == full_id;
+                }
+                else
+                {
+                    // only "--opt Value" or "--opt=Value" are valid
+                    return current_arg.substr(0, full_id.size()) == full_id && // prefix is the same
+                           (current_arg.size() == full_id.size() || current_arg[full_id.size()] == '='); // space or `=`
+                }
             }));
     }
 

test/unit/argument_parser/format_parse_test.cpp

@@ -60,16 +60,6 @@ TEST(parse_type_test, add_option_long_id)
     EXPECT_TRUE((testing::internal::GetCapturedStderr()).empty());
     EXPECT_EQ(option_value, "option_string");
 
-    // add with no space
-    const char * argv2[] = {"./argument_parser_test", "--string-optionoption_string"};
-    seqan3::argument_parser parser2{"test_parser", 2, argv2, false};
-    parser2.add_option(option_value, 'S', "string-option", "this is a string option.");
-
-    testing::internal::CaptureStderr();
-    EXPECT_NO_THROW(parser2.parse());
-    EXPECT_TRUE((testing::internal::GetCapturedStderr()).empty());
-    EXPECT_EQ(option_value, "option_string");
-
     const char * argv3[] = {"./argument_parser_test", "--string-option=option_string"};
     seqan3::argument_parser parser3{"test_parser", 2, argv3, false};
     parser3.add_option(option_value, 's', "string-option", "this is a string option.");
@@ -936,3 +926,46 @@ TEST(parse_test, subcommand_argument_parser_success)
                      seqan3::argument_parser_error);
     }
 }
+
+TEST(parse_test, issue1544)
+{
+    {   // wrong separation of long value:
+        std::string option_value;
+        const char * argv[] = {"./argument_parser_test", "--foohallo"};
+        seqan3::argument_parser parser{"test_parser", 2, argv, false};
+        parser.add_option(option_value, 'f', "foo", "this is a string option.");
+
+        EXPECT_THROW(parser.parse(), seqan3::unknown_option);
+    }
+
+    {   // unknown option (`--foo-bar`) that has a prefix of a known option (`--foo`)
+        std::string option_value;
+        const char * argv[] = {"./argument_parser_test", "--foo", "hallo", "--foo-bar", "ballo"};
+        seqan3::argument_parser parser{"test_parser", 5, argv, false};
+        parser.add_option(option_value, 'f', "foo", "this is a string option.");
+
+        EXPECT_THROW(parser.parse(), seqan3::unknown_option);
+    }
+
+    {   // known option (`--foo-bar`) that has a prefix of a unknown option (`--foo`)
+        std::string option_value;
+        const char * argv[] = {"./argument_parser_test", "--foo", "hallo", "--foo-bar", "ballo"};
+        seqan3::argument_parser parser{"test_parser", 5, argv, false};
+        parser.add_option(option_value, 'f', "foo-bar", "this is a string option.");
+
+        EXPECT_THROW(parser.parse(), seqan3::unknown_option);
+    }
+
+    {   // known option (`--foo`) is a prefix of another known option (`--foo-bar`)
+        std::string foo_option_value;
+        std::string foobar_option_value;
+        const char * argv[] = {"./argument_parser_test", "--foo", "hallo", "--foo-bar", "ballo"};
+        seqan3::argument_parser parser{"test_parser", 5, argv, false};
+        parser.add_option(foo_option_value, 'f', "foo", "this is a prefix of foobar.");
+        parser.add_option(foobar_option_value, 'b', "foo-bar", "this has prefix foo.");
+
+        EXPECT_NO_THROW(parser.parse());
+        EXPECT_EQ(foo_option_value, "hallo");
+        EXPECT_EQ(foobar_option_value, "ballo");
+    }
+}