mirror of
https://github.com/github/spec-kit.git
synced 2026-07-04 04:45:43 +08:00
fix: fail loudly on an unknown workflow expression filter (#3074)
* fix: fail loudly on an unknown workflow expression filter
The expression evaluator's filter dispatch fell through to `return value`
for any unregistered filter, so a typo'd or unsupported filter such as
`{{ items | length }}` rendered the value unchanged with no error and the
run completed — a silent wrong result.
Raise a clear ValueError instead, naming the offending filter and the valid
ones, mirroring the strict handling already used for `from_json`. The five
registered filters (default/join/map/contains/from_json) are unchanged; the
`name(arg)` form of an unknown filter is now caught too.
* fix: distinguish a misused registered filter from an unknown one; cover map
Address the review feedback on the unknown-filter fail-loud path:
- A *registered* filter used in an unsupported form (e.g. `| join` or
`| map` with no argument) raised the misleading "unknown filter
'<name>'" — the filter is registered, the syntax isn't. It now raises
a message naming it as a known filter misused. A new
`_REGISTERED_FILTERS` constant drives the distinction.
- `test_registered_filters_unaffected` now also exercises `map('attr')`,
which it previously claimed to cover but didn't. Add
`test_registered_filter_unsupported_form_raises` to pin the new path.
* fix: include the no-arg default form in the filter-error hint
Copilot review: the hint listed default('x') but omitted the valid
no-argument default form (| default), which this module supports.
This commit is contained in:
@@ -12,6 +12,19 @@ import re
|
||||
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)'.
|
||||
|
||||
@@ -342,6 +342,73 @@ class TestExpressions:
|
||||
"{{ 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
|
||||
|
||||
Reference in New Issue
Block a user