Skip to content

Commit a097fb3

Browse files
Introduce immutable hook option classes for type safety
Hook implementation and specification options are now represented by immutable classes (HookimplOptions, HookspecOptions) internally. This provides stronger type guarantees and prevents accidental mutation of option data after hook registration. For backward compatibility with pytest's custom parsing methods, the public parse_hookimpl_opts and parse_hookspec_opts methods continue to return TypedDict representations, while new internal helpers handle conversion to the immutable classes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent dbe2131 commit a097fb3

File tree

3 files changed

+266
-78
lines changed

3 files changed

+266
-78
lines changed

src/pluggy/_hooks.py

Lines changed: 209 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,185 @@ class HookimplOpts(TypedDict):
7474
specname: str | None
7575

7676

77+
@final
78+
class HookspecOptions:
79+
"""Immutable hook specification options (internal).
80+
81+
This is the internal representation used by pluggy. The TypedDict
82+
:class:`HookspecOpts` is kept for backward compatibility with pytest.
83+
"""
84+
85+
__slots__ = ("firstresult", "historic", "warn_on_impl", "warn_on_impl_args")
86+
87+
#: Whether to stop at the first non-None result.
88+
firstresult: Final[bool] # type: ignore[misc]
89+
#: Whether this hook is :ref:`historic <historic>`.
90+
historic: Final[bool] # type: ignore[misc]
91+
#: A warning to emit when a hook implementation is registered.
92+
warn_on_impl: Final[Warning | None] # type: ignore[misc]
93+
#: Warnings to emit for specific arguments when a hook implementation is registered.
94+
warn_on_impl_args: Final[Mapping[str, Warning] | None] # type: ignore[misc]
95+
96+
def __init__(
97+
self,
98+
firstresult: bool = False,
99+
historic: bool = False,
100+
warn_on_impl: Warning | None = None,
101+
warn_on_impl_args: Mapping[str, Warning] | None = None,
102+
) -> None:
103+
object.__setattr__(self, "firstresult", firstresult)
104+
object.__setattr__(self, "historic", historic)
105+
object.__setattr__(self, "warn_on_impl", warn_on_impl)
106+
object.__setattr__(self, "warn_on_impl_args", warn_on_impl_args)
107+
108+
def __setattr__(self, name: str, value: object) -> None:
109+
raise AttributeError("HookspecOptions is immutable")
110+
111+
def __delattr__(self, name: str) -> None:
112+
raise AttributeError("HookspecOptions is immutable")
113+
114+
def __repr__(self) -> str:
115+
return (
116+
f"HookspecOptions(firstresult={self.firstresult!r}, "
117+
f"historic={self.historic!r}, "
118+
f"warn_on_impl={self.warn_on_impl!r}, "
119+
f"warn_on_impl_args={self.warn_on_impl_args!r})"
120+
)
121+
122+
def __eq__(self, other: object) -> bool:
123+
if not isinstance(other, HookspecOptions):
124+
return NotImplemented
125+
return (
126+
self.firstresult == other.firstresult
127+
and self.historic == other.historic
128+
and self.warn_on_impl == other.warn_on_impl
129+
and self.warn_on_impl_args == other.warn_on_impl_args
130+
)
131+
132+
def __hash__(self) -> int:
133+
return hash(
134+
(
135+
self.firstresult,
136+
self.historic,
137+
self.warn_on_impl,
138+
# warn_on_impl_args is a Mapping, convert to hashable
139+
tuple(self.warn_on_impl_args.items())
140+
if self.warn_on_impl_args
141+
else None,
142+
)
143+
)
144+
145+
@classmethod
146+
def from_opts(cls, opts: HookspecOpts) -> HookspecOptions:
147+
"""Create from a HookspecOpts TypedDict (for backward compatibility)."""
148+
return cls(
149+
firstresult=opts.get("firstresult", False),
150+
historic=opts.get("historic", False),
151+
warn_on_impl=opts.get("warn_on_impl"),
152+
warn_on_impl_args=opts.get("warn_on_impl_args"),
153+
)
154+
155+
156+
@final
157+
class HookimplOptions:
158+
"""Immutable hook implementation options (internal).
159+
160+
This is the internal representation used by pluggy. The TypedDict
161+
:class:`HookimplOpts` is kept for backward compatibility with pytest.
162+
"""
163+
164+
__slots__ = (
165+
"wrapper",
166+
"hookwrapper",
167+
"optionalhook",
168+
"tryfirst",
169+
"trylast",
170+
"specname",
171+
)
172+
173+
#: Whether this is a :ref:`wrapper <hookwrappers>`.
174+
wrapper: Final[bool] # type: ignore[misc]
175+
#: Whether this is an :ref:`old-style wrapper <old_style_hookwrappers>`.
176+
hookwrapper: Final[bool] # type: ignore[misc]
177+
#: Whether validation against a hook specification is :ref:`optional
178+
#: <optionalhook>`.
179+
optionalhook: Final[bool] # type: ignore[misc]
180+
#: Whether to try to order this hook implementation :ref:`first <callorder>`.
181+
tryfirst: Final[bool] # type: ignore[misc]
182+
#: Whether to try to order this hook implementation :ref:`last <callorder>`.
183+
trylast: Final[bool] # type: ignore[misc]
184+
#: The name of the hook specification to match, see :ref:`specname`.
185+
specname: Final[str | None] # type: ignore[misc]
186+
187+
def __init__(
188+
self,
189+
wrapper: bool = False,
190+
hookwrapper: bool = False,
191+
optionalhook: bool = False,
192+
tryfirst: bool = False,
193+
trylast: bool = False,
194+
specname: str | None = None,
195+
) -> None:
196+
object.__setattr__(self, "wrapper", wrapper)
197+
object.__setattr__(self, "hookwrapper", hookwrapper)
198+
object.__setattr__(self, "optionalhook", optionalhook)
199+
object.__setattr__(self, "tryfirst", tryfirst)
200+
object.__setattr__(self, "trylast", trylast)
201+
object.__setattr__(self, "specname", specname)
202+
203+
def __setattr__(self, name: str, value: object) -> None:
204+
raise AttributeError("HookimplOptions is immutable")
205+
206+
def __delattr__(self, name: str) -> None:
207+
raise AttributeError("HookimplOptions is immutable")
208+
209+
def __repr__(self) -> str:
210+
return (
211+
f"HookimplOptions(wrapper={self.wrapper!r}, "
212+
f"hookwrapper={self.hookwrapper!r}, "
213+
f"optionalhook={self.optionalhook!r}, "
214+
f"tryfirst={self.tryfirst!r}, "
215+
f"trylast={self.trylast!r}, "
216+
f"specname={self.specname!r})"
217+
)
218+
219+
def __eq__(self, other: object) -> bool:
220+
if not isinstance(other, HookimplOptions):
221+
return NotImplemented
222+
return (
223+
self.wrapper == other.wrapper
224+
and self.hookwrapper == other.hookwrapper
225+
and self.optionalhook == other.optionalhook
226+
and self.tryfirst == other.tryfirst
227+
and self.trylast == other.trylast
228+
and self.specname == other.specname
229+
)
230+
231+
def __hash__(self) -> int:
232+
return hash(
233+
(
234+
self.wrapper,
235+
self.hookwrapper,
236+
self.optionalhook,
237+
self.tryfirst,
238+
self.trylast,
239+
self.specname,
240+
)
241+
)
242+
243+
@classmethod
244+
def from_opts(cls, opts: HookimplOpts) -> HookimplOptions:
245+
"""Create from a HookimplOpts TypedDict (for backward compatibility)."""
246+
return cls(
247+
wrapper=opts.get("wrapper", False),
248+
hookwrapper=opts.get("hookwrapper", False),
249+
optionalhook=opts.get("optionalhook", False),
250+
tryfirst=opts.get("tryfirst", False),
251+
trylast=opts.get("trylast", False),
252+
specname=opts.get("specname"),
253+
)
254+
255+
77256
@final
78257
class HookspecMarker:
79258
"""Decorator for marking functions as hook specifications.
@@ -146,12 +325,12 @@ def __call__( # noqa: F811
146325
def setattr_hookspec_opts(func: _F) -> _F:
147326
if historic and firstresult:
148327
raise ValueError("cannot have a historic firstresult hook")
149-
opts: HookspecOpts = {
150-
"firstresult": firstresult,
151-
"historic": historic,
152-
"warn_on_impl": warn_on_impl,
153-
"warn_on_impl_args": warn_on_impl_args,
154-
}
328+
opts = HookspecOptions(
329+
firstresult=firstresult,
330+
historic=historic,
331+
warn_on_impl=warn_on_impl,
332+
warn_on_impl_args=warn_on_impl_args,
333+
)
155334
setattr(func, self.project_name + "_spec", opts)
156335
return func
157336

@@ -261,14 +440,14 @@ def __call__( # noqa: F811
261440
"""
262441

