Improved CommandSet instance resolution during argparse argument completion, fixes #967

CHANGELOG.md

@@ -1,3 +1,10 @@
+## 1.3.1 (August 6, 2020)
+* Bug Fixes
+    * Fixed issue determining whether an argparse completer function required a reference to a containing
+       CommandSet. Also resolves issues determining the correct CommandSet instance when calling the argparse 
+       argument completer function.  Manifested as a TypeError when using `cmd2.Cmd.path_complete` as a completer 
+       for an argparse-based command defined in a CommandSet
+
 ## 1.3.0 (August 4, 2020)
 * Enhancements
     * Added CommandSet - Enables defining a separate loadable module of commands to register/unregister

cmd2/__init__.py

@@ -30,8 +30,9 @@
 from .cmd2 import Cmd
 from .command_definition import CommandSet, with_default_category
 from .constants import COMMAND_NAME, DEFAULT_SHORTCUTS
-from .decorators import with_argument_list, with_argparser, with_argparser_and_unknown_args, with_category, as_subcommand_to
-from .exceptions import Cmd2ArgparseError, SkipPostcommandHooks
+from .decorators import with_argument_list, with_argparser, with_argparser_and_unknown_args, with_category, \
+    as_subcommand_to
+from .exceptions import Cmd2ArgparseError, SkipPostcommandHooks, CommandSetRegistrationError
 from . import plugin
 from .parsing import Statement
 from .py_bridge import CommandResult

cmd2/argparse_completer.py

@@ -25,7 +25,7 @@
 )
 from .command_definition import CommandSet
 from .table_creator import Column, SimpleTable
-from .utils import CompletionError, basic_complete
+from .utils import CompletionError, basic_complete, get_defining_class
 
 # If no descriptive header is supplied, then this will be used instead
 DEFAULT_DESCRIPTIVE_HEADER = 'Description'
