Skip to content

Commit d1815d2

Browse files
Replace internal hook implementation with HookimplConfiguration
- Update HookimplMarker to store HookimplConfiguration objects directly instead of HookimplOpts dicts on decorated functions - Add new internal _parse_hookimpl method that works with HookimplConfiguration and handles both new-style configs and legacy dict-based configs - Convert parse_hookimpl_opts to compatibility shim that returns None, allowing legacy projects like pytest to override while modern code uses structured configuration throughout - Update test to use direct attribute access instead of dict .get() method - Eliminate redundant dict-to-config conversions in plugin registration 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
1 parent 6091306 commit d1815d2

File tree

3 files changed

+69
-36
lines changed

3 files changed

+69
-36
lines changed

src/pluggy/_hooks.py

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -338,15 +338,15 @@ def __call__( # noqa: F811
338338
"""
339339

340340
def setattr_hookimpl_opts(func: _F) -> _F:
341-
opts: HookimplOpts = {
342-
"wrapper": wrapper,
343-
"hookwrapper": hookwrapper,
344-
"optionalhook": optionalhook,
345-
"tryfirst": tryfirst,
346-
"trylast": trylast,
347-
"specname": specname,
348-
}
349-
setattr(func, self.project_name + "_impl", opts)
341+
config = HookimplConfiguration(
342+
wrapper=wrapper,
343+
hookwrapper=hookwrapper,
344+
optionalhook=optionalhook,
345+
tryfirst=tryfirst,
346+
trylast=trylast,
347+
specname=specname,
348+
)
349+
setattr(func, self.project_name + "_impl", config)
350350
return func
351351

352352
if function is None:
@@ -362,8 +362,8 @@ def get_hookconfig(self, func: Callable[..., object]) -> HookimplConfiguration:
362362
:raises AttributeError: If the function is not decorated with this marker
363363
"""
364364
attr_name = self.project_name + "_impl"
365-
opts: HookimplOpts = getattr(func, attr_name)
366-
return HookimplConfiguration.from_opts(opts)
365+
config: HookimplConfiguration = getattr(func, attr_name)
366+
return config
367367

368368

369369
def normalize_hookimpl_opts(opts: HookimplOpts) -> None:

src/pluggy/_manager.py

Lines changed: 57 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -155,47 +155,80 @@ def register(self, plugin: _Plugin, name: str | None = None) -> str | None:
155155

156156
# register matching hook implementations of the plugin
157157
for name in dir(plugin):
158-
hookimpl_opts = self.parse_hookimpl_opts(plugin, name)
159-
if hookimpl_opts is not None:
160-
normalize_hookimpl_opts(hookimpl_opts)
158+
hookimpl_config = self._parse_hookimpl(plugin, name)
159+
if hookimpl_config is not None:
161160
method: _HookImplFunction[object] = getattr(plugin, name)
162-
hookimpl_config = HookimplConfiguration.from_opts(hookimpl_opts)
163161
hookimpl = HookImpl(plugin, plugin_name, method, hookimpl_config)
164-
name = hookimpl_opts.get("specname") or name
165-
hook: HookCaller | None = getattr(self.hook, name, None)
162+
hook_name = hookimpl_config.specname or name
163+
hook: HookCaller | None = getattr(self.hook, hook_name, None)
166164
if hook is None:
167-
hook = HookCaller(name, self._hookexec)
168-
setattr(self.hook, name, hook)
165+
hook = HookCaller(hook_name, self._hookexec)
166+
setattr(self.hook, hook_name, hook)
169167
elif hook.has_spec():
170168
self._verify_hook(hook, hookimpl)
171169
hook._maybe_apply_history(hookimpl)
172170
hook._add_hookimpl(hookimpl)
173171
return plugin_name
174172

173+
def _parse_hookimpl(
174+
self, plugin: _Plugin, name: str
175+
) -> HookimplConfiguration | None:
176+
"""Internal method to parse hook implementation configuration.
177+
178+
This method uses the new HookimplConfiguration type internally.
179+
Falls back to the legacy parse_hookimpl_opts method for compatibility.
180+
181+
:param plugin: The plugin object to inspect
182+
:param name: The attribute name to check for hook implementation
183+
:returns: HookimplConfiguration if found, None otherwise
184+
"""
185+
try:
186+
method: object = getattr(plugin, name)
187+
except Exception: # pragma: no cover
188+
return None
189+
190+
if not inspect.isroutine(method):
191+
return None
192+
193+
try:
194+
# Try to get hook implementation configuration directly
195+
impl_attr = getattr(method, self.project_name + "_impl", None)
196+
except Exception: # pragma: no cover
197+
impl_attr = None
198+
199+
if impl_attr is not None:
200+
# Check if it's already a HookimplConfiguration (new style)
201+
if isinstance(impl_attr, HookimplConfiguration):
202+
return impl_attr
203+
# Handle legacy dict-based configuration
204+
elif isinstance(impl_attr, dict):
205+
return HookimplConfiguration.from_opts(impl_attr) # type: ignore
206+
207+
# Fall back to legacy parse_hookimpl_opts for compatibility
208+
# (e.g. pytest override)
209+
legacy_opts = self.parse_hookimpl_opts(plugin, name)
210+
if legacy_opts is not None:
211+
normalize_hookimpl_opts(legacy_opts)
212+
return HookimplConfiguration.from_opts(legacy_opts)
213+
214+
return None
215+
175216
def parse_hookimpl_opts(self, plugin: _Plugin, name: str) -> HookimplOpts | None:
176217
"""Try to obtain a hook implementation from an item with the given name
177218
in the given plugin which is being searched for hook impls.
178219
179220
:returns:
180221
The parsed hookimpl options, or None to skip the given item.
181222
182-
This method can be overridden by ``PluginManager`` subclasses to
183-
customize how hook implementation are picked up. By default, returns the
184-
options for items decorated with :class:`HookimplMarker`.
223+
.. deprecated::
224+
Customizing hook implementation parsing by overriding this method is
225+
deprecated. This method is only kept as a compatibility shim for
226+
legacy projects like pytest. New code should use the standard
227+
:class:`HookimplMarker` decorators.
185228
"""
186-
method: object = getattr(plugin, name)
187-
if not inspect.isroutine(method):
188-
return None
189-
try:
190-
res: HookimplOpts | None = getattr(
191-
method, self.project_name + "_impl", None
192-
)
193-
except Exception: # pragma: no cover
194-
res = {} # type: ignore[assignment] #pragma: no cover
195-
if res is not None and not isinstance(res, dict):
196-
# false positive
197-
res = None # type:ignore[unreachable] #pragma: no cover
198-
return res
229+
# Compatibility shim - only overridden by legacy projects like pytest
230+
# Modern hook implementations are handled by _parse_hookimpl
231+
return None
199232

200233
def unregister(
201234
self, plugin: _Plugin | None = None, name: str | None = None

testing/test_hookcaller.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -337,7 +337,7 @@ def he_myhook1(arg1) -> None:
337337
pass
338338

339339
if val:
340-
assert he_myhook1.example_impl.get(name)
340+
assert getattr(he_myhook1.example_impl, name)
341341
else:
342342
assert not hasattr(he_myhook1, name)
343343

0 commit comments

Comments
 (0)