263442
def setattr_hookimpl_opts(func: _F) -> _F:
264-
opts: HookimplOpts = {
265-
"wrapper": wrapper,
266-
"hookwrapper": hookwrapper,
267-
"optionalhook": optionalhook,
268-
"tryfirst": tryfirst,
269-
"trylast": trylast,
270-
"specname": specname,
271-
}
443+
opts = HookimplOptions(
444+
wrapper=wrapper,
445+
hookwrapper=hookwrapper,
446+
optionalhook=optionalhook,
447+
tryfirst=tryfirst,
448+
trylast=trylast,
449+
specname=specname,
450+
)
272451
setattr(func, self.project_name + "_impl", opts)
273452
return func
274453

@@ -278,15 +457,6 @@ def setattr_hookimpl_opts(func: _F) -> _F:
278457
return setattr_hookimpl_opts(function)
279458

280459

281-
def normalize_hookimpl_opts(opts: HookimplOpts) -> None:
282-
opts.setdefault("tryfirst", False)
283-
opts.setdefault("trylast", False)
284-
opts.setdefault("wrapper", False)
285-
opts.setdefault("hookwrapper", False)
286-
opts.setdefault("optionalhook", False)
287-
opts.setdefault("specname", None)
288-
289-
290460
_PYPY = hasattr(sys, "pypy_version_info")
291461

