From 36f03b2aeae38d14b8f3f90e56560a767ec4d26b Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Fri, 26 Jul 2019 16:09:23 +0300 Subject: [PATCH] Avoid cyclic dependency by merging callers.py into hooks.py The two modules have a type-level cyclic dependency: hooks.py uses _multicall and _legacymulticall from callers.py. callers.py uses HookImpl from hooks.py. This suggests that they're better off combined. The _Result functionality from callers.py is independent and general, so move it to its own module _result.py. Backward compatibility: I did not find any external project which imports directly from pluggy.callers. It exposes two potential import candidates: - HookCallError: exposed from the top-level, so no reason to reach inside. - _Result: it's prefixed and hidden, so no guarantees about that. --- docs/api_reference.rst | 4 +- docs/index.rst | 6 +- src/pluggy/__init__.py | 3 +- src/pluggy/_result.py | 64 +++++++++++ src/pluggy/_tracing.py | 2 +- src/pluggy/callers.py | 208 ----------------------------------- src/pluggy/hooks.py | 142 +++++++++++++++++++++++- testing/benchmark.py | 3 +- testing/test_deprecations.py | 2 +- testing/test_multicall.py | 3 +- 10 files changed, 215 insertions(+), 222 deletions(-) create mode 100644 src/pluggy/_result.py delete mode 100644 src/pluggy/callers.py diff --git a/docs/api_reference.rst b/docs/api_reference.rst index dc1ee284..52a7b487 100644 --- a/docs/api_reference.rst +++ b/docs/api_reference.rst @@ -9,8 +9,8 @@ Api Reference :show-inheritance: -.. automethod:: pluggy.callers._Result.get_result +.. automethod:: pluggy._result._Result.get_result -.. automethod:: pluggy.callers._Result.force_result +.. automethod:: pluggy._result._Result.force_result .. automethod:: pluggy.hooks._HookCaller.call_extra diff --git a/docs/index.rst b/docs/index.rst index 724cef58..00bc80c1 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -360,11 +360,11 @@ be implemented as generator function with a single ``yield`` in its body: if config.use_defaults: outcome.force_result(defaults) -The generator is `sent`_ a :py:class:`pluggy.callers._Result` object which can +The generator is `sent`_ a :py:class:`pluggy._result._Result` object which can be assigned in the ``yield`` expression and used to override or inspect the final result(s) returned back to the caller using the -:py:meth:`~pluggy.callers._Result.force_result` or -:py:meth:`~pluggy.callers._Result.get_result` methods. +:py:meth:`~pluggy._result._Result.force_result` or +:py:meth:`~pluggy._result._Result.get_result` methods. .. note:: Hook wrappers can **not** return results (as per generator function diff --git a/src/pluggy/__init__.py b/src/pluggy/__init__.py index fb4f991a..076ae555 100644 --- a/src/pluggy/__init__.py +++ b/src/pluggy/__init__.py @@ -14,5 +14,4 @@ ] from .manager import PluginManager, PluginValidationError -from .callers import HookCallError -from .hooks import HookspecMarker, HookimplMarker +from .hooks import HookspecMarker, HookimplMarker, HookCallError diff --git a/src/pluggy/_result.py b/src/pluggy/_result.py new file mode 100644 index 00000000..a99bbac0 --- /dev/null +++ b/src/pluggy/_result.py @@ -0,0 +1,64 @@ +import sys +import warnings + + +if sys.version_info < (3,): + exec( + """ +def _reraise(cls, val, tb): + raise cls, val, tb +""" + ) + + +class _Result(object): + def __init__(self, result, excinfo): + self._result = result + self._excinfo = excinfo + + @property + def excinfo(self): + return self._excinfo + + @property + def result(self): + """Get the result(s) for this hook call (DEPRECATED in favor of ``get_result()``).""" + msg = "Use get_result() which forces correct exception handling" + warnings.warn(DeprecationWarning(msg), stacklevel=2) + return self._result + + @classmethod + def from_call(cls, func): + __tracebackhide__ = True + result = excinfo = None + try: + result = func() + except BaseException: + excinfo = sys.exc_info() + + return cls(result, excinfo) + + def force_result(self, result): + """Force the result(s) to ``result``. + + If the hook was marked as a ``firstresult`` a single value should + be set otherwise set a (modified) list of results. Any exceptions + found during invocation will be deleted. + """ + self._result = result + self._excinfo = None + + def get_result(self): + """Get the result(s) for this hook call. + + If the hook was marked as a ``firstresult`` only a single value + will be returned otherwise a list of results. + """ + __tracebackhide__ = True + if self._excinfo is None: + return self._result + else: + ex = self._excinfo + if sys.version_info >= (3,): + raise ex[1].with_traceback(ex[2]) + _reraise(*ex) # noqa diff --git a/src/pluggy/_tracing.py b/src/pluggy/_tracing.py index cccae08e..dfcc8740 100644 --- a/src/pluggy/_tracing.py +++ b/src/pluggy/_tracing.py @@ -1,7 +1,7 @@ """ Tracing utils """ -from .callers import _Result +from ._result import _Result class TagTracer(object): diff --git a/src/pluggy/callers.py b/src/pluggy/callers.py deleted file mode 100644 index e7ea464b..00000000 --- a/src/pluggy/callers.py +++ /dev/null @@ -1,208 +0,0 @@ -""" -Call loop machinery -""" -import sys -import warnings - -_py3 = sys.version_info > (3, 0) - - -if not _py3: - exec( - """ -def _reraise(cls, val, tb): - raise cls, val, tb -""" - ) - - -def _raise_wrapfail(wrap_controller, msg): - co = wrap_controller.gi_code - raise RuntimeError( - "wrap_controller at %r %s:%d %s" - % (co.co_name, co.co_filename, co.co_firstlineno, msg) - ) - - -class HookCallError(Exception): - """ Hook was called wrongly. """ - - -class _Result(object): - def __init__(self, result, excinfo): - self._result = result - self._excinfo = excinfo - - @property - def excinfo(self): - return self._excinfo - - @property - def result(self): - """Get the result(s) for this hook call (DEPRECATED in favor of ``get_result()``).""" - msg = "Use get_result() which forces correct exception handling" - warnings.warn(DeprecationWarning(msg), stacklevel=2) - return self._result - - @classmethod - def from_call(cls, func): - __tracebackhide__ = True - result = excinfo = None - try: - result = func() - except BaseException: - excinfo = sys.exc_info() - - return cls(result, excinfo) - - def force_result(self, result): - """Force the result(s) to ``result``. - - If the hook was marked as a ``firstresult`` a single value should - be set otherwise set a (modified) list of results. Any exceptions - found during invocation will be deleted. - """ - self._result = result - self._excinfo = None - - def get_result(self): - """Get the result(s) for this hook call. - - If the hook was marked as a ``firstresult`` only a single value - will be returned otherwise a list of results. - """ - __tracebackhide__ = True - if self._excinfo is None: - return self._result - else: - ex = self._excinfo - if _py3: - raise ex[1].with_traceback(ex[2]) - _reraise(*ex) # noqa - - -def _wrapped_call(wrap_controller, func): - """ Wrap calling to a function with a generator which needs to yield - exactly once. The yield point will trigger calling the wrapped function - and return its ``_Result`` to the yield point. The generator then needs - to finish (raise StopIteration) in order for the wrapped call to complete. - """ - try: - next(wrap_controller) # first yield - except StopIteration: - _raise_wrapfail(wrap_controller, "did not yield") - call_outcome = _Result.from_call(func) - try: - wrap_controller.send(call_outcome) - _raise_wrapfail(wrap_controller, "has second yield") - except StopIteration: - pass - return call_outcome.get_result() - - -class _LegacyMultiCall(object): - """ execute a call into multiple python functions/methods. """ - - # XXX note that the __multicall__ argument is supported only - # for pytest compatibility reasons. It was never officially - # supported there and is explicitely deprecated since 2.8 - # 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, firstresult=False): - self.hook_impls = hook_impls - self.caller_kwargs = kwargs # come from _HookCaller.__call__() - self.caller_kwargs["__multicall__"] = self - self.firstresult = firstresult - - def execute(self): - caller_kwargs = self.caller_kwargs - self.results = results = [] - firstresult = self.firstresult - - while self.hook_impls: - hook_impl = self.hook_impls.pop() - try: - args = [caller_kwargs[argname] for argname in hook_impl.argnames] - 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: - return res - results.append(res) - - if not firstresult: - return results - - 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.caller_kwargs) - - -def _legacymulticall(hook_impls, caller_kwargs, firstresult=False): - return _LegacyMultiCall( - hook_impls, caller_kwargs, firstresult=firstresult - ).execute() - - -def _multicall(hook_impls, caller_kwargs, firstresult=False): - """Execute a call into multiple python functions/methods and return the - result(s). - - ``caller_kwargs`` comes from _HookCaller.__call__(). - """ - __tracebackhide__ = True - results = [] - excinfo = None - try: # run impl and wrapper setup functions in a loop - teardowns = [] - try: - for hook_impl in reversed(hook_impls): - try: - args = [caller_kwargs[argname] for argname in hook_impl.argnames] - 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: - try: - gen = hook_impl.function(*args) - next(gen) # first yield - teardowns.append(gen) - except StopIteration: - _raise_wrapfail(gen, "did not yield") - else: - res = hook_impl.function(*args) - if res is not None: - results.append(res) - if firstresult: # halt further impl calls - break - except BaseException: - excinfo = sys.exc_info() - finally: - if firstresult: # first result hooks return a single value - outcome = _Result(results[0] if results else None, excinfo) - else: - outcome = _Result(results, excinfo) - - # run all wrapper post-yield blocks - for gen in reversed(teardowns): - try: - gen.send(outcome) - _raise_wrapfail(gen, "has second yield") - except StopIteration: - pass - - return outcome.get_result() diff --git a/src/pluggy/hooks.py b/src/pluggy/hooks.py index 1476d703..3f7dcaf8 100644 --- a/src/pluggy/hooks.py +++ b/src/pluggy/hooks.py @@ -4,7 +4,8 @@ import inspect import sys import warnings -from .callers import _legacymulticall, _multicall + +from ._result import _Result class HookspecMarker(object): @@ -360,3 +361,142 @@ def __init__(self, namespace, name, opts): self.opts = opts self.argnames = ["__multicall__"] + list(self.argnames) self.warn_on_impl = opts.get("warn_on_impl") + + +def _raise_wrapfail(wrap_controller, msg): + co = wrap_controller.gi_code + raise RuntimeError( + "wrap_controller at %r %s:%d %s" + % (co.co_name, co.co_filename, co.co_firstlineno, msg) + ) + + +class HookCallError(Exception): + """ Hook was called wrongly. """ + + +def _wrapped_call(wrap_controller, func): + """ Wrap calling to a function with a generator which needs to yield + exactly once. The yield point will trigger calling the wrapped function + and return its ``_Result`` to the yield point. The generator then needs + to finish (raise StopIteration) in order for the wrapped call to complete. + """ + try: + next(wrap_controller) # first yield + except StopIteration: + _raise_wrapfail(wrap_controller, "did not yield") + call_outcome = _Result.from_call(func) + try: + wrap_controller.send(call_outcome) + _raise_wrapfail(wrap_controller, "has second yield") + except StopIteration: + pass + return call_outcome.get_result() + + +class _LegacyMultiCall(object): + """ execute a call into multiple python functions/methods. """ + + # XXX note that the __multicall__ argument is supported only + # for pytest compatibility reasons. It was never officially + # supported there and is explicitely deprecated since 2.8 + # 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, firstresult=False): + self.hook_impls = hook_impls + self.caller_kwargs = kwargs # come from _HookCaller.__call__() + self.caller_kwargs["__multicall__"] = self + self.firstresult = firstresult + + def execute(self): + caller_kwargs = self.caller_kwargs + self.results = results = [] + firstresult = self.firstresult + + while self.hook_impls: + hook_impl = self.hook_impls.pop() + try: + args = [caller_kwargs[argname] for argname in hook_impl.argnames] + 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: + return res + results.append(res) + + if not firstresult: + return results + + 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.caller_kwargs) + + +def _legacymulticall(hook_impls, caller_kwargs, firstresult=False): + return _LegacyMultiCall( + hook_impls, caller_kwargs, firstresult=firstresult + ).execute() + + +def _multicall(hook_impls, caller_kwargs, firstresult=False): + """Execute a call into multiple python functions/methods and return the + result(s). + + ``caller_kwargs`` comes from _HookCaller.__call__(). + """ + __tracebackhide__ = True + results = [] + excinfo = None + try: # run impl and wrapper setup functions in a loop + teardowns = [] + try: + for hook_impl in reversed(hook_impls): + try: + args = [caller_kwargs[argname] for argname in hook_impl.argnames] + 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: + try: + gen = hook_impl.function(*args) + next(gen) # first yield + teardowns.append(gen) + except StopIteration: + _raise_wrapfail(gen, "did not yield") + else: + res = hook_impl.function(*args) + if res is not None: + results.append(res) + if firstresult: # halt further impl calls + break + except BaseException: + excinfo = sys.exc_info() + finally: + if firstresult: # first result hooks return a single value + outcome = _Result(results[0] if results else None, excinfo) + else: + outcome = _Result(results, excinfo) + + # run all wrapper post-yield blocks + for gen in reversed(teardowns): + try: + gen.send(outcome) + _raise_wrapfail(gen, "has second yield") + except StopIteration: + pass + + return outcome.get_result() diff --git a/testing/benchmark.py b/testing/benchmark.py index aa8de929..a952701c 100644 --- a/testing/benchmark.py +++ b/testing/benchmark.py @@ -3,8 +3,7 @@ """ import pytest from pluggy import HookspecMarker, HookimplMarker -from pluggy.hooks import HookImpl -from pluggy.callers import _multicall, _legacymulticall +from pluggy.hooks import HookImpl, _multicall, _legacymulticall hookspec = HookspecMarker("example") hookimpl = HookimplMarker("example") diff --git a/testing/test_deprecations.py b/testing/test_deprecations.py index 7151921b..008f98bf 100644 --- a/testing/test_deprecations.py +++ b/testing/test_deprecations.py @@ -2,7 +2,7 @@ Deprecation warnings testing roundup. """ import pytest -from pluggy.callers import _Result +from pluggy._result import _Result from pluggy import PluginManager, HookimplMarker, HookspecMarker hookspec = HookspecMarker("example") diff --git a/testing/test_multicall.py b/testing/test_multicall.py index eaf534a0..36d821be 100644 --- a/testing/test_multicall.py +++ b/testing/test_multicall.py @@ -1,7 +1,6 @@ import pytest from pluggy import HookCallError, HookspecMarker, HookimplMarker -from pluggy.hooks import HookImpl -from pluggy.callers import _multicall, _legacymulticall +from pluggy.hooks import HookImpl, _multicall, _legacymulticall hookspec = HookspecMarker("example")