Skip to content

Commit 6338f75

Browse files
feat(implementation): add NormalImpl, WrapperImpl, and CompletionHook setup API
Complete design step 04: - HookImpl becomes a base class storing hookimpl_config (deprecated .opts alias kept) with arg binding moved to _get_call_args. - NormalImpl / WrapperImpl subclasses validate their configuration; HookimplConfiguration.create_hookimpl() returns the right subclass (fixing the try-claude footgun of bare HookImpl for normals). - WrapperImpl.setup_and_get_completion_hook() runs wrapper setup and returns a CompletionHook (runtime-checkable Protocol) that owns teardown, adapting old-style hookwrappers uniformly. - Registration and call_extra construct impls via create_hookimpl; multicall binds args via _get_call_args. Full dual-sequence multicall rewiring lands with design step 05. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 5b654ff commit 6338f75

11 files changed

Lines changed: 433 additions & 22 deletions

File tree

changelog/707.feature.rst

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
Hook implementations are now represented by dedicated types:
2+
:class:`pluggy.NormalImpl` for normal implementations and
3+
:class:`pluggy.WrapperImpl` for (old- and new-style) wrappers, both
4+
subclasses of :class:`pluggy.HookImpl`.
5+
``HookimplConfiguration.create_hookimpl()`` selects the appropriate
6+
subclass, and ``WrapperImpl.setup_and_get_completion_hook()`` exposes
7+
wrapper setup/teardown as a ``CompletionHook`` callback.
8+
``HookImpl`` now stores its configuration as ``hookimpl_config``; the old
9+
``opts`` attribute remains as a deprecated alias property.

docs/api_reference.rst

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,14 @@ API Reference
4040
.. autoclass:: pluggy.HookImpl()
4141
:members:
4242

43+
.. autoclass:: pluggy.NormalImpl()
44+
:show-inheritance:
45+
:members:
46+
47+
.. autoclass:: pluggy.WrapperImpl()
48+
:show-inheritance:
49+
:members:
50+
4351
.. autoclass:: pluggy.HookspecConfiguration()
4452
:members:
4553

src/pluggy/__init__.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
"HookspecOpts",
1010
"HookimplOpts",
1111
"HookImpl",
12+
"NormalImpl",
13+
"WrapperImpl",
1214
"HookRelay",
1315
"HookspecMarker",
1416
"HookimplMarker",
@@ -23,6 +25,8 @@
2325
from ._hooks import HookimplMarker
2426
from ._hooks import HookRelay
2527
from ._hooks import HookspecMarker
28+
from ._hooks import NormalImpl
29+
from ._hooks import WrapperImpl
2630
from ._manager import PluginManager
2731
from ._manager import PluginValidationError
2832
from ._pytest_compat import HookimplOpts

