Shortcut for for statement

lib/liquid/context.rb

@@ -12,12 +12,12 @@ module Liquid
   #
   #   context['bob']  #=> nil  class Context
   class Context
-    attr_reader :scopes, :errors, :registers, :environments, :resource_limits
+    attr_reader :scope, :errors, :registers, :environments, :resource_limits
     attr_accessor :exception_renderer, :template_name, :partial, :global_filter, :strict_variables, :strict_filters
 
     def initialize(environments = {}, outer_scope = {}, registers = {}, rethrow_errors = false, resource_limits = nil)
       @environments     = [environments].flatten
-      @scopes           = [(outer_scope || {})]
+      @scope            = outer_scope || {}
       @registers        = registers
       @errors           = []
       @partial          = false
@@ -25,8 +25,6 @@ module Liquid
       @resource_limits  = resource_limits || ResourceLimits.new(Template.default_resource_limits)
       squash_instance_assigns_with_environments
 
-      @this_stack_used = false
-
       self.exception_renderer = Template.default_exception_renderer
       if rethrow_errors
         self.exception_renderer = ->(e) { raise }
@@ -35,6 +33,8 @@ module Liquid
       @interrupts = []
       @filters = []
       @global_filter = nil
+
+      @stack_level = 0
     end
 
     def warnings
@@ -86,21 +86,9 @@ module Liquid
       strainer.invoke(method, *args).to_liquid
     end
 
-    # Push new local scope on the stack. use <tt>Context#stack</tt> instead
-    def push(new_scope = {})
-      @scopes.unshift(new_scope)
-      raise StackLevelError, "Nesting too deep".freeze if @scopes.length > Block::MAX_DEPTH
-    end
-
     # Merge a hash of variables in the current local scope
     def merge(new_scopes)
-      @scopes[0].merge!(new_scopes)
+      new_scopes.each { |k, v| self[k] = v }
-    end
-
-    # Pop from the stack. use <tt>Context#stack</tt> instead
-    def pop
-      raise ContextError if @scopes.size == 1
-      @scopes.shift
     end
 
     # Pushes a new local scope on the stack, pops it at the end of the block
@@ -111,32 +99,20 @@ module Liquid
     #   end
     #
     #   context['var]  #=> nil
-    def stack(new_scope = nil)
+    def stack(*variable_names)
-      old_stack_used = @this_stack_used
+      @stack_level += 1
-      if new_scope
+      raise StackLevelError, "Nesting too deep".freeze if @stack_level > Block::MAX_DEPTH
-        push(new_scope)
-        @this_stack_used = true
-      else
-        @this_stack_used = false
-      end
 
+      begin
         yield
       ensure
-      pop if @this_stack_used
+        @stack_level -= 1
-      @this_stack_used = old_stack_used
       end
-
-    def clear_instance_assigns
-      @scopes[0] = {}
     end
 
     # Only allow String, Numeric, Hash, Array, Proc, Boolean or <tt>Liquid::Drop</tt>
     def []=(key, value)
-      unless @this_stack_used
+      (@scope[key] ||= [nil]) << value
-        @this_stack_used = true
-        push({})
-      end
-      @scopes[0][key] = value
     end
 
     # Look up variable, either resolve directly after considering the name. We can directly handle
@@ -151,6 +127,29 @@ module Liquid
       evaluate(Expression.parse(expression))
     end
 
+    def unset(key)
+      if @scope[key].size <= 1
+        @scope.delete(key)
+      else
+        @scope[key].pop
+      end
+    end
+
+    def set_root(key, val)
+      @scope[key] ||= []
+      @scope[key][0] = val
+    end
+
+    def set_level(key, val, int)
+      @scope[key] ||= []
+      @scope[key][int] = val
+    end
+
+    def create_level(key)
+      (@scope[key] ||= [nil]) << nil
+      @scope[key].size - 1
+    end
+
     def key?(key)
       self[key] != nil
     end
@@ -161,27 +160,23 @@ module Liquid
 
     # Fetches an object starting at the local scope and then moving up the hierachy
     def find_variable(key, raise_on_not_found: true)
-      # This was changed from find() to find_index() because this is a very hot
+      trigger = false
-      # path and find_index() is optimized in MRI to reduce object allocation
+      value = @scope[key]
-      index = @scopes.find_index { |s| s.key?(key) }
+      scope = @scope unless value.nil?
-      scope = @scopes[index] if index
+      trigger = true unless value.nil?
-
-      variable = nil
 
       if scope.nil?
