Check for trap states and states that can't reach a final state (#410)

* Add check for stuck states and states that don't reach a final state.
* Document state transition checks, and strict_states flag behaviour.

Author: twoolie

docs/states.md

@@ -21,6 +21,57 @@ A {ref}`StateMachine` should have one and only one `initial` {ref}`state`.
 The initial {ref}`state` is entered when the machine starts and the corresponding entering
 state {ref}`actions` are called if defined.
 
+## State Transitions
+
+All states should have at least one transition to and from another state.
+
+If any states are unreachable from the initial state, an `InvalidDefinition` exception will be thrown.
+
+```py
+>>> from statemachine import StateMachine, State
+
+>>> class TrafficLightMachine(StateMachine):
+...     "A workflow machine"
+...     red = State('Red', initial=True, value=1)
+...     green = State('Green', value=2)
+...     orange = State('Orange', value=3)
+...     hazard = State('Hazard', value=4)
+...
+...     cycle = red.to(green) | green.to(orange) | orange.to(red)
+...     blink = hazard.to.itself()
+Traceback (most recent call last):
+...
+InvalidDefinition: There are unreachable states. The statemachine graph should have a single component. Disconnected states: ['hazard']
+```
+
+`StateMachine` will also check that all non-final states have an outgoing transition, and warn you if any states would result in
+the statemachine becoming trapped in a non-final state with no further transitions possible.
+
+```{note}
+This will currently issue a warning, but can be turned into an exception by setting `strict_states=True` on the class.
+```
+
+```py
+>>> from statemachine import StateMachine, State
+
+>>> class TrafficLightMachine(StateMachine, strict_states=True):
+...     "A workflow machine"
+...     red = State('Red', initial=True, value=1)
+...     green = State('Green', value=2)
+...     orange = State('Orange', value=3)
+...     hazard = State('Hazard', value=4)
+...
+...     cycle = red.to(green) | green.to(orange) | orange.to(red)
+...     fault = red.to(hazard) | green.to(hazard) | orange.to(hazard)
+Traceback (most recent call last):
+...
+InvalidDefinition: All non-final states should have at least one outgoing transition. These states have no outgoing transition: ['hazard']
+```
+
+```{warning}
+`strict_states=True` will become the default behaviour in future versions.
+```
+
 
 (final-state)=
 ## Final state
@@ -47,7 +98,35 @@ InvalidDefinition: Cannot declare transitions from final state. Invalid state(s)
 
 ```
 
-You can retrieve all final states.
+If you mark any states as final, `StateMachine` will check that all non-final states have a path to reach at least one final state.
+
+```{note}
+This will currently issue a warning, but can be turned into an exception by setting `strict_states=True` on the class.
+```
+
+```py
+>>> class CampaignMachine(StateMachine, strict_states=True):
+...     "A workflow machine"
+...     draft = State('Draft', initial=True, value=1)
+...     producing = State('Being produced', value=2)
+...     abandoned = State('Abandoned', value=3)
+...     closed = State('Closed', final=True, value=4)
+...
+...     add_job = draft.to.itself() | producing.to.itself()
+...     produce = draft.to(producing)
+...     abandon = producing.to(abandoned) | abandoned.to(abandoned)
+...     deliver = producing.to(closed)
+Traceback (most recent call last):
+...
+InvalidDefinition: All non-final states should have at least one path to a final state. These states have no path to a final state: ['abandoned']
+
+```
+
+```{warning}
+`strict_states=True` will become the default behaviour in future versions.
+```
+
+You can query a list of all final states from your statemachine.
 
 ```py
 >>> class CampaignMachine(StateMachine):
@@ -65,6 +144,9 @@ You can retrieve all final states.
 >>> machine.final_states
 [State('Closed', id='closed', value=3, initial=False, final=True)]
 
+>>> machine.current_state in machine.final_states
+False
+
 ```
 
 ## States from Enum types

docs/transitions.md

@@ -219,8 +219,8 @@ an action that `echoes` back the parameters informed.
 ```
 
 
