Skip to content

Commit d509871

Browse files
Replace internal hook implementation with HookspecConfiguration
- Add HookspecConfiguration class mirroring HookimplConfiguration pattern - Replace HookspecOpts TypedDict usage with structured class internally - Add backward compatibility for HookCaller.set_specification method - Support both positional/keyword HookspecOpts and new HookspecConfiguration - Add comprehensive tests for new configuration system - Deprecate parse_hookspec_opts method to return None by default 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
1 parent 8cbd593 commit d509871

File tree

4 files changed

+340
-35
lines changed

4 files changed

+340
-35
lines changed

src/pluggy/__init__.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
"HookCallError",
77
"HookspecOpts",
88
"HookimplOpts",
9+
"HookspecConfiguration",
910
"HookimplConfiguration",
1011
"HookImpl",
1112
"HookRelay",
@@ -22,6 +23,7 @@
2223
from ._hooks import HookimplMarker
2324
from ._hooks import HookimplOpts
2425
from ._hooks import HookRelay
26+
from ._hooks import HookspecConfiguration
2527
from ._hooks import HookspecMarker
2628
from ._hooks import HookspecOpts
2729
from ._manager import PluginManager

src/pluggy/_hooks.py

Lines changed: 113 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,62 @@ class HookimplOpts(TypedDict):
7575
specname: str | None
7676

7777

78+
@final
79+
class HookspecConfiguration:
80+
"""Configuration class for hook specifications.
81+
82+
This class is intended to replace HookspecOpts in future versions.
83+
It provides a more structured and extensible way to configure hook specifications.
84+
"""
85+
86+
__slots__ = (
87+
"firstresult",
88+
"historic",
89+
"warn_on_impl",
90+
"warn_on_impl_args",
91+
)
92+
93+
def __init__(
94+
self,
95+
firstresult: bool = False,
96+
historic: bool = False,
97+
warn_on_impl: Warning | None = None,
98+
warn_on_impl_args: Mapping[str, Warning] | None = None,
99+
) -> None:
100+
"""Initialize hook specification configuration.
101+
102+
:param firstresult:
103+
Whether the hook is :ref:`first result only <firstresult>`.
104+
:param historic:
105+
Whether the hook is :ref:`historic <historic>`.
106+
:param warn_on_impl:
107+
Whether the hook :ref:`warns when implemented <warn_on_impl>`.
108+
:param warn_on_impl_args:
109+
Whether the hook warns when :ref:`certain arguments are requested
110+
<warn_on_impl>`.
111+
"""
112+
if historic and firstresult:
113+
raise ValueError("cannot have a historic firstresult hook")
114+
#: Whether the hook is :ref:`first result only <firstresult>`.
115+
self.firstresult: Final = firstresult
116+
#: Whether the hook is :ref:`historic <historic>`.
117+
self.historic: Final = historic
118+
#: Whether the hook :ref:`warns when implemented <warn_on_impl>`.
119+
self.warn_on_impl: Final = warn_on_impl
120+
#: Whether the hook warns when :ref:`certain arguments are requested
121+
#: <warn_on_impl>`.
122+
self.warn_on_impl_args: Final = warn_on_impl_args
123+
124+
def __repr__(self) -> str:
125+
attrs = []
126+
for slot in self.__slots__:
127+
value = getattr(self, slot)
128+
if value:
129+
attrs.append(f"{slot}={value!r}")
130+
attrs_str = ", ".join(attrs)
131+
return f"HookspecConfiguration({attrs_str})"
132+
133+
78134
@final
79135
class HookimplConfiguration:
80136
"""Configuration class for hook implementations.
@@ -226,22 +282,31 @@ def __call__( # noqa: F811
226282
"""
227283

228284
def setattr_hookspec_opts(func: _F) -> _F:
229-
if historic and firstresult:
230-
raise ValueError("cannot have a historic firstresult hook")
231-
opts: HookspecOpts = {
232-
"firstresult": firstresult,
233-
"historic": historic,
234-
"warn_on_impl": warn_on_impl,
235-
"warn_on_impl_args": warn_on_impl_args,
236-
}
237-
setattr(func, self.project_name + "_spec", opts)
285+
config = HookspecConfiguration(
286+
firstresult=firstresult,
287+
historic=historic,
288+
warn_on_impl=warn_on_impl,
289+
warn_on_impl_args=warn_on_impl_args,
290+
)
291+
setattr(func, self.project_name + "_spec", config)
238292
return func
239293

240294
if function is not None:
241295
return setattr_hookspec_opts(function)
242296
else:
243297
return setattr_hookspec_opts
244298