-        @environments.each do |e|
+        index = @environments.find_index do |e|
           variable = lookup_and_evaluate(e, key, raise_on_not_found: raise_on_not_found)
           # When lookup returned a value OR there is no value but the lookup also did not raise
           # then it is the value we are looking for.
-          if !variable.nil? || @strict_variables && raise_on_not_found
+          !variable.nil? || @strict_variables && raise_on_not_found
-            scope = e
-            break
-          end
-        end
         end
 
-      scope ||= @environments.last || @scopes.last
+        scope = @environments[index || -1]
-      variable ||= lookup_and_evaluate(scope, key, raise_on_not_found: raise_on_not_found)
+      end
+
+      variable ||= lookup_and_evaluate(scope, key, trigger, raise_on_not_found: raise_on_not_found)
 
       variable = variable.to_liquid
       variable.context = self if variable.respond_to?(:context=)
@@ -189,12 +184,16 @@ module Liquid
       variable
     end
 
-    def lookup_and_evaluate(obj, key, raise_on_not_found: true)
+    def lookup_and_evaluate(obj, key, trigger = false, raise_on_not_found: true)
       if @strict_variables && raise_on_not_found && obj.respond_to?(:key?) && !obj.key?(key)
         raise Liquid::UndefinedVariable, "undefined variable #{key}"
       end
 
-      value = obj[key]
+      value = if trigger == true
+        obj[key][-1]
+      else
+        obj[key]
+      end
 
       if value.is_a?(Proc) && obj.respond_to?(:[]=)
         obj[key] = (value.arity == 0) ? value.call : value.call(self)
@@ -213,10 +212,10 @@ module Liquid
     end
 
     def squash_instance_assigns_with_environments
-      @scopes.last.each_key do |k|
+      @scope.each_key do |k|
         @environments.each do |env|
           if env.key?(k)
-            scopes.last[k] = lookup_and_evaluate(env, k)
+            @scope[k] = [lookup_and_evaluate(env, k)]
             break
           end
         end

lib/liquid/tags/assign.rb

@@ -24,7 +24,7 @@ module Liquid
 
     def render(context)
       val = @from.render(context)
-      context.scopes.last[@to] = val
+      context.set_root(@to, val)
       context.resource_limits.assign_score += assign_score_of(val)
       ''.freeze
     end

lib/liquid/tags/capture.rb

@@ -24,7 +24,7 @@ module Liquid
 
     def render(context)
       output = super
-      context.scopes.last[@to] = output
+      context.set_root(@to, output)
       context.resource_limits.assign_score += output.bytesize
       ''.freeze
     end

lib/liquid/tags/case.rb

@@ -39,7 +39,6 @@ module Liquid
     end
 
     def render(context)
-      context.stack do
       execute_else_block = true
 
       output = ''
@@ -51,9 +50,9 @@ module Liquid
           output << block.attachment.render(context)
         end
       end
+
       output
     end
-    end
 
     private
 

lib/liquid/tags/cycle.rb

@@ -34,7 +34,6 @@ module Liquid
     def render(context)
       context.registers[:cycle] ||= {}
 
-      context.stack do
       key = context.evaluate(@name)
       iteration = context.registers[:cycle][key].to_i
       result = context.evaluate(@variables[iteration])
@@ -43,7 +42,6 @@ module Liquid
       context.registers[:cycle][key] = iteration
       result
     end
-    end
 
     private
 

lib/liquid/tags/for.rb

@@ -156,19 +156,19 @@ module Liquid
 
       result = ''
 
-      context.stack do
+      context.stack('forloop', @variable_name) do
         loop_vars = Liquid::ForloopDrop.new(@name, length, for_stack[-1])
 
         for_stack.push(loop_vars)
 
         begin
           context['forloop'.freeze] = loop_vars
+          level = context.create_level(@variable_name)
 
           segment.each do |item|
-            context[@variable_name] = item
+            context.set_level(@variable_name, item, level)
             result << @for_block.render(context)
             loop_vars.send(:increment!)
-
             # Handle any interrupts if they exist.
             if context.interrupt?
               interrupt = context.pop_interrupt
@@ -176,7 +176,10 @@ module Liquid
               next if interrupt.is_a? ContinueInterrupt
             end
           end
+          context.unset(@variable_name)
+          context.unset('forloop'.freeze)
         ensure
