Skip to content

Commit 47262ee

Browse files
Separate normal and wrapper hook implementations with distinct types
This change introduces type-safe separation between normal hook implementations and wrapper implementations by: - Creating WrapperImpl subclass for wrapper/hookwrapper implementations - Modifying _multicall to handle normal and wrapper implementations separately - Updating hook callers to maintain separate lists and enforce type constraints - Adding factory method in HookimplConfiguration for proper type instantiation - Ensuring historic hooks reject wrapper implementations at registration time The separation improves type safety, clarifies execution flow, and maintains backward compatibility while providing better error messages for misuse. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
1 parent 9f060e1 commit 47262ee

File tree

2 files changed

+54
-88
lines changed

2 files changed

+54
-88
lines changed

src/pluggy/_callers.py

Lines changed: 42 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@
1111
from typing import NoReturn
1212
import warnings
1313

14-
from ._hooks import HookImpl
15-
from ._result import HookCallError
14+
from ._hook_callers import HookImpl
15+
from ._hook_callers import WrapperImpl
1616
from ._result import Result
1717
from ._warnings import PluggyTeardownRaisedWarning
1818

@@ -23,7 +23,7 @@
2323

2424

2525
def run_old_style_hookwrapper(
26-
hook_impl: HookImpl, hook_name: str, args: Sequence[object]
26+
hook_impl: WrapperImpl, hook_name: str, args: Sequence[object]
2727
) -> Teardown:
2828
"""
2929
backward compatibility wrapper to run a old style hookwrapper as a wrapper
@@ -64,18 +64,19 @@ def _raise_wrapfail(
6464

6565

6666
def _warn_teardown_exception(
67-
hook_name: str, hook_impl: HookImpl, e: BaseException
67+
hook_name: str, hook_impl: WrapperImpl, e: BaseException
6868
) -> None:
6969
msg = "A plugin raised an exception during an old-style hookwrapper teardown.\n"
7070
msg += f"Plugin: {hook_impl.plugin_name}, Hook: {hook_name}\n"
7171
msg += f"{type(e).__name__}: {e}\n"
7272
msg += "For more information see https://pluggy.readthedocs.io/en/stable/api_reference.html#pluggy.PluggyTeardownRaisedWarning" # noqa: E501
73-
warnings.warn(PluggyTeardownRaisedWarning(msg), stacklevel=6)
73+
warnings.warn(PluggyTeardownRaisedWarning(msg), stacklevel=7)
7474

7575

7676
def _multicall(
7777
hook_name: str,
78-
hook_impls: Sequence[HookImpl],
78+
normal_impls: Sequence[HookImpl],
79+
wrapper_impls: Sequence[WrapperImpl],
7980
caller_kwargs: Mapping[str, object],
8081
firstresult: bool,
8182
) -> object | list[object]:
@@ -87,81 +88,41 @@ def _multicall(
8788
__tracebackhide__ = True
8889
results: list[object] = []
8990
exception = None
90-
try: # run impl and wrapper setup functions in a loop
91-
teardowns: list[Teardown] = []
92-
try:
93-
for hook_impl in reversed(hook_impls):
94-
try:
95-
args = [caller_kwargs[argname] for argname in hook_impl.argnames]
96-
except KeyError as e:
97-
# coverage bug - this is tested
98-
for argname in hook_impl.argnames: # pragma: no cover
99-
if argname not in caller_kwargs:
100-
raise HookCallError(
101-
f"hook call must provide argument {argname!r}"
102-
) from e
103-
104-
if hook_impl.hookwrapper:
105-
function_gen = run_old_style_hookwrapper(hook_impl, hook_name, args)
106-
107-
next(function_gen) # first yield
108-
teardowns.append(function_gen)
109-
110-
elif hook_impl.wrapper:
111-
try:
112-
# If this cast is not valid, a type error is raised below,
113-
# which is the desired response.
114-
res = hook_impl.function(*args)
115-
function_gen = cast(Generator[None, object, object], res)
116-
next(function_gen) # first yield
117-
teardowns.append(function_gen)
118-
except StopIteration:
119-
_raise_wrapfail(function_gen, "did not yield")
120-
else:
121-
res = hook_impl.function(*args)
122-
if res is not None:
123-
results.append(res)
124-
if firstresult: # halt further impl calls
125-
break
126-
except BaseException as exc:
127-
exception = exc
128-
finally:
129-
if firstresult: # first result hooks return a single value
130-
result = results[0] if results else None
131-
else:
132-
result = results
133-
134-
# run all wrapper post-yield blocks
135-
for teardown in reversed(teardowns):
136-
try:
137-
if exception is not None:
138-
try:
139-
teardown.throw(exception)
140-
except RuntimeError as re:
141-
# StopIteration from generator causes RuntimeError
142-
# even for coroutine usage - see #544
143-
if (
144-
isinstance(exception, StopIteration)
145-
and re.__cause__ is exception
146-
):
147-
teardown.close()
148-
continue
149-
else:
150-
raise
151-
else:
152-
teardown.send(result)
153-
# Following is unreachable for a well behaved hook wrapper.
154-
# Try to force finalizers otherwise postponed till GC action.
155-
# Note: close() may raise if generator handles GeneratorExit.
156-
teardown.close()
157-
except StopIteration as si:
158-
result = si.value
159-
exception = None
160-
continue
161-
except BaseException as e:
162-
exception = e
163-
continue
164-
_raise_wrapfail(teardown, "has second yield")
91+
92+
# Set up wrapper completion hooks
93+
from ._hook_callers import CompletionHook
94+
95+
completion_hooks: list[CompletionHook] = []
96+
97+
try:
98+
# Set up all wrappers and collect their completion hooks
99+
for wrapper_impl in reversed(wrapper_impls):
100+
completion_hook = wrapper_impl.setup_and_get_completion_hook(
101+
hook_name, caller_kwargs
102+
)
103+
completion_hooks.append(completion_hook)
104+
105+
# Process normal implementations (in reverse order for correct execution)
106+
# Caller ensures normal_impls contains only non-wrapper implementations
107+
for normal_impl in reversed(normal_impls):
108+
args = normal_impl._get_call_args(caller_kwargs)
109+
res = normal_impl.function(*args)
110+
if res is not None:
111+
results.append(res)
112+
if firstresult: # halt further impl calls
113+
break
114+
except BaseException as exc:
115+
exception = exc
116+
117+
# Determine final result before teardown
118+
if firstresult: # first result hooks return a single value
119+
result = results[0] if results else None
120+
else:
121+
result = results
122+
123+
# Run completion hooks in reverse order (LIFO - Last In, First Out)
124+
for completion_hook in reversed(completion_hooks):
125+
result, exception = completion_hook(result, exception)
165126

166127
if exception is not None:
167128
raise exception

src/pluggy/_hook_callers.py

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,12 @@ def _insert_hookimpl_into_list(
6060
target_list.insert(i + 1, hookimpl)
6161

6262

63-
def _insert_hookimpl_into_list(hookimpl: HookImpl, target_list: list[HookImpl]) -> None:
63+
_T_HookImpl = TypeVar("_T_HookImpl", bound="HookImpl")
64+
65+
66+
def _insert_hookimpl_into_list(
67+
hookimpl: _T_HookImpl, target_list: MutableSequence[_T_HookImpl]
68+
) -> None:
6469
"""Insert a hookimpl into the target list maintaining proper ordering.
6570
6671
The ordering is: [trylast, normal, tryfirst]
@@ -549,9 +554,11 @@ def __call__(self, **kwargs: object) -> Any:
549554
if self.spec:
550555
self.spec.verify_all_args_are_provided(kwargs)
551556
firstresult = self.spec.config.firstresult if self.spec else False
552-
# Get the hookexec from the original
553557
hookexec = getattr(self._orig, "_hookexec")
554-
return hookexec(self.name, self.get_hookimpls(), kwargs, firstresult)
558+
559+
normal_impls = self._get_filtered(self._orig._normal_hookimpls)
560+
wrapper_impls = self._get_filtered(self._orig._wrapper_hookimpls)
561+
return hookexec(self.name, normal_impls, wrapper_impls, kwargs, firstresult)
555562

