Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Encapsulate spec definitions with a class #57

Merged
merged 2 commits into from
Aug 19, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 19 additions & 12 deletions pluggy/hooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -202,28 +202,22 @@ def __init__(self, name, hook_execute, specmodule_or_class=None, spec_opts=None)
self._wrappers = []
self._nonwrappers = []
self._hookexec = hook_execute
self._specmodule_or_class = None
self.argnames = None
self.kwargnames = None
self.multicall = _multicall
self.spec_opts = spec_opts or {}
self.spec = 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 self._specmodule_or_class is not None
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.update(spec_opts)
self.spec = HookSpec(specmodule_or_class, self.name, spec_opts)
if spec_opts.get("historic"):
self._call_history = []
self.warn_on_impl = spec_opts.get("warn_on_impl")

def is_historic(self):
return hasattr(self, "_call_history")
Expand Down Expand Up @@ -273,8 +267,10 @@ def __call__(self, *args, **kwargs):
if args:
raise TypeError("hook calling supports only keyword arguments")
assert not self.is_historic()
if self.argnames:
notincall = set(self.argnames) - set(["__multicall__"]) - set(kwargs.keys())
if self.spec and self.spec.argnames:
notincall = (
set(self.spec.argnames) - set(["__multicall__"]) - set(kwargs.keys())
)
if notincall:
warnings.warn(
"Argument(s) {} which are declared in the hookspec "
Expand Down Expand Up @@ -344,3 +340,14 @@ def __init__(self, plugin, plugin_name, function, hook_impl_opts):

def __repr__(self):
return "<HookImpl plugin_name=%r, plugin=%r>" % (self.plugin_name, self.plugin)


class HookSpec(object):
def __init__(self, namespace, name, opts):
self.namespace = namespace
self.function = function = getattr(namespace, name)
self.name = name
self.argnames, self.kwargnames = varnames(function)
self.opts = opts
self.argnames = ["__multicall__"] + list(self.argnames)
self.warn_on_impl = opts.get("warn_on_impl")
12 changes: 7 additions & 5 deletions pluggy/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,9 @@ def __init__(self, project_name, implprefix=None):
)
self._implprefix = implprefix
self._inner_hookexec = lambda hook, methods, kwargs: hook.multicall(
methods, kwargs, firstresult=hook.spec_opts.get("firstresult")
methods,
kwargs,
firstresult=hook.spec.opts.get("firstresult") if hook.spec else False,
)

def _hookexec(self, hook, methods, kwargs):
Expand Down Expand Up @@ -215,10 +217,10 @@ def _verify_hook(self, hook, hookimpl):
"Plugin %r\nhook %r\nhistoric incompatible to hookwrapper"
% (hookimpl.plugin_name, hook.name),
)
if hook.warn_on_impl:
_warn_for_function(hook.warn_on_impl, hookimpl.function)
if hook.spec.warn_on_impl:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like there'll be a bug here if there's no spec.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, nm. This is never called unless a spec exists.

_warn_for_function(hook.spec.warn_on_impl, hookimpl.function)
# positional arg checking
notinspec = set(hookimpl.argnames) - set(hook.argnames)
notinspec = set(hookimpl.argnames) - set(hook.spec.argnames)
if notinspec:
raise PluginValidationError(
hookimpl.plugin,
Expand Down Expand Up @@ -325,7 +327,7 @@ def subset_hook_caller(self, name, remove_plugins):
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
orig.name, orig._hookexec, orig.spec.namespace, orig.spec.opts
)
for hookimpl in orig._wrappers + orig._nonwrappers:
plugin = hookimpl.plugin
Expand Down
2 changes: 1 addition & 1 deletion testing/benchmark.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ def MC(methods, kwargs, callertype, firstresult=False):
for method in methods:
f = HookImpl(None, "<temp>", method, method.example_impl)
hookfuncs.append(f)
return callertype(hookfuncs, kwargs, {"firstresult": firstresult})
return callertype(hookfuncs, kwargs, firstresult=firstresult)


@hookimpl
Expand Down
6 changes: 3 additions & 3 deletions testing/test_hookcaller.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,9 +169,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"])
Expand Down