diff --git a/cmd2/argparse_utils.py b/cmd2/argparse_utils.py index c01086401..b71ba7275 100644 --- a/cmd2/argparse_utils.py +++ b/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( @@ -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( @@ -751,6 +752,12 @@ 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 placeholder_parser = subparsers_action.add_parser(subcommand, **add_parser_kwargs) @@ -783,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.""" diff --git a/cmd2/cmd2.py b/cmd2/cmd2.py index 0d80dd007..325985bbd 100644 --- a/cmd2/cmd2.py +++ b/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: @@ -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) diff --git a/cmd2/decorators.py b/cmd2/decorators.py index 204161b44..d743b7b0b 100644 --- a/cmd2/decorators.py +++ b/cmd2/decorators.py @@ -360,9 +360,9 @@ def as_subcommand_to( :param subcommand: Subcommand name :param parser: instance of Cmd2ArgumentParser or a callable that returns a Cmd2ArgumentParser for this subcommand :param help: Help message for this subcommand which displays in the list of subcommands of the command we are adding to. - This is passed as the help argument to subparsers.add_parser(). - :param aliases: Alternative names for this subcommand. This is passed as the alias argument to - subparsers.add_parser(). + If not None, this is passed as the 'help' argument to subparsers.add_parser(). + :param aliases: Alternative names for this subcommand. If a non-empty sequence is provided, it is passed + as the 'aliases' argument to subparsers.add_parser(). :param add_parser_kwargs: other registration-specific kwargs for add_parser() (e.g. deprecated [Python 3.13+]) :return: a decorator which configures the target function to be a subcommand handler diff --git a/tests/test_argparse_utils.py b/tests/test_argparse_utils.py index 0bdf932f6..3be2263f4 100644 --- a/tests/test_argparse_utils.py +++ b/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 @@ -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="Subcommand 'sub' already exists for 'root'"): + root_parser.attach_subcommand([], "sub", sub_parser) + def test_subcommand_attachment_parser_class_override() -> None: class MyParser(Cmd2ArgumentParser): diff --git a/tests/test_cmd2.py b/tests/test_cmd2.py index 39aad2d27..330ab97b7 100644 --- a/tests/test_cmd2.py +++ b/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 @@ -4606,9 +4613,14 @@ 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 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="Subcommand 'sub' already exists for 'test'"): + app.attach_subcommand("test", "sub", cmd2.Cmd2ArgumentParser())