+
           for_stack.pop
         end
       end

lib/liquid/tags/if.rb

@@ -40,7 +40,6 @@ module Liquid
     end
 
     def render(context)
-      context.stack do
       @blocks.each do |block|
         if block.evaluate(context)
           return block.attachment.render(context)
@@ -48,7 +47,6 @@ module Liquid
       end
       ''.freeze
     end
-    end
 
     private
 

lib/liquid/tags/ifchanged.rb

@@ -1,7 +1,6 @@
 module Liquid
   class Ifchanged < Block
     def render(context)
-      context.stack do
       output = super
 
       if output != context.registers[:ifchanged]
@@ -12,7 +11,6 @@ module Liquid
       end
     end
   end
-  end
 
   Template.register_tag('ifchanged'.freeze, Ifchanged)
 end

lib/liquid/tags/include.rb

@@ -60,7 +60,7 @@ module Liquid
       begin
         context.template_name = template_name
         context.partial = true
-        context.stack do
+        context.stack(context_variable_name, *@attributes.keys) do
           @attributes.each do |key, value|
             context[key] = context.evaluate(value)
           end

lib/liquid/tags/table_row.rb

@@ -31,7 +31,7 @@ module Liquid
       cols = context.evaluate(@attributes['cols'.freeze]).to_i
 
       result = "<tr class=\"row1\">\n"
-      context.stack do
+      context.stack('tablerowloop', @variable_name) do
         tablerowloop = Liquid::TablerowloopDrop.new(length, cols)
         context['tablerowloop'.freeze] = tablerowloop
 

lib/liquid/tags/unless.rb

@@ -7,7 +7,6 @@ module Liquid
   #
   class Unless < If
     def render(context)
-      context.stack do
       # First condition is interpreted backwards ( if not )
       first_block = @blocks.first
       unless first_block.evaluate(context)
@@ -24,7 +23,6 @@ module Liquid
       ''.freeze
     end
   end
-  end
 
   Template.register_tag('unless'.freeze, Unless)
 end

performance/shopify/comment_form.rb

@@ -15,7 +15,7 @@ class CommentForm < Liquid::Block
   def render(context)
     article = context[@variable_name]
 
