From 98610db30fa75da9704a4990e672ae78e0563668 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 22 Jun 2026 20:21:33 +0000 Subject: [PATCH] di: fail loud when unresolvable type hints would disable injection DI reads Use/From markers out of resolved type hints, and get_type_hints resolves the whole signature atomically: one unresolvable name anywhere - typically a return annotation whose inner type is imported only under 'if TYPE_CHECKING:' (e.g. TaskResult[MyResult]) - drops every hint, markers included. The resolver then fell back to {} and DI injected nothing, with no error: a silent footgun that forced a copy-pasted warning comment into every session file. When the substitute-Any fallback still can't resolve and the signature has a parameter carrying a Use(...)/From(...) marker, raise the new TypeHintResolutionError pointing at the culprit. With no DI marker at stake (e.g. only an unresolvable return annotation on a plain function), keep degrading to {} so nothing previously-working breaks. Closes #112 --- protest/di/hints.py | 49 ++++++++++++++- protest/exceptions.py | 28 +++++++++ tests/di/test_hints.py | 135 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 210 insertions(+), 2 deletions(-) create mode 100644 tests/di/test_hints.py diff --git a/protest/di/hints.py b/protest/di/hints.py index c61e301..9d2fe0a 100644 --- a/protest/di/hints.py +++ b/protest/di/hints.py @@ -62,7 +62,10 @@ def make() -> HeavyType: ... import contextlib import inspect import re -from typing import Any, get_type_hints +from typing import Annotated, Any, get_args, get_origin, get_type_hints + +from protest.di.markers import From, Use +from protest.exceptions import TypeHintResolutionError def get_type_hints_compat(func: Any) -> dict[str, Any]: @@ -103,16 +106,58 @@ def _get_type_hints_substituting_any( ``Any`` is only used as a placeholder so resolution can complete; the DI system reads the ``Use(...)``/``From(...)`` marker out of the ``Annotated[...]``, not the underlying type. + + If resolution still can't complete and the signature has a parameter + carrying a ``Use(...)``/``From(...)`` marker, raise + ``TypeHintResolutionError``: dropping the hints there would silently + disable that injection, so we fail loud and point at the culprit. With + no DI marker at stake (e.g. only an unresolvable return annotation), + keep degrading to ``{}`` - nothing is silently lost. """ localns = dict(localns) + last_error: Exception | None = None for _ in range(20): try: return get_type_hints(func, localns=localns, include_extras=True) except NameError as exc: + last_error = exc match = re.search(r"name '(\w+)' is not defined", str(exc)) if not match: break localns[match.group(1)] = Any - except Exception: + except Exception as exc: + last_error = exc break + if last_error is not None and _has_di_marker_param(func): + raise TypeHintResolutionError( + getattr(func, "__name__", repr(func)), + getattr(func, "__annotations__", {}), + last_error, + ) return {} + + +def _has_di_marker_param(func: Any) -> bool: + """True if any non-return parameter carries a Use/From marker. + + Handles both stringified annotations (PEP 563 - substring match on the + raw string) and live ``Annotated[...]`` objects (inspect the metadata). + Used to decide whether an unresolvable signature would silently disable + injection. + """ + try: + params = inspect.signature(func).parameters + except (TypeError, ValueError): + return False + for param in params.values(): + ann = param.annotation + if ann is inspect.Parameter.empty: + continue + if isinstance(ann, str): + if "Use(" in ann or "From(" in ann: + return True + elif get_origin(ann) is Annotated and any( + isinstance(meta, (Use, From)) for meta in get_args(ann)[1:] + ): + return True + return False diff --git a/protest/exceptions.py b/protest/exceptions.py index e4df84f..70923f0 100644 --- a/protest/exceptions.py +++ b/protest/exceptions.py @@ -44,6 +44,34 @@ def __init__(self, fixture_name: str): super().__init__(f"Fixture '{fixture_name}' is not registered.") +class TypeHintResolutionError(ProTestError): + """Raised when a function's type hints can't be resolved at runtime and + that failure would silently disable dependency injection. + + DI reads the ``Use(...)``/``From(...)`` markers out of resolved type + hints. ``get_type_hints`` resolves the whole signature atomically, so a + single unresolvable name anywhere - typically a return annotation whose + inner type is imported only under ``if TYPE_CHECKING:`` - drops every + hint, markers included, and injection would silently inject nothing. + Raising points at the culprit instead of degrading silently. + """ + + def __init__( + self, func_name: str, annotations: dict[str, object], original: Exception + ): + self.func_name = func_name + self.original = original + rendered = ", ".join(f"{k}: {v!r}" for k, v in annotations.items()) + super().__init__( + f"Could not resolve type hints for '{func_name}' ({original}). " + f"A type referenced in its signature is not importable at runtime " + f"- is it imported only under `if TYPE_CHECKING:`? It must be " + f"importable at runtime (module level, not TYPE_CHECKING) for " + f"dependency injection to resolve the Use(...)/From(...) markers; " + f"otherwise injection silently does nothing. Annotations: {rendered}" + ) + + class ParameterizedFixtureError(ProTestError): def __init__(self, fixture_name: str, param_names: list[str]): params = ", ".join(param_names) diff --git a/tests/di/test_hints.py b/tests/di/test_hints.py new file mode 100644 index 0000000..b977ac1 --- /dev/null +++ b/tests/di/test_hints.py @@ -0,0 +1,135 @@ +"""Tests for the type-hint resolver (`get_type_hints_compat`), issue #112. + +DI reads Use/From markers out of resolved type hints. get_type_hints resolves +the whole signature atomically, so a single unresolvable name (typically a +return annotation imported only under `if TYPE_CHECKING:`) drops every hint, +markers included. That used to silently disable injection; now it raises +TypeHintResolutionError when a DI marker is at stake, and keeps degrading to +{} when nothing is silently lost. +""" + +from __future__ import annotations + +import importlib.util +import sys +import textwrap +from typing import TYPE_CHECKING + +import pytest + +from protest.di.hints import get_type_hints_compat +from protest.exceptions import TypeHintResolutionError + +if TYPE_CHECKING: + from pathlib import Path + + +def _load(tmp_path: Path, name: str, source: str) -> object: + """Write a module to tmp_path, import it, return the named eval function.""" + inner = tmp_path / "inner112.py" + if not inner.exists(): + inner.write_text( + "from dataclasses import dataclass\n" + "@dataclass\n" + "class MyResult:\n" + " value: str = ''\n" + "def my_fixture() -> str:\n" + " return 'injected'\n" + ) + mod_path = tmp_path / f"{name}.py" + mod_path.write_text(textwrap.dedent(source)) + + sys.path.insert(0, str(tmp_path)) + try: + spec = importlib.util.spec_from_file_location(name, mod_path) + assert spec and spec.loader + module = importlib.util.module_from_spec(spec) + sys.modules[name] = module + spec.loader.exec_module(module) + return module.my_eval + finally: + sys.path.remove(str(tmp_path)) + + +class TestSilentInjectionFailureNowLoud: + def test_unresolvable_return_with_di_marker_raises(self, tmp_path: Path) -> None: + """A TYPE_CHECKING-only return type that breaks resolution, on a + signature with a Use() param, raises instead of dropping the dep.""" + func = _load( + tmp_path, + "sess_raise", + """ + from __future__ import annotations + from typing import TYPE_CHECKING, Annotated + from protest import Use + from inner112 import my_fixture + + if TYPE_CHECKING: + import inner112 + + def my_eval( + dep: Annotated[str, Use(my_fixture)], + ) -> "inner112.MyResult": + return None + """, + ) + with pytest.raises(TypeHintResolutionError) as excinfo: + get_type_hints_compat(func) + msg = str(excinfo.value) + assert "my_eval" in msg + assert "TYPE_CHECKING" in msg + + def test_typecheck_only_generic_inner_still_resolves(self, tmp_path: Path) -> None: + """TaskResult[MyResult] with MyResult under TYPE_CHECKING resolves via + the substitute-Any fallback - the Use() dep is still injected.""" + func = _load( + tmp_path, + "sess_ok", + """ + from __future__ import annotations + from typing import TYPE_CHECKING, Annotated + from protest import Use + from protest.evals.types import TaskResult + from inner112 import my_fixture + + if TYPE_CHECKING: + from inner112 import MyResult + + def my_eval( + dep: Annotated[str, Use(my_fixture)], + ) -> TaskResult[MyResult]: + return TaskResult(output="x") + """, + ) + hints = get_type_hints_compat(func) + assert "dep" in hints + + def test_unresolvable_return_without_di_marker_degrades( + self, tmp_path: Path + ) -> None: + """No DI marker at stake -> keep degrading to {}, don't raise: a plain + function with a TYPE_CHECKING-only return annotation stays usable.""" + func = _load( + tmp_path, + "sess_plain", + """ + from __future__ import annotations + from typing import TYPE_CHECKING + + if TYPE_CHECKING: + import inner112 + + def my_eval() -> "inner112.MyResult": + return None + """, + ) + assert get_type_hints_compat(func) == {} + + +class TestResolvableSignaturesUnaffected: + def test_plain_resolvable_signature(self) -> None: + def my_eval(x: int, y: str) -> bool: + return True + + hints = get_type_hints_compat(my_eval) + assert hints == {"x": int, "y": str, "return": bool}