From f9c1743bb5bab7b7dcdf5fc36bacf8b1d8b431bb Mon Sep 17 00:00:00 2001
From: ochafik <ochafik@google.com>
Date: Fri, 27 Sep 2024 03:36:49 +0100
Subject: [PATCH] `minja`: fix iterables

---
 common/minja.hpp     | 32 +++++++++++++++++++++++++++-----
 tests/test-minja.cpp |  7 ++++++-
 2 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/common/minja.hpp b/common/minja.hpp
index 91a9f669e..eaee57ed1 100644
--- a/common/minja.hpp
+++ b/common/minja.hpp
@@ -249,6 +249,7 @@ public:
   bool is_number_float() const { return primitive_.is_number_float(); }
   bool is_number() const { return primitive_.is_number(); }
   bool is_string() const { return primitive_.is_string(); }
+  bool is_iterable() const { return is_array() || is_object() || is_string(); }
 
   bool is_primitive() const { return !array_ && !object_ && !callable_; }
   bool is_hashable() const { return is_primitive(); }
@@ -262,6 +263,28 @@ public:
     return false;
   }
 
+  void for_each(const std::function<void(Value &)> & callback) const {
+    if (is_null())
+      throw std::runtime_error("Undefined value or reference");
+    if (array_) {
+      for (auto& item : *array_) {
+        callback(item);
+      }
+    } else if (object_) {
+      for (auto & item : *object_) {
+        Value key(item.first);
+        callback(key);
+      }
+    } else if (is_string()) {
+      for (char c : primitive_.get<std::string>()) {
+        auto val = Value(std::string(1, c));
+        callback(val);
+      }
+    } else {
+      throw std::runtime_error("Value is not iterable: " + dump());
+    }
+  }
+
   bool to_bool() const {
     if (is_null()) return false;
     if (is_boolean()) return get<bool>();
@@ -829,16 +852,15 @@ public:
       std::function<void(Value&)> visit = [&](Value& iter) {
           auto filtered_items = Value::array();
           if (!iter.is_null()) {
-            if (!iterable_value.is_array()) {
+            if (!iterable_value.is_iterable()) {
               throw std::runtime_error("For loop iterable must be iterable: " + iterable_value.dump());
             }
-            for (size_t i = 0, n = iter.size(); i < n; ++i) {
-                auto item = iter.at(i);
+            iterable_value.for_each([&](Value & item) {
                 destructuring_assign(var_names, context, item);
                 if (!condition || condition->evaluate(context).to_bool()) {
                   filtered_items.push_back(item);
                 }
-            }
+            });
           }
           if (filtered_items.empty()) {
             if (else_body) {
@@ -1115,7 +1137,7 @@ public:
               if (name == "number") return l.is_number();
               if (name == "string") return l.is_string();
               if (name == "mapping") return l.is_object();
-              if (name == "iterable") return l.is_array();
+              if (name == "iterable") return l.is_iterable();
               if (name == "sequence") return l.is_array();
               if (name == "defined") return !l.is_null();
               throw std::runtime_error("Unknown type for 'is' operator: " + name);
diff --git a/tests/test-minja.cpp b/tests/test-minja.cpp
index d4c66714d..e7d3265d4 100644
--- a/tests/test-minja.cpp
+++ b/tests/test-minja.cpp
@@ -119,6 +119,11 @@ static void test_error_contains(const std::string & template_str, const json & b
     cmake -B build -DCMAKE_BUILD_TYPE=Release && cmake --build build -t test-minja -j && ./build/bin/test-minja
 */
 int main() {
+    test_render(R"({{ {} is mapping }},{{ '' is mapping }})", {}, {}, "True,False");
+    test_render(R"({{ {} is iterable }},{{ '' is iterable }})", {}, {}, "True,True");   
+    test_render(R"({% for x in ["a", "b"] %}{{ x }},{% endfor %})", {}, {}, "a,b,");
+    test_render(R"({% for x in {"a": 1, "b": 2} %}{{ x }},{% endfor %})", {}, {}, "a,b,");
+    test_render(R"({% for x in "ab" %}{{ x }},{% endfor %})", {}, {}, "a,b,");
     test_render(R"({{ 'foo bar'.title() }})", {}, {}, "Foo Bar");
     test_render(R"({{ 1 | safe }})", {}, {}, "1");
     test_render(R"({{ 'abc'.endswith('bc') }},{{ ''.endswith('a') }})", {}, {}, "True,False");
@@ -261,7 +266,7 @@ int main() {
                 {{- x | tojson -}},
             {%- endfor -%}
         )", {}, {},
-        R"(1,1.2,"a",True,True,False,False,null,[],[1],[1, 2],{},{"a": 1},{"1": "b"},)");
+        R"(1,1.2,"a",true,true,false,false,null,[],[1],[1, 2],{},{"a": 1},{"1": "b"},)");
     test_render(
         R"(
             {%- set n = namespace(value=1, title='') -%}