diff --git a/src/specify_cli/workflows/expressions.py b/src/specify_cli/workflows/expressions.py index 5fb036ebf2..9f504d7b31 100644 --- a/src/specify_cli/workflows/expressions.py +++ b/src/specify_cli/workflows/expressions.py @@ -552,6 +552,31 @@ def _apply_filter(value: Any, filter_expr: str, namespace: dict[str, Any]) -> An _leaf_sink: ContextVar[list[str] | None] = ContextVar("_leaf_sink", default=None) +def _is_wrapped_in_parens(text: str) -> bool: + """True when *text* is one parenthesised group, brackets and all. + + ``(a or b)`` is; ``(a) and (b)`` is not, because the opening paren closes + before the end. Quote-aware, so ``('(')`` does not count its own literal. + """ + if not (text.startswith("(") and text.endswith(")")): + return False + quote: str | None = None + depth = 0 + for index, ch in enumerate(text): + if quote is not None: + if ch == quote: + quote = None + elif ch in ("'", '"'): + quote = ch + elif ch == "(": + depth += 1 + elif ch == ")": + depth -= 1 + if depth == 0: + return index == len(text) - 1 + return False + + def _evaluate_simple_expression(expr: str, namespace: dict[str, Any]) -> Any: """Evaluate a simple expression against the namespace. @@ -573,6 +598,16 @@ def _evaluate_simple_expression(expr: str, namespace: dict[str, Any]) -> Any: if expr[:1] in ("'", '"') and expr.find(expr[0], 1) == len(expr) - 1: return expr[1:-1] + # A parenthesised group. The operator scans below deliberately skip over + # bracketed text so an operator inside a quoted or nested operand is not + # split on -- which also means nothing ever looked inside a group that + # wraps the WHOLE expression. `(a or b) and c` split at the top-level + # `and`, then evaluated `(a or b)` as a dot path, found no such key, and + # returned None: the `or` was never evaluated and the whole thing read + # false. Unwrap here so grouping means what it says. + if _is_wrapped_in_parens(expr): + return _evaluate_simple_expression(expr[1:-1], namespace) + # Handle pipe filters. Detect the pipe at the top level only, so a literal # '|' inside a quoted operand (e.g. `inputs.x == 'a|b'`) or nested brackets is # not mistaken for a filter separator — mirroring the operator parsing below. diff --git a/tests/test_workflows.py b/tests/test_workflows.py index d5869887fb..d00c3d0141 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -1003,6 +1003,33 @@ def test_boolean_literal(self): assert evaluate_expression("{{ true }}", ctx) is True assert evaluate_expression("{{ false }}", ctx) is False + def test_parenthesised_grouping(self): + """A parenthesised group is evaluated, not read as a dot path. + + The operator scans skip bracketed text so an operator inside an + operand is not split on. Nothing unwrapped a group spanning the whole + expression, so ``(a or b) and c`` split at the top-level ``and`` and + then looked up ``(a or b)`` as a key, got ``None``, and read false -- + adding parentheses to make precedence explicit silently inverted the + result. + """ + from specify_cli.workflows.expressions import evaluate_expression + from specify_cli.workflows.base import StepContext + + ctx = StepContext(inputs={"a": True, "b": False, "c": True, "n": 5}) + + assert evaluate_expression("{{ (inputs.a or inputs.b) and inputs.c }}", ctx) is True + assert evaluate_expression("{{ (inputs.b or inputs.b) and inputs.c }}", ctx) is False + assert evaluate_expression("{{ (inputs.n) }}", ctx) == 5 + assert evaluate_expression("{{ (inputs.n > 1) }}", ctx) is True + assert evaluate_expression("{{ ((inputs.n)) }}", ctx) == 5 + # A group is still only unwrapped when it spans the whole expression. + assert evaluate_expression("{{ (inputs.a) and (inputs.b) }}", ctx) is False + assert evaluate_expression("{{ (inputs.n) | default(9) }}", ctx) == 5 + # A parenthesis inside a string literal is not a group. + assert evaluate_expression("{{ 'a(b' }}", ctx) == "a(b" + assert evaluate_expression("{{ ('(') }}", ctx) == "(" + def test_list_indexing(self): from specify_cli.workflows.expressions import evaluate_expression from specify_cli.workflows.base import StepContext diff --git a/tests/unit/test_condition_expression_block.py b/tests/unit/test_condition_expression_block.py index 528798d5e2..8f85650631 100644 --- a/tests/unit/test_condition_expression_block.py +++ b/tests/unit/test_condition_expression_block.py @@ -963,20 +963,14 @@ def test_gate_reads_the_shared_indexed_segment_definition(monkeypatch): def test_gate_reports_the_leaves_the_evaluator_actually_reached(monkeypatch): """The gate's operands come from the evaluator's own walk, not a second parse. - If `_evaluate_simple_expression` stops treating something as a leaf — which - is what unwrapping a parenthesised group does — the gate stops checking it, - with no change to the gate itself. + What `_evaluate_simple_expression` treats as a leaf is what the gate checks. + Unwrapping a parenthesised group takes that group off the list, so the gate + stops checking it, with no change to the gate itself. """ grouped = "(inputs.a or inputs.b) and inputs.c" - assert expressions._unresolvable_term(grouped) is not None - - real = expressions._evaluate_simple_expression - - def unwrapping(expr, namespace): - stripped = expr.strip() - if stripped.startswith("(") and stripped.endswith(")"): - return unwrapping(stripped[1:-1], namespace) - return real(expr, namespace) - - monkeypatch.setattr(expressions, "_evaluate_simple_expression", unwrapping) assert expressions._unresolvable_term(grouped) is None + + # Stop the evaluator unwrapping the group and it becomes a leaf again, so + # the gate goes back to reporting it as a name it cannot resolve. + monkeypatch.setattr(expressions, "_is_wrapped_in_parens", lambda text: False) + assert expressions._unresolvable_term(grouped) is not None