From 6ad9fd3e10680bede01dfec4a13b546f6b35723b Mon Sep 17 00:00:00 2001 From: Tyler Goodlet Date: Thu, 17 Nov 2016 18:52:58 -0500 Subject: [PATCH] Add defaults support to enable deprecation Add support for using declared keyword argument values from both hookspecs and hookimpls. The current logic will inspect the hookimpl and, if it contains defaults, values will be first looked up from the caller provided data and if not defined will be taken from the hookspec's declared defaults. If the spec does not define defaults the value is taken from the hookimpl's defaults as is expected under normal function call semantics. Resolves #15 --- pluggy.py | 89 ++++++++++++++++++++++----------- testing/test_method_ordering.py | 6 +-- testing/test_multicall.py | 2 +- 3 files changed, 65 insertions(+), 32 deletions(-) diff --git a/pluggy.py b/pluggy.py index 6906e415..afde685b 100644 --- a/pluggy.py +++ b/pluggy.py @@ -1,5 +1,6 @@ import sys import inspect +import copy import warnings __version__ = '0.5.0' @@ -274,10 +275,12 @@ def __init__(self, project_name, implprefix=None): self.trace = _TagTracer().get("pluginmanage") self.hook = _HookRelay(self.trace.root.get("hook")) self._implprefix = implprefix - self._inner_hookexec = lambda hook, methods, kwargs: \ - _MultiCall( - methods, kwargs, specopts=hook.spec_opts, hook=hook - ).execute() + self._inner_hookexec = lambda hook, methods, kwargs: _MultiCall( + methods, + kwargs, + firstresult=hook.spec.opts['firstresult'] if hook.spec else False, + hook=hook + ).execute() def _hookexec(self, hook, methods, kwargs): # called from all hookcaller instances. @@ -424,7 +427,7 @@ def _verify_hook(self, hook, hookimpl): (hookimpl.plugin_name, hook.name)) # positional arg checking - notinspec = set(hookimpl.argnames) - set(hook.argnames) + notinspec = set(hookimpl.argnames) - set(hook.spec.argnames) if notinspec: raise PluginValidationError( "Plugin %r for hook %r\nhookimpl definition: %s\n" @@ -517,8 +520,8 @@ def subset_hook_caller(self, name, remove_plugins): orig = getattr(self.hook, name) plugins_to_remove = [plug for plug in remove_plugins if hasattr(plug, name)] if plugins_to_remove: - hc = _HookCaller(orig.name, orig._hookexec, orig._specmodule_or_class, - orig.spec_opts) + hc = _HookCaller(orig.name, orig._hookexec, orig.spec.namespace, + orig.spec.opts) for hookimpl in (orig._wrappers + orig._nonwrappers): plugin = hookimpl.plugin if plugin not in plugins_to_remove: @@ -539,29 +542,43 @@ class _MultiCall: # so we can remove it soon, allowing to avoid the below recursion # in execute() and simplify/speed up the execute loop. - def __init__(self, hook_impls, kwargs, specopts={}, hook=None): - self.hook = hook + def __init__(self, hook_impls, kwargs, firstresult=False, hook=None): self.hook_impls = hook_impls self.caller_kwargs = kwargs # come from _HookCaller.__call__() self.caller_kwargs["__multicall__"] = self - self.specopts = hook.spec_opts if hook else specopts + self.firstresult = firstresult + self.hook = hook + self.spec = hook.spec if hook else None def execute(self): caller_kwargs = self.caller_kwargs self.results = results = [] - firstresult = self.specopts.get("firstresult") + firstresult = self.firstresult + spec = self.spec while self.hook_impls: hook_impl = self.hook_impls.pop() + implkwargs = hook_impl.kwargs try: args = [caller_kwargs[argname] for argname in hook_impl.argnames] + # get any caller provided kwargs declared in our + # hookimpl and fail over to the spec's value if provided + if implkwargs: + kwargs = copy.copy(implkwargs) + if spec: + kwargs.update(spec.kwargs) + + args += [caller_kwargs.get(argname, kwargs[argname]) + for argname in hook_impl.kwargnames] except KeyError: for argname in hook_impl.argnames: if argname not in caller_kwargs: raise HookCallError( "hook call must provide argument %r" % (argname,)) + if hook_impl.hookwrapper: return _wrapped_call(hook_impl.function(*args), self.execute) + res = hook_impl.function(*args) if res is not None: if firstresult: @@ -645,28 +662,23 @@ def __init__(self, name, hook_execute, specmodule_or_class=None, spec_opts=None) self._wrappers = [] self._nonwrappers = [] self._hookexec = hook_execute - self.argnames = None - self.kwargnames = None + self.spec = None + self._call_history = None if specmodule_or_class is not None: assert spec_opts is not None self.set_specification(specmodule_or_class, spec_opts) def has_spec(self): - return hasattr(self, "_specmodule_or_class") + return self.spec is not None def set_specification(self, specmodule_or_class, spec_opts): assert not self.has_spec() - self._specmodule_or_class = specmodule_or_class - specfunc = getattr(specmodule_or_class, self.name) - # get spec arg signature - argnames, self.kwargnames = varnames(specfunc) - self.argnames = ["__multicall__"] + list(argnames) - self.spec_opts = spec_opts + self.spec = HookSpec(specmodule_or_class, self.name, spec_opts) if spec_opts.get("historic"): self._call_history = [] def is_historic(self): - return hasattr(self, "_call_history") + return self._call_history is not None def _remove_plugin(self, plugin): def remove(wrappers): @@ -702,13 +714,14 @@ def __repr__(self): def __call__(self, **kwargs): assert not self.is_historic() - notincall = set(self.argnames) - set(kwargs.keys()) - if notincall: - warnings.warn( - "Positional arg(s) %s are declared in the hookspec " - "but can not be found in this hook call" % notincall, - FutureWarning - ) + if self.spec: + notincall = set(self.spec.argnames) - set(kwargs.keys()) + if notincall: + warnings.warn( + "Positional arg(s) %s are declared in the hookspec " + "but can not be found in this hook call" % notincall, + FutureWarning + ) return self._hookexec(self, self._nonwrappers + self._wrappers, kwargs) def call_historic(self, proc=None, kwargs=None): @@ -747,10 +760,30 @@ def _maybe_apply_history(self, method): proc(res[0]) +class HookSpec: + def __init__(self, namespace, name, hook_spec_opts): + self.namespace = namespace + self.function = function = getattr(namespace, name) + self.name = name + self.argnames, self.kwargnames = varnames(function) + self.kwargvalues = inspect.getargspec(function).defaults + self.kwargs = dict( + ((name, value) for name, value in + zip(self.kwargnames, inspect.getargspec(function).defaults)) + ) if self.kwargvalues else {} + self.opts = hook_spec_opts + self.argnames = ["__multicall__"] + list(self.argnames) + + class HookImpl: def __init__(self, plugin, plugin_name, function, hook_impl_opts): self.function = function self.argnames, self.kwargnames = varnames(self.function) + self.kwargvalues = inspect.getargspec(function).defaults + self.kwargs = dict( + ((name, value) for name, value in + zip(self.kwargnames, inspect.getargspec(function).defaults)) + ) if self.kwargvalues else {} self.plugin = plugin self.opts = hook_impl_opts self.plugin_name = plugin_name diff --git a/testing/test_method_ordering.py b/testing/test_method_ordering.py index c8bd39b1..56fcb045 100644 --- a/testing/test_method_ordering.py +++ b/testing/test_method_ordering.py @@ -163,9 +163,9 @@ def he_myhook3(arg1): pass pm.add_hookspecs(HookSpec) - assert not pm.hook.he_myhook1.spec_opts["firstresult"] - assert pm.hook.he_myhook2.spec_opts["firstresult"] - assert not pm.hook.he_myhook3.spec_opts["firstresult"] + assert not pm.hook.he_myhook1.spec.opts["firstresult"] + assert pm.hook.he_myhook2.spec.opts["firstresult"] + assert not pm.hook.he_myhook3.spec.opts["firstresult"] @pytest.mark.parametrize('name', ["hookwrapper", "optionalhook", "tryfirst", "trylast"]) diff --git a/testing/test_multicall.py b/testing/test_multicall.py index 0d668049..a0890f52 100644 --- a/testing/test_multicall.py +++ b/testing/test_multicall.py @@ -22,7 +22,7 @@ def MC(methods, kwargs, firstresult=False): for method in methods: f = HookImpl(None, "", method, method.example_impl) hookfuncs.append(f) - return _MultiCall(hookfuncs, kwargs, {"firstresult": firstresult}) + return _MultiCall(hookfuncs, kwargs, firstresult=firstresult) def test_call_passing():