Fixed inconsistent behavior between Python versions by always checking for duplicate subcommands.

Author: kmvanbrunt

cmd2/argparse_utils.py

@@ -516,7 +516,7 @@ def _SubParsersAction_remove_parser(  # noqa: N802
     :raises ValueError: if the subcommand doesn't exist
     """
     if name not in self._name_parser_map:
-        raise ValueError(f"Subcommand '{name}' not found")
+        raise ValueError(f"Subcommand '{name}' does not exist")
 
     subparser = self._name_parser_map[name]
 
@@ -684,12 +684,12 @@ def update_prog(self, prog: str) -> None:
         # add_parser() will have the correct prog value.
         subparsers_action._prog_prefix = self._build_subparsers_prog_prefix(positionals)
 
-        # subparsers_action.choices includes aliases. Since primary names are inserted first,
-        # we skip already updated parsers to ensure primary names are used in 'prog'.
+        # subparsers_action._name_parser_map includes aliases. Since primary names are inserted
+        # first, we skip already updated parsers to ensure primary names are used in 'prog'.
         updated_parsers: set[Cmd2ArgumentParser] = set()
 
         # Set the prog value for each subcommand's parser
-        for subcmd_name, subcmd_parser in subparsers_action.choices.items():
+        for subcmd_name, subcmd_parser in subparsers_action._name_parser_map.items():
             if subcmd_parser in updated_parsers:
                 continue
 
@@ -707,9 +707,9 @@ def find_parser(self, subcommand_path: Iterable[str]) -> "Cmd2ArgumentParser":
         parser = self
         for name in subcommand_path:
             subparsers_action = parser.get_subparsers_action()
-            if name not in subparsers_action.choices:
-                raise ValueError(f"Subcommand '{name}' not found in '{parser.prog}'")
-            parser = subparsers_action.choices[name]
+            if name not in subparsers_action._name_parser_map:
+                raise ValueError(f"Subcommand '{name}' does not exist for '{parser.prog}'")
+            parser = subparsers_action._name_parser_map[name]
         return parser
 
     def attach_subcommand(
@@ -752,12 +752,14 @@ def attach_subcommand(
                 f"Received: '{type(subcommand_parser).__name__}'."
             )
 
+        # Do not overwrite existing subcommands or aliases
+        all_names = (subcommand, *add_parser_kwargs.get("aliases", ()))
+        for name in all_names:
+            if name in subparsers_action._name_parser_map:
+                raise ValueError(f"Subcommand '{name}' already exists for '{target_parser.prog}'")
+
         # Use add_parser to register the subcommand name and any aliases
-        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
+        placeholder_parser = subparsers_action.add_parser(subcommand, **add_parser_kwargs)
 
         # To ensure accurate usage strings, recursively update 'prog' values
         # within the injected parser to match its new location in the command hierarchy.
@@ -788,7 +790,7 @@ def detach_subcommand(self, subcommand_path: Iterable[str], subcommand: str) ->
                 subparsers_action.remove_parser(subcommand),  # type: ignore[attr-defined]
             )
         except ValueError:
-            raise ValueError(f"Subcommand '{subcommand}' not found in '{target_parser.prog}'") from None
+            raise ValueError(f"Subcommand '{subcommand}' does not exist for '{target_parser.prog}'") from None
 
     def error(self, message: str) -> NoReturn:
         """Override that applies custom formatting to the error message."""

cmd2/cmd2.py

@@ -1174,7 +1174,7 @@ def get_root_parser_and_subcmd_path(self, command: str) -> tuple[Cmd2ArgumentPar
         # Search for the base command function and verify it has an argparser defined
         command_func = self.get_command_func(root_command)
         if command_func is None:
-            raise ValueError(f"Root command '{root_command}' not found")
+            raise ValueError(f"Root command '{root_command}' does not exist")
 
         root_parser = self.command_parsers.get(command_func)
         if root_parser is None:

tests/test_argparse_utils.py

@@ -453,13 +453,13 @@ def test_subcommand_attachment_errors() -> None:
     root_parser.add_subparsers()
 
     # Verify ValueError when path is invalid (find_parser() fails)
-    with pytest.raises(ValueError, match="Subcommand 'nonexistent' not found"):
+    with pytest.raises(ValueError, match="Subcommand 'nonexistent' does not exist for 'root'"):
         root_parser.attach_subcommand(["nonexistent"], "anything", child_parser)
-    with pytest.raises(ValueError, match="Subcommand 'nonexistent' not found"):
+    with pytest.raises(ValueError, match="Subcommand 'nonexistent' does not exist for 'root'"):
         root_parser.detach_subcommand(["nonexistent"], "anything")
 
     # Verify ValueError when path is valid but subcommand name is wrong
-    with pytest.raises(ValueError, match="Subcommand 'fake' not found in 'root'"):
+    with pytest.raises(ValueError, match="Subcommand 'fake' does not exist for 'root'"):
         root_parser.detach_subcommand([], "fake")
 
     # Verify TypeError when attaching a non-Cmd2ArgumentParser type
@@ -470,7 +470,7 @@ def test_subcommand_attachment_errors() -> None:
     # 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"):
+    with pytest.raises(ValueError, match="Subcommand 'sub' already exists for 'root'"):
         root_parser.attach_subcommand([], "sub", sub_parser)
 
 

tests/test_cmd2.py

@@ -4613,7 +4613,7 @@ def do_no_argparse(self, _statement: cmd2.Statement) -> None:
         app.attach_subcommand("", "sub", cmd2.Cmd2ArgumentParser())
 
     # Test non-existent command
-    with pytest.raises(ValueError, match="Root command 'fake' not found"):
+    with pytest.raises(ValueError, match="Root command 'fake' does not exist"):
         app.attach_subcommand("fake", "sub", cmd2.Cmd2ArgumentParser())
 
     # Test command that doesn't use argparse
@@ -4622,5 +4622,5 @@ def do_no_argparse(self, _statement: cmd2.Statement) -> None:
 
     # Test duplicate subcommand
     app.attach_subcommand("test", "sub", cmd2.Cmd2ArgumentParser())
-    with pytest.raises(ValueError, match="conflicting subparser: sub"):
+    with pytest.raises(ValueError, match="Subcommand 'sub' already exists for 'test'"):
         app.attach_subcommand("test", "sub", cmd2.Cmd2ArgumentParser())