diff --git a/src/specify_cli/workflows/expressions.py b/src/specify_cli/workflows/expressions.py index 38a29890ae..e30346f019 100644 --- a/src/specify_cli/workflows/expressions.py +++ b/src/specify_cli/workflows/expressions.py @@ -400,6 +400,22 @@ def _apply_filter(value: Any, filter_expr: str, namespace: dict[str, Any]) -> An # branch above. The greedy ``.+`` still handles literal ``)`` and ``|`` # inside quoted args. filter_match = re.fullmatch(r"(\w+)\((.+)\)", filter_expr) + # A multi-argument call is not a supported form: every filter here takes + # exactly one argument, and the whole captured argument text was handed to + # ``_evaluate_simple_expression`` as ONE expression. "1, 2" is not a valid + # expression, so it evaluated to None -- making ``default(1, 2)`` return + # None (silently wrong) and ``join(",", "extra")`` raise a message blaming + # the separator rather than the extra argument. Fall through to the + # unsupported-form error below instead, which names the filter and lists + # the accepted forms. + # + # Use ``_find_top_level``, the same scanner the operator splitting uses: it + # skips commas inside quotes AND inside nested brackets, so a single + # argument that happens to contain a comma still works -- ``join(", ")``, + # ``default("a, b")``, and the list/dict literals the evaluator supports + # (``default([1, 2])``, ``default({"a": 1, "b": 2})``). + if filter_match and _find_top_level(filter_match.group(2), ",") != -1: + filter_match = None if filter_match: fname = filter_match.group(1) farg = _evaluate_simple_expression(filter_match.group(2).strip(), namespace) diff --git a/tests/test_workflows.py b/tests/test_workflows.py index afd70adecf..ef4aa265e2 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -711,6 +711,71 @@ def test_filter_call_with_trailing_tokens_fails_loudly(self): StepContext(inputs={"tags": ["a", "b"]}), ) + def test_multi_argument_filter_call_fails_loudly(self): + """A second argument must be reported, not silently mis-evaluated. + + The whole captured argument text was handed to + `_evaluate_simple_expression` as ONE expression. `"1, 2"` is not a valid + expression, so it evaluated to None — making `default(1, 2)` return None + (silently wrong) and `join(",", "extra")` raise a message blaming the + separator rather than the extra argument. + """ + import pytest + from specify_cli.workflows.expressions import evaluate_expression + from specify_cli.workflows.base import StepContext + + with pytest.raises(ValueError, match="unsupported form"): + evaluate_expression( + "{{ inputs.missing | default(1, 2) }}", StepContext(inputs={}) + ) + with pytest.raises(ValueError, match="unsupported form"): + evaluate_expression( + '{{ inputs.tags | join(",", "extra") }}', + StepContext(inputs={"tags": ["a", "b"]}), + ) + + def test_single_argument_containing_a_comma_still_works(self): + """The multi-argument check must skip quotes AND nested brackets. + + A single argument may legitimately contain a comma in two ways: + + * inside quotes — `join(", ")`, `default("a, b")` + * inside a bracketed literal — `default([1, 2])`, which the expression + evaluator supports and which resolves to a real list + + so the check uses `_find_top_level` (the same scanner the operator + splitting uses) rather than a quote-only scan. + """ + from specify_cli.workflows.expressions import evaluate_expression + from specify_cli.workflows.base import StepContext + + ctx = StepContext(inputs={"tags": ["a", "b"]}) + assert evaluate_expression('{{ inputs.tags | join(", ") }}', ctx) == "a, b" + assert evaluate_expression('{{ inputs.tags | join(",") }}', ctx) == "a,b" + assert ( + evaluate_expression('{{ inputs.missing | default("a, b") }}', ctx) + == "a, b" + ) + # List literals: a comma inside brackets is not an argument separator. + assert evaluate_expression( + "{{ inputs.missing | default([1, 2]) }}", ctx + ) == [1, 2] + assert evaluate_expression( + "{{ inputs.missing | default([1,2]) }}", ctx + ) == [1, 2] + assert evaluate_expression("{{ inputs.missing | default([]) }}", ctx) == [] + + def test_multi_argument_after_a_literal_is_still_rejected(self): + """A real second argument is rejected even when the first is a literal.""" + import pytest + from specify_cli.workflows.expressions import evaluate_expression + from specify_cli.workflows.base import StepContext + + with pytest.raises(ValueError, match="unsupported form"): + evaluate_expression( + "{{ inputs.missing | default([1,2], 3) }}", StepContext(inputs={}) + ) + def test_chained_filters_apply_left_to_right(self): # Filters chain: each filter's result feeds the next. `map` yields a # list and `join` is the only filter that renders a list to a string,