Merge pull request #562 from Shopify/use-find_variable-for-parentloop

Use custom stack for forloop references

lib/liquid/tags/for.rb

@@ -67,7 +67,8 @@ module Liquid
     end
 
     def render(context)
-      context.registers[:for] ||= Hash.new(0)
+      for_offsets = context.registers[:for] ||= Hash.new(0)
+      for_stack = context.registers[:for_stack] ||= []
 
       collection = context.evaluate(@collection_name)
       collection = collection.to_a if collection.is_a?(Range)
@@ -76,7 +77,7 @@ module Liquid
       return render_else(context) unless iterable?(collection)
 
       from = if @from == :continue
-        context.registers[:for][@name].to_i
+        for_offsets[@name].to_i
       else
         context.evaluate(@from).to_i
       end
@@ -95,14 +96,15 @@ module Liquid
       length = segment.length
 
       # Store our progress through the collection for the continue flag
-      context.registers[:for][@name] = from + segment.length
+      for_offsets[@name] = from + segment.length
 
-      parent_loop = context['forloop'.freeze]
+      parent_loop = for_stack.last
+      for_stack.push(nil)
 
       context.stack do
         segment.each_with_index do |item, index|
           context[@variable_name] = item
-          context['forloop'.freeze] = {
+          loop_vars = {
             'name'.freeze       => @name,
             'length'.freeze     => length,
             'index'.freeze      => index + 1,
@@ -114,6 +116,9 @@ module Liquid
             'parentloop'.freeze => parent_loop
           }
 
+          context['forloop'.freeze] = loop_vars
+          for_stack[-1] = loop_vars
+
           result << @for_block.render(context)
 
           # Handle any interrupts if they exist.
@@ -124,7 +129,10 @@ module Liquid
           end
         end
       end
+
       result
+    ensure
+      for_stack.pop
     end
 
     protected

test/integration/error_handling_test.rb

@@ -1,24 +1,5 @@
 require 'test_helper'
 
-class ErrorDrop < Liquid::Drop
-  def standard_error
-    raise Liquid::StandardError, 'standard error'
-  end
-
-  def argument_error
-    raise Liquid::ArgumentError, 'argument error'
-  end
-
-  def syntax_error
-    raise Liquid::SyntaxError, 'syntax error'
-  end
-
-  def exception
-    raise Exception, 'exception'
-  end
-
-end
-
 class ErrorHandlingTest < Minitest::Test
   include Liquid
 

test/integration/tags/for_tag_test.rb

@@ -387,4 +387,14 @@ HERE
     assert_template_result(expected, template, loader_assigns)
     assert_template_result(expected, template, array_assigns)
   end
+
+  def test_for_cleans_up_registers
+    context = Context.new(ErrorDrop.new)
+
+    assert_raises(StandardError) do
+      Liquid::Template.parse('{% for i in (1..2) %}{{ standard_error }}{% endfor %}').render!(context)
+    end
+
+    assert context.registers[:for_stack].empty?
+  end
 end

test/test_helper.rb

@@ -88,3 +88,22 @@ class ThingWithToLiquid
     'foobar'
   end
 end
+
+class ErrorDrop < Liquid::Drop
+  def standard_error
+    raise Liquid::StandardError, 'standard error'
+  end
+
+  def argument_error
+    raise Liquid::ArgumentError, 'argument error'
+  end
+
+  def syntax_error
+    raise Liquid::SyntaxError, 'syntax error'
+  end
+
+  def exception
+    raise Exception, 'exception'
+  end
+end
+