From 9c42a7b9fa9ed011ca687c0b7d9e01516993e42f Mon Sep 17 00:00:00 2001 From: Fernando Macedo Date: Tue, 13 Oct 2026 23:59:01 -0300 Subject: [PATCH 1/4] fix: run the effect of an explicit initial transition and stop adding a default one Refs #587 Refs #692 Signed-off-by: Fernando Macedo --- statemachine/engines/async_.py | 2 +- statemachine/engines/base.py | 4 +- statemachine/factory.py | 14 +- tests/io/test_native_behavior.py | 33 +++ tests/test_statechart_initial_transition.py | 223 ++++++++++++++++++++ 5 files changed, 269 insertions(+), 7 deletions(-) create mode 100644 tests/test_statechart_initial_transition.py diff --git a/statemachine/engines/async_.py b/statemachine/engines/async_.py index ae1955b0..b4db6090 100644 --- a/statemachine/engines/async_.py +++ b/statemachine/engines/async_.py @@ -277,7 +277,7 @@ async def _enter_states( initial_transition = initial_transitions.get(target.id) if initial_transition is not None: result += await self.sm._callbacks.async_call( - initial_transition.on.key, *args, **kwargs + initial_transition.on.key, *args, on_error=on_error_transition, **kwargs ) # Handle default history states diff --git a/statemachine/engines/base.py b/statemachine/engines/base.py index c457957f..eac3796e 100644 --- a/statemachine/engines/base.py +++ b/statemachine/engines/base.py @@ -651,7 +651,9 @@ def _enter_states( initial_transition = initial_transitions.get(target.id) if initial_transition is not None: - result += self.sm._callbacks.call(initial_transition.on.key, *args, **kwargs) + result += self.sm._callbacks.call( + initial_transition.on.key, *args, on_error=on_error_transition, **kwargs + ) # Handle default history states default_history_transitions = [ diff --git a/statemachine/factory.py b/statemachine/factory.py index 606553ff..db84712c 100644 --- a/statemachine/factory.py +++ b/statemachine/factory.py @@ -153,6 +153,15 @@ def _initials_by_document_order( # noqa: C901 if s.initial: initials.append(s) + if parent and not parent.parallel: + explicit_initials = [t for t in parent.transitions if t.initial] + if len(explicit_initials) > 1: + raise InvalidDefinition( + _("State '{0}' has more than one initial transition.").format(parent.id) + ) + if explicit_initials: + return + if not initials and states: initial = states[0] initial._initial = True @@ -161,11 +170,6 @@ def _initials_by_document_order( # noqa: C901 if not parent: return - # If parent already has a multi-target initial transition (e.g., from SCXML initial - # attribute targeting multiple parallel regions), don't create default initial transitions. - if any(t for t in parent.transitions if t.initial and len(t.targets) > 1): - return - for initial in initials: if not any(t for t in parent.transitions if t.initial and t.target == initial): parent.to(initial, initial=True) diff --git a/tests/io/test_native_behavior.py b/tests/io/test_native_behavior.py index b7e28d31..485765cf 100644 --- a/tests/io/test_native_behavior.py +++ b/tests/io/test_native_behavior.py @@ -62,3 +62,36 @@ async def test_safe_expression_guards(self, sm_runner): sm = await sm_runner.start(load(doc, format="yaml")) await sm_runner.send(sm, "go") assert "high" in sm.configuration_values + + async def test_explicit_initial_transition_to_descendant_runs_its_effect(self, sm_runner): + doc = """ + datamodel: + - {id: trace, expr: "''"} + states: + d: + enter: + - assign: {location: trace, expr: "trace + 'enter_d;'"} + transitions: + - target: d212 + initial: true + on: + - assign: {location: trace, expr: "trace + 'effect;'"} + states: + d1: {} + d2: + enter: + - assign: {location: trace, expr: "trace + 'enter_d2;'"} + states: + d21: + states: + d211: {initial: true} + d212: + enter: + - assign: {location: trace, expr: "trace + 'enter_d212;'"} + """ + chart = load(doc, format="yaml", validate=True) + sm = await sm_runner.start(chart) + + assert [t.target.id for t in chart.d.transitions if t.initial] == ["d212"] + assert set(sm.configuration_values) == {"d", "d2", "d21", "d212"} + assert sm.model.trace == "enter_d;effect;enter_d2;enter_d212;" diff --git a/tests/test_statechart_initial_transition.py b/tests/test_statechart_initial_transition.py new file mode 100644 index 00000000..49cd25a5 --- /dev/null +++ b/tests/test_statechart_initial_transition.py @@ -0,0 +1,223 @@ +"""Explicit initial transitions of compound states (refs #587).""" + +import pytest +from statemachine.exceptions import InvalidDefinition + +from statemachine import HistoryState +from statemachine import State +from statemachine import StateChart + + +class DeepInitial(StateChart): + class d(State.Compound): + class d1(State.Compound): + d11 = State(initial=True) + + class d2(State.Compound): + class d21(State.Compound): + d211 = State(initial=True) + d212 = State() + x = d211.to(d212) + + to_d2 = d1.to(d2) + to_d1 = d2.to(d1) + + d.to(d.d2.d21.d212, initial=True, on="init_action") + out = State() + leave = d.to(out) + back = out.to(d) + + def __init__(self, *args, **kwargs): + self.log = [] + super().__init__(*args, **kwargs) + + def init_action(self): + self.log.append("init_action") + + def on_enter_state(self, state): + self.log.append(f"enter:{state.id}") + + +class TestExplicitInitialTransition: + def test_is_the_only_initial_transition_of_the_compound(self): + initials = [t for t in DeepInitial.d.transitions if t.initial] + + assert [t.target.id for t in initials] == ["d212"] + + def test_no_child_is_marked_initial_by_document_order(self): + assert not DeepInitial.d.d1.initial + + async def test_effect_runs_between_parent_entry_and_child_entries(self, sm_runner): + sm = await sm_runner.start(DeepInitial) + + assert set(sm.configuration_values) == {"d", "d2", "d21", "d212"} + assert sm.log == ["enter:d", "init_action", "enter:d2", "enter:d21", "enter:d212"] + + async def test_effect_runs_again_on_reentry(self, sm_runner): + sm = await sm_runner.start(DeepInitial) + await sm_runner.send(sm, "leave") + sm.log.clear() + + await sm_runner.send(sm, "back") + + assert set(sm.configuration_values) == {"d", "d2", "d21", "d212"} + assert sm.log == ["enter:d", "init_action", "enter:d2", "enter:d21", "enter:d212"] + + async def test_effect_receives_event_arguments(self, sm_runner): + received = [] + + class Chart(StateChart): + class d(State.Compound): + a = State() + b = State() + back = b.to(a) + + out = State(initial=True) + enter = out.to(d) + d.to(d.b, initial=True, on=lambda value: received.append(value)) + + sm = await sm_runner.start(Chart) + await sm_runner.send(sm, "enter", value=42) + + assert set(sm.configuration_values) == {"d", "b"} + assert received == [42] + + def test_subclass_keeps_a_single_initial_transition(self): + class Child(DeepInitial): + pass + + sm = Child() + + assert [t.target.id for t in Child.d.transitions if t.initial] == ["d212"] + assert sm.log == ["enter:d", "init_action", "enter:d2", "enter:d21", "enter:d212"] + + +class TestConflictingInitialDefinitions: + def test_two_explicit_initial_transitions_are_rejected(self): + with pytest.raises(InvalidDefinition, match="d.*more than one initial transition"): + + class Chart(StateChart): + class d(State.Compound): + a = State() + b = State() + + d.to(d.a, initial=True) + d.to(d.b, initial=True) + + def test_initial_child_targeted_by_explicit_initial_transition_is_accepted(self): + class Chart(StateChart): + class d(State.Compound): + a = State() + b = State(initial=True) + back = b.to(a) + + d.to(d.b, initial=True, on="effect") + + def effect(self): + self.ran = True + + sm = Chart() + + assert [t.target.id for t in Chart.d.transitions if t.initial] == ["b"] + assert set(sm.configuration_values) == {"d", "b"} + assert sm.ran + + def test_initial_child_containing_the_explicit_target_is_accepted(self): + class Chart(StateChart): + class d(State.Compound): + a = State() + + class b(State.Compound, initial=True): + b1 = State(initial=True) + b2 = State() + step = b2.to(b1) + + back = b.to(a) + + d.to(d.b.b2, initial=True) + + sm = Chart() + + assert set(sm.configuration_values) == {"d", "b", "b2"} + + +class TestInitialTransitionEffectErrors: + async def test_error_in_effect_becomes_error_execution_and_entry_completes(self, sm_runner): + class Chart(StateChart): + class d(State.Compound): + a = State() + b = State() + back = b.to(a) + + out = State(initial=True) + go = out.to(d) + d.to(d.b, initial=True, on="effect") + + def effect(self): + raise ValueError("effect failed") + + sm = await sm_runner.start(Chart) + await sm_runner.send(sm, "go") + + assert set(sm.configuration_values) == {"d", "b"} + + async def test_error_execution_handler_sees_the_effect_error(self, sm_runner): + class Chart(StateChart): + class d(State.Compound): + a = State() + b = State() + back = b.to(a) + + out = State(initial=True) + go = out.to(d) + d.to(d.b, initial=True, on="effect") + error_execution = d.to(out, on="handle") + + def __init__(self, *args, **kwargs): + self.errors = [] + super().__init__(*args, **kwargs) + + def effect(self): + raise ValueError("effect failed") + + def handle(self, error): + self.errors.append(str(error)) + + sm = await sm_runner.start(Chart) + await sm_runner.send(sm, "go") + + assert sm.errors == ["effect failed"] + assert set(sm.configuration_values) == {"out"} + + +class TestInitialTransitionToHistory: + async def test_effect_runs_once_per_entry(self, sm_runner): + class Chart(StateChart): + class d(State.Compound): + h = HistoryState() + a = State() + b = State() + step = a.to(b) + h.to(a) + + d.to(d.h, initial=True, on="effect") + out = State() + leave = d.to(out) + back = out.to(d) + + def __init__(self, *args, **kwargs): + self.effects = 0 + super().__init__(*args, **kwargs) + + def effect(self): + self.effects += 1 + + sm = await sm_runner.start(Chart) + assert sm.effects == 1 + + await sm_runner.send(sm, "step") + await sm_runner.send(sm, "leave") + await sm_runner.send(sm, "back") + + assert sm.effects == 2 + assert set(sm.configuration_values) == {"d", "b"} From d00cada493a98bc7150e338c9ebad7287368ca53 Mon Sep 17 00:00:00 2001 From: Fernando Macedo Date: Wed, 14 Oct 2026 00:00:08 -0300 Subject: [PATCH 2/4] docs: explicit initial transition of a compound state Refs #587 Refs #692 Signed-off-by: Fernando Macedo --- docs/states.md | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/docs/states.md b/docs/states.md index a7fbb409..f63abe73 100644 --- a/docs/states.md +++ b/docs/states.md @@ -126,6 +126,54 @@ True Entering a compound activates both the parent and its `initial` child. You can query whether a state is compound using the `is_compound` property. +### Explicit initial transition + +To enter a compound at a nested descendant, or to run an action each time the compound +is entered by default, declare its initial transition explicitly with +`parent.to(target, initial=True)`. The target can be any descendant, and the `on` +action runs after the parent's `enter` callbacks and before any child is entered: + +```py +>>> class Trip(StateChart): +... class travel(State.Compound): +... class planning(State.Compound): +... dates = State(initial=True) +... route = State() +... next_step = dates.to(route) +... booked = State() +... book = planning.to(booked) +... replan = booked.to(planning) +... travel.to(travel.planning.route, initial=True, on="load_saved_route") +... home = State() +... cancel = travel.to(home) +... restart = home.to(travel) +... +... def __init__(self): +... self.log = [] +... super().__init__() +... +... def load_saved_route(self): +... self.log.append("load saved route") +... +... def on_enter_state(self, state): +... self.log.append(f"enter {state.id}") + +>>> sm = Trip() +>>> sm.log +['enter travel', 'load saved route', 'enter planning', 'enter route'] + +>>> sm.send("cancel") +>>> sm.log.clear() +>>> sm.send("restart") +>>> sm.log +['enter travel', 'load saved route', 'enter planning', 'enter route'] + +``` + +An explicit initial transition replaces the default one: the first child is no longer +picked by document order. A compound state accepts a single initial transition, so +declaring a second one raises `InvalidDefinition`. + ```{seealso} See {ref}`done-state-events` for completion events when a compound state's final child is reached. From 20cb9a8ae55efe681e175f563411bc9e2a003927 Mon Sep 17 00:00:00 2001 From: Fernando Macedo Date: Wed, 14 Oct 2026 23:22:24 -0300 Subject: [PATCH 3/4] feat: run on_init_ on the initial transition of a compound state A method named on_init_() is resolved as an action of the state's initial transition, explicit or by document order. It runs after the state's enter callbacks and only on default entry, giving the UML statechart init action a callback convention. Closes #587 Signed-off-by: Fernando Macedo --- docs/actions.md | 5 +- docs/releases/3.3.0.md | 27 ++++++ docs/states.md | 42 +++++++++ statemachine/transition.py | 2 + tests/test_statechart_initial_transition.py | 98 +++++++++++++++++++++ 5 files changed, 173 insertions(+), 1 deletion(-) diff --git a/docs/actions.md b/docs/actions.md index e3c32137..4988d7d7 100644 --- a/docs/actions.md +++ b/docs/actions.md @@ -52,7 +52,10 @@ sequence of **groups**. Each group runs to completion before the next one starts * - Enter - `on_enter_state()`, `on_enter_()` - entering state - - Runs once per state being entered, from ancestor to child. + - Runs once per state being entered, from ancestor to child. A compound entered + by default then runs its initial transition: its `on` action, then + `on_init_()`, before any child is entered. Errors there follow the + transition `on` rules. See {ref}`explicit-initial-transition`. * - Invoke - `on_invoke_state()`, `on_invoke_()` - `target` diff --git a/docs/releases/3.3.0.md b/docs/releases/3.3.0.md index 163556be..b8e38645 100644 --- a/docs/releases/3.3.0.md +++ b/docs/releases/3.3.0.md @@ -28,6 +28,16 @@ its output. See [](../contributing.md#uml-cases-based-on-pssm). [#697](https://github.com/fgmacedo/python-statemachine/pull/697). +### Initial transition actions + +A compound state can now run an action on its initial transition, the "init" of UML +statecharts. Declare the transition explicitly with `on=`, or define a method named +`on_init_()`. Either one runs after the state's `enter` callbacks and before any +child is entered, and only when the compound is entered by default. See +[](../states.md#explicit-initial-transition). + +[#694](https://github.com/fgmacedo/python-statemachine/pull/694). + ## Behavior changes to check when upgrading Each item below fixes behavior that did not match the documented contract. Code that relied @@ -52,6 +62,10 @@ on the old behavior can still behave differently or fail after upgrading: loop, as before, or, when the invocation itself processes `done.invoke`, through the channels above, where it used to become `error.execution`. See {ref}`release-3.3.0-done-invoke`. +- A compound state with an explicit initial transition (`parent.to(child, initial=True)`) + no longer also gets a document-order one. A chart whose first child was reachable only + through that second transition now fails the reachability check with `InvalidDefinition`. + See {ref}`release-3.3.0-explicit-initial`. ## Deprecations in 3.3.0 @@ -134,3 +148,16 @@ handler. An error raised by a handler's `on_finalize` goes through the same erro See [](../invoke.md#error-handling). [#698](https://github.com/fgmacedo/python-statemachine/pull/698). + +(release-3.3.0-explicit-initial)= + +### Explicit initial transition of a compound state + +An explicit initial transition of a compound state, such as +`d.to(d.d2.d21, initial=True, on="action")`, entered the right descendant but dropped its +`on` action, and the compound also got a second, document-order initial transition. The +action now runs after the compound's `enter` callbacks and before any child is entered, and +the explicit transition is the compound's only initial one. See +[](../states.md#explicit-initial-transition). + +[#694](https://github.com/fgmacedo/python-statemachine/pull/694). diff --git a/docs/states.md b/docs/states.md index f63abe73..7def81a2 100644 --- a/docs/states.md +++ b/docs/states.md @@ -126,6 +126,8 @@ True Entering a compound activates both the parent and its `initial` child. You can query whether a state is compound using the `is_compound` property. +(explicit-initial-transition)= + ### Explicit initial transition To enter a compound at a nested descendant, or to run an action each time the compound @@ -174,6 +176,46 @@ An explicit initial transition replaces the default one: the first child is no l picked by document order. A compound state accepts a single initial transition, so declaring a second one raises `InvalidDefinition`. +A method named `on_init_()` runs as part of the initial transition of that +compound state, whether the transition is explicit or picked by document order. It +runs after the state's `enter` callbacks and after the transition's `on` action, and +only when the compound is entered by default: a transition that targets one of its +descendants directly skips it. A parallel state never runs its own `on_init_()`; +each of its regions runs theirs. + +To run an initial action for the whole chart, before any substate is entered, wrap the +states in a top-level compound and give it an explicit initial transition: + +```py +>>> class Device(StateChart): +... class top(State.Compound): +... class idle(State.Compound): +... waiting = State() +... class active(State.Compound): +... class job(State.Compound): +... running = State() +... stop = active.to(idle) +... top.to(top.active, initial=True, on="reset_counters") +... +... def __init__(self): +... self.log = [] +... super().__init__() +... +... def reset_counters(self): +... self.log.append("top init") +... +... def on_init_active(self): +... self.log.append("active init") +... +... def on_enter_state(self, state): +... self.log.append(f"enter {state.id}") + +>>> sm = Device() +>>> sm.log +['enter top', 'top init', 'enter active', 'active init', 'enter job', 'enter running'] + +``` + ```{seealso} See {ref}`done-state-events` for completion events when a compound state's final child is reached. diff --git a/statemachine/transition.py b/statemachine/transition.py index c068fae3..55e5243e 100644 --- a/statemachine/transition.py +++ b/statemachine/transition.py @@ -123,6 +123,8 @@ def _setup(self): before("before_transition", priority=CallbackPriority.GENERIC, is_convention=True) on("on_transition", priority=CallbackPriority.GENERIC, is_convention=True) + if self.initial: + on(f"on_init_{self.source.id}", priority=CallbackPriority.NAMING, is_convention=True) for event in self._events: same_event_cond = event.is_same_event diff --git a/tests/test_statechart_initial_transition.py b/tests/test_statechart_initial_transition.py index 49cd25a5..f487ca7d 100644 --- a/tests/test_statechart_initial_transition.py +++ b/tests/test_statechart_initial_transition.py @@ -221,3 +221,101 @@ def effect(self): assert sm.effects == 2 assert set(sm.configuration_values) == {"d", "b"} + + +class InitConvention(StateChart): + class d(State.Compound): + class d1(State.Compound): + d11 = State(initial=True) + + class d2(State.Compound): + class d21(State.Compound): + d211 = State(initial=True) + + to_d2 = d1.to(d2) + to_d211 = d1.to(d2.d21.d211) + + d.to(d.d1, initial=True, on="explicit_effect") + + def __init__(self, *args, **kwargs): + self.log = [] + super().__init__(*args, **kwargs) + + def explicit_effect(self): + self.log.append("explicit") + + def on_init_d(self): + self.log.append("init:d") + + def on_init_d2(self): + self.log.append("init:d2") + + def on_init_d21(self): + self.log.append("init:d21") + + def on_enter_state(self, state): + self.log.append(f"enter:{state.id}") + + +class TestOnInitNamingConvention: + async def test_runs_after_explicit_effect_and_before_children(self, sm_runner): + sm = await sm_runner.start(InitConvention) + + assert sm.log == ["enter:d", "explicit", "init:d", "enter:d1", "enter:d11"] + + async def test_runs_on_the_default_initial_transition(self, sm_runner): + sm = await sm_runner.start(InitConvention) + sm.log.clear() + + await sm_runner.send(sm, "to_d2") + + assert sm.log == ["enter:d2", "init:d2", "enter:d21", "init:d21", "enter:d211"] + + async def test_skipped_when_a_transition_targets_a_substate(self, sm_runner): + sm = await sm_runner.start(InitConvention) + sm.log.clear() + + await sm_runner.send(sm, "to_d211") + + assert sm.log == ["enter:d2", "enter:d21", "enter:d211"] + + async def test_resolved_on_listeners(self, sm_runner): + class Listener: + def __init__(self): + self.inits = [] + + def on_init_d2(self, target): + self.inits.append(target.id) + + listener = Listener() + sm = await sm_runner.start(InitConvention, listeners=[listener]) + + await sm_runner.send(sm, "to_d2") + + assert listener.inits == ["d2"] + + async def test_parallel_state_runs_it_per_region(self, sm_runner): + class Chart(StateChart): + class p(State.Parallel): + class r1(State.Compound): + a = State() + + class r2(State.Compound): + b = State() + + def __init__(self, *args, **kwargs): + self.inits = [] + super().__init__(*args, **kwargs) + + def on_init_p(self): + self.inits.append("p") + + def on_init_r1(self): + self.inits.append("r1") + + def on_init_r2(self): + self.inits.append("r2") + + sm = await sm_runner.start(Chart) + + assert sm.inits == ["r1", "r2"] From bbc2b7539ff537fe2724ab0212df46201c79a3c3 Mon Sep 17 00:00:00 2001 From: Fernando Macedo Date: Fri, 16 Oct 2026 19:31:31 -0300 Subject: [PATCH 4/4] feat: reject callback names bound both to an event and to a state A method named on_init_ is also the on_ action of an event named init_, so it would run for both. The class build now finds those names once per class and checks the class, the model and every listener attached by the constructor or add_listener against them: on_init_ names raise InvalidDefinition, and the existing on_enter_, on_exit_ and on_invoke_ overlaps emit a DeprecationWarning until 4.0. validate_callback_name_conflicts = False opts out. Refs #587 Signed-off-by: Fernando Macedo --- docs/releases/3.3.0.md | 13 +- docs/states.md | 3 +- docs/validations.md | 36 ++++ statemachine/callback_names.py | 58 +++++++ statemachine/factory.py | 23 +++ statemachine/statemachine.py | 41 ++++- tests/test_callback_name_conflicts.py | 226 ++++++++++++++++++++++++++ tests/test_statechart_error.py | 4 +- 8 files changed, 390 insertions(+), 14 deletions(-) create mode 100644 statemachine/callback_names.py create mode 100644 tests/test_callback_name_conflicts.py diff --git a/docs/releases/3.3.0.md b/docs/releases/3.3.0.md index b8e38645..dfe6704b 100644 --- a/docs/releases/3.3.0.md +++ b/docs/releases/3.3.0.md @@ -34,7 +34,9 @@ A compound state can now run an action on its initial transition, the "init" of statecharts. Declare the transition explicitly with `on=`, or define a method named `on_init_()`. Either one runs after the state's `enter` callbacks and before any child is entered, and only when the compound is entered by default. See -[](../states.md#explicit-initial-transition). +[](../states.md#explicit-initial-transition). A class that also has an event +`init_` is rejected, since the method would be bound to both; see +{ref}`callback-name-conflicts`. [#694](https://github.com/fgmacedo/python-statemachine/pull/694). @@ -69,7 +71,14 @@ on the old behavior can still behave differently or fail after upgrading: ## Deprecations in 3.3.0 -Deprecated APIs are removed in 4.0. Nothing is deprecated in 3.3.0 yet. +Deprecated APIs are removed in 4.0. + +- A method whose name is bound both to an event and to a state callback, such as + `on_enter_x` with an event `enter_x` and a state `x`, runs for both. Defining one on the + class, the model or a listener now emits a `DeprecationWarning`, and will raise + `InvalidDefinition` in 4.0. The same overlap with the new `on_init_` raises + `InvalidDefinition` already. Set `validate_callback_name_conflicts = False` to keep the + old behavior. See {ref}`callback-name-conflicts`. ## Bug fixes in 3.3.0 diff --git a/docs/states.md b/docs/states.md index 7def81a2..3dede940 100644 --- a/docs/states.md +++ b/docs/states.md @@ -181,7 +181,8 @@ compound state, whether the transition is explicit or picked by document order. runs after the state's `enter` callbacks and after the transition's `on` action, and only when the compound is entered by default: a transition that targets one of its descendants directly skips it. A parallel state never runs its own `on_init_()`; -each of its regions runs theirs. +each of its regions runs theirs. An event named `init_` would bind the same method +name, so it raises `InvalidDefinition` (see {ref}`callback-name-conflicts`). To run an initial action for the whole chart, before any substate is entered, wrap the states in a top-level compound and give it an explicit initial transition: diff --git a/docs/validations.md b/docs/validations.md index cc1010dd..e45adc4a 100644 --- a/docs/validations.md +++ b/docs/validations.md @@ -144,6 +144,41 @@ All non-final states should have at least one path to a final state. These state Disable with `validate_final_reachability = False`. +(callback-name-conflicts)= + +### Callback names bound to an event and to a state + +A method name can match two naming conventions at once: `on_init_payment` is the action +of an event `init_payment` and also the {ref}`initial action ` +of a compound state `payment`. Such a method would run for both. When the class, its +model or a listener defines one, an `on_init_` name raises `InvalidDefinition`: + +```py +>>> try: +... class Checkout(StateChart): +... cart = State(initial=True) +... class payment(State.Compound): +... choosing = State() +... paid = State(final=True) +... pay = choosing.to(paid) +... init_payment = cart.to(payment) +... +... def on_init_payment(self): +... pass +... except InvalidDefinition as e: +... print(e) +Checkout defines on_init_payment, bound both to an event and to the initial transition of a compound state. Rename the event or the method. + +``` + +The same overlap with `on_enter_`, `on_exit_` or `on_invoke_` (an +event `enter_x` and a state `x`, for instance) emits a `DeprecationWarning` and will raise +in 4.0. Listeners are checked when they are attached, by the constructor or by +`add_listener()`. + +Disable with `validate_callback_name_conflicts = False`; the method then runs for both. + + ### Internal transition targets Internal transitions must target the same state (self) or a descendant — @@ -317,6 +352,7 @@ Expressions support `and`, `or`, `not`, and parentheses. See | Unreachable states | Class definition| `validate_disconnected_states` | | Trap states | Class definition| `validate_trap_states` | | Final state reachability | Class definition| `validate_final_reachability` | +| Callback names bound twice | Class definition, listeners | `validate_callback_name_conflicts` | | Internal transition targets | Class definition| No | | Initial transitions have no cond | Class definition| No | | `donedata` on final states only | Class definition| No | diff --git a/statemachine/callback_names.py b/statemachine/callback_names.py new file mode 100644 index 00000000..5b56e816 --- /dev/null +++ b/statemachine/callback_names.py @@ -0,0 +1,58 @@ +import warnings +from collections.abc import Iterable +from typing import TYPE_CHECKING + +from .callbacks import CallbackPriority +from .exceptions import InvalidDefinition +from .graph import iterate_states_and_transitions +from .i18n import _ + +if TYPE_CHECKING: + from .state import State + +_INIT_PREFIX = "on_init_" + + +def ambiguous_callback_names(states: "Iterable[State]") -> frozenset[str]: + """Names that both an event convention (``on_``) and a state convention + (``on_enter_``, ``on_init_``, ...) bind, such as ``on_init_x`` for an + event ``init_x`` and a compound state ``x``.""" + event_names: set[str] = set() + state_names: set[str] = set() + for visited in iterate_states_and_transitions(states): + for spec in visited._specs: + if not spec.is_convention or spec.priority != CallbackPriority.NAMING: + continue + if spec.cond is None: + state_names.add(spec.func) + else: + event_names.add(spec.func) + return frozenset(event_names & state_names) + + +def check_callback_names(owner: str, names: Iterable[str], stacklevel: int): + """Reject or deprecate the ambiguous callback names that ``owner`` defines. + + ``stacklevel`` counts from the caller, as in :func:`warnings.warn`.""" + names = sorted(names) + if not names: + return + + rejected = [name for name in names if name.startswith(_INIT_PREFIX)] + if rejected: + raise InvalidDefinition( + _( + "{owner} defines {names}, bound both to an event and to the initial " + "transition of a compound state. Rename the event or the method." + ).format(owner=owner, names=", ".join(rejected)) + ) + + warnings.warn( + _( + "{owner} defines {names}, bound both to an event and to a state callback, so " + "it runs for both. This will raise InvalidDefinition in 4.0. " + "Rename the event or the method." + ).format(owner=owner, names=", ".join(names)), + DeprecationWarning, + stacklevel=stacklevel + 1, + ) diff --git a/statemachine/factory.py b/statemachine/factory.py index db84712c..975ca19a 100644 --- a/statemachine/factory.py +++ b/statemachine/factory.py @@ -2,6 +2,8 @@ from typing import Any from . import registry +from .callback_names import ambiguous_callback_names +from .callback_names import check_callback_names from .callbacks import CallbackGroup from .callbacks import CallbackPriority from .callbacks import CallbackSpecList @@ -47,6 +49,13 @@ class StateMachineMetaclass(type): """If ``True`` and final states exist, non-final states without a path to any final state raise ``InvalidDefinition``.""" + validate_callback_name_conflicts: bool = True + """If ``True``, a method that the machine, its model or a listener defines under a name + bound both to an event (``on_``) and to a state (``on_enter_``, + ``on_exit_``, ``on_invoke_``, ``on_init_``) is reported: + ``on_init_`` raises ``InvalidDefinition``, the others emit a + ``DeprecationWarning``.""" + def __init__( cls, name: str, @@ -64,6 +73,7 @@ def __init__( """Map of ``state.value`` to the corresponding :ref:`state`.""" cls._abstract = True + cls._ambiguous_callback_names: frozenset[str] = frozenset() cls._events: dict[Event, None] = {} # used Dict to preserve order and avoid duplicates cls._protected_attrs: set = set() cls._events_to_update: dict[Event, list[Event]] = {} @@ -103,6 +113,7 @@ def __init__( cls._check() cls._setup() cls._unpack_builders_callbacks(attrs) + cls._check_callback_names() cls._expand_docstring() _STATECHART_RE = re.compile(r"\{statechart:(\w+)\}") @@ -352,6 +363,18 @@ def _check_disconnected_state(cls): ).format([s.id for s in states]) ) + def _check_callback_names(cls): + cls._ambiguous_callback_names = ( + ambiguous_callback_names(cls.states) + if cls.validate_callback_name_conflicts + else frozenset() + ) + check_callback_names( + cls.__name__, + (name for name in cls._ambiguous_callback_names if hasattr(cls, name)), + stacklevel=3, + ) + def _setup(cls): for visited in iterate_states_and_transitions(cls.states): visited._setup() diff --git a/statemachine/statemachine.py b/statemachine/statemachine.py index 639a21e0..d0fafbe0 100644 --- a/statemachine/statemachine.py +++ b/statemachine/statemachine.py @@ -1,4 +1,5 @@ import warnings +from collections.abc import Iterable from collections.abc import MutableSet from inspect import isawaitable from typing import TYPE_CHECKING @@ -9,6 +10,7 @@ from statemachine.orderedset import OrderedSet +from .callback_names import check_callback_names from .callbacks import SPECS_ALL from .callbacks import SPECS_SAFE from .callbacks import CallbackSpecList @@ -137,6 +139,7 @@ class StateChart(Generic[TModel], metaclass=StateMachineMetaclass): _protected_attrs: set _specs: CallbackSpecList _class_listeners: list[Any] + _ambiguous_callback_names: frozenset[str] prepare: SpecListGrouper def __init__( @@ -271,9 +274,9 @@ def __setstate__(self, state: dict[str, Any]) -> None: # _listeners already contained both class-level and runtime listeners # when serialized, so just re-register them all. - self._register_callbacks([]) + self._register_callbacks([], check_names=False) if listeners: - self.add_listener(*listeners.values()) + self._attach_listeners(listeners.values(), check_names=False) self._engine = self._get_engine() self._engine.start() @@ -315,15 +318,17 @@ def _add_listener(self, listeners: "Listeners", allowed_references: SpecReferenc return self - def _register_callbacks(self, listeners: list[object]): + def _register_callbacks(self, listeners: list[object], check_names: bool = True): + external = ( + Listener.from_obj(self.model, skip_attrs={self.state_field}), + *(Listener.from_obj(listener) for listener in listeners), + ) + if check_names: + self._check_listener_names(external, stacklevel=3) self._listeners.update({id(listener): listener for listener in listeners}) self._add_listener( Listeners.from_listeners( - ( - Listener.from_obj(self, skip_attrs=self._protected_attrs), - Listener.from_obj(self.model, skip_attrs={self.state_field}), - *(Listener.from_obj(listener) for listener in listeners), - ) + (Listener.from_obj(self, skip_attrs=self._protected_attrs), *external) ) ) @@ -357,12 +362,30 @@ def add_listener(self, *listeners): :ref:`listeners`. """ + return self._attach_listeners(listeners, check_names=True) + + def _attach_listeners(self, listeners: "Iterable[object]", check_names: bool): + listeners = list(listeners) + added = [Listener.from_obj(listener) for listener in listeners] + if check_names: + self._check_listener_names(added, stacklevel=3) self._listeners.update({id(listener): listener for listener in listeners}) return self._add_listener( - Listeners.from_listeners(Listener.from_obj(listener) for listener in listeners), + Listeners.from_listeners(added), allowed_references=SPECS_SAFE, ) + def _check_listener_names(self, listeners: "Iterable[Listener]", stacklevel: int): + ambiguous = self._ambiguous_callback_names + if not ambiguous: + return + for listener in listeners: + check_callback_names( + type(listener.obj).__name__, + ambiguous & listener.all_attrs, + stacklevel=stacklevel + 1, + ) + def _repr_html_(self): return f'
{self._repr_svg_()}
' diff --git a/tests/test_callback_name_conflicts.py b/tests/test_callback_name_conflicts.py new file mode 100644 index 00000000..8a387817 --- /dev/null +++ b/tests/test_callback_name_conflicts.py @@ -0,0 +1,226 @@ +"""A method name bound both by an event convention and by a state convention (refs #587).""" + +import copy +import pickle +import warnings + +import pytest +from statemachine.event import Event +from statemachine.exceptions import InvalidDefinition + +from statemachine import State +from statemachine import StateChart + + +def build_init_collision(**attrs): + class Chart(StateChart): + a = State(initial=True) + + class x(State.Compound): + x1 = State() + done = State(final=True) + finish = x1.to(done) + + init_x = a.to(x) + vars().update(attrs) + + return Chart + + +def on_init_x(self): + pass + + +class TestInitConvention: + def test_method_on_the_class_is_rejected(self): + with pytest.raises(InvalidDefinition, match="on_init_x"): + build_init_collision(on_init_x=on_init_x) + + def test_event_with_explicit_id_is_rejected(self): + with pytest.raises(InvalidDefinition, match="on_init_x"): + + class Chart(StateChart): + a = State(initial=True) + + class x(State.Compound): + x1 = State() + done = State(final=True) + finish = x1.to(done) + + start = Event(a.to(x), id="init_x") + + def on_init_x(self): + pass + + def test_without_the_method_the_class_is_accepted(self): + sm = build_init_collision()() + + sm.send("init_x") + + assert "x1" in sm.configuration_values + + def test_listener_is_rejected_on_construction(self): + class Listener: + def on_init_x(self): + pass + + with pytest.raises(InvalidDefinition, match="on_init_x"): + build_init_collision()(listeners=[Listener()]) + + def test_listener_is_rejected_on_add_listener(self): + class Listener: + def on_init_x(self): + pass + + sm = build_init_collision()() + + with pytest.raises(InvalidDefinition, match="on_init_x"): + sm.add_listener(Listener()) + + def test_model_is_rejected(self): + class Model: + state = None + + def on_init_x(self): + pass + + with pytest.raises(InvalidDefinition, match="on_init_x"): + build_init_collision()(model=Model()) + + def test_opt_out_binds_the_method_to_both(self): + calls = [] + + Chart = build_init_collision( + validate_callback_name_conflicts=False, + on_init_x=lambda self, event: calls.append(str(event)), + ) + sm = Chart() + + sm.send("init_x") + + assert calls == ["init_x", "init_x"] + + def test_opt_out_accepts_listeners(self): + class Listener: + def on_init_x(self): + pass + + Chart = build_init_collision(validate_callback_name_conflicts=False) + + Chart(listeners=[Listener()]).add_listener(Listener()) + + +def build_state_collision(prefix, **attrs): + class Chart(StateChart): + a = State(initial=True) + x = State() + back = x.to(a) + vars()[f"{prefix}_x"] = a.to(x) + vars().update(attrs) + + return Chart + + +@pytest.mark.parametrize("prefix", ["enter", "exit", "invoke"]) +class TestExistingStateConventions: + def test_method_on_the_class_is_deprecated(self, prefix): + with pytest.warns(DeprecationWarning, match=f"on_{prefix}_x"): + build_state_collision(prefix, **{f"on_{prefix}_x": lambda self: None}) + + def test_listener_is_deprecated(self, prefix): + Listener = type("Listener", (), {f"on_{prefix}_x": lambda self: None}) + Chart = build_state_collision(prefix) + + with pytest.warns(DeprecationWarning, match=f"on_{prefix}_x"): + Chart(listeners=[Listener()]) + + def test_class_is_warned_once_not_per_instance(self, prefix): + with pytest.warns(DeprecationWarning, match=f"on_{prefix}_x"): + Chart = build_state_collision(prefix, **{f"on_{prefix}_x": lambda self: None}) + + with warnings.catch_warnings(): + warnings.simplefilter("error") + Chart() + Chart() + + def test_opt_out_is_silent(self, prefix): + with warnings.catch_warnings(): + warnings.simplefilter("error") + build_state_collision( + prefix, + validate_callback_name_conflicts=False, + **{f"on_{prefix}_x": lambda self: None}, + )() + + +class TestNestedStateBody: + def test_init_method_in_the_compound_body_is_rejected(self): + with pytest.raises(InvalidDefinition, match="on_init_x"): + + class Chart(StateChart): + a = State(initial=True) + + class x(State.Compound): + x1 = State() + done = State(final=True) + finish = x1.to(done) + + def on_init_x(self): + pass + + init_x = a.to(x) + + def test_enter_method_in_the_compound_body_is_deprecated(self): + with pytest.warns(DeprecationWarning, match="on_enter_x1"): + + class Chart(StateChart): + a = State(initial=True) + + class x(State.Compound): + x1 = State() + done = State(final=True) + finish = x1.to(done) + + def on_enter_x1(self): + pass + + enter_x1 = a.to(x.x1) + + +class EnterListener: + def on_enter_x(self): + pass + + +class RestorableChart(StateChart): + a = State(initial=True) + x = State() + back = x.to(a) + enter_x = a.to(x) + + +class TestRestoredMachine: + @pytest.mark.parametrize("restore", [copy.deepcopy, lambda sm: pickle.loads(pickle.dumps(sm))]) + def test_is_not_warned_again(self, restore): + with pytest.warns(DeprecationWarning, match="on_enter_x"): + sm = RestorableChart(listeners=[EnterListener()]) + + with warnings.catch_warnings(): + warnings.simplefilter("error") + restored = restore(sm) + + assert [type(listener) for listener in restored.active_listeners] == [EnterListener] + + +class TestRejectedListener: + def test_is_not_attached(self): + class Listener: + def on_init_x(self): + pass + + sm = build_init_collision()() + + with pytest.raises(InvalidDefinition): + sm.add_listener(Listener()) + + assert sm.active_listeners == [] diff --git a/tests/test_statechart_error.py b/tests/test_statechart_error.py index ea96755f..254b4720 100644 --- a/tests/test_statechart_error.py +++ b/tests/test_statechart_error.py @@ -22,7 +22,7 @@ class realm(State.Compound): safe = State(initial=True) danger = State() - enter_danger = safe.to(danger) + go_danger = safe.to(danger) def on_enter_danger(self): raise RuntimeError("Balrog awakens!") @@ -31,7 +31,7 @@ def on_enter_danger(self): error_execution = Event(realm.to(error_state), id="error.execution") sm = await sm_runner.start(CompoundError) - await sm_runner.send(sm, "enter_danger") + await sm_runner.send(sm, "go_danger") assert {"error_state"} == set(sm.configuration_values) async def test_error_in_parallel_region_isolation(self, sm_runner):