292462

@@ -395,7 +565,7 @@ def __init__(
395565
name: str,
396566
hook_execute: _HookExec,
397567
specmodule_or_class: _Namespace | None = None,
398-
spec_opts: HookspecOpts | None = None,
568+
spec_opts: HookspecOptions | None = None,
399569
) -> None:
400570
""":meta private:"""
401571
#: Name of the hook getting called.
@@ -424,15 +594,15 @@ def has_spec(self) -> bool:
424594
def set_specification(
425595
self,
426596
specmodule_or_class: _Namespace,
427-
spec_opts: HookspecOpts,
597+
spec_opts: HookspecOptions,
428598
) -> None:
429599
if self.spec is not None:
430600
raise ValueError(
431601
f"Hook {self.spec.name!r} is already registered "
432602
f"within namespace {self.spec.namespace}"
433603
)
434604
self.spec = HookSpec(specmodule_or_class, self.name, spec_opts)
435-
if spec_opts.get("historic"):
605+
if spec_opts.historic:
436606
self._call_history = []
437607

438608
def is_historic(self) -> bool:
@@ -509,7 +679,7 @@ def __call__(self, **kwargs: object) -> Any:
509679
"Cannot directly call a historic hook - use call_historic instead."
510680
)
511681
self._verify_all_args_are_provided(kwargs)
512-
firstresult = self.spec.opts.get("firstresult", False) if self.spec else False
682+
firstresult = self.spec.opts.firstresult if self.spec else False
513683
# Copy because plugins may register other plugins during iteration (#438).
514684
return self._hookexec(self.name, self._hookimpls.copy(), kwargs, firstresult)
515685

