chore: more edge case fix

Author: KelvinChung2000

cmd2/annotated.py

@@ -2000,25 +2000,16 @@ def build_parser_from_function(
         mutually_exclusive_groups=mutually_exclusive_groups,
     )
 
-    # ``argument_default=argparse.SUPPRESS`` removes an absent argument from the parsed namespace.
-    # That is safe only for arguments that are always supplied (required) or carry their own default;
-    # an *omittable* argument with no default (e.g. a ``T | None`` positional -> nargs='?') would be
-    # dropped when absent, leaving the function without a keyword argument it expects.  ``*args`` is
-    # exempt: the invocation path substitutes an empty tuple for it.  Reject the combination here,
-    # mirroring the per-argument ``default=argparse.SUPPRESS`` rejection.
+    # ``argument_default=argparse.SUPPRESS`` drops an absent argument from the parsed namespace.
+    # @with_annotated builds the call from the function signature, so every declared parameter is
+    # expected at invocation -- an argument vanishing from the namespace can never be valid here.
+    # Reject it outright, mirroring the per-argument ``default=argparse.SUPPRESS`` rejection.
     if parser_kwargs.get("argument_default") is argparse.SUPPRESS:
-        dropped = [
-            arg.name
-            for arg in resolved
-            if arg.default is _UNSET and arg.omittable and not arg.required and not arg.is_variadic
-        ]
-        if dropped:
-            raise TypeError(
-                f"argument_default=argparse.SUPPRESS is not supported by @with_annotated for {func.__qualname__}: "
-                f"it would drop {dropped!r} from the parsed namespace when absent, but the function expects "
-                f"{'them' if len(dropped) > 1 else 'it'} as a keyword argument. Give each an explicit default or "
-                f"make it required, or drop argument_default=argparse.SUPPRESS."
-            )
+        raise TypeError(
+            f"argument_default=argparse.SUPPRESS is not supported by @with_annotated for {func.__qualname__}: "
+            f"it drops absent arguments from the parsed namespace, but every parameter built from the "
+            f"signature is expected at invocation. Drop argument_default=argparse.SUPPRESS."
+        )
 
     # Build the group lookup (member references already validated by _resolve_parameters).
     target_for, argument_group_for = _build_argument_group_targets(parser, groups=groups)
@@ -2138,7 +2129,7 @@ def handler(self_arg: Any, ns: Any) -> Any:
         filtered = _filtered_namespace_kwargs(ns, accepted=_accepted)
         if constants.NS_ATTR_SUBCOMMAND_FUNC in filtered:
             cmd2_h = filtered[constants.NS_ATTR_SUBCOMMAND_FUNC]
