diff --git a/src/specify_cli/workflows/expressions.py b/src/specify_cli/workflows/expressions.py index 6259b59de0..ca10b24d1b 100644 --- a/src/specify_cli/workflows/expressions.py +++ b/src/specify_cli/workflows/expressions.py @@ -12,6 +12,19 @@ from typing import Any +# The filters the expression evaluator recognizes. Used to tell a +# *registered* filter used in an unsupported form (e.g. `| join` with no +# argument) apart from a genuinely unknown filter name, so each raises an +# error that names the real problem. +_REGISTERED_FILTERS: tuple[str, ...] = ( + "default", + "join", + "map", + "contains", + "from_json", +) + + # -- Custom filters ------------------------------------------------------- def _filter_default(value: Any, default_value: Any = "") -> Any: @@ -192,7 +205,27 @@ def _evaluate_simple_expression(expr: str, namespace: dict[str, Any]) -> Any: filter_name = filter_expr.strip() if filter_name == "default": return _filter_default(value) - return value + # No recognized filter matched. Fail loudly rather than silently + # returning the unfiltered value: a passthrough turns a mis-typed or + # unsupported filter into a wrong result with no signal. Mirrors the + # strict `from_json` handling above. Distinguish a *registered* filter + # used in an unsupported form (e.g. `| join` or `| map` with no + # argument) from a genuinely unknown filter name, so the message names + # the real problem instead of calling a known filter "unknown". + leading_name = re.match(r"\w+", filter_expr) + name = leading_name.group(0) if leading_name else filter_expr + expected = ( + "expected one of default or default('x'), join('sep'), " + "map('attr'), contains('s'), or from_json" + ) + if name in _REGISTERED_FILTERS: + raise ValueError( + f"filter '{name}' used in an unsupported form (got " + f"'| {filter_expr}'): {expected}" + ) + raise ValueError( + f"unknown filter '{name}': {expected} (got '| {filter_expr}')" + ) # Boolean operators — parse 'or' first (lower precedence) so that # 'a or b and c' is evaluated as 'a or (b and c)'. diff --git a/tests/test_workflows.py b/tests/test_workflows.py index a87c09cf05..bfb3c1bbe1 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -342,6 +342,73 @@ def test_filter_from_json_rejects_malformed_forms(self): "{{ steps.emit.output.stdout | " + bad + " }}", ctx ) + def test_filter_unknown_name_raises(self): + # An unregistered filter name must fail loudly rather than silently + # returning the unfiltered value (which hides a typo / unsupported + # filter as a wrong result). + import pytest + from specify_cli.workflows.expressions import evaluate_expression + from specify_cli.workflows.base import StepContext + + ctx = StepContext(inputs={"items": [1, 2, 3]}) + with pytest.raises(ValueError, match="unknown filter 'length'"): + evaluate_expression("{{ inputs.items | length }}", ctx) + + def test_filter_unknown_name_with_args_raises(self): + # The unknown-filter path must also catch the `name(arg)` form, which + # otherwise falls through the recognized-args branch silently. + import pytest + from specify_cli.workflows.expressions import evaluate_expression + from specify_cli.workflows.base import StepContext + + ctx = StepContext(inputs={"text": "hello"}) + with pytest.raises(ValueError, match="unknown filter 'upper'"): + evaluate_expression("{{ inputs.text | upper('x') }}", ctx) + + def test_registered_filters_unaffected(self): + # Regression: all five registered filters keep working unchanged. + from specify_cli.workflows.expressions import evaluate_expression + from specify_cli.workflows.base import StepContext + + ctx = StepContext( + inputs={ + "tags": ["a", "b", "c"], + "text": "hello world", + "missing": "", + "rows": [{"id": "a"}, {"id": "b"}], + }, + steps={"emit": {"output": {"stdout": '{"n": 1}'}}}, + ) + assert ( + evaluate_expression("{{ inputs.missing | default('fb') }}", ctx) == "fb" + ) + assert evaluate_expression("{{ inputs.tags | join(', ') }}", ctx) == "a, b, c" + assert evaluate_expression("{{ inputs.rows | map('id') }}", ctx) == ["a", "b"] + assert ( + evaluate_expression("{{ inputs.text | contains('world') }}", ctx) is True + ) + assert evaluate_expression( + "{{ steps.emit.output.stdout | from_json }}", ctx + ) == {"n": 1} + + def test_registered_filter_unsupported_form_raises(self): + # A *registered* filter used in an unsupported form (e.g. `| join` with + # no argument) must fail loudly with a message that names it as a known + # filter misused, not as an "unknown filter". + import pytest + from specify_cli.workflows.expressions import evaluate_expression + from specify_cli.workflows.base import StepContext + + ctx = StepContext(inputs={"tags": ["a", "b", "c"]}) + with pytest.raises( + ValueError, match="filter 'join' used in an unsupported form" + ): + evaluate_expression("{{ inputs.tags | join }}", ctx) + with pytest.raises( + ValueError, match="filter 'map' used in an unsupported form" + ): + evaluate_expression("{{ inputs.tags | map }}", ctx) + def test_condition_evaluation(self): from specify_cli.workflows.expressions import evaluate_condition from specify_cli.workflows.base import StepContext