From ca2e2037b68ae7d821113d538110e6ce6903864c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sigbj=C3=B8rn=20Skj=C3=A6ret?= Date: Wed, 30 Sep 2026 15:33:26 +0200 Subject: [PATCH] jinja : support coerced array attributes (#29574) * support coerced array attributes * add tests --- common/jinja/runtime.cpp | 10 ++-- common/jinja/value.cpp | 119 +++++++++++++++++++++------------------ common/jinja/value.h | 6 ++ tests/test-jinja.cpp | 56 ++++++++++++++++-- 4 files changed, 125 insertions(+), 66 deletions(-) diff --git a/common/jinja/runtime.cpp b/common/jinja/runtime.cpp index e2cab8aa64..a44d63c0cc 100644 --- a/common/jinja/runtime.cpp +++ b/common/jinja/runtime.cpp @@ -888,7 +888,7 @@ value member_expression::execute_impl(context & ctx) const { JJ_DEBUG("Accessed property '%s' value, got type: %s", key.c_str(), val->type().c_str()); } else if (is_val(object) || is_val(object)) { - if (is_val(property)) { + if (is_val(property) || is_val(property)) { int64_t index = property->as_int(); JJ_DEBUG("Accessing %s index %d", object->type().c_str(), (int)index); if (is_val(object)) { @@ -911,8 +911,6 @@ value member_expression::execute_impl(context & ctx) const { JJ_DEBUG("Accessing %s built-in '%s'", is_val(object) ? "array" : "string", key.c_str()); val = try_builtin_func(ctx, key, object, true); - } else { - throw std::runtime_error("Cannot access property with non-string/non-number: got " + property->type()); } } else { if (!is_val(property)) { @@ -926,10 +924,10 @@ value member_expression::execute_impl(context & ctx) const { value_t::stats_t::mark_used(val); value_t::stats_t::mark_used(object); value_t::stats_t::mark_used(property); - if (is_val(property)) { - object->stats.ops.insert("array_access"); - } else if (is_val(property)) { + if (is_val(object) || is_val(property) || is_val(property) || is_val(property) || is_val(property)) { object->stats.ops.insert("object_access"); + } else if (is_val(property) || is_val(property)) { + object->stats.ops.insert("array_access"); } } diff --git a/common/jinja/value.cpp b/common/jinja/value.cpp index 10130a6b02..6edd8b96f2 100644 --- a/common/jinja/value.cpp +++ b/common/jinja/value.cpp @@ -149,6 +149,13 @@ static value test_type_fn(const func_args & args) { JJ_DEBUG("test_type_fn: type=%s, %s or %s result=%d", typeid(T).name(), typeid(U).name(), typeid(V).name(), is_type ? 1 : 0); return mk_val(is_type); } +template +static value test_type_fn(const func_args & args) { + args.ensure_count(1); + bool is_type = is_val(args.get_pos(0)) || is_val(args.get_pos(0)) || is_val(args.get_pos(0)) || is_val(args.get_pos(0)); + JJ_DEBUG("test_type_fn: type=%s, %s, %s or %s result=%d", typeid(T).name(), typeid(U).name(), typeid(V).name(), typeid(W).name(), is_type ? 1 : 0); + return mk_val(is_type); +} template static value test_compare_fn(const func_args & args) { args.ensure_count(2, 2); @@ -261,6 +268,30 @@ static value tojson(const func_args & args) { return mk_val(json_str); } +static value & get_attribute(const value & val, const value & attr, value & default_val) { + if (!attr->is_undefined()) { + if (is_val(val)) { + value idx = attr; + + if (is_val(attr)) { + const std::string s = attr->as_string().str(); + if (!s.empty() && std::all_of(s.begin(), s.end(), [](unsigned char c) { return std::isdigit(c); })) { + try { + idx = mk_val(std::stoll(s)); + } catch (...) { + idx = mk_val(); + } + } + } + + return val->at(idx, default_val); + } else if (is_val(val)) { + return val->at(attr, default_val); + } + } + return default_val; +} + template static value selectattr(const func_args & args) { args.ensure_count(2, 4); @@ -274,10 +305,7 @@ static value selectattr(const func_args & args) { if (args.count() == 2) { // example: array | selectattr("active") for (const auto & item : arr) { - if (!is_val(item)) { - throw raised_exception("selectattr: item is not an object"); - } - value attr_val = item->at(attribute, val_default); + value attr_val = get_attribute(item, attribute, val_default); bool is_selected = attr_val->as_bool(); if constexpr (is_reject) is_selected = !is_selected; if (is_selected) out->push_back(item); @@ -318,10 +346,7 @@ static value selectattr(const func_args & args) { } auto test_fn = it->second; for (const auto & item : arr) { - if (!is_val(item)) { - throw raised_exception("selectattr: item is not an object"); - } - value attr_val = item->at(attribute, val_default); + value attr_val = get_attribute(item, attribute, val_default); func_args test_args(args.ctx); test_args.push_back(attr_val); // attribute value test_args.push_back(extra_arg); // extra argument @@ -478,8 +503,8 @@ const func_builtins & global_builtins() { {"test_is_integer", test_type_fn}, {"test_is_float", test_type_fn}, {"test_is_number", test_type_fn}, - {"test_is_iterable", test_type_fn}, - {"test_is_sequence", test_type_fn}, + {"test_is_iterable", test_type_fn}, + {"test_is_sequence", test_type_fn}, {"test_is_mapping", test_type_fn}, {"test_is_lower", [](const func_args & args) -> value { args.ensure_vals(); @@ -1068,22 +1093,14 @@ const func_builtins & value_array_t::get_builtins() const { } value val_delim = args.get_kwarg_or_pos("d", 1); value attribute = args.get_kwarg_or_pos("attribute", 2); + value undef = mk_val(); const auto & arr = args.get_pos(0)->as_array(); - const bool attr_is_int = is_val(attribute); - if (!attribute->is_undefined() && !is_val(attribute) && !attr_is_int) { - throw raised_exception("join() attribute must be string or integer"); - } - const int64_t attr_int = attr_is_int ? attribute->as_int() : 0; const std::string delim = val_delim->is_undefined() ? "" : val_delim->as_string().str(); std::string result; for (size_t i = 0; i < arr.size(); ++i) { value val_arr = arr[i]; if (!attribute->is_undefined()) { - if (attr_is_int && is_val(val_arr)) { - val_arr = val_arr->at(attr_int); - } else if (!attr_is_int && is_val(val_arr)) { - val_arr = val_arr->at(attribute); - } + val_arr = get_attribute(val_arr, attribute, undef); } if (!is_val(val_arr) && !is_val(val_arr) && !is_val(val_arr)) { throw raised_exception("join() can only join arrays of strings or numerics"); @@ -1115,21 +1132,11 @@ const func_builtins & value_array_t::get_builtins() const { } value val = args.get_pos(0); value attribute = args.get_kwarg_or_pos("attribute", 1); - const bool attr_is_int = is_val(attribute); - if (!is_val(attribute) && !attr_is_int) { - throw raised_exception("map: attribute must be string or integer"); - } - const int64_t attr_int = attr_is_int ? attribute->as_int() : 0; value default_val = args.get_kwarg("default", mk_val()); auto out = mk_val(); auto arr = val->as_array(); for (const auto & item : arr) { - value attr_val; - if (attr_is_int) { - attr_val = is_val(item) ? item->at(attr_int, default_val) : default_val; - } else { - attr_val = is_val(item) ? item->at(attribute, default_val) : default_val; - } + value attr_val = get_attribute(item, attribute, default_val); out->push_back(attr_val); } return is_val(val) ? mk_val(std::move(out->as_array())) : out; @@ -1166,22 +1173,14 @@ const func_builtins & value_array_t::get_builtins() const { // FIXME: sorting is currently always case sensitive //const bool case_sensitive = val_case->as_bool(); // undefined == false const bool reverse = val_reverse->as_bool(); // undefined == false - const bool attr_is_int = is_val(attribute); - const int64_t attr_int = attr_is_int ? attribute->as_int() : 0; + value undef = mk_val(); std::vector arr = val->as_array(); // copy std::sort(arr.begin(), arr.end(),[&](const value & a, const value & b) { value val_a = a; value val_b = b; if (!attribute->is_undefined()) { - if (attr_is_int && is_val(a) && is_val(b)) { - val_a = a->at(attr_int); - val_b = b->at(attr_int); - } else if (!attr_is_int && is_val(a) && is_val(b)) { - val_a = a->at(attribute); - val_b = b->at(attribute); - } else { - throw raised_exception("sort: unsupported object attribute comparison between " + a->type() + " and " + b->type()); - } + val_a = get_attribute(a, attribute, undef); + val_b = get_attribute(b, attribute, undef); } return value_compare(val_a, val_b, reverse ? value_compare_op::gt : value_compare_op::lt); }); @@ -1199,19 +1198,23 @@ const func_builtins & value_array_t::get_builtins() const { args.ensure_vals(); value val_case = args.get_kwarg_or_pos("case_sensitive", 1); value attribute = args.get_kwarg_or_pos("attribute", 2); - if (!attribute->is_undefined()) { - throw not_implemented_exception("min: attribute not implemented"); - } // FIXME: min is currently always case sensitive (void) val_case; + value undef = mk_val(); const auto & arr = args.get_pos(0)->as_array(); if (arr.empty()) { - return mk_val(); + return undef; } value result = arr[0]; - for (size_t i = 1; i < arr.size(); ++i) { - if (value_compare(arr[i], result, value_compare_op::lt)) { - result = arr[i]; + for (const auto & item : arr) { + value val_arr = item; + value val_cmp = result; + if (!attribute->is_undefined()) { + val_arr = get_attribute(val_arr, attribute, undef); + val_cmp = get_attribute(val_cmp, attribute, undef); + } + if (value_compare(val_arr, val_cmp, value_compare_op::lt)) { + result = item; } } return result; @@ -1221,19 +1224,23 @@ const func_builtins & value_array_t::get_builtins() const { args.ensure_vals(); value val_case = args.get_kwarg_or_pos("case_sensitive", 1); value attribute = args.get_kwarg_or_pos("attribute", 2); - if (!attribute->is_undefined()) { - throw not_implemented_exception("max: attribute not implemented"); - } // FIXME: max is currently always case sensitive (void) val_case; + value undef = mk_val(); const auto & arr = args.get_pos(0)->as_array(); if (arr.empty()) { - return mk_val(); + return undef; } value result = arr[0]; - for (size_t i = 1; i < arr.size(); ++i) { - if (value_compare(arr[i], result, value_compare_op::gt)) { - result = arr[i]; + for (const auto & item : arr) { + value val_arr = item; + value val_cmp = result; + if (!attribute->is_undefined()) { + val_arr = get_attribute(val_arr, attribute, undef); + val_cmp = get_attribute(val_cmp, attribute, undef); + } + if (value_compare(val_arr, val_cmp, value_compare_op::gt)) { + result = item; } } return result; diff --git a/common/jinja/value.h b/common/jinja/value.h index 4926fb6801..142943dfd7 100644 --- a/common/jinja/value.h +++ b/common/jinja/value.h @@ -433,6 +433,12 @@ struct value_array_t : public value_t { } return val_arr[index]; } + virtual value & at(const value & index, value & default_val) override { + if (!is_val(index) && !is_val(index)) { + return default_val; + } + return at(index->as_int(), default_val); + } virtual const func_builtins & get_builtins() const override; virtual bool is_hashable() const override { if (std::all_of(val_arr.begin(), val_arr.end(), [&](auto & val) -> bool { diff --git a/tests/test-jinja.cpp b/tests/test-jinja.cpp index 891b785c4f..7649e36246 100644 --- a/tests/test-jinja.cpp +++ b/tests/test-jinja.cpp @@ -452,6 +452,18 @@ static void test_expressions(testing & t) { "c" ); + test_template(t, "array bool access", + "{{ items[true] }}", + {{"items", json::array({"a", "b", "c"})}}, + "b" + ); + + test_template(t, "array non-index access", + "{{ items[1.0] is undefined }}", + {{"items", json::array({"a", "b", "c"})}}, + "True" + ); + test_template(t, "array slice", "{{ items[1:-1]|string }}", {{"items", json::array({"a", "b", "c"})}}, @@ -735,6 +747,16 @@ static void test_filters(testing & t) { "xyz" ); + test_template(t, "sort with numeric-like attribute", + "{{ items|sort(attribute='01')|join(attribute=1) }}", + {{"items", json::array({ + json::array({1, "z"}), + json::array({2, "x"}), + json::array({3, "y"}), + })}}, + "xyz" + ); + test_template(t, "join", "{{ items|join(', ') }}", {{"items", json::array({"a", "b", "c"})}}, @@ -1590,6 +1612,16 @@ static void test_array_methods(testing & t) { "b c " ); + test_template(t, "array|selectattr numeric-like with operator", + "{% for item in items|selectattr('0', 'gt', 1) %}{{ item.1 }} {% endfor %}", + {{"items", json::array({ + json::array({3, "z"}), + json::array({1, "x"}), + json::array({2, "y"}), + })}}, + "z y " + ); + test_template(t, "array|tojson", "{{ arr|tojson }}", {{"arr", json::array({1, 2, 3})}}, @@ -1654,6 +1686,12 @@ static void test_array_methods(testing & t) { "123" ); + test_template(t, "array|join numeric-like attribute", + "{{ arr|join(attribute='0') }}", + {{"arr", json::array({json::array({1}), json::array({2}), json::array({3})})}}, + "123" + ); + test_template(t, "array.pop() last", "{{ arr.pop() }}-{{ arr|join(',') }}", {{"arr", json::array({"a", "b", "c"})}}, @@ -1712,6 +1750,16 @@ static void test_array_methods(testing & t) { "10 20 30 " ); + test_template(t, "array|map with numeric-like attribute", + "{% for v in arr|map(attribute='1') %}{{ v }} {% endfor %}", + {{"arr", json::array({ + json::array({10, "x"}), + json::array({20, "y"}), + json::array({30, "z"}), + })}}, + "x y z " + ); + test_template(t, "array|map with negative attribute", "{% for v in arr|map(attribute=-1) %}{{ v }} {% endfor %}", {{"arr", json::array({ @@ -1741,21 +1789,21 @@ static void test_array_methods(testing & t) { ); test_template(t, "array|min attribute", - "{{ items|min(attribute='x') }}", + "{{ items|min(attribute='x')|tojson }}", {{"items", json::array({ json({{"x", 2}}), json({{"x", 1}}), })}}, - "{'x': 1}" + "{\"x\": 1}" ); test_template(t, "array|max attribute", - "{{ items|max(attribute='x') }}", + "{{ items|max(attribute='x')|tojson }}", {{"items", json::array({ json({{"x", 2}}), json({{"x", 1}}), })}}, - "{'x': 2}" + "{\"x\": 2}" ); // not used by any chat templates