src/pluggy/_caller.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -224,10 +224,10 @@ def call_extra(
224224
"Cannot directly call a historic hook - use call_historic instead."
225225
)
226226
self._verify_all_args_are_provided(kwargs)
227-
opts = HookimplConfiguration()
227+
config = HookimplConfiguration()
228228
hookimpls = self._hookimpls.copy()
229229
for method in methods:
230-
hookimpl = HookImpl(None, "<temp>", method, opts)
230+
hookimpl = config.create_hookimpl(None, "<temp>", method)
231231
# Find last non-tryfirst nonwrapper method.
232232
i = len(hookimpls) - 1
233233
while i >= 0 and (

src/pluggy/_config.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,14 @@
88
from typing import Any
99
from typing import Final
1010
from typing import final
11+
from typing import TYPE_CHECKING
12+
13+
14+
if TYPE_CHECKING:
15+
from ._implementation import _HookImplFunction
16+
from ._implementation import _Plugin
17+
from ._implementation import NormalImpl
18+
from ._implementation import WrapperImpl
1119

1220

1321
@final
@@ -98,6 +106,27 @@ def __init__(
98106
#: The name of the hook specification to match, see :ref:`specname`.
99107
self.specname = specname
100108

109+
def create_hookimpl(
110+
self,
111+
plugin: _Plugin,
112+
plugin_name: str,
113+
function: _HookImplFunction[object],
114+
) -> NormalImpl | WrapperImpl:
115+
"""Create the appropriate :class:`HookImpl` subclass for this
116+
configuration.
117+
118+
Wrapper configurations produce a :class:`WrapperImpl`; all others
119+
produce a :class:`NormalImpl`.
120+
"""
121+
# Local import to avoid a circular import with the implementation
122+
# module.
123+
from ._implementation import NormalImpl
124+
from ._implementation import WrapperImpl
125+
126+
if self.wrapper or self.hookwrapper:
127+
return WrapperImpl(plugin, plugin_name, function, self)
128+
return NormalImpl(plugin, plugin_name, function, self)
129+
101130
def __repr__(self) -> str:
102131
attrs = [
103132
f"{slot}={getattr(self, slot)!r}"

src/pluggy/_execution.py

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
import warnings
1515

1616
from ._implementation import HookImpl
17-
from ._result import HookCallError
1817
from ._result import Result
1918
from ._warnings import PluggyTeardownRaisedWarning
2019

@@ -96,12 +95,7 @@ def _multicall(
9695
teardowns: list[Teardown] = []
9796
try: # run impl and wrapper setup functions in a loop
9897
for hook_impl in reversed(hook_impls):
99-
try:
100-
args = [caller_kwargs[argname] for argname in hook_impl.argnames]
101-
except KeyError as e:
102-
raise HookCallError(
103-
f"hook call must provide argument {e.args[0]!r}"
104-
) from e
98+
args = hook_impl._get_call_args(caller_kwargs)
10599

106100
if hook_impl.hookwrapper:
107101
function_gen = run_old_style_hookwrapper(hook_impl, hook_name, args)

src/pluggy/_hooks.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,10 @@
2222
from ._decorators import varnames
2323
from ._implementation import _HookImplFunction
2424
from ._implementation import _Plugin
25+
from ._implementation import CompletionHook
2526
from ._implementation import HookImpl
27+
from ._implementation import NormalImpl
28+
from ._implementation import WrapperImpl
2629

2730

2831
__all__ = [
@@ -38,6 +41,9 @@
3841
"_HookRelay",
3942
"_SubsetHookCaller",
4043
"HookImpl",
44+
"NormalImpl",
45+
"WrapperImpl",
46+
"CompletionHook",
4147
"_HookImplFunction",
4248
"_Namespace",
4349
"_Plugin",

src/pluggy/_implementation.py

Lines changed: 154 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,18 @@
66

77
from collections.abc import Callable
88
from collections.abc import Generator
9+
from collections.abc import Mapping
10+
from typing import cast
911
from typing import Final
1012
from typing import final
13+
from typing import Protocol
14+
from typing import runtime_checkable
1115
from typing import TypeAlias
1216
from typing import TypeVar
1317

1418
from ._config import HookimplConfiguration
1519
from ._decorators import varnames
20+
from ._result import HookCallError
1621
from ._result import Result
1722

1823

@@ -22,16 +27,30 @@
2227
_HookImplFunction: TypeAlias = Callable[..., _T | Generator[None, Result[_T], None]]
2328

2429

25-
@final
30+
@runtime_checkable
31+
class CompletionHook(Protocol):
32+
"""Teardown callback returned by :meth:`WrapperImpl.setup_and_get_completion_hook`.
33+
34+
Receives the current ``(result, exception)`` outcome of the hook call and
35+
returns the possibly replaced ``(result, exception)`` pair.
36+
"""
37+
38+
def __call__(
39+
self,
40+
result: object | list[object] | None,
41+
exception: BaseException | None,
42+
) -> tuple[object | list[object] | None, BaseException | None]: ...
43+
44+
2645
class HookImpl:
27-
"""A hook implementation in a :class:`HookCaller`."""
46+
"""Base class for hook implementations in a :class:`HookCaller`."""
2847

2948
__slots__ = (
3049
"function",
3150
"argnames",
3251
"kwargnames",
3352
"plugin",
34-
"opts",
53+
"hookimpl_config",
3554
"plugin_name",
3655
"wrapper",
3756
"hookwrapper",
@@ -45,7 +64,7 @@ def __init__(
4564
plugin: _Plugin,
4665
plugin_name: str,
4766
function: _HookImplFunction[object],
48-
hook_impl_opts: HookimplConfiguration,
67+
hook_impl_config: HookimplConfiguration,
4968
) -> None:
5069
""":meta private:"""
5170
#: The hook implementation function.
@@ -59,23 +78,147 @@ def __init__(
5978
self.plugin: Final = plugin
6079
#: The :class:`HookimplConfiguration` used to configure this hook
6180
#: implementation.
62-
self.opts: Final = hook_impl_opts
81+
self.hookimpl_config: Final = hook_impl_config
6382
#: The name of the plugin which defined this hook implementation.
6483
self.plugin_name: Final = plugin_name
6584
#: Whether the hook implementation is a :ref:`wrapper <hookwrapper>`.
66-
self.wrapper: Final = hook_impl_opts.wrapper
85+
self.wrapper: Final = hook_impl_config.wrapper
6786
#: Whether the hook implementation is an :ref:`old-style wrapper
6887
#: <old_style_hookwrappers>`.
69-
self.hookwrapper: Final = hook_impl_opts.hookwrapper
88+
self.hookwrapper: Final = hook_impl_config.hookwrapper
7089
#: Whether validation against a hook specification is :ref:`optional
7190
#: <optionalhook>`.
72-
self.optionalhook: Final = hook_impl_opts.optionalhook
91+
self.optionalhook: Final = hook_impl_config.optionalhook
7392
#: Whether to try to order this hook implementation :ref:`first
7493
#: <callorder>`.
75-
self.tryfirst: Final = hook_impl_opts.tryfirst
94+
self.tryfirst: Final = hook_impl_config.tryfirst
7695
#: Whether to try to order this hook implementation :ref:`last
7796
#: <callorder>`.
78-
self.trylast: Final = hook_impl_opts.trylast
97+
self.trylast: Final = hook_impl_config.trylast
98+
99+
@property
100+
def opts(self) -> HookimplConfiguration:
101+
"""Alias for :attr:`hookimpl_config`.
102+
103+
.. deprecated::
104+
Use :attr:`hookimpl_config` instead.
105+
"""
106+
return self.hookimpl_config
107+
108+
def _get_call_args(self, caller_kwargs: Mapping[str, object]) -> list[object]:
109+
"""Extract the positional arguments for calling this hook implementation.
110+
111+
:raises HookCallError: If a required argument is missing.
112+
"""
113+
try:
114+
return [caller_kwargs[argname] for argname in self.argnames]
115+
except KeyError as e:
116+
raise HookCallError(f"hook call must provide argument {e.args[0]!r}") from e
79117

80118
def __repr__(self) -> str:
81-
return f"<HookImpl plugin_name={self.plugin_name!r}, plugin={self.plugin!r}>"
119+
return (
120+
f"<{type(self).__name__} "
121+
f"plugin_name={self.plugin_name!r}, plugin={self.plugin!r}>"
122+
)
123+
124+
125+
@final
126+
class NormalImpl(HookImpl):
127+
"""A normal (non-wrapper) hook implementation in a :class:`HookCaller`."""
128+
129+
def __init__(
130+
self,
131+
plugin: _Plugin,
132+
plugin_name: str,
133+
function: _HookImplFunction[object],
134+
hook_impl_config: HookimplConfiguration,
135+
) -> None:
136+
""":meta private:"""
137+
if hook_impl_config.wrapper or hook_impl_config.hookwrapper:
138+
raise ValueError(
139+
"NormalImpl cannot be used for wrapper implementations. "
140+
"Use WrapperImpl instead."
141+
)
142+
super().__init__(plugin, plugin_name, function, hook_impl_config)
143+
144+
145+
@final
146+
class WrapperImpl(HookImpl):
147+
"""A wrapper hook implementation in a :class:`HookCaller`."""
148+
149+
def __init__(
150+
self,
151+
plugin: _Plugin,
152+
plugin_name: str,
153+
function: _HookImplFunction[object],
154+
hook_impl_config: HookimplConfiguration,
155+
) -> None:
156+
""":meta private:"""
157+
if not (hook_impl_config.wrapper or hook_impl_config.hookwrapper):
158+
raise ValueError(
159+
"WrapperImpl can only be used for wrapper implementations. "
160+
"Use NormalImpl for normal implementations."
161+
)
162+
super().__init__(plugin, plugin_name, function, hook_impl_config)
163+
164+
def setup_and_get_completion_hook(
165+
self, hook_name: str, caller_kwargs: Mapping[str, object]
166+
) -> CompletionHook:
167+
"""Run the wrapper setup phase and return its :class:`CompletionHook`.
168+
169+
Old-style hookwrappers and new-style wrappers are handled uniformly by
170+
adapting old-style wrappers via ``run_old_style_hookwrapper``.
171+
172+
The returned completion hook performs the teardown: it sends the
173+
current outcome into the wrapper generator (or throws the current
174+
exception) and returns the possibly replaced ``(result, exception)``
175+
pair.
176+
"""
177+
# Local import to avoid a circular import with the execution module.
178+
from ._execution import _raise_wrapfail
179+
from ._execution import run_old_style_hookwrapper
180+
181+
args = self._get_call_args(caller_kwargs)
182+
183+
wrapper_gen: Generator[None, object, object]
184+
if self.hookwrapper:
185+
wrapper_gen = run_old_style_hookwrapper(self, hook_name, args)
186+
else:
187+
wrapper_gen = cast(Generator[None, object, object], self.function(*args))
188+
189+
try:
190+
next(wrapper_gen) # first yield / setup phase
191+
except StopIteration:
192+
_raise_wrapfail(wrapper_gen, "did not yield")
193+
194+
def completion_hook(
195+
result: object | list[object] | None, exception: BaseException | None
196+
) -> tuple[object | list[object] | None, BaseException | None]:
197+
try:
198+
if exception is not None:
199+
try:
200+
wrapper_gen.throw(exception)
201+
except RuntimeError as re:
202+
# StopIteration from generator causes RuntimeError
203+
# even for coroutine usage - see #544
204+
if (
205+
isinstance(exception, StopIteration)
206+
and re.__cause__ is exception
207+
):
208+
wrapper_gen.close()
209+
return result, exception
210+
else:
211+
raise
212+
else:
213+
wrapper_gen.send(result)
214+
# Following is unreachable for a well behaved hook wrapper.
215+
# Try to force finalizers otherwise postponed till GC action.
216+
# Note: close() may raise if generator handles GeneratorExit.
217+
wrapper_gen.close()
218+
_raise_wrapfail(wrapper_gen, "has second yield")
219+
except StopIteration as si:
220+
return si.value, None
221+
except BaseException as e:
222+
return result, e
223+
224+
return completion_hook

src/pluggy/_manager.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ def register(self, plugin: _Plugin, name: str | None = None) -> str | None:
150150
hookimpl_config = self._discover_hookimpl_configuration(plugin, name)
151151
if hookimpl_config is not None:
152152
method: _HookImplFunction[object] = getattr(plugin, name)
153-
hookimpl = HookImpl(plugin, plugin_name, method, hookimpl_config)
153+
hookimpl = hookimpl_config.create_hookimpl(plugin, plugin_name, method)
154154
name = hookimpl_config.specname or name
155155
hook: HookCaller | None = getattr(self.hook, name, None)
156156
if hook is None:

testing/test_details.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ def myhook(self):
193193
plugin = Plugin()
194194
pname = pm.register(plugin)
195195
assert repr(pm.hook.myhook.get_hookimpls()[0]) == (
196-
f"<HookImpl plugin_name={pname!r}, plugin={plugin!r}>"
196+
f"<NormalImpl plugin_name={pname!r}, plugin={plugin!r}>"
197197
)
198198

199199

0 commit comments

Comments
 (0)