-This action is executed before the transition associated with `cycle` event is activated, so you
-can also raise an exception at this point to stop a transition to occur.
+This action is executed before the transition associated with `cycle` event is activated.
+You can raise an exception at this point to stop a transition from completing.
 
 ```py
 >>> machine.current_state.id

statemachine/factory.py

@@ -1,3 +1,4 @@
+import warnings
 from typing import TYPE_CHECKING
 from typing import Any
 from typing import Dict
@@ -18,7 +19,15 @@
 
 
 class StateMachineMetaclass(type):
-    def __init__(cls, name: str, bases: Tuple[type], attrs: Dict[str, Any]):
+    "Metaclass for constructing StateMachine classes"
+
+    def __init__(
+        cls,
+        name: str,
+        bases: Tuple[type],
+        attrs: Dict[str, Any],
+        strict_states: bool = False,
+    ) -> None:
         super().__init__(name, bases, attrs)
         registry.register(cls)
         cls.name = cls.__name__
@@ -27,6 +36,7 @@ def __init__(cls, name: str, bases: Tuple[type], attrs: Dict[str, Any]):
         """Map of ``state.value`` to the corresponding :ref:`state`."""
 
         cls._abstract = True
+        cls._strict_states = strict_states
         cls._events: Dict[str, Event] = {}
 
         cls.add_inherited(bases)
@@ -66,14 +76,16 @@ def _check(cls):
         cls._check_initial_state()
         cls._check_final_states()
         cls._check_disconnected_state()
+        cls._check_trap_states()
+        cls._check_reachable_final_states()
 
     def _check_initial_state(cls):
         initials = [s for s in cls.states if s.initial]
         if len(initials) != 1:
             raise InvalidDefinition(
                 _(
                     "There should be one and only one initial state. "
-                    "Your currently have these: {!r}"
+                    "You currently have these: {!r}"
                 ).format([s.id for s in initials])
             )
 
@@ -89,6 +101,40 @@ def _check_final_states(cls):
                 ).format([s.id for s in final_state_with_invalid_transitions])
             )
 
+    def _check_trap_states(cls):
+        trap_states = [s for s in cls.states if not s.final and not s.transitions]
+        if trap_states:
+            message = _(
+                "All non-final states should have at least one outgoing transition. "
+                "These states have no outgoing transition: {!r}"
+            ).format([s.id for s in trap_states])
+            if cls._strict_states:
+                raise InvalidDefinition(message)
+            else:
+                warnings.warn(message, UserWarning, stacklevel=1)
+
+    def _check_reachable_final_states(cls):
+        if not any(s.final for s in cls.states):
+            return  # No need to check final reachability
+        disconnected_states = cls._states_without_path_to_final_states()
+        if disconnected_states:
+            message = _(
+                "All non-final states should have at least one path to a final state. "
+                "These states have no path to a final state: {!r}"
+            ).format([s.id for s in disconnected_states])
+            if cls._strict_states:
+                raise InvalidDefinition(message)
+            else:
+                warnings.warn(message, UserWarning, stacklevel=1)
+
+    def _states_without_path_to_final_states(cls):
+        return [
+            state
+            for state in cls.states
+            if not state.final
+            and not any(s.final for s in visit_connected_states(state))
+        ]
+
     def _disconnected_states(cls, starting_state):
         visitable_states = set(visit_connected_states(starting_state))
         return set(cls.states) - visitable_states

statemachine/statemachine.py

@@ -84,6 +84,10 @@ def __init__(
         self._setup(initial_transition)
         self._activate_initial_state(initial_transition)
 
+    def __init_subclass__(cls, strict_states: bool = False):
+        cls._strict_states = strict_states
+        super().__init_subclass__()
+
     if TYPE_CHECKING:
         """Makes mypy happy with dynamic created attributes"""
 

tests/conftest.py

@@ -23,7 +23,7 @@ class CampaignMachine(StateMachine):
         "A workflow machine"
         draft = State(initial=True)
         producing = State("Being produced")
-        closed = State()
+        closed = State(final=True)
 
         add_job = draft.to(draft) | producing.to(producing)
         produce = draft.to(producing)
@@ -42,7 +42,7 @@ class CampaignMachine(StateMachine):
         "A workflow machine"
         draft = State(initial=True)
         producing = State("Being produced")
-        closed = State()
+        closed = State(final=True)
 
         add_job = draft.to(draft) | producing.to(producing)
         produce = draft.to(producing, validators="can_produce")
@@ -84,7 +84,7 @@ class CampaignMachineWithKeys(StateMachine):
         "A workflow machine"
         draft = State(initial=True, value=1)
         producing = State("Being produced", value=2)
-        closed = State(value=3)
+        closed = State(value=3, final=True)
 
         add_job = draft.to(draft) | producing.to(producing)
         produce = draft.to(producing)
@@ -164,7 +164,7 @@ class ApprovalMachine(StateMachine):
         accepted = State()
         rejected = State()
 
-        completed = State()
+        completed = State(final=True)
 
         validate = requested.to(accepted, cond="is_ok") | requested.to(rejected)
 

tests/test_callbacks.py

@@ -184,7 +184,7 @@ def race_uppercase(race):
         assert race_uppercase("Hobbit") == "HOBBIT"
 
     def test_decorate_unbounded_machine_methods(self):
-        class MiniHeroJourneyMachine(StateMachine):
+        class MiniHeroJourneyMachine(StateMachine, strict_states=False):
 
             ordinary_world = State(initial=True)
             call_to_adventure = State()

tests/test_multiple_destinations.py

@@ -52,8 +52,8 @@ def test_transition_to_first_that_executes_if_multiple_targets():
     class ApprovalMachine(StateMachine):
         "A workflow"
         requested = State(initial=True)
-        accepted = State()
-        rejected = State()
+        accepted = State(final=True)
+        rejected = State(final=True)
 
         validate = requested.to(accepted, rejected)
 
@@ -70,8 +70,8 @@ def never_will_pass(event_data):
     class ApprovalMachine(StateMachine):
         "A workflow"
         requested = State(initial=True)
-        accepted = State()
-        rejected = State()
+        accepted = State(final=True)
+        rejected = State(final=True)
 
         validate = (
             requested.to(accepted, cond=never_will_pass)
@@ -97,8 +97,8 @@ def test_check_invalid_reference_to_conditions():
     class ApprovalMachine(StateMachine):
         "A workflow"
         requested = State(initial=True)
-        accepted = State()
-        rejected = State()
+        accepted = State(final=True)
+        rejected = State(final=True)
 
         validate = requested.to(accepted, cond="not_found_condition") | requested.to(
             rejected
@@ -114,7 +114,7 @@ class ApprovalMachine(StateMachine):
         requested = State(initial=True)
         accepted = State()
         rejected = State()
-        completed = State()
+        completed = State(final=True)
 
         validate = (
             requested.to(accepted, cond="is_ok")
@@ -179,8 +179,8 @@ def test_multiple_values_returned_with_multiple_targets():
     class ApprovalMachine(StateMachine):
         "A workflow"
         requested = State(initial=True)
-        accepted = State()
-        denied = State()
+        accepted = State(final=True)
+        denied = State(final=True)
 
         @requested.to(accepted, denied)
         def validate(self):
@@ -206,7 +206,7 @@ def test_multiple_targets_using_or_starting_from_same_origin(
 ):
     class InvoiceStateMachine(StateMachine):
         unpaid = State(initial=True)
-        paid = State()
+        paid = State(final=True)
         failed = State()
 
         pay = (

tests/test_rtc.py

@@ -13,7 +13,7 @@ def chained_after_sm_class():  # noqa: C901
     class ChainedSM(StateMachine):
         a = State(initial=True)
         b = State()
-        c = State()
+        c = State(final=True)
 
         t1 = a.to(b, after="t1") | b.to(c)
 
@@ -52,7 +52,7 @@ class ChainedSM(StateMachine):
         s1 = State(initial=True)
         s2 = State()
         s3 = State()
-        s4 = State()
+        s4 = State(final=True)
 
         t1 = s1.to(s2)
         t2a = s2.to(s2)

tests/test_statemachine.py

@@ -54,7 +54,7 @@ def test_machine_should_activate_initial_state():
     class CampaignMachine(StateMachine):
         "A workflow machine"
         producing = State()
-        closed = State()
+        closed = State(final=True)
         draft = State(initial=True)
 
         add_job = draft.to(draft) | producing.to(producing)
@@ -381,7 +381,7 @@ def test_state_value_is_correct():
     STATE_NEW = 0
     STATE_DRAFT = 1
 
-    class ValueTestModel(StateMachine):
+    class ValueTestModel(StateMachine, strict_states=False):
         new = State(STATE_NEW, value=STATE_NEW, initial=True)
         draft = State(STATE_DRAFT, value=STATE_DRAFT)
 

tests/test_statemachine_bounded_transitions.py

@@ -18,7 +18,7 @@ def state_machine(event_mock):
     class CampaignMachine(StateMachine):
         draft = State(initial=True)
         producing = State()
-        closed = State()
+        closed = State(final=True)
 
         add_job = draft.to(draft) | producing.to(producing)
         produce = draft.to(producing)