-            if isinstance(cmd2_h, functools.partial) and cmd2_h.func is handler:
+            if isinstance(cmd2_h, functools.partial) and getattr(cmd2_h.func, "__func__", cmd2_h.func) is handler:
                 filtered[constants.NS_ATTR_SUBCOMMAND_FUNC] = None
         return _invoke_command_func(
             func, self_arg, filtered, leading_names=_leading_names, var_positional_name=_var_positional_name

docs/features/annotated.md

@@ -363,14 +363,11 @@ def do_run(self, verbose: bool = False, quiet: bool = False): ...
 ```
 
 `parents=` mirrors argparse's standard parents mechanism for sharing argument definitions across
-parsers. `argument_default=argparse.SUPPRESS` is accepted only when no argument could be stranded by
-it: it removes an absent argument from the parsed namespace, which is safe for an argument that is
-always supplied (a required option, a mandatory positional) or that carries its own default, but not
-for an _omittable_ argument with no default (for example a `T | None` positional, which becomes
-`nargs='?'`). If any such argument is present, `@with_annotated` raises `TypeError` rather than let
-the function be called missing a keyword argument it expects (mirroring the per-argument
-`default=argparse.SUPPRESS` rejection). `*args` is exempt, since the invocation path substitutes an
-empty tuple for it.
+parsers. `argument_default=argparse.SUPPRESS` is not supported and raises `TypeError`. It removes an
+absent argument from the parsed namespace, but `@with_annotated` builds the call from the function
+signature, so every declared parameter is expected at invocation; an argument vanishing from the
+namespace can never be valid here (mirroring the per-argument `default=argparse.SUPPRESS`
+rejection). Any other `argument_default` value is forwarded to the parser unchanged.
 
 The remaining argparse kwargs cover less-common needs but are wired through unchanged:
 

tests/test_annotated.py

@@ -1186,6 +1186,28 @@ class MyParser(cmd2.Cmd2ArgumentParser):
         parser = build_parser_from_function(_make_func(str), parser_class=MyParser)
         assert isinstance(parser, MyParser)
 
+    def test_default_parser_class(self) -> None:
+        """With no parser_class, the parser is an instance of the configured default."""
+        from cmd2 import argparse_utils
+
+        parser = build_parser_from_function(_make_func(str))
+        assert type(parser) is argparse_utils.DEFAULT_ARGUMENT_PARSER
+
+    def test_default_parser_class_follows_current_default(self, monkeypatch) -> None:
+        """The default is resolved at call time, never a copy captured at import.
+
+        ``set_default_argument_parser`` rebinds ``argparse_utils.DEFAULT_ARGUMENT_PARSER`` at runtime;
+        a build issued afterwards must honor the new value.
+        """
+        from cmd2 import argparse_utils
+
+        class MyDefaultParser(cmd2.Cmd2ArgumentParser):
+            pass
+
+        monkeypatch.setattr(argparse_utils, "DEFAULT_ARGUMENT_PARSER", MyDefaultParser)
+        parser = build_parser_from_function(_make_func(str))
+        assert type(parser) is MyDefaultParser
+
     def test_completer_class(self) -> None:
         from cmd2.argparse_completer import ArgparseCompleter
 
@@ -2309,6 +2331,114 @@ def test_subcommand_help(self, subcmd_app) -> None:
         assert "member" in help_text
 
 
+class _OptionalIntermediateApp(cmd2.Cmd):
+    """An intermediate subcommand that is itself a base command with an *optional* subcommand."""
+
+    @with_annotated(base_command=True)
+    def do_opt(self, cmd2_subcommand_func) -> None:
+        if cmd2_subcommand_func:
+            cmd2_subcommand_func()
+
+    @with_annotated(subcommand_to="opt", base_command=True, subcommand_required=False, help="optional middle")
+    def opt_mid(self, cmd2_subcommand_func) -> None:
+        # The guard must blank out the self-referential handler so this runs exactly once.
+        self.poutput("mid:none" if cmd2_subcommand_func is None else "mid:recurse")
+        if cmd2_subcommand_func:
+            cmd2_subcommand_func()
+
+    @with_annotated(subcommand_to="opt mid", help="leaf")
+    def opt_mid_leaf(self, name: str) -> None:
+        self.poutput(f"leaf:{name}")
+
+
+@pytest.fixture
+def opt_app() -> _OptionalIntermediateApp:
+    return _OptionalIntermediateApp()
+
+
+class TestOptionalIntermediateSubcommand:
+    def test_intermediate_without_deeper_subcommand_runs_once(self, opt_app) -> None:
+        """The recursion guard blanks the self-referential handler: the body runs once with None."""
+        out, _err = run_cmd(opt_app, "opt mid")
+        assert out == ["mid:none"]
+
+    def test_deeper_subcommand_still_dispatches(self, opt_app) -> None:
+        """Blanking the self-reference must not break dispatch to a genuine deeper subcommand."""
+        out, _err = run_cmd(opt_app, "opt mid leaf Bob")
+        assert out == ["leaf:Bob"]
+
+    def test_guard_blanks_only_subcommand_func(self) -> None:
+        """The guard must null *only* cmd2_subcommand_func, leaving the intermediate's own args intact."""
+
+        class App(cmd2.Cmd):
+            @with_annotated(base_command=True)
+            def do_opt(self, cmd2_subcommand_func) -> None:
+                if cmd2_subcommand_func:
+                    cmd2_subcommand_func()
+
+            @with_annotated(subcommand_to="opt", base_command=True, subcommand_required=False)
+            def opt_mid(self, cmd2_subcommand_func, verbose: bool = False) -> None:
+                self.poutput(f"none={cmd2_subcommand_func is None}:verbose={verbose}")
+                if cmd2_subcommand_func:
+                    cmd2_subcommand_func()
+
+        out, _err = run_cmd(App(), "opt mid --verbose")
+        assert out == ["none=True:verbose=True"]
+
+    def test_guard_fires_at_deeper_nesting_level(self) -> None:
+        """The guard must work past two levels: the deepest *selected* optional base runs once."""
+
+        class App(cmd2.Cmd):
+            @with_annotated(base_command=True)
+            def do_a(self, cmd2_subcommand_func) -> None:
+                if cmd2_subcommand_func:
+                    cmd2_subcommand_func()
+
+            @with_annotated(subcommand_to="a", base_command=True, subcommand_required=False)
+            def a_b(self, cmd2_subcommand_func) -> None:
+                if cmd2_subcommand_func:
+                    cmd2_subcommand_func()
+
+            @with_annotated(subcommand_to="a b", base_command=True, subcommand_required=False)
+            def a_b_c(self, cmd2_subcommand_func) -> None:
+                self.poutput(f"c:none={cmd2_subcommand_func is None}")
+                if cmd2_subcommand_func:
+                    cmd2_subcommand_func()
+
+            @with_annotated(subcommand_to="a b c", help="leaf")
+            def a_b_c_leaf(self, name: str) -> None:
+                self.poutput(f"leaf:{name}")
+
+        app = App()
+        assert run_cmd(app, "a b c")[0] == ["c:none=True"]
+        assert run_cmd(app, "a b c leaf Z")[0] == ["leaf:Z"]
+
+    def test_guard_works_for_commandset_subcommand(self) -> None:
+        """The handler is a CommandSet-bound method here; unwrapping to __func__ must still match."""
+
+        class _Grp(cmd2.CommandSet):
+            @cmd2.with_category("grp")
+            @with_annotated(base_command=True)
+            def do_grp(self, cmd2_subcommand_func) -> None:
+                if cmd2_subcommand_func:
+                    cmd2_subcommand_func()
+
+            @with_annotated(subcommand_to="grp", base_command=True, subcommand_required=False)
+            def grp_mid(self, cmd2_subcommand_func) -> None:
+                self._cmd.poutput(f"mid:none={cmd2_subcommand_func is None}")
+                if cmd2_subcommand_func:
+                    cmd2_subcommand_func()
+
+            @with_annotated(subcommand_to="grp mid", help="leaf")
+            def grp_mid_leaf(self, name: str) -> None:
+                self._cmd.poutput(f"leaf:{name}")
+
+        app = cmd2.Cmd(auto_load_commands=False)
+        app.register_command_set(_Grp())
+        assert run_cmd(app, "grp mid")[0] == ["mid:none=True"]
+        assert run_cmd(app, "grp mid leaf Q")[0] == ["leaf:Q"]
+
+
 class TestSubcommandValidation:
     def test_subcommand_aliases_none_raises(self) -> None:
         """aliases=None is off-spec (it must be a Sequence[str]); reject it with a clear message."""
@@ -3282,21 +3412,6 @@ def test_argument_default_passthrough(self) -> None:
         parser = build_parser_from_function(_make_func(str), argument_default=sentinel)
         assert parser.argument_default == sentinel
 
-    def test_argument_default_suppress_works_with_explicit_defaults(self) -> None:
-        """``argument_default=SUPPRESS`` is safe when every argument sets its own default.
-
-        Every ``@with_annotated`` argument either is positional (always supplied) or
-        has an explicit default, so SUPPRESS at the parser level can't drop a kwarg
-        the function expects.
-        """
-
-        def func(self, name: str, count: int = 1) -> None: ...
-
-        parser = build_parser_from_function(func, argument_default=argparse.SUPPRESS)
-        ns = parser.parse_args(["alice"])
-        assert ns.name == "alice"
-        assert ns.count == 1
-
     def test_decorator_passes_parser_kwargs(self) -> None:
         @with_annotated(prog="myprog", usage="usage line")
         def do_run(self, name: str) -> None: ...
@@ -3481,27 +3596,34 @@ def test_const_on_untyped_param_not_validated(self) -> None:
 
 
 class TestArgumentDefaultSuppressGuard:
-    """``argument_default=argparse.SUPPRESS`` is rejected when it would drop an omittable argument."""
+    """``argument_default=argparse.SUPPRESS`` is rejected outright by @with_annotated.
 
-    def test_suppress_with_optional_positional_rejected(self) -> None:
-        def do_t(self, x: int | None): ...
+    SUPPRESS drops an absent argument from the parsed namespace, but @with_annotated builds the call
+    from the signature, so every declared parameter is expected at invocation -- a vanished argument
+    can never be valid.  The rejection is unconditional (it never inspects the signature), so one
+    direct-build case and one subcommand-registration case cover it.
+    """
+
+    def test_suppress_rejected(self) -> None:
+        def do_t(self, a: int, b: str = "x"): ...
 
         with pytest.raises(TypeError, match="SUPPRESS"):
             build_parser_from_function(do_t, argument_default=argparse.SUPPRESS)
 
-    def test_suppress_safe_when_all_args_required_or_defaulted(self) -> None:
-        def do_t(self, a: int, b: str = "x"): ...
+    def test_suppress_rejected_in_subcommand(self) -> None:
+        """The subcommand path shares the same builder, so the rejection fires at registration too."""
 
-        # ``a`` is always supplied (required positional); ``b`` carries its own default -> safe.
-        parser = build_parser_from_function(do_t, argument_default=argparse.SUPPRESS)
-        assert parser is not None
+        class App(cmd2.Cmd):
+            @with_annotated(base_command=True)
+            def do_calc(self, cmd2_subcommand_func) -> None:
+                if cmd2_subcommand_func:
+                    cmd2_subcommand_func()
 
-    def test_suppress_safe_with_var_positional(self) -> None:
-        def do_t(self, *vals: int): ...
+            @with_annotated(subcommand_to="calc", argument_default=argparse.SUPPRESS, help="sum values")
+            def calc_sum(self, value: str = "x") -> None: ...
 
-        # *args is substituted with () by the invocation path, so SUPPRESS cannot strand it.
-        parser = build_parser_from_function(do_t, argument_default=argparse.SUPPRESS)
-        assert parser is not None
+        with pytest.raises(TypeError, match="SUPPRESS"):
+            App()
 
 
 class TestHelpKwargReserved: