Merge pull request #250 from wildfireapp/correct-if-nodelists

Correct if-statement nodelist.

History.md

@@ -3,6 +3,7 @@
 ## 3.0.0 / not yet released / branch "master"
 
 * ...
+* Make if, for & case tags return complete and consistent nodelists, see #250 [Nick Jones, dntj]
 * Prevent arbitrary method invocation on condition objects, see #274 [Dylan Thacker-Smith, dylanahsmith]
 * Don't call to_sym when creating conditions for security reasons, see #273 [Bouke van der Bijl, bouk]
 * Fix resource counting bug with respond_to?(:length), see #263 [Florian Weingarten, fw42]

lib/liquid/tag.rb

@@ -1,7 +1,7 @@
 module Liquid
   class Tag
-    attr_accessor :nodelist, :options
+    attr_accessor :options
-    attr_reader :warnings
+    attr_reader :nodelist, :warnings
 
     def self.new_with_options(tag_name, markup, tokens, options)
       # Forgive me Matz for I have sinned. I know this code is weird

lib/liquid/tags/case.rb

@@ -15,6 +15,10 @@ module Liquid
       super
     end
 
+    def nodelist
+      @blocks.map(&:attachment).flatten
+    end
+
     def unknown_tag(tag, markup, tokens)
       @nodelist = []
       case tag

lib/liquid/tags/for.rb

@@ -52,6 +52,14 @@ module Liquid
       super
     end
 
+    def nodelist
+      if @else_block
+        @for_block + @else_block
+      else
+        @for_block
+      end
+    end
+
     def unknown_tag(tag, markup, tokens)
       return super unless tag == 'else'
       @nodelist = @else_block = []

lib/liquid/tags/if.rb

@@ -20,6 +20,10 @@ module Liquid
       super
     end
 
+    def nodelist
+      @blocks.map(&:attachment).flatten
+    end
+
     def unknown_tag(tag, markup, tokens)
       if ['elsif', 'else'].include?(tag)
         push_block(tag, markup)

test/liquid/tags/case_tag_test.rb

@@ -0,0 +1,10 @@
+require 'test_helper'
+
+class CaseTagTest < Test::Unit::TestCase
+  include Liquid
+
+  def test_case_nodelist
+    template = Liquid::Template.parse('{% case var %}{% when true %}WHEN{% else %}ELSE{% endcase %}')
+    assert_equal ['WHEN', 'ELSE'], template.root.nodelist[0].nodelist
+  end
+end # CaseTest

test/liquid/tags/for_tag_test.rb

@@ -295,6 +295,16 @@ HERE
     assert_template_result(expected, template, assigns)
   end
 
+  def test_for_nodelist
+    template = Liquid::Template.parse('{% for item in items %}FOR{% endfor %}')
+    assert_equal ['FOR'], template.root.nodelist[0].nodelist
+  end
+
+  def test_for_else_nodelist
+    template = Liquid::Template.parse('{% for item in items %}FOR{% else %}ELSE{% endfor %}')
+    assert_equal ['FOR', 'ELSE'], template.root.nodelist[0].nodelist
+  end
+
   class LoaderDrop < Liquid::Drop
     attr_accessor :each_called, :load_slice_called
 

test/liquid/tags/if_else_tag_test.rb

@@ -158,6 +158,11 @@ class IfElseTagTest < Test::Unit::TestCase
                            %({% if 'gnomeslab-and-or-liquid' contains 'gnomeslab-and-or-liquid' %}yes{% endif %}))
   end
 
+  def test_if_nodelist
+    template = Liquid::Template.parse('{% if true %}IF{% else %}ELSE{% endif %}')
+    assert_equal ['IF', 'ELSE'], template.root.nodelist[0].nodelist
+  end
+
   def test_operators_are_whitelisted
     assert_raise(SyntaxError) do
       assert_template_result('', %({% if 1 or throw or or 1 %}yes{% endif %}))