299+
def _get_hookconfig(self, func: Callable[..., object]) -> HookspecConfiguration:
300+
"""Extract hook specification configuration from a decorated function.
301+
302+
:param func: A function decorated with this HookspecMarker
303+
:return: HookspecConfiguration object containing the hook specification options
304+
:raises AttributeError: If the function is not decorated with this marker
305+
"""
306+
attr_name = self.project_name + "_spec"
307+
config: HookspecConfiguration = getattr(func, attr_name)
308+
return config
309+
245310

246311
@final
247312
class HookimplMarker:
@@ -486,7 +551,7 @@ def __init__(
486551
name: str,
487552
hook_execute: _HookExec,
488553
specmodule_or_class: _Namespace | None = None,
489-
spec_opts: HookspecOpts | None = None,
554+
spec_config: HookspecConfiguration | None = None,
490555
) -> None:
491556
""":meta private:"""
492557
#: Name of the hook getting called.
@@ -504,8 +569,8 @@ def __init__(
504569
# TODO: Document, or make private.
505570
self.spec: HookSpec | None = None
506571
if specmodule_or_class is not None:
507-
assert spec_opts is not None
508-
self.set_specification(specmodule_or_class, spec_opts)
572+
assert spec_config is not None
573+
self.set_specification(specmodule_or_class, spec_config=spec_config)
509574

510575
# TODO: Document, or make private.
511576
def has_spec(self) -> bool:
@@ -515,15 +580,39 @@ def has_spec(self) -> bool:
515580
def set_specification(
516581
self,
517582
specmodule_or_class: _Namespace,
518-
spec_opts: HookspecOpts,
583+
_spec_opts_or_config: HookspecOpts | HookspecConfiguration | None = None,
584+
*,
585+
spec_opts: HookspecOpts | None = None,
586+
spec_config: HookspecConfiguration | None = None,
519587
) -> None:
520588
if self.spec is not None:
521589
raise ValueError(
522590
f"Hook {self.spec.name!r} is already registered "
523591
f"within namespace {self.spec.namespace}"
524592
)
525-
self.spec = HookSpec(specmodule_or_class, self.name, spec_opts)
526-
if spec_opts.get("historic"):
593+
594+
# Handle the dual parameter - set the appropriate typed parameter
595+
if _spec_opts_or_config is not None:
596+
assert spec_opts is None and spec_config is None, (
597+
"Cannot provide both positional and keyword spec arguments"
598+
)
599+
600+
if isinstance(_spec_opts_or_config, dict):
601+
spec_opts = _spec_opts_or_config
602+
else:
603+
spec_config = _spec_opts_or_config
604+
605+
# Require exactly one of the typed parameters to be set
606+
if spec_opts is not None:
607+
assert spec_config is None, "Cannot provide both spec_opts and spec_config"
608+
final_config = HookspecConfiguration(**spec_opts)
609+
elif spec_config is not None:
610+
final_config = spec_config
611+
else:
612+
raise TypeError("Must provide either spec_opts or spec_config")
613+
614+
self.spec = HookSpec(specmodule_or_class, self.name, final_config)
615+
if final_config.historic:
527616
self._call_history = []
528617

529618
def is_historic(self) -> bool:
@@ -600,7 +689,7 @@ def __call__(self, **kwargs: object) -> Any:
600689
"Cannot directly call a historic hook - use call_historic instead."
601690
)
602691
self._verify_all_args_are_provided(kwargs)
603-
firstresult = self.spec.opts.get("firstresult", False) if self.spec else False
692+
firstresult = self.spec.config.firstresult if self.spec else False
604693
# Copy because plugins may register other plugins during iteration (#438).
605694
return self._hookexec(self.name, self._hookimpls.copy(), kwargs, firstresult)
606695

@@ -655,7 +744,7 @@ def call_extra(
655744
):
656745
i -= 1
657746
hookimpls.insert(i + 1, hookimpl)
658-
firstresult = self.spec.opts.get("firstresult", False) if self.spec else False
747+
firstresult = self.spec.config.firstresult if self.spec else False
659748
return self._hookexec(self.name, hookimpls, kwargs, firstresult)
660749

661750
def _maybe_apply_history(self, method: HookImpl) -> None:
@@ -786,16 +875,18 @@ class HookSpec:
786875
"name",
787876
"argnames",
788877
"kwargnames",
789-
"opts",
878+
"config",
790879
"warn_on_impl",
791880
"warn_on_impl_args",
792881
)
793882