@@ -569,12 +569,43 @@ def _complete_for_arg(self, arg_action: argparse.Action,
         kwargs = {}
         if isinstance(arg_choices, ChoicesCallable):
             if arg_choices.is_method:
-                cmd_set = getattr(self._parser, constants.PARSER_ATTR_COMMANDSET, cmd_set)
-                if cmd_set is not None:
-                    if isinstance(cmd_set, CommandSet):
-                        # If command is part of a CommandSet, `self` should be the CommandSet and Cmd will be next
-                        if cmd_set is not None:
-                            args.append(cmd_set)
+                # figure out what class the completer was defined in
+                completer_class = get_defining_class(arg_choices.to_call)
+
+                # Was there a defining class identified? If so, is it a sub-class of CommandSet?
+                if completer_class is not None and issubclass(completer_class, CommandSet):
+                    # Since the completer function is provided as an unbound function, we need to locate the instance
+                    # of the CommandSet to pass in as `self` to emulate a bound method call.
+                    # We're searching for candidates that match the completer function's parent type in this order:
+                    #   1. Does the CommandSet registered with the command's argparser match as a subclass?
+                    #   2. Do any of the registered CommandSets in the Cmd2 application exactly match the type?
+                    #   3. Is there a registered CommandSet that is is the only matching subclass?
+
+                    # Now get the CommandSet associated with the current command/subcommand argparser
+                    parser_cmd_set = getattr(self._parser, constants.PARSER_ATTR_COMMANDSET, cmd_set)
+                    if isinstance(parser_cmd_set, completer_class):
+                        # Case 1: Parser's CommandSet is a sub-class of the completer function's CommandSet
+                        cmd_set = parser_cmd_set
+                    else:
+                        # Search all registered CommandSets
+                        cmd_set = None
+                        candidate_sets = []  # type: List[CommandSet]
+                        for installed_cmd_set in self._cmd2_app._installed_command_sets:
+                            if type(installed_cmd_set) == completer_class:
+                                # Case 2: CommandSet is an exact type match for the completer's CommandSet
+                                cmd_set = installed_cmd_set
+                                break
+
+                            # Add candidate for Case 3:
+                            if isinstance(installed_cmd_set, completer_class):
+                                candidate_sets.append(installed_cmd_set)
+                        if cmd_set is None and len(candidate_sets) == 1:
+                            # Case 3: There exists exactly 1 CommandSet that is a subclass of the completer's CommandSet
+                            cmd_set = candidate_sets[0]
+                    if cmd_set is None:
+                        # No cases matched, raise an error
+                        raise CompletionError('Could not find CommandSet instance matching defining type for completer')
+                    args.append(cmd_set)
                 args.append(self._cmd2_app)
 
             # Check if arg_choices.to_call expects arg_tokens

cmd2/exceptions.py

@@ -25,6 +25,10 @@ class Cmd2ArgparseError(SkipPostcommandHooks):
 
 
 class CommandSetRegistrationError(Exception):
+    """
+    Exception that can be thrown when an error occurs while a CommandSet is being added or removed
+    from a cmd2 application.
+    """
     pass
 
 ############################################################################################################

cmd2/utils.py

@@ -3,15 +3,17 @@
 
 import collections
 import collections.abc as collections_abc
+import functools
 import glob
+import inspect
 import os
 import re
 import subprocess
 import sys
 import threading
 import unicodedata
 from enum import Enum
-from typing import Any, Callable, Dict, Iterable, List, Optional, TextIO, Union
+from typing import Any, Callable, Dict, Iterable, List, Optional, TextIO, Type, Union
 
 from . import constants
 
@@ -1037,3 +1039,30 @@ def categorize(func: Union[Callable, Iterable[Callable]], category: str) -> None
             setattr(item, constants.CMD_ATTR_HELP_CATEGORY, category)
     else:
         setattr(func, constants.CMD_ATTR_HELP_CATEGORY, category)
+
+
+def get_defining_class(meth: Callable) -> Optional[Type]:
+    """
+    Attempts to resolve the class that defined a method.
+
+    Inspired by implementation published here:
+        https://stackoverflow.com/a/25959545/1956611
+
+    :param meth: method to inspect
+    :return: class type in which the supplied method was defined. None if it couldn't be resolved.
+    """
+    if isinstance(meth, functools.partial):
+        return get_defining_class(meth.func)
+    if inspect.ismethod(meth) or (inspect.isbuiltin(meth)
+                                  and getattr(meth, '__self__') is not None
+                                  and getattr(meth.__self__, '__class__')):
+        for cls in inspect.getmro(meth.__self__.__class__):
+            if meth.__name__ in cls.__dict__:
+                return cls
+        meth = getattr(meth, '__func__', meth)  # fallback to __qualname__ parsing
+    if inspect.isfunction(meth):
+        cls = getattr(inspect.getmodule(meth),
+                      meth.__qualname__.split('.<locals>', 1)[0].rsplit('.', 1)[0])
+        if isinstance(cls, type):
+            return cls
+    return getattr(meth, '__objclass__', None)  # handle special descriptor objects

docs/api/exceptions.rst

@@ -9,3 +9,6 @@ Custom cmd2 exceptions
 
 .. autoclass:: cmd2.exceptions.Cmd2ArgparseError
     :members:
+
+.. autoclass:: cmd2.exceptions.CommandSetRegistrationError
+    :members:

docs/features/modular_commands.rst

@@ -8,7 +8,8 @@ Cmd2 also enables developers to modularize their command definitions into Comman
 a logical grouping of commands within an cmd2 application. By default, all CommandSets will be discovered and loaded
 automatically when the cmd2.Cmd class is instantiated with this mixin. This also enables the developer to
 dynamically add/remove commands from the cmd2 application. This could be useful for loadable plugins that
-add additional capabilities.
+add additional capabilities. Additionally, it allows for object-oriented encapsulation and garbage collection of state
+that is specific to a CommandSet.
 
 Features
 ~~~~~~~~

tasks.py

@@ -70,10 +70,10 @@ def pytest(context, junit=False, pty=True, base=False, isolated=False):
             tests_cmd = command_str + ' tests'
             context.run(tests_cmd, pty=pty)
         if isolated:
-            for root, dirnames, _ in os.walk(str(TASK_ROOT/'isolated_tests')):
+            for root, dirnames, _ in os.walk(str(TASK_ROOT/'tests_isolated')):
                 for dir in dirnames:
                     if dir.startswith('test_'):
-                        context.run(command_str + ' isolated_tests/' + dir)
+                        context.run(command_str + ' tests_isolated/' + dir)
 
 
 namespace.add_task(pytest)

tests/test_utils_defining_class.py

@@ -0,0 +1,83 @@
+# coding=utf-8
+# flake8: noqa E302
+"""
+Unit testing for get_defining_class in cmd2/utils.py module.
+"""
+import functools
+
+import cmd2.utils as cu
+
+
+class ParentClass(object):
+    def func_with_overrides(self):
+        pass
+
+    def parent_only_func(self, param1, param2):
+        pass
+
+
+class ChildClass(ParentClass):
+    def func_with_overrides(self):
+        super(ChildClass, self).func_with_overrides()
+
+    def child_function(self):
+        pass
+
+    lambda1 = lambda: 1
+
+    lambda2 = (lambda: lambda: 2)()
+
+    @classmethod
+    def class_method(cls):
+        pass
+
+    @staticmethod
+    def static_meth():
+        pass
+
+
+def func_not_in_class():
+    pass
+
+
+def test_get_defining_class():
+    parent_instance = ParentClass()
+    child_instance = ChildClass()
+
+    # validate unbound class functions
+    assert cu.get_defining_class(ParentClass.func_with_overrides) is ParentClass
+    assert cu.get_defining_class(ParentClass.parent_only_func) is ParentClass
+    assert cu.get_defining_class(ChildClass.func_with_overrides) is ChildClass
+    assert cu.get_defining_class(ChildClass.parent_only_func) is ParentClass
+    assert cu.get_defining_class(ChildClass.child_function) is ChildClass
+    assert cu.get_defining_class(ChildClass.class_method) is ChildClass
+    assert cu.get_defining_class(ChildClass.static_meth) is ChildClass
+
+    # validate bound class methods
+    assert cu.get_defining_class(parent_instance.func_with_overrides) is ParentClass
+    assert cu.get_defining_class(parent_instance.parent_only_func) is ParentClass
+    assert cu.get_defining_class(child_instance.func_with_overrides) is ChildClass
+    assert cu.get_defining_class(child_instance.parent_only_func) is ParentClass
+    assert cu.get_defining_class(child_instance.child_function) is ChildClass
+    assert cu.get_defining_class(child_instance.class_method) is ChildClass
+    assert cu.get_defining_class(child_instance.static_meth) is ChildClass
+
+    # bare functions resolve to nothing
+    assert cu.get_defining_class(func_not_in_class) is None
+
+    # lambdas and nested lambdas
+    assert cu.get_defining_class(ChildClass.lambda1) is ChildClass
+    assert cu.get_defining_class(ChildClass.lambda2) is ChildClass
+    assert cu.get_defining_class(ChildClass().lambda1) is ChildClass
+    assert cu.get_defining_class(ChildClass().lambda2) is ChildClass
+
+    # partials
+    partial_unbound = functools.partial(ParentClass.parent_only_func, 1)
+    nested_partial_unbound = functools.partial(partial_unbound, 2)
+    assert cu.get_defining_class(partial_unbound) is ParentClass
+    assert cu.get_defining_class(nested_partial_unbound) is ParentClass
+
+    partial_bound =  functools.partial(parent_instance.parent_only_func, 1)
+    nested_partial_bound =  functools.partial(partial_bound, 2)
+    assert cu.get_defining_class(partial_bound) is ParentClass
+    assert cu.get_defining_class(nested_partial_bound) is ParentClass

isolated_tests/test_commandset/conftest.py → tests_isolated/test_commandset/conftest.py

@@ -10,6 +10,7 @@
 from pytest import fixture
 
 import cmd2
+from cmd2_ext_test import ExternalTestMixin
 from cmd2.utils import StdSim
 
 # Prefer statically linked gnureadline if available (for macOS compatibility due to issues with libedit)
@@ -194,3 +195,21 @@ def get_endidx():
         with mock.patch.object(readline, 'get_begidx', get_begidx):
             with mock.patch.object(readline, 'get_endidx', get_endidx):
                 return app.complete(text, 0)
+
+
+class WithCommandSets(ExternalTestMixin, cmd2.Cmd):
+    """Class for testing custom help_* methods which override docstring help."""
+    def __init__(self, *args, **kwargs):
+        super(WithCommandSets, self).__init__(*args, **kwargs)
+
+
+@fixture
+def command_sets_app():
+    app = WithCommandSets()
+    return app
+
+
+@fixture()
+def command_sets_manual():
+    app = WithCommandSets(auto_load_commands=False)
+    return app