Skip to content

Commit

Permalink
Future warn about hookimpl to call mis-matches
Browse files Browse the repository at this point in the history
Warn when either a hook call doesn't match a hookspec.
Extend `varnames()` to return the list of kwarg names.

Relates to #15
  • Loading branch information
Tyler Goodlet committed Nov 15, 2016
1 parent cac9657 commit b61f9a2
Showing 1 changed file with 54 additions and 30 deletions.
84 changes: 54 additions & 30 deletions pluggy.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
"""
import sys
import inspect
import warnings

__version__ = '0.5.0'

Expand All @@ -75,6 +76,14 @@
_py3 = sys.version_info > (3, 0)


class PluginValidationError(Exception):
""" plugin failed validation. """


class HookCallError(Exception):
""" Hook was called wrongly. """


class HookspecMarker:
""" Decorator helper class for marking functions as hook specifications.
Expand Down Expand Up @@ -331,7 +340,9 @@ def __init__(self, project_name, implprefix=None):
self.hook = _HookRelay(self.trace.root.get("hook"))
self._implprefix = implprefix
self._inner_hookexec = lambda hook, methods, kwargs: \
_MultiCall(methods, kwargs, hook.spec_opts).execute()
_MultiCall(
methods, kwargs, specopts=hook.spec_opts, hook=hook
).execute()

def _hookexec(self, hook, methods, kwargs):
# called from all hookcaller instances.
Expand Down Expand Up @@ -477,14 +488,16 @@ def _verify_hook(self, hook, hookimpl):
"Plugin %r\nhook %r\nhistoric incompatible to hookwrapper" %
(hookimpl.plugin_name, hook.name))

for arg in hookimpl.argnames:
if arg not in hook.argnames:
raise PluginValidationError(
"Plugin %r\nhook %r\nargument %r not available\n"
"plugin definition: %s\n"
"available hookargs: %s" %
(hookimpl.plugin_name, hook.name, arg,
_formatdef(hookimpl.function), ", ".join(hook.argnames)))
# positional arg checking
notinspec = set(hookimpl.argnames), set(hook.argnames)
if notinspec:
raise PluginValidationError(
"Plugin %r for hook %r\nhookimpl definition: %s\n"
"Positional args %s are declared in the hookimpl but "
"can not be found in the hookspec" %
(hookimpl.plugin_name, hook.name,
_formatdef(hookimpl.function), notinspec)
)

def check_pending(self):
""" Verify that all hooks which have not been verified against
Expand Down Expand Up @@ -591,24 +604,25 @@ 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={}):
def __init__(self, hook_impls, kwargs, specopts={}, hook=None):
self.hook = hook
self.hook_impls = hook_impls
self.kwargs = kwargs
self.kwargs["__multicall__"] = self
self.specopts = specopts
self.caller_kwargs = kwargs # come from _HookCaller.__call__()
self.caller_kwargs["__multicall__"] = self
self.specopts = hook.spec_opts if hook else specopts

def execute(self):
all_kwargs = self.kwargs
caller_kwargs = self.caller_kwargs
self.results = results = []
firstresult = self.specopts.get("firstresult")

while self.hook_impls:
hook_impl = self.hook_impls.pop()
try:
args = [all_kwargs[argname] for argname in hook_impl.argnames]
args = [caller_kwargs[argname] for argname in hook_impl.argnames]
except KeyError:
for argname in hook_impl.argnames:
if argname not in all_kwargs:
if argname not in caller_kwargs:
raise HookCallError(
"hook call must provide argument %r" % (argname,))
if hook_impl.hookwrapper:
Expand All @@ -626,7 +640,7 @@ def __repr__(self):
status = "%d meths" % (len(self.hook_impls),)
if hasattr(self, "results"):
status = ("%d results, " % len(self.results)) + status
return "<_MultiCall %s, kwargs=%r>" % (status, self.kwargs)
return "<_MultiCall %s, kwargs=%r>" % (status, self.caller_kwargs)


def varnames(func):
Expand Down Expand Up @@ -659,7 +673,11 @@ def varnames(func):
return ()

args, defaults = spec.args, spec.defaults
args = args[:-len(defaults)] if defaults else args
if defaults:
index = -len(defaults)
args, defaults = args[:index], args[index:]
else:
defaults = []

# strip any implicit instance arg
if args:
Expand All @@ -670,10 +688,10 @@ def varnames(func):

assert "self" not in args # best naming practises check?
try:
cache["_varnames"] = args
cache["_varnames"] = args, defaults
except TypeError:
pass
return tuple(args)
return args, defaults


class _HookRelay:
Expand All @@ -692,6 +710,8 @@ 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
if specmodule_or_class is not None:
assert spec_opts is not None
self.set_specification(specmodule_or_class, spec_opts)
Expand All @@ -703,7 +723,8 @@ 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)
argnames = varnames(specfunc)
# get spec arg signature
argnames, self.kwargnames = varnames(specfunc)
self.argnames = ["__multicall__"] + list(argnames)
self.spec_opts = spec_opts
if spec_opts.get("historic"):
Expand All @@ -723,6 +744,8 @@ def remove(wrappers):
raise ValueError("plugin %r not found" % (plugin,))

def _add_hookimpl(self, hookimpl):
"""A an implementation to the callback chain.
"""
if hookimpl.hookwrapper:
methods = self._wrappers
else:
Expand All @@ -744,6 +767,13 @@ 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
)
return self._hookexec(self, self._nonwrappers + self._wrappers, kwargs)

def call_historic(self, proc=None, kwargs=None):
Expand Down Expand Up @@ -773,6 +803,8 @@ def call_extra(self, methods, kwargs):
self._nonwrappers, self._wrappers = old

def _maybe_apply_history(self, method):
"""Apply call history to a new hookimpl if it is marked as historic.
"""
if self.is_historic():
for kwargs, proc in self._call_history:
res = self._hookexec(self, [method], kwargs)
Expand All @@ -783,21 +815,13 @@ def _maybe_apply_history(self, method):
class HookImpl:
def __init__(self, plugin, plugin_name, function, hook_impl_opts):
self.function = function
self.argnames = varnames(self.function)
self.argnames, self.kwargnames = varnames(self.function)
self.plugin = plugin
self.opts = hook_impl_opts
self.plugin_name = plugin_name
self.__dict__.update(hook_impl_opts)


class PluginValidationError(Exception):
""" plugin failed validation. """


class HookCallError(Exception):
""" Hook was called wrongly. """


if hasattr(inspect, 'signature'):
def _formatdef(func):
return "%s%s" % (
Expand Down

0 comments on commit b61f9a2

Please sign in to comment.