794-
def __init__(self, namespace: _Namespace, name: str, opts: HookspecOpts) -> None:
883+
def __init__(
884+
self, namespace: _Namespace, name: str, config: HookspecConfiguration
885+
) -> None:
795886
self.namespace = namespace
796887
self.function: Callable[..., object] = getattr(namespace, name)
797888
self.name = name
798889
self.argnames, self.kwargnames = varnames(self.function)
799-
self.opts = opts
800-
self.warn_on_impl = opts.get("warn_on_impl")
801-
self.warn_on_impl_args = opts.get("warn_on_impl_args")
890+
self.config = config
891+
self.warn_on_impl = config.warn_on_impl
892+
self.warn_on_impl_args = config.warn_on_impl_args

src/pluggy/_manager.py

Lines changed: 46 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
from ._hooks import HookimplConfiguration
2525
from ._hooks import HookimplOpts
2626
from ._hooks import HookRelay
27+
from ._hooks import HookspecConfiguration
2728
from ._hooks import HookspecOpts
2829
from ._result import Result
2930

@@ -213,6 +214,39 @@ def _parse_hookimpl(
213214

214215
return None
215216

217+
def _parse_hookspec(
218+
self, module_or_class: _Namespace, name: str
219+
) -> HookspecConfiguration | None:
220+
"""Internal method to parse hook specification configuration.
221+
222+
:param module_or_class: The module or class to inspect
223+
:param name: The attribute name to check for hook specification
224+
:returns: HookspecConfiguration if found, None otherwise
225+
"""
226+
try:
227+
method: object = getattr(module_or_class, name)
228+
except Exception: # pragma: no cover
229+
return None
230+
231+
if not inspect.isroutine(method):
232+
return None
233+
234+
try:
235+
# Get hook specification configuration directly
236+
spec_attr = getattr(method, self.project_name + "_spec", None)
237+
except Exception: # pragma: no cover
238+
spec_attr = None
239+
240+
if isinstance(spec_attr, HookspecConfiguration):
241+
return spec_attr
242+
243+
# Fall back to legacy parse_hookspec_opts for compatibility
244+
legacy_opts = self.parse_hookspec_opts(module_or_class, name)
245+
if legacy_opts is not None:
246+
return HookspecConfiguration(**legacy_opts)
247+
248+
return None
249+
216250
def parse_hookimpl_opts(self, plugin: _Plugin, name: str) -> HookimplOpts | None:
217251
"""Try to obtain a hook implementation from an item with the given name
218252
in the given plugin which is being searched for hook impls.
@@ -289,15 +323,15 @@ def add_hookspecs(self, module_or_class: _Namespace) -> None:
289323
"""
290324
names = []
291325
for name in dir(module_or_class):
292-
spec_opts = self.parse_hookspec_opts(module_or_class, name)
293-
if spec_opts is not None:
326+
spec_config = self._parse_hookspec(module_or_class, name)
327+
if spec_config is not None:
294328
hc: HookCaller | None = getattr(self.hook, name, None)
295329
if hc is None:
296-
hc = HookCaller(name, self._hookexec, module_or_class, spec_opts)
330+
hc = HookCaller(name, self._hookexec, module_or_class, spec_config)
297331
setattr(self.hook, name, hc)
298332
else:
299333
# Plugins registered this hook without knowing the spec.
300-
hc.set_specification(module_or_class, spec_opts)
334+
hc.set_specification(module_or_class, spec_config)
301335
for hookfunction in hc.get_hookimpls():
302336
self._verify_hook(hc, hookfunction)
303337
names.append(name)
@@ -317,13 +351,15 @@ def parse_hookspec_opts(
317351
The parsed hookspec options for defining a hook, or None to skip the
318352
given item.
319353
320-
This method can be overridden by ``PluginManager`` subclasses to
321-
customize how hook specifications are picked up. By default, returns the
322-
options for items decorated with :class:`HookspecMarker`.
354+
.. deprecated::
355+
Customizing hook specification parsing by overriding this method is
356+
deprecated. This method is only kept as a compatibility shim for
357+
legacy projects. New code should use the standard
358+
:class:`HookspecMarker` decorators.
323359
"""
324-
method = getattr(module_or_class, name)
325-
opts: HookspecOpts | None = getattr(method, self.project_name + "_spec", None)
326-
return opts
360+
# Compatibility shim - only overridden by legacy projects
361+
# Modern hook specifications are handled by _parse_hookspec
362+
return None
327363

328364
def get_plugins(self) -> set[Any]:
329365
"""Return a set of all registered plugin objects."""

0 commit comments

Comments
 (0)