Repository: fgmacedo/python-statemachine

Issue:
Guards and conditions passed as strings really necessary?
* Python State Machine version: 2.1.2
* Python version: 3.11.4
* Operating System: Windows 10

### Description

I wonder why we have to give the "unless" parameter and similar as strings. It hinders me from using automatic renaming in my IDE which does not recognize function names as strings.

### What I Did

So what I am using currently is following hack:
```
step |= measure_spot_center.to(move_spot_center, unless=spot_center_ok.__name__)
```
Using  `__name__`  to pass a string just to be resolved back to the functions feels very unnerving to do.
Is there a reason, why this has to be that way?


Discussion:
sonarcloud[bot]: ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=fgmacedo_python-statemachine&pullRequest=431) **Quality Gate passed**  
Issues  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [4 New issues](https://sonarcloud.io/project/issues?id=fgmacedo_python-statemachine&pullRequest=431&resolved=false&inNewCodePeriod=true)  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/component_measures?id=fgmacedo_python-statemachine&pullRequest=431&metric=new_accepted_issues&view=list)

Measures  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=fgmacedo_python-statemachine&pullRequest=431&resolved=false&inNewCodePeriod=true)  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/no-data-16px.png '') No data about Coverage  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=fgmacedo_python-statemachine&pullRequest=431&metric=new_duplicated_lines_density&view=list)  
  
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=fgmacedo_python-statemachine&pullRequest=431)


codecov[bot]: ## [Codecov](https://app.codecov.io/gh/fgmacedo/python-statemachine/pull/431?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Fernando+Macedo) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 100.00%. Comparing base [(`d195b4a`)](https://app.codecov.io/gh/fgmacedo/python-statemachine/commit/d195b4a5ab9f068e617ae529aa30214e953a50e1?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Fernando+Macedo) to head [(`5727721`)](https://app.codecov.io/gh/fgmacedo/python-statemachine/pull/431?dropdown=coverage&src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Fernando+Macedo).


<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff            @@
##           develop      #431   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           20        20           
  Lines         1174      1199   +25     
  Branches       166       175    +9     
