Validate subparser class and remove default subcommand title (#1636)

* Improved attach_subcommand() by validating that attached parsers are
  Cmd2ArgumentParser instances and match the subparsers group's
  parser_class.

* Removed default subcommands title from add_subparsers() to align
  with standard argparse behavior.

Author: kmvanbrunt

CHANGELOG.md

@@ -78,6 +78,9 @@ prompt is displayed.
       the Enhancements section below for details).
     - Removed `Cmd.undoc_header` since all commands are now considered categorized.
     - Renamed `Cmd.cmd_func()` to `Cmd.get_command_func()`.
+    - `cmd2` no longer sets a default title for a subparsers group. If you desire a title, you will
+      need to pass one in like this `parser.add_subparsers(title="subcommands")`. This is standard
+      `argparse` behavior.
 - Enhancements
     - New `cmd2.Cmd` parameters
         - **auto_suggest**: (boolean) if `True`, provide fish shell style auto-suggestions. These

cmd2/argparse_custom.py

@@ -772,25 +772,6 @@ def __init__(
         self.description: RenderableType | None  # type: ignore[assignment]
         self.epilog: RenderableType | None  # type: ignore[assignment]
 
-    def add_subparsers(  # type: ignore[override]
-        self,
-        **kwargs: Any,
-    ) -> "argparse._SubParsersAction[Cmd2ArgumentParser]":
-        """Override for improved defaults and type safety.
-
-        This override does two things.
-        1. Sets a default title if one was not given.
-        2. Narrows the return type to provide better IDE autocompletion
-           and type safety for `Cmd2ArgumentParser` instances.
-
-        :param kwargs: additional keyword arguments
-        :return: _SubParsersAction which stores Cmd2ArgumentParsers
-        """
-        if 'title' not in kwargs:
-            kwargs['title'] = 'subcommands'
-
-        return super().add_subparsers(**kwargs)
-
     def _get_subparsers_action(self) -> "argparse._SubParsersAction[Cmd2ArgumentParser]":
         """Get the _SubParsersAction for this parser if it exists.
 
@@ -890,7 +871,7 @@ def _find_parser(self, subcommand_path: Iterable[str]) -> 'Cmd2ArgumentParser':
         """Find a parser in the hierarchy based on a sequence of subcommand names.
 
         :param subcommand_path: sequence of subcommand names leading to the target parser
-        :return: the discovered Cmd2ArgumentParser
+        :return: the discovered parser
         :raises ValueError: if any subcommand in the path is not found or a level doesn't support subcommands
         """
         parser = self
@@ -905,34 +886,54 @@ def attach_subcommand(
         self,
         subcommand_path: Iterable[str],
         subcommand: str,
-        parser: 'Cmd2ArgumentParser',
+        subcommand_parser: 'Cmd2ArgumentParser',
         **add_parser_kwargs: Any,
     ) -> None:
         """Attach a parser as a subcommand to a command at the specified path.
 
         :param subcommand_path: sequence of subcommand names leading to the parser that will
                                 host the new subcommand. An empty sequence indicates this parser.
         :param subcommand: name of the new subcommand
-        :param parser: the parser to attach
+        :param subcommand_parser: the parser to attach
         :param add_parser_kwargs: additional arguments for the subparser registration (e.g. help, aliases)
+        :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
         """
+        if not isinstance(subcommand_parser, Cmd2ArgumentParser):
+            raise TypeError(
+                f"The attached parser must be an instance of 'Cmd2ArgumentParser' (or a subclass). "
+                f"Received: '{type(subcommand_parser).__name__}'."
+            )
+
         target_parser = self._find_parser(subcommand_path)
         subparsers_action = target_parser._get_subparsers_action()
 
+        # Verify the parser is compatible with the 'parser_class' configured for this
+        # subcommand group. We use isinstance() here to allow for subclasses, providing
+        # more flexibility than the standard add_parser() factory approach which enforces
+        # a specific class.
+        if not isinstance(subcommand_parser, subparsers_action._parser_class):
+            raise TypeError(
+                f"The attached parser must be an instance of '{subparsers_action._parser_class.__name__}' "
+                f"(or a subclass) to match the 'parser_class' configured for this subcommand group. "
+                f"Received: '{type(subcommand_parser).__name__}'."
+            )
+
         # Use add_parser to register the subcommand name and any aliases
-        new_parser = subparsers_action.add_parser(subcommand, **add_parser_kwargs)
+        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.
-        parser.update_prog(new_parser.prog)
+        subcommand_parser.update_prog(placeholder_parser.prog)
 
         # Replace the parser created by add_parser() with our pre-configured one
-        subparsers_action._name_parser_map[subcommand] = parser
+        subparsers_action._name_parser_map[subcommand] = subcommand_parser
 
         # Remap any aliases to our pre-configured parser
         for alias in add_parser_kwargs.get("aliases", ()):
-            subparsers_action._name_parser_map[alias] = parser
+            subparsers_action._name_parser_map[alias] = subcommand_parser
 
     def detach_subcommand(self, subcommand_path: Iterable[str], subcommand: str) -> 'Cmd2ArgumentParser':
         """Detach a subcommand from a command at the specified path.

cmd2/cmd2.py

@@ -1179,20 +1179,23 @@ def attach_subcommand(
         self,
         command: str,
         subcommand: str,
-        parser: Cmd2ArgumentParser,
+        subcommand_parser: Cmd2ArgumentParser,
         **add_parser_kwargs: Any,
     ) -> None:
         """Attach a parser as a subcommand to a command at the specified path.
 
         :param command: full command path (space-delimited) leading to the parser that will
                         host the new subcommand (e.g. 'foo bar')
         :param subcommand: name of the new subcommand
-        :param parser: the parser to attach
+        :param subcommand_parser: the parser to attach
         :param add_parser_kwargs: additional arguments for the subparser registration (e.g. help, aliases)
+        :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
         """
         root_parser, subcommand_path = self._get_root_parser_and_subcmd_path(command)
-        root_parser.attach_subcommand(subcommand_path, subcommand, parser, **add_parser_kwargs)
+        root_parser.attach_subcommand(subcommand_path, subcommand, subcommand_parser, **add_parser_kwargs)
 
     def detach_subcommand(self, command: str, subcommand: str) -> Cmd2ArgumentParser:
         """Detach a subcommand from a command at the specified path.
@@ -3726,7 +3729,7 @@ def _build_alias_parser() -> Cmd2ArgumentParser:
             "See Also",
             "macro",
         )
-        alias_parser.add_subparsers(metavar='SUBCOMMAND', required=True)
+        alias_parser.add_subparsers(title="subcommands", metavar="SUBCOMMAND", required=True)
 
         return alias_parser
 
@@ -3942,7 +3945,7 @@ def _build_macro_parser() -> Cmd2ArgumentParser:
             "See Also",
             "alias",
         )
-        macro_parser.add_subparsers(metavar='SUBCOMMAND', required=True)
+        macro_parser.add_subparsers(title="subcommands", metavar="SUBCOMMAND", required=True)
 
         return macro_parser
 

cmd2/decorators.py

@@ -370,7 +370,7 @@ def as_subcommand_to(
     Example:
     ```py
     base_parser = cmd2.Cmd2ArgumentParser()
-    base_parser.add_subparsers(metavar='SUBCOMMAND', required=True)
+    base_parser.add_subparsers(title="subcommands", metavar="SUBCOMMAND", required=True)
     sub_parser = cmd2.Cmd2ArgumentParser()
 
     class MyApp(cmd2.Cmd):

tests/test_argparse_custom.py

@@ -425,6 +425,37 @@ def test_subcommand_attachment_errors() -> None:
     with pytest.raises(ValueError, match="Subcommand 'fake' not found in 'root'"):
         root_parser.detach_subcommand([], "fake")
 
+    # Verify TypeError when attaching a non-Cmd2ArgumentParser type
+    ap_parser = argparse.ArgumentParser(prog="non-cmd2-parser")
+    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]
+
+
+def test_subcommand_attachment_parser_class_override() -> None:
+    class MyParser(Cmd2ArgumentParser):
+        pass
+
+    class MySubParser(MyParser):
+        pass
+
+    root_parser = Cmd2ArgumentParser(prog="root")
+
+    # Explicitly override parser_class for this subparsers action
+    root_parser.add_subparsers(parser_class=MyParser)
+
+    # Attaching a MyParser instance should succeed
+    my_parser = MyParser(prog="sub")
+    root_parser.attach_subcommand([], "sub", my_parser)
+
+    # Attaching a MySubParser instance should also succeed (isinstance check)
+    my_sub_parser = MySubParser(prog="sub2")
+    root_parser.attach_subcommand([], "sub2", my_sub_parser)
+
+    # Attaching a standard Cmd2ArgumentParser instance should fail
+    standard_parser = Cmd2ArgumentParser(prog="standard")
+    with pytest.raises(TypeError, match=r"must be an instance of 'MyParser' \(or a subclass\)"):
+        root_parser.attach_subcommand([], "fail", standard_parser)
+
 
 def test_completion_items_as_choices(capsys) -> None:
     """Test cmd2's patch to Argparse._check_value() which supports CompletionItems as choices.