Skip to content

Commit 37e6c1a

Browse files
authored
fix: handle VAR_POSITIONAL and kwargs precedence in _fast_bind cache (#556)
The _fast_bind method had three bugs when replaying cached binding templates: 1. VAR_POSITIONAL (*args) params were stored as a single value (args[i]) instead of a tuple (args[i:]), causing TypeError when BoundArguments.args tried to extend() a non-iterable value. 2. KEYWORD_ONLY params after *args were not processed at all — the loop would break at the VAR_POSITIONAL and skip remaining params. 3. POSITIONAL_OR_KEYWORD params that share a name with a kwarg key should prefer the kwargs value (matching _full_bind behavior), but _fast_bind always used the positional arg. These bugs only manifested when the bind cache was shared across tests via the module-level signature_cache (different callables with the same signature sharing a SignatureAdapter instance). Fixes issues observed in SCXML W3C tests (test179, test527, test529, test562, test578) when run together with the bind cache from #550.
1 parent 1fd4b7e commit 37e6c1a

2 files changed

Lines changed: 115 additions & 5 deletions

File tree

statemachine/signature.py

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
from typing import Tuple
1414

1515
BindCacheKey = Tuple[int, FrozenSet[str]]
16-
BindTemplate = Tuple[Tuple[str, ...], Optional[str]] # noqa: UP007
16+
BindTemplate = Tuple[Tuple[str, ...], Optional[str], Optional[str]] # noqa: UP007
1717

1818

1919
def _make_key(method):
@@ -89,12 +89,25 @@ def _fast_bind(
8989
kwargs: dict[str, Any],
9090
template: BindTemplate,
9191
) -> BoundArguments:
92-
param_names, kwargs_param_name = template
92+
param_names, kwargs_param_name, var_positional_name = template
9393
arguments: dict[str, Any] = {}
94+
past_var_positional = False
9495

9596
for i, name in enumerate(param_names):
96-
if i < len(args):
97-
arguments[name] = args[i]
97+
if name == var_positional_name:
98+
# Collect all remaining positional args into a tuple
99+
arguments[name] = args[i:]
100+
past_var_positional = True
101+
elif past_var_positional:
102+
# After *args, remaining params are keyword-only
103+
arguments[name] = kwargs.get(name)
104+
elif i < len(args):
105+
# Match _full_bind: if param is also in kwargs, kwargs wins
106+
# (POSITIONAL_OR_KEYWORD params prefer kwargs over positional args)
107+
if name in kwargs:
108+
arguments[name] = kwargs[name]
109+
else:
110+
arguments[name] = args[i]
98111
else:
99112
arguments[name] = kwargs.get(name)
100113

@@ -124,6 +137,7 @@ def _full_bind( # noqa: C901
124137
parameters_ex: Any = ()
125138
kwargs_param = None
126139
kwargs_param_name: str | None = None
140+
var_positional_name: str | None = None
127141

128142
while True:
129143
# Let's iterate through the positional arguments and corresponding
@@ -192,6 +206,7 @@ def _full_bind( # noqa: C901
192206
values.extend(arg_vals)
193207
arguments[param.name] = tuple(values)
194208
param_names_used.append(param.name)
209+
var_positional_name = param.name
195210
break
196211

197212
if param.name in kwargs and param.kind != Parameter.POSITIONAL_ONLY:
@@ -236,7 +251,7 @@ def _full_bind( # noqa: C901
236251
# 'ignoring we got an unexpected keyword argument'
237252
pass
238253

239-
template: BindTemplate = (tuple(param_names_used), kwargs_param_name)
254+
template: BindTemplate = (tuple(param_names_used), kwargs_param_name, var_positional_name)
240255
self._bind_cache[cache_key] = template
241256

242257
return BoundArguments(self, arguments) # type: ignore[arg-type]

tests/test_signature.py

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
from functools import partial
33

44
import pytest
5+
56
from statemachine.dispatcher import callable_method
67
from statemachine.signature import SignatureAdapter
78

@@ -196,3 +197,97 @@ def test_kwargs_only_receives_unmatched_keys_with_positional(self):
196197

197198
result2 = wrapped("X", target="Y")
198199
assert result2 == ("X", {"target": "Y"})
200+
201+
def test_var_positional_collected_as_tuple(self):
202+
"""VAR_POSITIONAL (*args) must be collected into a tuple on cache hit."""
203+
204+
def fn(*args, **kwargs):
205+
return args, kwargs
206+
207+
wrapped = callable_method(fn)
208+
209+
result1 = wrapped(1, 2, 3, key="val")
210+
assert result1 == ((1, 2, 3), {"key": "val"})
211+
212+
result2 = wrapped(4, 5, key="other")
213+
assert result2 == ((4, 5), {"key": "other"})
214+
215+
def test_keyword_only_after_var_positional(self):
216+
"""KEYWORD_ONLY params after *args must be extracted from kwargs on cache hit."""
217+
218+
def fn(*args, event, **kwargs):
219+
return args, event, kwargs
220+
221+
wrapped = callable_method(fn)
222+
223+
result1 = wrapped(100, event="ev1", source="s0")
224+
assert result1 == ((100,), "ev1", {"source": "s0"})
225+
226+
result2 = wrapped(200, event="ev2", source="s1")
227+
assert result2 == ((200,), "ev2", {"source": "s1"})
228+
229+
def test_positional_or_keyword_prefers_kwargs_over_positional(self):
230+
"""When a POSITIONAL_OR_KEYWORD param is in both args and kwargs, kwargs wins."""
231+
232+
def fn(event, source, target):
233+
return event, source, target
234+
235+
wrapped = callable_method(fn)
236+
237+
# 1st call: positional arg provided but 'event' also in kwargs -> kwargs wins
238+
result1 = wrapped("discarded_content", event="ev1", source="s0", target="t0")
239+
assert result1 == ("ev1", "s0", "t0")
240+
241+
# 2nd call: cache hit, same behavior expected
242+
result2 = wrapped("other_content", event="ev2", source="s1", target="t1")
243+
assert result2 == ("ev2", "s1", "t1")
244+
245+
def test_empty_var_positional(self):
246+
"""Empty *args is handled correctly on cache hit."""
247+
248+
def fn(*args, **kwargs):
249+
return args, kwargs
250+
251+
wrapped = callable_method(fn)
252+
253+
# 1st call with args
254+
result1 = wrapped(1, key="val")
255+
assert result1 == ((1,), {"key": "val"})
256+
257+
# 2nd call: only kwargs, no positional args — different cache key (len=0)
258+
result2 = wrapped(key="val2")
259+
assert result2 == ((), {"key": "val2"})
260+
261+
# 3rd call: hits cache for len=0
262+
result3 = wrapped(key="val3")
263+
assert result3 == ((), {"key": "val3"})
264+
265+
def test_named_params_before_var_positional(self):
266+
"""Named params before *args are filled correctly on cache hit."""
267+
268+
def fn(a, b, *args, **kwargs):
269+
return a, b, args, kwargs
270+
271+
wrapped = callable_method(fn)
272+
273+
result1 = wrapped(1, 2, 3, 4, key="val")
274+
assert result1 == (1, 2, (3, 4), {"key": "val"})
275+
276+
result2 = wrapped(10, 20, 30, key="val2")
277+
assert result2 == (10, 20, (30,), {"key": "val2"})
278+
279+
def test_kwargs_wins_with_var_positional_present(self):
280+
"""POSITIONAL_OR_KEYWORD before *args prefers kwargs when name matches."""
281+
282+
def fn(event, *args, **kwargs):
283+
return event, args, kwargs
284+
285+
wrapped = callable_method(fn)
286+
287+
# 1st call: 'event' in both positional and kwargs — kwargs wins
288+
result1 = wrapped("discarded", "extra", event="ev1", key="a")
289+
assert result1 == ("ev1", ("extra",), {"key": "a"})
290+
291+
# 2nd call: cache hit, same behavior
292+
result2 = wrapped("other", "more", event="ev2", key="b")
293+
assert result2 == ("ev2", ("more",), {"key": "b"})

0 commit comments

Comments
 (0)