=========================================
+ Hits          1174      1199   +25     
```

| [Flag](https://app.codecov.io/gh/fgmacedo/python-statemachine/pull/431/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Fernando+Macedo) | Coverage Δ | |
|---|---|---|
| [unittests](https://app.codecov.io/gh/fgmacedo/python-statemachine/pull/431/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Fernando+Macedo) | `100.00% <100.00%> (ø)` | |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Fernando+Macedo#carryforward-flags-in-the-pull-request-comment) to find out more.


</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/fgmacedo/python-statemachine/pull/431?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Fernando+Macedo).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Fernando+Macedo).


Patch:
diff --git a/statemachine/dispatcher.py b/statemachine/dispatcher.py
index 4c3bc54..cc1cb29 100644
--- a/statemachine/dispatcher.py
+++ b/statemachine/dispatcher.py
@@ -19,12 +19,7 @@ class ObjectConfig(namedtuple("ObjectConfig", "obj skip_attrs resolver_id")):
         if isinstance(obj, ObjectConfig):
             return obj
         else:
-            return cls(obj, set(skip_attrs) if skip_attrs else set(), str(id(obj)))
-
-    def getattr(self, attr):
-        if attr in self.skip_attrs:
-            return
-        return getattr(self.obj, attr, None)
+            return cls(obj, skip_attrs or set(), str(id(obj)))
 
 
 class WrapSearchResult:
@@ -88,29 +83,74 @@ class EventSearchResult(WrapSearchResult):
         return wrapper
 
 
-def search_callable(attr, *configs) -> Generator[WrapSearchResult, None, None]:
-    if callable(attr) or isinstance(attr, property):
-        yield CallableSearchResult(attr, attr, None)
-    else:
-        for config in configs:
-            func = config.getattr(attr)
-            if func is None:
-                continue
-            if not callable(func):
-                yield AttributeCallableSearchResult(attr, config.obj, config.resolver_id)
+def _search_callable_attr_is_property(
+    attr, configs: tuple[ObjectConfig]
+) -> "WrapSearchResult | None":
+    # if the attr is a property, we'll try to find the object that has the
+    # property on the configs
+    attr_name = attr.fget.__name__
+    for obj, _skip_attrs, resolver_id in configs:
+        func = getattr(type(obj), attr_name, None)
+        if func is not None and func is attr:
+            return AttributeCallableSearchResult(attr_name, obj, resolver_id)
+    return None
 
-            if getattr(func, "_is_sm_event", False):
-                yield EventSearchResult(attr, func, config.resolver_id)
 
-            yield CallableSearchResult(attr, func, config.resolver_id)
+def _search_callable_attr_is_callable(attr, configs: tuple[ObjectConfig]) -> WrapSearchResult:
+    # if the attr is an unbounded method, we'll try to find the bounded method
+    # on the configs
+    if not hasattr(attr, "__self__"):
+        for obj, _skip_attrs, resolver_id in configs:
+            func = getattr(obj, attr.__name__, None)
+            if func is not None and func.__func__ is attr:
+                return CallableSearchResult(attr.__name__, func, resolver_id)
 
+    return CallableSearchResult(attr, attr, None)
+
+
+def _search_callable_in_configs(
+    attr, configs: tuple[ObjectConfig]
+) -> Generator[WrapSearchResult, None, None]:
+    for obj, skip_attrs, resolver_id in configs:
+        if attr in skip_attrs:
+            continue
+
+        if not hasattr(obj, attr):
+            continue
+
+        func = getattr(obj, attr)
+        if not callable(func):
+            yield AttributeCallableSearchResult(attr, obj, resolver_id)
+
+        if getattr(func, "_is_sm_event", False):
+            yield EventSearchResult(attr, func, resolver_id)
+
+        yield CallableSearchResult(attr, func, resolver_id)
 
-def resolver_factory(*objects):
-    """Factory that returns a configured resolver."""
 
-    objects = [ObjectConfig.from_obj(obj) for obj in objects]
+def search_callable(attr, configs: tuple) -> Generator[WrapSearchResult, None, None]:  # noqa: C901
+    if isinstance(attr, property):
+        result = _search_callable_attr_is_property(attr, configs)
+        if result is not None:
+            yield result
+        return
+
+    if callable(attr):
+        yield _search_callable_attr_is_callable(attr, configs)
+        return
+
+    yield from _search_callable_in_configs(attr, configs)
+
+
+def resolver_factory(objects: tuple[ObjectConfig]):
+    """Factory that returns a configured resolver."""
 
     def wrapper(attr) -> Generator[WrapSearchResult, None, None]:
-        yield from search_callable(attr, *objects)
+        yield from search_callable(attr, objects)
 
     return wrapper
+
+
+def resolver_factory_from_objects(*objects: tuple[Any]):
+    configs = tuple(ObjectConfig.from_obj(o) for o in objects)
+    return resolver_factory(configs)
diff --git a/statemachine/signature.py b/statemachine/signature.py
index 60f5aeb..34624d7 100644
--- a/statemachine/signature.py
+++ b/statemachine/signature.py
@@ -9,6 +9,7 @@ from typing import Any
 
 def _make_key(method):
     method = method.func if isinstance(method, partial) else method
+    method = method.fget if isinstance(method, property) else method
     if isinstance(method, MethodType):
         return hash(
             (
diff --git a/statemachine/state.py b/statemachine/state.py
index 37f386a..0703d7e 100644
--- a/statemachine/state.py
+++ b/statemachine/state.py
@@ -139,6 +139,9 @@ class State:
             f"initial={self.initial!r}, final={self.final!r})"
         )
 
+    def __str__(self):
+        return self.name
+
     def __get__(self, machine, owner):
         if machine is None:
             return self
diff --git a/statemachine/statemachine.py b/statemachine/statemachine.py
index 6a436a5..2786e46 100644
--- a/statemachine/statemachine.py
+++ b/statemachine/statemachine.py
@@ -81,7 +81,6 @@ class StateMachine(metaclass=StateMachineMetaclass):
         if self._abstract:
             raise InvalidDefinition(_("There are no states or transitions."))
 
-        self._initial_transition = Transition(None, self._get_initial_state(), event="__initial__")
         self._setup()
         self._activate_initial_state()
 
@@ -125,16 +124,17 @@ class StateMachine(metaclass=StateMachineMetaclass):
         if self.current_state_value is None:
             # send an one-time event `__initial__` to enter the current state.
             # current_state = self.current_state
-            self._initial_transition.before.clear()
-            self._initial_transition.on.clear()
-            self._initial_transition.after.clear()
+            initial_transition = Transition(None, self._get_initial_state(), event="__initial__")
+            initial_transition.before.clear()
+            initial_transition.on.clear()
+            initial_transition.after.clear()
 
             event_data = EventData(
                 trigger_data=TriggerData(
                     machine=self,
-                    event=self._initial_transition.event,
+                    event=initial_transition.event,
                 ),
-                transition=self._initial_transition,
+                transition=initial_transition,
             )
             self._activate(event_data)
 
@@ -157,29 +157,31 @@ class StateMachine(metaclass=StateMachineMetaclass):
             yield state
             yield from state.transitions
 
+    def _build_register(self, observers):
+        return partial(self._callbacks_registry.register, resolver=resolver_factory(observers))
+
     def _setup(self):
-        machine = ObjectConfig.from_obj(self, skip_attrs=self._get_protected_attrs())
-        model = ObjectConfig.from_obj(self.model, skip_attrs={self.state_field})
-        add_observer_visitor = self._build_observers_visitor(machine, model)
+        register = self._build_register(
+            (
+                ObjectConfig.from_obj(self, skip_attrs=self._get_protected_attrs()),
+                ObjectConfig.from_obj(self.model, skip_attrs={self.state_field}),
+            )
+        )
         check_callbacks = self._callbacks_registry.check
 
         for visited in self._iterate_states_and_transitions():
             visited._setup()
 
         for visited in self._iterate_states_and_transitions():
-            add_observer_visitor(visited)
+            visited._add_observer(register)
 
         for visited in self._iterate_states_and_transitions():
-            visited._check_callbacks(check_callbacks)
-
-    def _build_observers_visitor(self, *observers):
-        resolver = resolver_factory(*observers)
-        _register = partial(self._callbacks_registry.register, resolver=resolver)
-
-        def add_observer_visitor(visited):
-            visited._add_observer(_register)
-
-        return add_observer_visitor
+            try:
+                visited._check_callbacks(check_callbacks)
+            except Exception as err:
+                raise InvalidDefinition(
+                    f"Error on {visited!s} when resolving callbacks: {err}"
+                ) from err
 
     def add_observer(self, *observers):
         """Add an observer.
@@ -192,10 +194,12 @@ class StateMachine(metaclass=StateMachineMetaclass):
             :ref:`observers`.
         """
         self._observers.update({o: None for o in observers})
+        observers = tuple(ObjectConfig.from_obj(o) for o in observers)
 
-        add_observer_visitor = self._build_observers_visitor(*observers)
+        register = self._build_register(observers)
         for visited in self._iterate_states_and_transitions():
-            add_observer_visitor(visited)
+            visited._add_observer(register)
+
         return self
 
     def _repr_html_(self):
diff --git a/statemachine/transition.py b/statemachine/transition.py
index e84c4ff..2c68181 100644
--- a/statemachine/transition.py
+++ b/statemachine/transition.py
@@ -73,6 +73,9 @@ class Transition:
             f"internal={self.internal!r})"
         )
 
+    def __str__(self):
+        return f"transition {self.event!s} from {self.source!s} to {self.target!s}"
+
     def _setup(self):
         before = self.before.add
         on = self.on.add


Test patch:
diff --git a/tests/examples/order_control_rich_model_machine.py b/tests/examples/order_control_rich_model_machine.py
index 8b7c2d5..35be54f 100644
--- a/tests/examples/order_control_rich_model_machine.py
+++ b/tests/examples/order_control_rich_model_machine.py
@@ -8,7 +8,7 @@ An StateMachine that demonstrates :ref:`Actions` being used on a rich model.
 
 from statemachine import State
 from statemachine import StateMachine
-from statemachine.exceptions import AttrNotFound
+from statemachine.exceptions import InvalidDefinition
 
 
 class Order:
@@ -59,11 +59,16 @@ class OrderControl(StateMachine):
 
 try:
     control = OrderControl()
-except AttrNotFound as e:
+except InvalidDefinition as e:
     assert (  # noqa: PT017
-        str(e) == "Did not found name 'payment_received' from model or statemachine"
+        str(e)
+        == (
+            "Error on transition process_order from Processing to Shipping when resolving "
+            "callbacks: Did not found name 'payment_received' from model or statemachine"
+        )
     )
 
+
 # %%
 # Now initializing with a proper ``order`` instance.
 
diff --git a/tests/test_callbacks.py b/tests/test_callbacks.py
index 5a2ecfe..7028f16 100644
--- a/tests/test_callbacks.py
+++ b/tests/test_callbacks.py
@@ -9,7 +9,7 @@ from statemachine.callbacks import CallbackMeta
 from statemachine.callbacks import CallbackMetaList
 from statemachine.callbacks import CallbacksExecutor
 from statemachine.callbacks import CallbacksRegistry
-from statemachine.dispatcher import resolver_factory
+from statemachine.dispatcher import resolver_factory_from_objects
 from statemachine.exceptions import InvalidDefinition
 
 
@@ -23,7 +23,9 @@ def ObjectWithCallbacks():
                 ["life_meaning", "name", "a_method"],
             )
             self.registry = CallbacksRegistry()
-            self.executor = self.registry.register(self.callbacks, resolver=resolver_factory(self))
+            self.executor = self.registry.register(
+                self.callbacks, resolver=resolver_factory_from_objects(self)
+            )
 
         @property
         def life_meaning(self):
@@ -49,7 +51,7 @@ class TestCallbacksMachinery:
         obj = MyObject()
 
         meta_list.add(obj.do_something)
-        executor.add(meta_list, resolver_factory(obj))
+        executor.add(meta_list, resolver_factory_from_objects(obj))
 
         executor.call(1, 2, 3, a="x", b="y")
 
@@ -80,7 +82,7 @@ class TestCallbacksMachinery:
         callbacks.add("my_method").add("other_method")
         callbacks.add("last_one")
 
-        registry.register(callbacks, resolver_factory(obj))
+        registry.register(callbacks, resolver_factory_from_objects(obj))
 
         registry[callbacks].call(1, 2, 3, a="x", b="y")
 
@@ -111,7 +113,7 @@ class TestCallbacksMachinery:
         callbacks = CallbackMetaList()
         registry = CallbacksRegistry()
 
-        register = partial(registry.register, resolver=resolver_factory(self))
+        register = partial(registry.register, resolver=resolver_factory_from_objects(self))
 
         callbacks.add(
             "this_does_no_exist",
@@ -139,7 +141,7 @@ class TestCallbacksMachinery:
             return {"key": "value"}
 
         callbacks.add([func1, func2, func3])
-        registry.register(callbacks, resolver_factory(object()))
+        registry.register(callbacks, resolver_factory_from_objects(object()))
 
         results = registry[callbacks].call(1, 2, 3, a="x", b="y")
 
@@ -170,7 +172,7 @@ class TestCallbacksAsDecorator:
         def race_uppercase(race):
             return race.upper()
 
-        x.registry.register(x.callbacks, resolver=resolver_factory(x))
+        x.registry.register(x.callbacks, resolver=resolver_factory_from_objects(x))
 
         assert x.executor.call(hero="Gandalf", race="Maia") == [
             42,
diff --git a/tests/test_dispatcher.py b/tests/test_dispatcher.py
index 5758112..0f181ae 100644
--- a/tests/test_dispatcher.py
+++ b/tests/test_dispatcher.py
@@ -1,7 +1,7 @@
 import pytest
 
 from statemachine.dispatcher import ObjectConfig
-from statemachine.dispatcher import resolver_factory
+from statemachine.dispatcher import resolver_factory_from_objects
 from statemachine.dispatcher import search_callable
 
 
@@ -46,14 +46,14 @@ class TestEnsureCallable:
     def test_return_same_object_if_already_a_callable(self):
         model = Person("Frodo", "Bolseiro")
         expected = model.get_full_name
-        actual = next(search_callable(expected)).wrap()
+        actual = next(search_callable(expected, [])).wrap()
         assert actual.__name__ == expected.__name__
         assert actual.__doc__ == expected.__doc__
 
     def test_retrieve_a_method_from_its_name(self, args, kwargs):
         model = Person("Frodo", "Bolseiro")
         expected = model.get_full_name
-        method = next(search_callable("get_full_name", ObjectConfig.from_obj(model))).wrap()
+        method = next(search_callable("get_full_name", [ObjectConfig.from_obj(model)])).wrap()
 
         assert method.__name__ == expected.__name__
         assert method.__doc__ == expected.__doc__
@@ -61,13 +61,13 @@ class TestEnsureCallable:
 
     def test_retrieve_a_callable_from_a_property_name(self, args, kwargs):
         model = Person("Frodo", "Bolseiro")
-        method = next(search_callable("first_name", ObjectConfig.from_obj(model))).wrap()
+        method = next(search_callable("first_name", [ObjectConfig.from_obj(model)])).wrap()
 
         assert method(*args, **kwargs) == "Frodo"
 
     def test_retrieve_callable_from_a_property_name_that_should_keep_reference(self, args, kwargs):
         model = Person("Frodo", "Bolseiro")
-        method = next(search_callable("first_name", ObjectConfig.from_obj(model))).wrap()
+        method = next(search_callable("first_name", [ObjectConfig.from_obj(model)])).wrap()
 
         model.first_name = "Bilbo"
 
@@ -88,7 +88,7 @@ class TestResolverFactory:
         person = Person("Frodo", "Bolseiro", "cpf")
         org = Organization("The Lord fo the Rings", "cnpj")
 
-        resolver = resolver_factory(org, person)
+        resolver = resolver_factory_from_objects(org, person)
         resolved_method = next(resolver(attr)).wrap()
         assert resolved_method() == expected_value
 
@@ -107,6 +107,6 @@ class TestResolverFactory:
 
         org_config = ObjectConfig.from_obj(org, {"get_full_name"})
 
-        resolver = resolver_factory(org_config, person)
+        resolver = resolver_factory_from_objects(org_config, person)
         resolved_method = next(resolver(attr)).wrap()
         assert resolved_method() == expected_value
diff --git a/tests/test_transition_list.py b/tests/test_transition_list.py
index acfe03b..a43a26c 100644
--- a/tests/test_transition_list.py
+++ b/tests/test_transition_list.py
@@ -2,7 +2,7 @@ import pytest
 
 from statemachine import State
 from statemachine.callbacks import CallbacksRegistry
-from statemachine.dispatcher import resolver_factory
+from statemachine.dispatcher import resolver_factory_from_objects
 
 
 def test_transition_list_or_operator():
@@ -58,6 +58,6 @@ class TestDecorators:
         transition = s1.transitions[0]
         callback_list = getattr(transition, list_attr_name)
 
-        registry.register(callback_list, resolver_factory(object()))
+        registry.register(callback_list, resolver_factory_from_objects(object()))
 
         assert registry[callback_list].call() == [expected_value]
diff --git a/tests/testcases/issue417.md b/tests/testcases/issue417.md
new file mode 100644
index 0000000..cc7bb67
--- /dev/null
+++ b/tests/testcases/issue417.md
@@ -0,0 +1,98 @@
+### Issue 417
+
+A StateMachine that exercises the derived example given on issue
+#[417](https://github.com/fgmacedo/python-statemachine/issues/417).
+
+In this example, the condition callback must be registered using a method by reference, not by it's name.
+Just to be sure, we've added a lot of variations.
+
+```py
+>>> from statemachine import State
+>>> from statemachine import StateMachine
+
+
+>>> class ExampleStateMachine(StateMachine):
+...     created = State(initial=True)
+...     started = State(final=True)
+...
+...     def can_be_started(self) -> bool:
+...         return self.counter > 0
+...
+...     @property
+...     def can_be_started_as_property(self) -> bool:
+...         return self.counter > 1
+...
+...     @property
+...     def can_be_started_as_property_str(self) -> bool:
+...         return self.counter > 2
+...
+...     start = created.to(
+...          started,
+...          cond=[can_be_started, can_be_started_as_property, "can_be_started_as_property_str"]
+...     )
+...
+...     def __init__(self, counter: bool = False):
+...         self.counter = counter
+...         super().__init__()
+...
+...     def on_start(self):
+...         print("started")
+...
+
+>>> def test_machine(counter):
+...     sm = ExampleStateMachine(counter)
+...     sm.start()
+
+```
+
+Expected output:
+
+```py
+>>> test_machine(0)
+Traceback (most recent call last):
+...
+statemachine.exceptions.TransitionNotAllowed: Can't start when in Created.
+
+>>> test_machine(3)
+started
+
+```
+
+## Invalid scenarios
+
+Should raise an exception if the property is not found on the correct objects:
+
+
+```py
+
+>>> class StrangeObject:
+...     @property
+...     def this_cannot_resolve(self) -> bool:
+...         return True
+
+
+
+>>> class ExampleStateMachine(StateMachine):
+...     created = State(initial=True)
+...     started = State(final=True)
+...
+...     start = created.to(
+...          started,
+...          cond=[StrangeObject.this_cannot_resolve]
+...     )
+...
+
+>>> def test_machine():
+...     sm = ExampleStateMachine()
+...     sm.start()
+
+```
+
+Expected output:
+
+```py
+>>> test_machine()
+Traceback (most recent call last):
+...
+statemachine.exceptions.InvalidDefinition: Error on transition start from Created to Started when resolving callbacks: Did not found name ... from model or statemachine
+```