fix: Any must not collide with concrete types under beartype>=0.23 - #296
fix: Any must not collide with concrete types under beartype>=0.23#296nstarman wants to merge 2 commits into
Conversation
Since beartype 0.23 (beartype#616, closing beartype#530), `is_subhint(Any, T)` is `True` for every `T` -- a deliberate change matching mypy's two-way assignability semantics for `Any`. Combined with `TypeHint.__eq__`'s antisymmetric syllogism (`is_subhint(a, b) and is_subhint(b, a) => a == b`), this makes `beartype.door.TypeHint(Any) == TypeHint(T)` also `True` for every `T`. Unannotated parameters resolve to `Any` (`_signature.py`'s `_extract_signature`), so `Signature.__eq__`/`Resolver.register()`'s redefinition-detection now treats any unannotated method as "the same signature" as the next concretely-typed method registered on the same dispatcher -- silently overwriting one with the other, regardless of registration order. This is severe enough to destroy plum's own built-in identity-conversion fallback in `_promotion.py` the moment `plum` finishes importing, breaking `plum.convert()` for any type without a specific registered converter. The same root cause also corrupts ordinary dispatch specificity: `Signature.__le__`'s raw subhint comparisons now let `Any` compare as subhint-and-supersubhint of concrete types too, so a generic `f(x)` fallback and a specific `f(x: int)` overload become mutually "comparable but neither more specific", and whichever was registered *last* wins arbitrarily instead of the genuinely more specific one. Add `_type_hints_equal`/`_type_hint_le` in `_type.py`: thin wrappers around beartype's `TypeHint` equality/subhint checks that special-case `Any` so it never collapses onto (or below) an unrelated concrete type, while leaving beartype's semantics untouched everywhere else (including plum's own public `issubclass`/`isinstance`, and ordinary type-hint equivalence like `Union[int, bool] == int`). Use these in `Signature.__eq__`/`__le__` (both the types and varargs comparisons) and in `add_promotion_rule`'s reverse-rule-skip check -- the three places `_signature.py`/`_promotion.py` rely on `TypeHint` equality/ subhint checks for bookkeeping (redefinition detection, specificity ordering) rather than genuine assignability. Fixes beartype#295.
Coverage Report for CI Build 31473258192Coverage increased (+0.005%) to 99.499%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
Per ponytail-review on beartype#296: _type_hints_equal duplicated _type_hint_le's is-Any branch and TypeHintWrapper call instead of being built from it via the same antisymmetry syllogism beartype's own TypeHint equality uses. Also trims both source and test docstrings, which restated the same beartype#530/#616 backstory in full three more times. No behavior change (full suite green under beartype 0.22.9 and 0.23.0rc0, mypy clean).
There was a problem hiding this comment.
Pull request overview
This PR fixes plum’s signature bookkeeping semantics under beartype>=0.23, where Any became two-way assignable and thus could incorrectly compare equal (and as-specific) to concrete types, breaking method registration and specificity ordering.
Changes:
- Added
_type_hint_le/_type_hints_equalhelpers to keepAnydistinct for signature bookkeeping while preserving beartype’s assignability semantics elsewhere. - Updated
Signature.__eq__andSignature.__le__to use the new helpers for redefinition detection and specificity ordering. - Added regression tests covering registration-order independence and the new helper semantics.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/test_type.py | Adds tests pinning Any-specific equality/subhint behavior for bookkeeping helpers. |
| tests/test_resolver.py | Adds regression test ensuring Signature(Any) never collides with Signature(int) and dispatch remains order-independent. |
| src/plum/_type.py | Introduces _type_hint_le and _type_hints_equal wrappers around beartype.door.TypeHint comparisons with an Any special-case. |
| src/plum/_signature.py | Switches signature equality and specificity ordering to use the new helpers to prevent Any collisions. |
| src/plum/_promotion.py | Uses _type_hints_equal for reverse-rule skip logic to avoid Any being mistaken for a concrete type. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| def __hash__(self) -> int: | ||
| return hash((Signature, *self.types, self.varargs)) |
There was a problem hiding this comment.
Good catch, but this predates #296 and isn't something this PR introduces or worsens — it's reproducible on unmodified master with stable beartype==0.22.9, nothing to do with the Any fix here:
from typing import Union
from plum import Signature as Sig
s1, s2 = Sig(Union[int, bool]), Sig(int)
s1 == s2 # True (test_signature.py::test_equality already asserts this)
hash(s1) == hash(s2) # False -- hash/eq contract violatedIt's inherited straight from beartype.door.TypeHint itself: TypeHint.__hash__ is hash(self._hint), not consistent with its own __eq__ (TypeHint(Union[int, bool]) == TypeHint(int) is True, but the two hash differently). Routing Signature.__hash__ through TypeHintWrapper(...) wouldn't fix anything since it reduces to the identical call. A real fix needs a canonical form for semantically-equivalent-but-structurally-different hints, which is a separate, bigger problem than this PR's scope (and arguably belongs upstream in beartype.door.TypeHint, or requires plum to maintain its own hint canonicalization).
For what it's worth, this PR doesn't make it worse — for the specific Any case, it actually removes one instance of the exact same violation: under beartype>=0.23 before this fix, Signature(Any) == Signature(int) was True while their hashes still differed (an even broader case of this bug, since Any collided with every type).
Filed as its own tracking issue so it doesn't get lost: #297.
Fixes #295.
Problem
Since
beartype0.23.0rc0,is_subhint(Any, T)isTruefor everyT— a deliberate change (beartype#616, closing beartype#530) matching mypy's two-way assignability semantics forAny.TypeHint.__eq__is built from the antisymmetric syllogismis_subhint(a, b) and is_subhint(b, a) ⇒ a == b, so this also makesbeartype.door.TypeHint(Any) == TypeHint(T)Truefor everyT.plumrelies onTypeHintequality/subhint checks for two different things that need to stay separate:Signature.match,resolve()'s value-level checks viais_bearable) — assignability semantics are correct here, and are untouched by this PR.Resolver.register()'s redefinition detection, viaSignature.__eq__) and "is this signature strictly more specific than that one" (Signature.__le__'s specificity ordering) — these needAnyto behave as a distinct, least-specific type, not something that collapses onto (or below) arbitrary concrete types.Since unannotated parameters resolve to
Any(_extract_signature), the bug surfaces two ways:Registration silently overwrites the wrong method.
Resolver.register()treats a newly-registered concretely-typed method as "redefining" any earlier unannotated method (or vice versa), regardless of registration order, and clobbers it in place — with noMethodRedefinitionWarning(warn_redefinitiondefaults toFalse). This is severe enough to destroy plum's own built-in identity-conversion fallback in_promotion.pythe momentplumfinishes importing, breakingplum.convert()globally for any type without a specific registered converter:Dispatch specificity becomes registration-order-dependent. A generic
f(x)fallback and a specificf(x: int)overload become mutually "comparable but neither strictly more specific" (Sig(int) <= Sig(Any)andSig(Any) <= Sig(int)bothTrue), so whichever was registered last wins arbitrarily:(Registering
g(x)beforeg(x: int)instead gives the "correct" answer only by accident, because the earlier bug already destroyed one of the two methods on registration.)Both were originally found via quax, where
plum.convert(jnp.asarray(1.0), jax.Array)started failing the same way in its "pre-release deps" CI job. Neither reproduction involvesjax/quaxat all — see #295 for the full trace.Fix
Add two small helpers in
_type.py:_type_hints_equal(x, y)— likeTypeHint(x) == TypeHint(y), exceptAnyis considered equal only toAnyitself._type_hint_le(x, y)— likeTypeHint(x) <= TypeHint(y), exceptAnyis only a subhint ofAnyitself (soX <= Anystill holds for allX, preservingAnyas the top/least-specific type, butAny <= Tno longer holds for concreteT).Both are thin wrappers with no behavior change for anything not involving
Any, sobeartype's (correct, upstream-endorsed) assignability semantics forAnyare left completely untouched everywhere else — includingSignature.match/is_bearable, plum's own publicplum.issubclass/plum.isinstance, and ordinary type-hint-equivalence cases likeUnion[int, bool] == int(still passes, seetest_equalityintest_signature.py).Use these in the three spots that need bookkeeping-equality rather than assignability:
Signature.__eq__(types + varargs)Signature.__le__'s two branches (types + varargs, in both the equality fast-path and the subhint fallback)add_promotion_rule's reverse-rule-skip check in_promotion.pyTesting
test_type_hints_equal/test_type_hint_leintests/test_type.py, andtest_register_any_never_collides_with_concrete_type(parametrized over registration order) intests/test_resolver.py, all directly pinning the scenarios above.beartype==0.23.0rc0before this fix: 29 tests fail (plum._resolver.NotFoundLookupErrorand similar) that all pass onbeartype<0.23. After this fix, underbeartype==0.23.0rc0: 200 passed, only the one pre-existing, unrelatedtest_methodlist_reprfailure remains (a path-length rendering artifact in my local checkout path, reproduces identically on unmodifiedmasterregardless ofbeartypeversion).beartype<0.23too — no regression for the currently-releasedbeartype.ruff check/ruff format --checkclean;mypyerror count unchanged (23, matching unmodifiedmaster) after wrapping the two new comparisons inbool(...)to satisfyno-any-return.Happy to adjust naming/placement or split into a smaller PR (e.g. just the
__eq__/registration fix) if you'd prefer to review the specificity-ordering fix separately — I bundled them because they share the exact same root cause and helper functions.