Checking for Cmd2ArgumentParser instance in _build_parser().

Author: kmvanbrunt

cmd2/cmd2.py

@@ -896,21 +896,30 @@ def _build_parser(
         :param parent: object which owns the command using the parser.
                        When parser_builder is a classmethod, this function passes
                        parent's class to it.
-        :param parser_builder: means used to build the parser
+        :param parser_builder: an existing Cmd2ArgumentParser instance or a factory
+                               (callable, staticmethod, or classmethod) that returns one.
         :param prog: prog value to set in new parser
         :return: new parser
-        :raises TypeError: if parser_builder is invalid type
+        :raises TypeError: if parser_builder is an invalid type or if the factory fails
+                           to return a Cmd2ArgumentParser
         """
-        if isinstance(parser_builder, staticmethod):
-            parser = parser_builder.__func__()
-        elif isinstance(parser_builder, classmethod):
-            parser = parser_builder.__func__(parent.__class__)
-        elif callable(parser_builder):
-            parser = parser_builder()
-        elif isinstance(parser_builder, Cmd2ArgumentParser):
+        if isinstance(parser_builder, Cmd2ArgumentParser):
             parser = copy.deepcopy(parser_builder)
         else:
-            raise TypeError(f"Invalid type for parser_builder: {type(parser_builder)}")
+            # Try to build the parser with a factory
+            if isinstance(parser_builder, staticmethod):
+                parser = parser_builder.__func__()
+            elif isinstance(parser_builder, classmethod):
+                parser = parser_builder.__func__(parent.__class__)
+            elif callable(parser_builder):
+                parser = parser_builder()
+            else:
+                raise TypeError(f"Invalid type for parser_builder: {type(parser_builder)}")
+
+            # Verify the factory returned the required type
+            if not isinstance(parser, Cmd2ArgumentParser):
+                builder_name = getattr(parser_builder, "__name__", str(parser_builder))  # type: ignore[unreachable]
+                raise TypeError(f"The parser returned by '{builder_name}' must be a Cmd2ArgumentParser or a subclass of it")
 
         argparse_custom.set_parser_prog(parser, prog)
 

tests/test_argparse.py

@@ -247,10 +247,56 @@ def test_preservelist(argparse_app) -> None:
 
 def test_invalid_parser_builder(argparse_app):
     parser_builder = None
-    with pytest.raises(TypeError):
+    with pytest.raises(TypeError, match="Invalid type for parser_builder"):
         argparse_app._build_parser(argparse_app, parser_builder, "fake_prog")
 
 
+def test_invalid_parser_return_type(argparse_app):
+    def bad_builder():
+        return argparse.ArgumentParser()
+
+    with pytest.raises(TypeError, match="must be a Cmd2ArgumentParser or a subclass of it"):
+        argparse_app._build_parser(argparse_app, bad_builder, "fake_prog")
+
+
+def test_invalid_parser_return_type_staticmethod(argparse_app):
+    def bad_builder():
+        return argparse.ArgumentParser()
+
+    sm = staticmethod(bad_builder)
+
+    with pytest.raises(TypeError, match="must be a Cmd2ArgumentParser or a subclass of it"):
+        argparse_app._build_parser(argparse_app, sm, "fake_prog")
+
+
+def test_invalid_parser_return_type_classmethod(argparse_app):
+    def bad_builder(cls):
+        return argparse.ArgumentParser()
+
+    cm = classmethod(bad_builder)
+
+    with pytest.raises(TypeError, match="must be a Cmd2ArgumentParser or a subclass of it"):
+        argparse_app._build_parser(argparse_app, cm, "fake_prog")
+
+
+def test_invalid_parser_return_type_nameless_object(argparse_app):
+    # A class that is callable but has no __name__ attribute
+    class NamelessBuilder:
+        def __call__(self):
+            return argparse.ArgumentParser()
+
+    builder = NamelessBuilder()
+
+    # Verify __name__ is actually missing
+    assert not hasattr(builder, '__name__')
+
+    # The error message should now contain the string representation of the object
+    expected_msg = f"The parser returned by '{builder}' must be a Cmd2ArgumentParser"
+
+    with pytest.raises(TypeError, match=expected_msg):
+        argparse_app._build_parser(argparse_app, builder, "fake_prog")
+
+
 def _build_has_subcmd_parser() -> cmd2.Cmd2ArgumentParser:
     has_subcmds_parser = cmd2.Cmd2ArgumentParser(description="Tests as_subcmd_to decorator")
     has_subcmds_parser.add_subparsers(dest='subcommand', metavar='SUBCOMMAND', required=True)