@@ -550,14 +720,7 @@ def call_extra(
550720
"Cannot directly call a historic hook - use call_historic instead."
551721
)
552722
self._verify_all_args_are_provided(kwargs)
553-
opts: HookimplOpts = {
554-
"wrapper": False,
555-
"hookwrapper": False,
556-
"optionalhook": False,
557-
"trylast": False,
558-
"tryfirst": False,
559-
"specname": None,
560-
}
723+
opts = HookimplOptions()
561724
hookimpls = self._hookimpls.copy()
562725
for method in methods:
563726
hookimpl = HookImpl(None, "<temp>", method, opts)
@@ -571,7 +734,7 @@ def call_extra(
571734
):
572735
i -= 1
573736
hookimpls.insert(i + 1, hookimpl)
574-
firstresult = self.spec.opts.get("firstresult", False) if self.spec else False
737+
firstresult = self.spec.opts.firstresult if self.spec else False
575738
return self._hookexec(self.name, hookimpls, kwargs, firstresult)
576739

577740
def _maybe_apply_history(self, method: HookImpl) -> None:
@@ -658,7 +821,7 @@ def __init__(
658821
plugin: _Plugin,
659822
plugin_name: str,
660823
function: _HookImplFunction[object],
661-
hook_impl_opts: HookimplOpts,
824+
hook_impl_opts: HookimplOptions,
662825
) -> None:
663826
""":meta private:"""
664827
#: The hook implementation function.
@@ -670,24 +833,24 @@ def __init__(
670833
self.kwargnames: Final = kwargnames
671834
#: The plugin which defined this hook implementation.
672835
self.plugin: Final = plugin
673-
#: The :class:`HookimplOpts` used to configure this hook implementation.
836+
#: The :class:`HookimplOptions` used to configure this hook implementation.
674837
self.opts: Final = hook_impl_opts
675838
#: The name of the plugin which defined this hook implementation.
676839
self.plugin_name: Final = plugin_name
677840
#: Whether the hook implementation is a :ref:`wrapper <hookwrapper>`.
678-
self.wrapper: Final = hook_impl_opts["wrapper"]
841+
self.wrapper: Final = hook_impl_opts.wrapper
679842
#: Whether the hook implementation is an :ref:`old-style wrapper
680843
#: <old_style_hookwrappers>`.
681-
self.hookwrapper: Final = hook_impl_opts["hookwrapper"]
844+
self.hookwrapper: Final = hook_impl_opts.hookwrapper
682845
#: Whether validation against a hook specification is :ref:`optional
683846
#: <optionalhook>`.
684-
self.optionalhook: Final = hook_impl_opts["optionalhook"]
847+
self.optionalhook: Final = hook_impl_opts.optionalhook
685848
#: Whether to try to order this hook implementation :ref:`first
686849
#: <callorder>`.
687-
self.tryfirst: Final = hook_impl_opts["tryfirst"]
850+
self.tryfirst: Final = hook_impl_opts.tryfirst
688851
#: Whether to try to order this hook implementation :ref:`last
689852
#: <callorder>`.
690-
self.trylast: Final = hook_impl_opts["trylast"]
853+
self.trylast: Final = hook_impl_opts.trylast
691854

692855
def __repr__(self) -> str:
693856
return f"<HookImpl plugin_name={self.plugin_name!r}, plugin={self.plugin!r}>"
@@ -706,11 +869,11 @@ class HookSpec:
706869
"warn_on_impl_args",
707870
)
708871

709-
def __init__(self, namespace: _Namespace, name: str, opts: HookspecOpts) -> None:
872+
def __init__(self, namespace: _Namespace, name: str, opts: HookspecOptions) -> None:
710873
self.namespace = namespace
711874
self.function: Callable[..., object] = getattr(namespace, name)
712875
self.name = name
713876
self.argnames, self.kwargnames = varnames(self.function)
714877
self.opts = opts
715-
self.warn_on_impl = opts.get("warn_on_impl")
716-
self.warn_on_impl_args = opts.get("warn_on_impl_args")
878+
self.warn_on_impl = opts.warn_on_impl
879+
self.warn_on_impl_args = opts.warn_on_impl_args

0 commit comments

Comments
 (0)