556563
normal_impls = self._get_filtered(self._orig._normal_hookimpls)
557564
wrapper_impls = self._get_filtered(self._orig._wrapper_hookimpls)
@@ -576,9 +583,7 @@ def call_historic(
576583
if self.spec:
577584
self.spec.verify_all_args_are_provided(kwargs)
578585

579-
# If the original is a HistoricHookCaller, add to its history
580-
if hasattr(self._orig, "_call_history"):
581-
self._orig._call_history.append((kwargs, result_callback))
586+
self._orig._call_history.append((kwargs, result_callback))
582587

583588
# Execute with filtered hookimpls (historic hooks don't support wrappers)
584589
hookexec = getattr(self._orig, "_hookexec")
@@ -631,7 +636,6 @@ def __repr__(self) -> str:
631636
_SubsetHookCaller = SubsetHookCaller
632637

633638

634-
@final
635639
class HookImpl:
636640
"""Base class for hook implementations in a :class:`HookCaller`."""
637641

@@ -648,6 +652,7 @@ class HookImpl:
648652
"trylast",
649653
"hookimpl_config",
650654
)
655+
651656
function: Final[_HookImplFunction[object]]
652657
argnames: Final[tuple[str, ...]]
653658
kwargnames: Final[tuple[str, ...]]

0 commit comments

Comments
 (0)