-    context.stack do
+    context.stack('form') do
       context['form'] = {
         'posted_successfully?' => context.registers[:posted_successfully],
         'errors' => context['comment.errors'],

performance/shopify/paginate.rb

@@ -24,7 +24,7 @@ class Paginate < Liquid::Block
   def render(context)
     @context = context
 
-    context.stack do
+    context.stack('paginate') do
       current_page  = context['current_page'].to_i
 
       pagination = {

test/integration/drop_test.rb

@@ -1,14 +1,6 @@
 require 'test_helper'
 
 class ContextDrop < Liquid::Drop
-  def scopes
-    @context.scopes.size
-  end
-
-  def scopes_as_array
-    (1..@context.scopes.size).to_a
-  end
-
   def loop_pos
     @context['forloop.index']
   end
@@ -194,31 +186,6 @@ class DropsTest < Minitest::Test
     end
   end
 
-  def test_scope
-    assert_equal '1', Liquid::Template.parse('{{ context.scopes }}').render!('context' => ContextDrop.new)
-    assert_equal '2', Liquid::Template.parse('{%for i in dummy%}{{ context.scopes }}{%endfor%}').render!('context' => ContextDrop.new, 'dummy' => [1])
-    assert_equal '3', Liquid::Template.parse('{%for i in dummy%}{%for i in dummy%}{{ context.scopes }}{%endfor%}{%endfor%}').render!('context' => ContextDrop.new, 'dummy' => [1])
-  end
-
-  def test_scope_though_proc
-    assert_equal '1', Liquid::Template.parse('{{ s }}').render!('context' => ContextDrop.new, 's' => proc{ |c| c['context.scopes'] })
-    assert_equal '2', Liquid::Template.parse('{%for i in dummy%}{{ s }}{%endfor%}').render!('context' => ContextDrop.new, 's' => proc{ |c| c['context.scopes'] }, 'dummy' => [1])
-    assert_equal '3', Liquid::Template.parse('{%for i in dummy%}{%for i in dummy%}{{ s }}{%endfor%}{%endfor%}').render!('context' => ContextDrop.new, 's' => proc{ |c| c['context.scopes'] }, 'dummy' => [1])
-  end
-
-  def test_scope_with_assigns
-    assert_equal 'variable', Liquid::Template.parse('{% assign a = "variable"%}{{a}}').render!('context' => ContextDrop.new)
-    assert_equal 'variable', Liquid::Template.parse('{% assign a = "variable"%}{%for i in dummy%}{{a}}{%endfor%}').render!('context' => ContextDrop.new, 'dummy' => [1])
-    assert_equal 'test', Liquid::Template.parse('{% assign header_gif = "test"%}{{header_gif}}').render!('context' => ContextDrop.new)
-    assert_equal 'test', Liquid::Template.parse("{% assign header_gif = 'test'%}{{header_gif}}").render!('context' => ContextDrop.new)
-  end
-
-  def test_scope_from_tags
-    assert_equal '1', Liquid::Template.parse('{% for i in context.scopes_as_array %}{{i}}{% endfor %}').render!('context' => ContextDrop.new, 'dummy' => [1])
-    assert_equal '12', Liquid::Template.parse('{%for a in dummy%}{% for i in context.scopes_as_array %}{{i}}{% endfor %}{% endfor %}').render!('context' => ContextDrop.new, 'dummy' => [1])
-    assert_equal '123', Liquid::Template.parse('{%for a in dummy%}{%for a in dummy%}{% for i in context.scopes_as_array %}{{i}}{% endfor %}{% endfor %}{% endfor %}').render!('context' => ContextDrop.new, 'dummy' => [1])
-  end
-
   def test_access_context_from_drop
     assert_equal '123', Liquid::Template.parse('{%for a in dummy%}{{ context.loop_pos }}{% endfor %}').render!('context' => ContextDrop.new, 'dummy' => [1, 2, 3])
   end

test/integration/standard_filter_test.rb

@@ -405,11 +405,11 @@ class StandardFiltersTest < Minitest::Test
   def test_map_over_drops_returning_procs
     drops = [
       {
-        "proc" => ->{ "foo" },
+        "proc" => ->{ "foo" }
       },
       {
-        "proc" => ->{ "bar" },
+        "proc" => ->{ "bar" }
-      },
+      }
     ]
     templ = '{{ drops | map: "proc" }}'
     assert_template_result "foobar", templ, "drops" => drops

test/integration/tags/for_tag_test.rb

@@ -368,6 +368,23 @@ HERE
     assert_template_result(expected, template, assigns)
   end
 
+  def test_overwriting_internal_variable
+    template = <<-HEREDOC
+    {% assign forloop = 'first' %}
+
+    {% for item in items %}
+      {{ forloop }}
+      {% assign forloop = 'second' %}
+      {{ forloop }}
+    {% endfor %}
+
+    {{ forloop }}
+    HEREDOC
+
+    result = Liquid::Template.parse(template).render('items' => '1')
+    assert_equal 'Liquid::ForloopDrop Liquid::ForloopDrop second', result.split.map(&:strip).join(' ')
+  end
+
   class LoaderDrop < Liquid::Drop
     attr_accessor :each_called, :load_slice_called
 

test/integration/tags/if_else_tag_test.rb

@@ -176,7 +176,7 @@ class IfElseTagTest < Minitest::Test
       [false, true, true] => true,
       [false, true, false] => false,
       [false, false, true] => false,
-      [false, false, false] => false,
+      [false, false, false] => false
     }
 
     tests.each do |vals, expected|

test/unit/context_unit_test.rb

@@ -102,21 +102,6 @@ class ContextUnitTest < Minitest::Test
     assert_nil @context['does_not_exist']
   end
 
-  def test_scoping
-    @context.push
-    @context.pop
-
-    assert_raises(Liquid::ContextError) do
-      @context.pop
-    end
-
-    assert_raises(Liquid::ContextError) do
-      @context.push
-      @context.pop
-      @context.pop
-    end
-  end
-
   def test_length_query
     @context['numbers'] = [1, 2, 3, 4]
 
@@ -170,18 +155,12 @@ class ContextUnitTest < Minitest::Test
 
   def test_add_item_in_outer_scope
     @context['test'] = 'test'
-    @context.push
+
-    assert_equal 'test', @context['test']
+    @context.stack('test') do
-    @context.pop
       assert_equal 'test', @context['test']
     end
 
-  def test_add_item_in_inner_scope
-    @context.push
-    @context['test'] = 'test'
     assert_equal 'test', @context['test']
-    @context.pop
-    assert_nil @context['test']
   end
 
   def test_hierachical_data