From 7c8a269b4d5ef134834019bcc026b2043512669e Mon Sep 17 00:00:00 2001
From: Michael Go <michael.go@shopify.com>
Date: Thu, 14 Nov 2024 15:26:43 -0400
Subject: [PATCH] fix cycle tag not resetting

---
 lib/liquid/tags/cycle.rb                |  8 ++++-
 test/integration/tags/cycle_tag_test.rb | 48 +++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 1 deletion(-)
 create mode 100644 test/integration/tags/cycle_tag_test.rb

diff --git a/lib/liquid/tags/cycle.rb b/lib/liquid/tags/cycle.rb
index 06d788de..c2d94d5f 100644
--- a/lib/liquid/tags/cycle.rb
+++ b/lib/liquid/tags/cycle.rb
@@ -68,7 +68,13 @@ module Liquid
     def variables_from_string(markup)
       markup.split(',').collect do |var|
         var =~ /\s*(#{QuotedFragment})\s*/o
-        Regexp.last_match(1) ? parse_expression(Regexp.last_match(1)) : nil
+        next unless Regexp.last_match(1)
+
+        # Expression Parser returns cached objects, and we need to dup them to
+        # start the cycle over for each new cycle call.
+        # Liquid-C does not have a cache, so we don't need to dup the object.
+        var = parse_expression(Regexp.last_match(1))
+        var.is_a?(VariableLookup) ? var.dup : var
       end.compact
     end
 
diff --git a/test/integration/tags/cycle_tag_test.rb b/test/integration/tags/cycle_tag_test.rb
new file mode 100644
index 00000000..a034db17
--- /dev/null
+++ b/test/integration/tags/cycle_tag_test.rb
@@ -0,0 +1,48 @@
+# frozen_string_literal: true
+
+require 'test_helper'
+
+class CycleTagTest < Minitest::Test
+  def test_simple_cycle
+    template = <<~LIQUID
+      {%- cycle '1', '2', '3' -%}
+      {%- cycle '1', '2', '3' -%}
+      {%- cycle '1', '2', '3' -%}
+    LIQUID
+
+    assert_template_result("123", template)
+  end
+
+  def test_simple_cycle_inside_for_loop
+    template = <<~LIQUID
+      {%- for i in (1..3) -%}
+        {% cycle '1', '2', '3' %}
+      {%- endfor -%}
+    LIQUID
+
+    assert_template_result("123", template)
+  end
+
+  def test_cycle_with_variables_inside_for_loop
+    template = <<~LIQUID
+      {%- assign a = 1 -%}
+      {%- assign b = 2 -%}
+      {%- assign c = 3 -%}
+      {%- for i in (1..3) -%}
+        {% cycle a, b, c %}
+      {%- endfor -%}
+    LIQUID
+
+    assert_template_result("123", template)
+  end
+
+  def test_cycle_tag_always_resets_cycle
+    template = <<~LIQUID
+      {%- assign a = "1" -%}
+      {%- cycle a, "2" -%}
+      {%- cycle a, "2" -%}
+    LIQUID
+
+    assert_template_result("11", template)
+  end
+end