Raise ValueError in attach_subcommand() when the subcommand already exists.

Author: kmvanbrunt

cmd2/argparse_utils.py

@@ -729,7 +729,8 @@ def attach_subcommand(
         :raises TypeError: if subcommand_parser is not an instance of the following or their subclasses:
                            1. Cmd2ArgumentParser
                            2. The parser_class configured for the target subcommand group
-        :raises ValueError: if the command path is invalid or doesn't support subcommands
+        :raises ValueError: if the command path is invalid, doesn't support subcommands, or the
+                            subcommand already exists
         """
         if not isinstance(subcommand_parser, Cmd2ArgumentParser):
             raise TypeError(
@@ -752,7 +753,11 @@ def attach_subcommand(
             )
 
         # Use add_parser to register the subcommand name and any aliases
-        placeholder_parser = subparsers_action.add_parser(subcommand, **add_parser_kwargs)
+        try:
+            placeholder_parser = subparsers_action.add_parser(subcommand, **add_parser_kwargs)
+        except ArgumentError as ex:
+            # The subcommand already exists
+            raise ValueError(str(ex)) from None
 
         # To ensure accurate usage strings, recursively update 'prog' values
         # within the injected parser to match its new location in the command hierarchy.

cmd2/cmd2.py

@@ -1199,7 +1199,8 @@ def attach_subcommand(
         :raises TypeError: if subcommand_parser is not an instance of the following or their subclasses:
                            1. Cmd2ArgumentParser
                            2. The parser_class configured for the target subcommand group
-        :raises ValueError: if the command path is invalid or doesn't support subcommands
+        :raises ValueError: if the command path is invalid, doesn't support subcommands, or the
+                            subcommand already exists
         """
         root_parser, subcommand_path = self.get_root_parser_and_subcmd_path(command)
         root_parser.attach_subcommand(subcommand_path, subcommand, subcommand_parser, **add_parser_kwargs)

tests/test_argparse_utils.py

@@ -467,6 +467,12 @@ def test_subcommand_attachment_errors() -> None:
     with pytest.raises(TypeError, match=r"must be an instance of 'Cmd2ArgumentParser' \(or a subclass\)"):
         root_parser.attach_subcommand([], "sub", ap_parser)  # type: ignore[arg-type]
 
+    # Verify ValueError when subcommand name already exists
+    sub_parser = Cmd2ArgumentParser(prog="sub")
+    root_parser.attach_subcommand([], "sub", sub_parser)
+    with pytest.raises(ValueError, match="conflicting subparser: sub"):
+        root_parser.attach_subcommand([], "sub", sub_parser)
+
 
 def test_subcommand_attachment_parser_class_override() -> None:
     class MyParser(Cmd2ArgumentParser):

tests/test_cmd2.py

@@ -4596,6 +4596,13 @@ class SubcmdErrorApp(cmd2.Cmd):
         def __init__(self) -> None:
             super().__init__()
 
+        test_parser = cmd2.Cmd2ArgumentParser()
+        test_parser.add_subparsers(required=True)
+
+        @cmd2.with_argparser(test_parser)
+        def do_test(self, _statement: cmd2.Statement) -> None:
+            pass
+
         def do_no_argparse(self, _statement: cmd2.Statement) -> None:
             pass
 
@@ -4612,3 +4619,8 @@ def do_no_argparse(self, _statement: cmd2.Statement) -> None:
     # Test command that doesn't use argparse
     with pytest.raises(ValueError, match="Command 'no_argparse' does not use argparse"):
         app.attach_subcommand("no_argparse", "sub", cmd2.Cmd2ArgumentParser())
+
+    # Test duplicate subcommand
+    app.attach_subcommand("test", "sub", cmd2.Cmd2ArgumentParser())
+    with pytest.raises(ValueError, match="conflicting subparser: sub"):
+        app.attach_subcommand("test", "sub", cmd2.Cmd2ArgumentParser())