Treat ^ and v as operators even without surrounding whitespace

docs/releases/3.2.1.md

@@ -100,3 +100,51 @@ instead of at class-definition time.
 ```
 
 [#632](https://github.com/fgmacedo/python-statemachine/issues/632).
+
+### Operators without surrounding whitespace in `cond` expressions
+
+Boolean expressions used in `cond` / `unless` had a fast-path that returned the
+whole string as a single variable name when it looked operator-free. The check
+was too naive: it only looked for `!`, a literal space and `In(`, so any other
+operator written without surrounding whitespace was swallowed into a variable
+name. `cond="is_paid^is_shipped"` and `cond="items>0"` were resolved as
+variables literally named `is_paid^is_shipped` and `items>0`, failing with
+`InvalidDefinition: Did not found name ...` when the machine was built.
+
+The fast-path now triggers only for a lone Python identifier, so every other
+expression goes through the parser and whitespace is never required:
+
+```py
+>>> from statemachine import State, StateMachine
+
+>>> class Order(StateMachine):
+...     waiting = State(initial=True)
+...     completed = State(final=True)
+...
+...     complete = waiting.to(completed, cond="is_paid^items>0")
+...
+...     is_paid: bool = False
+...     items: int = 2
+
+>>> sm = Order()
+>>> sm.send("complete")
+Traceback (most recent call last):
+    ...
+statemachine.exceptions.TransitionNotAllowed: Can't complete when in Waiting.
+
+>>> sm.is_paid = True
+>>> sm.send("complete")
+>>> sm.completed.is_active
+True
+
+```
+
+Guards that are a single name (including a bare `v`) keep taking the fast-path,
+and `!=` keeps parsing as a comparison rather than a negation.
+
+As a side effect, a `cond` with a structure that is not valid in a boolean
+expression, such as `cond="user.age"`, now raises
+`InvalidDefinition: Failed to parse boolean expression 'user.age'` instead of
+reporting the whole string as a name that was not found.
+
+[#639](https://github.com/fgmacedo/python-statemachine/pull/639).

statemachine/dispatcher.py

@@ -14,6 +14,7 @@
 from .exceptions import InvalidDefinition
 from .i18n import _
 from .signature import SignatureAdapter
+from .spec_parser import UnsupportedExpression
 from .spec_parser import custom_and
 from .spec_parser import operator_mapping
 from .spec_parser import parse_boolean_expr
@@ -118,7 +119,10 @@ def build(self, spec: "CallbackSpec"):
 
         try:
             expression = parse_boolean_expr(spec.func, take_callback_partial, operator_mapping)
-        except SyntaxError as err:
+        except (SyntaxError, UnsupportedExpression) as err:
+            # ``UnsupportedExpression`` comes from the AST allowlist rejecting a node
+            # kind that cannot appear in a boolean expression (e.g. ``"user.age"``).
+            # Errors raised while resolving names are not parse errors and propagate.
             raise InvalidDefinition(
                 _("Failed to parse boolean expression '{}'").format(spec.func)
             ) from err

statemachine/spec_parser.py

@@ -19,6 +19,15 @@
 }
 
 
+class UnsupportedExpression(ValueError):
+    """The expression contains a structure that is not on the parser allowlist.
+
+    Distinguishes a rejected expression from errors raised by the ``variable_hook``
+    while resolving names, so callers can report each one properly. Inherits from
+    ``ValueError`` to keep the previous behavior for callers catching it.
+    """
+
+
 def _unique_key(left, right, operator) -> str:
     left_key = getattr(left, "unique_key", "")
     right_key = getattr(right, "unique_key", "")
@@ -128,7 +137,7 @@ def register(func):
     def get(cls, func_id):
         func_id = func_id.lower()
         if func_id not in cls.registry:
-            raise ValueError(f"Unsupported function: {func_id}")
+            raise UnsupportedExpression(f"Unsupported function: {func_id}")
         return cls.registry[func_id]
 
 
@@ -300,23 +309,27 @@ def recurse(child):
             # without underscore-attribute access or method calls, it cannot reach
             # type objects.
             if attr.startswith("_"):
-                raise ValueError(f"Attribute access to '{attr}' is not allowed")
+                raise UnsupportedExpression(f"Attribute access to '{attr}' is not allowed")
             return build_attribute(recurse(node.value), attr)
         case ast.Name(id=name):
             return variable_hook(name)
         case ast.Constant(value=value):
             return build_constant(value)
         case _:
-            raise ValueError(f"Unsupported expression structure: {node.__class__.__name__}")
+            raise UnsupportedExpression(
+                f"Unsupported expression structure: {node.__class__.__name__}"
+            )
 
 
 def parse_boolean_expr(expr, variable_hook, operator_mapping):
     """Parses the expression into an AST and build a custom expression tree"""
     if expr.strip() == "":
         raise SyntaxError("Empty expression")
 
-    # Optimization trying to avoid parsing the expression if not needed
-    if "!" not in expr and " " not in expr and "In(" not in expr:
+    # Optimization: a lone identifier can only be a variable name, so there is
+    # nothing to parse. Anything else (operators, comparisons, spaces, calls)
+    # goes through the parser.
+    if expr.isidentifier():
         return variable_hook(expr)
     expr = replace_operators(expr)
     tree = ast.parse(expr, mode="eval")

tests/test_conditions_algebra.py

@@ -66,3 +66,33 @@ class AnyConditionSM(StateChart):
 
     with pytest.raises(InvalidDefinition, match="Did not found name 'xxx'"):
         AnyConditionSM()
+
+
+def test_should_raise_invalid_definition_if_cond_has_unsupported_structure():
+    class AnyConditionSM(StateChart):
+        start = State(initial=True)
+        end = State(final=True)
+
+        submit = start.to(end, cond="user.age")
+
+    with pytest.raises(InvalidDefinition, match="Failed to parse boolean expression 'user.age'"):
+        AnyConditionSM()
+
+
+def test_should_not_mask_errors_raised_while_resolving_names():
+    # Resolving a name reads attributes from the model, which runs user code. An
+    # error raised there is not a parse error and must not be reported as one.
+    class Model:
+        @property
+        def is_ready(self):
+            raise ValueError("the model is not configured")
+
+    class AnyConditionSM(StateChart):
+        start = State(initial=True)
+        end = State(final=True)
+
+        submit = start.to(end, cond="is_ready")
+
+    model = Model()
+    with pytest.raises(ValueError, match="the model is not configured"):
+        AnyConditionSM(model)

tests/test_spec_parser.py

@@ -210,6 +210,35 @@ def test_classical_operators_name():
     )  # name reflects expression structure
 
 
+@pytest.mark.parametrize(
+    ("unspaced", "spaced", "expected"),
+    [
+        ("frodo_has_ring^sam_is_loyal", "frodo_has_ring ^ sam_is_loyal", True),
+        ("frodo_has_ring^sauron_alive", "frodo_has_ring ^ sauron_alive", False),
+        ("(frodo_has_ring)v(sauron_alive)", "(frodo_has_ring) v (sauron_alive)", True),
+        # "!" is an operator, but "!=" is not a negation: it must keep parsing
+        # as a comparison. Note the "!=51" (and not "!=50"): a misparse resolves
+        # the whole string as an unknown variable, which defaults to False, so a
+        # "!=50" case would pass for the wrong reason.
+        ("frodo_age!=51", "frodo_age != 51", True),
+        ("frodo_age>=50", "frodo_age >= 50", True),
+    ],
+)
+def test_operators_without_surrounding_spaces(unspaced, spaced, expected):
+    # Operators do not require surrounding whitespace, so an unspaced expression
+    # must parse just like its spaced equivalent instead of being swallowed into
+    # a single variable name.
+    got = parse_boolean_expr(unspaced, variable_hook, operator_mapping)()
+    want = parse_boolean_expr(spaced, variable_hook, operator_mapping)()
+    assert got is want is expected
+
+
+def test_bare_v_is_a_variable_name():
+    # "v" is only an operator between operands; a lone "v" is a plain variable.
+    expr = parse_boolean_expr("v", variable_hook, operator_mapping)
+    assert expr.__name__ == "v"
+
+
 def test_empty_expression():
     expr = ""
     with pytest.raises(SyntaxError):