From d789ec4175176127ca50e0e39e77540c5d7222d6 Mon Sep 17 00:00:00 2001
From: Stephen Paul Weber <stephen.weber@shopify.com>
Date: Mon, 10 Sep 2018 13:36:27 -0400
Subject: [PATCH 1/6] Liquid::Traversal

This enables traversal over whole document tree.
---
 lib/liquid/condition.rb            |   4 +-
 lib/liquid/tags/assign.rb          |   2 +
 lib/liquid/tags/case.rb            |   2 +
 lib/liquid/tags/cycle.rb           |   2 +
 lib/liquid/tags/for.rb             |   3 +-
 lib/liquid/tags/if.rb              |  10 +-
 lib/liquid/tags/include.rb         |   2 +
 lib/liquid/tags/table_row.rb       |   2 +
 lib/liquid/traversal.rb            | 118 +++++++++++++++
 test/integration/traversal_test.rb | 234 +++++++++++++++++++++++++++++
 10 files changed, 371 insertions(+), 8 deletions(-)
 create mode 100644 lib/liquid/traversal.rb
 create mode 100644 test/integration/traversal_test.rb

diff --git a/lib/liquid/condition.rb b/lib/liquid/condition.rb
index 3e79849..72bd2ee 100644
--- a/lib/liquid/condition.rb
+++ b/lib/liquid/condition.rb
@@ -29,7 +29,7 @@ module Liquid
       @@operators
     end
 
-    attr_reader :attachment
+    attr_reader :attachment, :child_condition
     attr_accessor :left, :operator, :right
 
     def initialize(left = nil, operator = nil, right = nil)
@@ -83,7 +83,7 @@ module Liquid
 
     protected
 
-    attr_reader :child_relation, :child_condition
+    attr_reader :child_relation
 
     private
 
diff --git a/lib/liquid/tags/assign.rb b/lib/liquid/tags/assign.rb
index f6cd5fa..ee6fa76 100644
--- a/lib/liquid/tags/assign.rb
+++ b/lib/liquid/tags/assign.rb
@@ -10,6 +10,8 @@ module Liquid
   class Assign < Tag
     Syntax = /(#{VariableSignature}+)\s*=\s*(.*)\s*/om
 
+    attr_reader :to, :from
+
     def initialize(tag_name, markup, options)
       super
       if markup =~ Syntax
diff --git a/lib/liquid/tags/case.rb b/lib/liquid/tags/case.rb
index 453b4d6..f55aa61 100644
--- a/lib/liquid/tags/case.rb
+++ b/lib/liquid/tags/case.rb
@@ -3,6 +3,8 @@ module Liquid
     Syntax     = /(#{QuotedFragment})/o
     WhenSyntax = /(#{QuotedFragment})(?:(?:\s+or\s+|\s*\,\s*)(#{QuotedFragment}.*))?/om
 
+    attr_reader :blocks, :left
+
     def initialize(tag_name, markup, options)
       super
       @blocks = []
diff --git a/lib/liquid/tags/cycle.rb b/lib/liquid/tags/cycle.rb
index ad116a6..6cf77a2 100644
--- a/lib/liquid/tags/cycle.rb
+++ b/lib/liquid/tags/cycle.rb
@@ -15,6 +15,8 @@ module Liquid
     SimpleSyntax = /\A#{QuotedFragment}+/o
     NamedSyntax  = /\A(#{QuotedFragment})\s*\:\s*(.*)/om
 
+    attr_reader :variables
+
     def initialize(tag_name, markup, options)
       super
       case markup
diff --git a/lib/liquid/tags/for.rb b/lib/liquid/tags/for.rb
index 6c95624..c529aae 100644
--- a/lib/liquid/tags/for.rb
+++ b/lib/liquid/tags/for.rb
@@ -46,8 +46,7 @@ module Liquid
   class For < Block
     Syntax = /\A(#{VariableSegment}+)\s+in\s+(#{QuotedFragment}+)\s*(reversed)?/o
 
-    attr_reader :collection_name
-    attr_reader :variable_name
+    attr_reader :collection_name, :variable_name, :limit, :from
 
     def initialize(tag_name, markup, options)
       super
diff --git a/lib/liquid/tags/if.rb b/lib/liquid/tags/if.rb
index 904369d..2a91741 100644
--- a/lib/liquid/tags/if.rb
+++ b/lib/liquid/tags/if.rb
@@ -14,21 +14,23 @@ module Liquid
     ExpressionsAndOperators = /(?:\b(?:\s?and\s?|\s?or\s?)\b|(?:\s*(?!\b(?:\s?and\s?|\s?or\s?)\b)(?:#{QuotedFragment}|\S+)\s*)+)/o
     BOOLEAN_OPERATORS = %w(and or)
 
+    attr_reader :blocks
+
     def initialize(tag_name, markup, options)
       super
       @blocks = []
       push_block('if'.freeze, markup)
     end
 
+    def nodelist
+      @blocks.map(&:attachment)
+    end
+
     def parse(tokens)
       while parse_body(@blocks.last.attachment, tokens)
       end
     end
 
-    def nodelist
-      @blocks.map(&:attachment)
-    end
-
     def unknown_tag(tag, markup, tokens)
       if ['elsif'.freeze, 'else'.freeze].include?(tag)
         push_block(tag, markup)
diff --git a/lib/liquid/tags/include.rb b/lib/liquid/tags/include.rb
index a800703..a334d83 100644
--- a/lib/liquid/tags/include.rb
+++ b/lib/liquid/tags/include.rb
@@ -16,6 +16,8 @@ module Liquid
   class Include < Tag
     Syntax = /(#{QuotedFragment}+)(\s+(?:with|for)\s+(#{QuotedFragment}+))?/o
 
+    attr_reader :template_name_expr, :variable_name_expr, :attributes
+
     def initialize(tag_name, markup, options)
       super
 
diff --git a/lib/liquid/tags/table_row.rb b/lib/liquid/tags/table_row.rb
index cfdef33..99d12ec 100644
--- a/lib/liquid/tags/table_row.rb
+++ b/lib/liquid/tags/table_row.rb
@@ -2,6 +2,8 @@ module Liquid
   class TableRow < Block
     Syntax = /(\w+)\s+in\s+(#{QuotedFragment}+)/o
 
+    attr_reader :variable_name, :collection_name, :attributes
+
     def initialize(tag_name, markup, options)
       super
       if markup =~ Syntax
diff --git a/lib/liquid/traversal.rb b/lib/liquid/traversal.rb
new file mode 100644
index 0000000..339fc2c
--- /dev/null
+++ b/lib/liquid/traversal.rb
@@ -0,0 +1,118 @@
+# frozen_string_literal: true
+
+module Liquid
+  class Traversal
+    def self.for(node, callbacks = Hash.new(proc {}))
+      kase = CASES.find { |(klass, _)| node.is_a?(klass) }&.last
+      (kase || self).new(node, callbacks)
+    end
+
+    def initialize(node, callbacks)
+      @node = node
+      @callbacks = callbacks
+    end
+
+    def callback_for(*classes, &block)
+      callback = block
+      callback = ->(node, _) { block.call(node) } if block.arity.abs == 1
+      callback = ->(_, _) { block.call } if block.arity.zero?
+      classes.each { |klass| @callbacks[klass] = callback }
+      self
+    end
+
+    def traverse(context = nil)
+      children.map do |node|
+        item, new_context = @callbacks[node.class].call(node, context)
+        [
+          item,
+          Traversal.for(node, @callbacks).traverse(new_context || context)
+        ]
+      end
+    end
+
+    protected
+
+    def children
+      @node.respond_to?(:nodelist) ? Array(@node.nodelist) : []
+    end
+
+    class Assign < Traversal
+      def children
+        [@node.from]
+      end
+    end
+
+    class Case < Traversal
+      def children
+        [@node.left] + @node.blocks
+      end
+    end
+
+    class Condition < Traversal
+      def children
+        [
+          @node.left, @node.right,
+          @node.child_condition, @node.attachment
+        ].compact
+      end
+    end
+
+    class Cycle < Traversal
+      def children
+        Array(@node.variables)
+      end
+    end
+
+    class For < Traversal
+      def children
+        (super + [@node.limit, @node.from, @node.collection_name]).compact
+      end
+    end
+
+    class If < Traversal
+      def children
+        @node.blocks
+      end
+    end
+
+    class Include < Traversal
+      def children
+        [
+          @node.template_name_expr,
+          @node.variable_name_expr
+        ] + @node.attributes.values
+      end
+    end
+
+    class TableRow < Traversal
+      def children
+        super + @node.attributes.values + [@node.collection_name]
+      end
+    end
+
+    class Variable < Traversal
+      def children
+        [@node.name] + @node.filters.flatten
+      end
+    end
+
+    class VariableLookup < Traversal
+      def children
+        @node.lookups
+      end
+    end
+
+    CASES = {
+      Liquid::Assign => Assign,
+      Liquid::Case => Case,
+      Liquid::Condition => Condition,
+      Liquid::Cycle => Cycle,
+      Liquid::For => For,
+      Liquid::If => If,
+      Liquid::Include => Include,
+      Liquid::TableRow => TableRow,
+      Liquid::Variable => Variable,
+      Liquid::VariableLookup => VariableLookup
+    }.freeze
+  end
+end
diff --git a/test/integration/traversal_test.rb b/test/integration/traversal_test.rb
new file mode 100644
index 0000000..6254cb9
--- /dev/null
+++ b/test/integration/traversal_test.rb
@@ -0,0 +1,234 @@
+# frozen_string_literal: true
+
+require 'test_helper'
+require 'liquid/traversal'
+
+class TraversalTest < Minitest::Test
+  include Liquid
+
+  def test_variable
+    assert_equal(
+      ["test"],
+      traversal(%({{ test }}))
+    )
+  end
+
+  def test_varible_with_filter
+    assert_equal(
+      ["test", "infilter"],
+      traversal(%({{ test | split: infilter }}))
+    )
+  end
+
+  def test_dynamic_variable
+    assert_equal(
+      ["test", "inlookup"],
+      traversal(%({{ test[inlookup] }}))
+    )
+  end
+
+  def test_if_condition
+    assert_equal(
+      ["test"],
+      traversal(%({% if test %}{% endif %}))
+    )
+  end
+
+  def test_complex_if_condition
+    assert_equal(
+      ["test"],
+      traversal(%({% if 1 == 1 and 2 == test %}{% endif %}))
+    )
+  end
+
+  def test_if_body
+    assert_equal(
+      ["test"],
+      traversal(%({% if 1 == 1 %}{{ test }}{% endif %}))
+    )
+  end
+
+  def test_unless_condition
+    assert_equal(
+      ["test"],
+      traversal(%({% unless test %}{% endunless %}))
+    )
+  end
+
+  def test_complex_unless_condition
+    assert_equal(
+      ["test"],
+      traversal(%({% unless 1 == 1 and 2 == test %}{% endunless %}))
+    )
+  end
+
+  def test_unless_body
+    assert_equal(
+      ["test"],
+      traversal(%({% unless 1 == 1 %}{{ test }}{% endunless %}))
+    )
+  end
+
+  def test_elsif_condition
+    assert_equal(
+      ["test"],
+      traversal(%({% if 1 == 1 %}{% elsif test %}{% endif %}))
+    )
+  end
+
+  def test_complex_elsif_condition
+    assert_equal(
+      ["test"],
+      traversal(%({% if 1 == 1 %}{% elsif 1 == 1 and 2 == test %}{% endif %}))
+    )
+  end
+
+  def test_elsif_body
+    assert_equal(
+      ["test"],
+      traversal(%({% if 1 == 1 %}{% elsif 2 == 2 %}{{ test }}{% endif %}))
+    )
+  end
+
+  def test_else_body
+    assert_equal(
+      ["test"],
+      traversal(%({% if 1 == 1 %}{% else %}{{ test }}{% endif %}))
+    )
+  end
+
+  def test_case_left
+    assert_equal(
+      ["test"],
+      traversal(%({% case test %}{% endcase %}))
+    )
+  end
+
+  def test_case_condition
+    assert_equal(
+      ["test"],
+      traversal(%({% case 1 %}{% when test %}{% endcase %}))
+    )
+  end
+
+  def test_case_when_body
+    assert_equal(
+      ["test"],
+      traversal(%({% case 1 %}{% when 2 %}{{ test }}{% endcase %}))
+    )
+  end
+
+  def test_case_else_body
+    assert_equal(
+      ["test"],
+      traversal(%({% case 1 %}{% else %}{{ test }}{% endcase %}))
+    )
+  end
+
+  def test_for_in
+    assert_equal(
+      ["test"],
+      traversal(%({% for x in test %}{% endfor %}))
+    )
+  end
+
+  def test_for_limit
+    assert_equal(
+      ["test"],
+      traversal(%({% for x in (1..5) limit: test %}{% endfor %}))
+    )
+  end
+
+  def test_for_offset
+    assert_equal(
+      ["test"],
+      traversal(%({% for x in (1..5) offset: test %}{% endfor %}))
+    )
+  end
+
+  def test_for_body
+    assert_equal(
+      ["test"],
+      traversal(%({% for x in (1..5) %}{{ test }}{% endfor %}))
+    )
+  end
+
+  def test_tablerow_in
+    assert_equal(
+      ["test"],
+      traversal(%({% tablerow x in test %}{% endtablerow %}))
+    )
+  end
+
+  def test_tablerow_limit
+    assert_equal(
+      ["test"],
+      traversal(%({% tablerow x in (1..5) limit: test %}{% endtablerow %}))
+    )
+  end
+
+  def test_tablerow_offset
+    assert_equal(
+      ["test"],
+      traversal(%({% tablerow x in (1..5) offset: test %}{% endtablerow %}))
+    )
+  end
+
+  def test_tablerow_body
+    assert_equal(
+      ["test"],
+      traversal(%({% tablerow x in (1..5) %}{{ test }}{% endtablerow %}))
+    )
+  end
+
+  def test_cycle
+    assert_equal(
+      ["test"],
+      traversal(%({% cycle test %}))
+    )
+  end
+
+  def test_assign
+    assert_equal(
+      ["test"],
+      traversal(%({% assign x = test %}))
+    )
+  end
+
+  def test_capture
+    assert_equal(
+      ["test"],
+      traversal(%({% capture x %}{{ test }}{% endcapture %}))
+    )
+  end
+
+  def test_include
+    assert_equal(
+      ["test"],
+      traversal(%({% include test %}))
+    )
+  end
+
+  def test_include_with
+    assert_equal(
+      ["test"],
+      traversal(%({% include "hai" with test %}))
+    )
+  end
+
+  def test_include_for
+    assert_equal(
+      ["test"],
+      traversal(%({% include "hai" for test %}))
+    )
+  end
+
+  private
+
+  def traversal(template)
+    ParseTreeVisitor
+      .for(Template.parse(template).root)
+      .add_callback_for(VariableLookup, &:name)
+      .visit.flatten.compact
+  end
+end

From c11fc656cf67365fefac7d1f71a7c172cefd6a5e Mon Sep 17 00:00:00 2001
From: Stephen Paul Weber <stephen.weber@shopify.com>
Date: Wed, 10 Oct 2018 09:49:14 -0400
Subject: [PATCH 2/6] Colocate Traversal classes with classes they traverse

This puts all knowledge of the traversal in the same file, and removes
the need for a CASES registry.
---
 lib/liquid.rb                      |  1 +
 lib/liquid/condition.rb            |  9 ++++
 lib/liquid/tags/assign.rb          |  6 +++
 lib/liquid/tags/case.rb            |  6 +++
 lib/liquid/tags/cycle.rb           |  6 +++
 lib/liquid/tags/for.rb             |  6 +++
 lib/liquid/tags/if.rb              |  6 +++
 lib/liquid/tags/include.rb         |  9 ++++
 lib/liquid/tags/table_row.rb       |  6 +++
 lib/liquid/traversal.rb            | 86 ++----------------------------
 lib/liquid/variable.rb             |  6 +++
 lib/liquid/variable_lookup.rb      |  6 +++
 test/integration/traversal_test.rb |  1 -
 13 files changed, 72 insertions(+), 82 deletions(-)

diff --git a/lib/liquid.rb b/lib/liquid.rb
index 7d9da26..eef582f 100644
--- a/lib/liquid.rb
+++ b/lib/liquid.rb
@@ -45,6 +45,7 @@ module Liquid
 end
 
 require "liquid/version"
+require 'liquid/traversal'
 require 'liquid/lexer'
 require 'liquid/parser'
 require 'liquid/i18n'
diff --git a/lib/liquid/condition.rb b/lib/liquid/condition.rb
index 72bd2ee..aca9420 100644
--- a/lib/liquid/condition.rb
+++ b/lib/liquid/condition.rb
@@ -128,6 +128,15 @@ module Liquid
         end
       end
     end
+
+    class Traversal < Liquid::Traversal
+      def children
+        [
+          @node.left, @node.right,
+          @node.child_condition, @node.attachment
+        ].compact
+      end
+    end
   end
 
   class ElseCondition < Condition
diff --git a/lib/liquid/tags/assign.rb b/lib/liquid/tags/assign.rb
index ee6fa76..53c9eb2 100644
--- a/lib/liquid/tags/assign.rb
+++ b/lib/liquid/tags/assign.rb
@@ -47,6 +47,12 @@ module Liquid
         1
       end
     end
+
+    class Traversal < Liquid::Traversal
+      def children
+        [@node.from]
+      end
+    end
   end
 
   Template.register_tag('assign'.freeze, Assign)
diff --git a/lib/liquid/tags/case.rb b/lib/liquid/tags/case.rb
index f55aa61..433d404 100644
--- a/lib/liquid/tags/case.rb
+++ b/lib/liquid/tags/case.rb
@@ -82,6 +82,12 @@ module Liquid
       block.attach(BlockBody.new)
       @blocks << block
     end
+
+    class Traversal < Liquid::Traversal
+      def children
+        [@node.left] + @node.blocks
+      end
+    end
   end
 
   Template.register_tag('case'.freeze, Case)
diff --git a/lib/liquid/tags/cycle.rb b/lib/liquid/tags/cycle.rb
index 6cf77a2..aaac686 100644
--- a/lib/liquid/tags/cycle.rb
+++ b/lib/liquid/tags/cycle.rb
@@ -53,6 +53,12 @@ module Liquid
         $1 ? Expression.parse($1) : nil
       end.compact
     end
+
+    class Traversal < Liquid::Traversal
+      def children
+        Array(@node.variables)
+      end
+    end
   end
 
   Template.register_tag('cycle', Cycle)
diff --git a/lib/liquid/tags/for.rb b/lib/liquid/tags/for.rb
index c529aae..68c1f92 100644
--- a/lib/liquid/tags/for.rb
+++ b/lib/liquid/tags/for.rb
@@ -191,6 +191,12 @@ module Liquid
     def render_else(context)
       @else_block ? @else_block.render(context) : ''.freeze
     end
+
+    class Traversal < Liquid::Traversal
+      def children
+        (super + [@node.limit, @node.from, @node.collection_name]).compact
+      end
+    end
   end
 
   Template.register_tag('for'.freeze, For)
diff --git a/lib/liquid/tags/if.rb b/lib/liquid/tags/if.rb
index 2a91741..54560c2 100644
--- a/lib/liquid/tags/if.rb
+++ b/lib/liquid/tags/if.rb
@@ -110,6 +110,12 @@ module Liquid
         Condition.new(a)
       end
     end
+
+    class Traversal < Liquid::Traversal
+      def children
+        @node.blocks
+      end
+    end
   end
 
   Template.register_tag('if'.freeze, If)
diff --git a/lib/liquid/tags/include.rb b/lib/liquid/tags/include.rb
index a334d83..23cc591 100644
--- a/lib/liquid/tags/include.rb
+++ b/lib/liquid/tags/include.rb
@@ -109,6 +109,15 @@ module Liquid
 
       file_system.read_template_file(context.evaluate(@template_name_expr))
     end
+
+    class Traversal < Liquid::Traversal
+      def children
+        [
+          @node.template_name_expr,
+          @node.variable_name_expr
+        ] + @node.attributes.values
+      end
+    end
   end
 
   Template.register_tag('include'.freeze, Include)
diff --git a/lib/liquid/tags/table_row.rb b/lib/liquid/tags/table_row.rb
index 99d12ec..3994553 100644
--- a/lib/liquid/tags/table_row.rb
+++ b/lib/liquid/tags/table_row.rb
@@ -50,6 +50,12 @@ module Liquid
       result << "</tr>\n"
       result
     end
+
+    class Traversal < Liquid::Traversal
+      def children
+        super + @node.attributes.values + [@node.collection_name]
+      end
+    end
   end
 
   Template.register_tag('tablerow'.freeze, TableRow)
diff --git a/lib/liquid/traversal.rb b/lib/liquid/traversal.rb
index 339fc2c..0cebd96 100644
--- a/lib/liquid/traversal.rb
+++ b/lib/liquid/traversal.rb
@@ -3,8 +3,11 @@
 module Liquid
   class Traversal
     def self.for(node, callbacks = Hash.new(proc {}))
-      kase = CASES.find { |(klass, _)| node.is_a?(klass) }&.last
-      (kase || self).new(node, callbacks)
+      if defined?(node.class::Traversal)
+        node.class::Traversal
+      else
+        self
+      end.new(node, callbacks)
     end
 
     def initialize(node, callbacks)
@@ -35,84 +38,5 @@ module Liquid
     def children
       @node.respond_to?(:nodelist) ? Array(@node.nodelist) : []
     end
-
-    class Assign < Traversal
-      def children
-        [@node.from]
-      end
-    end
-
-    class Case < Traversal
-      def children
-        [@node.left] + @node.blocks
-      end
-    end
-
-    class Condition < Traversal
-      def children
-        [
-          @node.left, @node.right,
-          @node.child_condition, @node.attachment
-        ].compact
-      end
-    end
-
-    class Cycle < Traversal
-      def children
-        Array(@node.variables)
-      end
-    end
-
-    class For < Traversal
-      def children
-        (super + [@node.limit, @node.from, @node.collection_name]).compact
-      end
-    end
-
-    class If < Traversal
-      def children
-        @node.blocks
-      end
-    end
-
-    class Include < Traversal
-      def children
-        [
-          @node.template_name_expr,
-          @node.variable_name_expr
-        ] + @node.attributes.values
-      end
-    end
-
-    class TableRow < Traversal
-      def children
-        super + @node.attributes.values + [@node.collection_name]
-      end
-    end
-
-    class Variable < Traversal
-      def children
-        [@node.name] + @node.filters.flatten
-      end
-    end
-
-    class VariableLookup < Traversal
-      def children
-        @node.lookups
-      end
-    end
-
-    CASES = {
-      Liquid::Assign => Assign,
-      Liquid::Case => Case,
-      Liquid::Condition => Condition,
-      Liquid::Cycle => Cycle,
-      Liquid::For => For,
-      Liquid::If => If,
-      Liquid::Include => Include,
-      Liquid::TableRow => TableRow,
-      Liquid::Variable => Variable,
-      Liquid::VariableLookup => VariableLookup
-    }.freeze
   end
 end
diff --git a/lib/liquid/variable.rb b/lib/liquid/variable.rb
index 5f88eb3..1258b2d 100644
--- a/lib/liquid/variable.rb
+++ b/lib/liquid/variable.rb
@@ -138,5 +138,11 @@ module Liquid
         raise error
       end
     end
+
+    class Traversal < Liquid::Traversal
+      def children
+        [@node.name] + @node.filters.flatten
+      end
+    end
   end
 end
diff --git a/lib/liquid/variable_lookup.rb b/lib/liquid/variable_lookup.rb
index 3ed4e4a..f1b7834 100644
--- a/lib/liquid/variable_lookup.rb
+++ b/lib/liquid/variable_lookup.rb
@@ -78,5 +78,11 @@ module Liquid
     def state
       [@name, @lookups, @command_flags]
     end
+
+    class Traversal < Liquid::Traversal
+      def children
+        @node.lookups
+      end
+    end
   end
 end
diff --git a/test/integration/traversal_test.rb b/test/integration/traversal_test.rb
index 6254cb9..7bcfac8 100644
--- a/test/integration/traversal_test.rb
+++ b/test/integration/traversal_test.rb
@@ -1,7 +1,6 @@
 # frozen_string_literal: true
 
 require 'test_helper'
-require 'liquid/traversal'
 
 class TraversalTest < Minitest::Test
   include Liquid

From ff727016ef946bccceadf4c98384f45889081ef2 Mon Sep 17 00:00:00 2001
From: Stephen Paul Weber <stephen.weber@shopify.com>
Date: Wed, 10 Oct 2018 09:51:37 -0400
Subject: [PATCH 3/6] s/callback_for/add_callback_for

---
 lib/liquid/traversal.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/liquid/traversal.rb b/lib/liquid/traversal.rb
index 0cebd96..d9d6fa8 100644
--- a/lib/liquid/traversal.rb
+++ b/lib/liquid/traversal.rb
@@ -15,7 +15,7 @@ module Liquid
       @callbacks = callbacks
     end
 
-    def callback_for(*classes, &block)
+    def add_callback_for(*classes, &block)
       callback = block
       callback = ->(node, _) { block.call(node) } if block.arity.abs == 1
       callback = ->(_, _) { block.call } if block.arity.zero?

From 7d13d882583bae1794a40be048a03662f35d5dde Mon Sep 17 00:00:00 2001
From: Stephen Paul Weber <stephen.weber@shopify.com>
Date: Thu, 11 Oct 2018 10:45:31 -0400
Subject: [PATCH 4/6] s/Traversal/ParseTreeVisitor

---
 lib/liquid.rb                                          |  2 +-
 lib/liquid/condition.rb                                |  2 +-
 lib/liquid/{traversal.rb => parse_tree_visitor.rb}     | 10 +++++-----
 lib/liquid/tags/assign.rb                              |  2 +-
 lib/liquid/tags/case.rb                                |  2 +-
 lib/liquid/tags/cycle.rb                               |  2 +-
 lib/liquid/tags/for.rb                                 |  2 +-
 lib/liquid/tags/if.rb                                  |  2 +-
 lib/liquid/tags/include.rb                             |  2 +-
 lib/liquid/tags/table_row.rb                           |  2 +-
 lib/liquid/variable.rb                                 |  2 +-
 lib/liquid/variable_lookup.rb                          |  2 +-
 .../{traversal_test.rb => parse_tree_visitor_test.rb}  |  2 +-
 13 files changed, 17 insertions(+), 17 deletions(-)
 rename lib/liquid/{traversal.rb => parse_tree_visitor.rb} (78%)
 rename test/integration/{traversal_test.rb => parse_tree_visitor_test.rb} (98%)

diff --git a/lib/liquid.rb b/lib/liquid.rb
index eef582f..770d2f9 100644
--- a/lib/liquid.rb
+++ b/lib/liquid.rb
@@ -45,7 +45,7 @@ module Liquid
 end
 
 require "liquid/version"
-require 'liquid/traversal'
+require 'liquid/parse_tree_visitor'
 require 'liquid/lexer'
 require 'liquid/parser'
 require 'liquid/i18n'
diff --git a/lib/liquid/condition.rb b/lib/liquid/condition.rb
index aca9420..3b51682 100644
--- a/lib/liquid/condition.rb
+++ b/lib/liquid/condition.rb
@@ -129,7 +129,7 @@ module Liquid
       end
     end
 
-    class Traversal < Liquid::Traversal
+    class ParseTreeVisitor < Liquid::ParseTreeVisitor
       def children
         [
           @node.left, @node.right,
diff --git a/lib/liquid/traversal.rb b/lib/liquid/parse_tree_visitor.rb
similarity index 78%
rename from lib/liquid/traversal.rb
rename to lib/liquid/parse_tree_visitor.rb
index d9d6fa8..62f6c7b 100644
--- a/lib/liquid/traversal.rb
+++ b/lib/liquid/parse_tree_visitor.rb
@@ -1,10 +1,10 @@
 # frozen_string_literal: true
 
 module Liquid
-  class Traversal
+  class ParseTreeVisitor
     def self.for(node, callbacks = Hash.new(proc {}))
-      if defined?(node.class::Traversal)
-        node.class::Traversal
+      if defined?(node.class::ParseTreeVisitor)
+        node.class::ParseTreeVisitor
       else
         self
       end.new(node, callbacks)
@@ -23,12 +23,12 @@ module Liquid
       self
     end
 
-    def traverse(context = nil)
+    def visit(context = nil)
       children.map do |node|
         item, new_context = @callbacks[node.class].call(node, context)
         [
           item,
-          Traversal.for(node, @callbacks).traverse(new_context || context)
+          ParseTreeVisitor.for(node, @callbacks).visit(new_context || context)
         ]
       end
     end
diff --git a/lib/liquid/tags/assign.rb b/lib/liquid/tags/assign.rb
index 53c9eb2..c8d0574 100644
--- a/lib/liquid/tags/assign.rb
+++ b/lib/liquid/tags/assign.rb
@@ -48,7 +48,7 @@ module Liquid
       end
     end
 
-    class Traversal < Liquid::Traversal
+    class ParseTreeVisitor < Liquid::ParseTreeVisitor
       def children
         [@node.from]
       end
diff --git a/lib/liquid/tags/case.rb b/lib/liquid/tags/case.rb
index 433d404..5036b27 100644
--- a/lib/liquid/tags/case.rb
+++ b/lib/liquid/tags/case.rb
@@ -83,7 +83,7 @@ module Liquid
       @blocks << block
     end
 
-    class Traversal < Liquid::Traversal
+    class ParseTreeVisitor < Liquid::ParseTreeVisitor
       def children
         [@node.left] + @node.blocks
       end
diff --git a/lib/liquid/tags/cycle.rb b/lib/liquid/tags/cycle.rb
index aaac686..17aa860 100644
--- a/lib/liquid/tags/cycle.rb
+++ b/lib/liquid/tags/cycle.rb
@@ -54,7 +54,7 @@ module Liquid
       end.compact
     end
 
-    class Traversal < Liquid::Traversal
+    class ParseTreeVisitor < Liquid::ParseTreeVisitor
       def children
         Array(@node.variables)
       end
diff --git a/lib/liquid/tags/for.rb b/lib/liquid/tags/for.rb
index 68c1f92..b69aa78 100644
--- a/lib/liquid/tags/for.rb
+++ b/lib/liquid/tags/for.rb
@@ -192,7 +192,7 @@ module Liquid
       @else_block ? @else_block.render(context) : ''.freeze
     end
 
-    class Traversal < Liquid::Traversal
+    class ParseTreeVisitor < Liquid::ParseTreeVisitor
       def children
         (super + [@node.limit, @node.from, @node.collection_name]).compact
       end
diff --git a/lib/liquid/tags/if.rb b/lib/liquid/tags/if.rb
index 54560c2..1451c25 100644
--- a/lib/liquid/tags/if.rb
+++ b/lib/liquid/tags/if.rb
@@ -111,7 +111,7 @@ module Liquid
       end
     end
 
-    class Traversal < Liquid::Traversal
+    class ParseTreeVisitor < Liquid::ParseTreeVisitor
       def children
         @node.blocks
       end
diff --git a/lib/liquid/tags/include.rb b/lib/liquid/tags/include.rb
index 23cc591..c9f2a28 100644
--- a/lib/liquid/tags/include.rb
+++ b/lib/liquid/tags/include.rb
@@ -110,7 +110,7 @@ module Liquid
       file_system.read_template_file(context.evaluate(@template_name_expr))
     end
 
-    class Traversal < Liquid::Traversal
+    class ParseTreeVisitor < Liquid::ParseTreeVisitor
       def children
         [
           @node.template_name_expr,
diff --git a/lib/liquid/tags/table_row.rb b/lib/liquid/tags/table_row.rb
index 3994553..7f391cf 100644
--- a/lib/liquid/tags/table_row.rb
+++ b/lib/liquid/tags/table_row.rb
@@ -51,7 +51,7 @@ module Liquid
       result
     end
 
-    class Traversal < Liquid::Traversal
+    class ParseTreeVisitor < Liquid::ParseTreeVisitor
       def children
         super + @node.attributes.values + [@node.collection_name]
       end
diff --git a/lib/liquid/variable.rb b/lib/liquid/variable.rb
index 1258b2d..c31bffe 100644
--- a/lib/liquid/variable.rb
+++ b/lib/liquid/variable.rb
@@ -139,7 +139,7 @@ module Liquid
       end
     end
 
-    class Traversal < Liquid::Traversal
+    class ParseTreeVisitor < Liquid::ParseTreeVisitor
       def children
         [@node.name] + @node.filters.flatten
       end
diff --git a/lib/liquid/variable_lookup.rb b/lib/liquid/variable_lookup.rb
index f1b7834..8f7ad46 100644
--- a/lib/liquid/variable_lookup.rb
+++ b/lib/liquid/variable_lookup.rb
@@ -79,7 +79,7 @@ module Liquid
       [@name, @lookups, @command_flags]
     end
 
-    class Traversal < Liquid::Traversal
+    class ParseTreeVisitor < Liquid::ParseTreeVisitor
       def children
         @node.lookups
       end
diff --git a/test/integration/traversal_test.rb b/test/integration/parse_tree_visitor_test.rb
similarity index 98%
rename from test/integration/traversal_test.rb
rename to test/integration/parse_tree_visitor_test.rb
index 7bcfac8..4ffef08 100644
--- a/test/integration/traversal_test.rb
+++ b/test/integration/parse_tree_visitor_test.rb
@@ -2,7 +2,7 @@
 
 require 'test_helper'
 
-class TraversalTest < Minitest::Test
+class ParseTreeVisitorTest < Minitest::Test
   include Liquid
 
   def test_variable

From 8217a8d86cbc4f5d0ea1c3f6e02dc271cbe3e73f Mon Sep 17 00:00:00 2001
From: Stephen Paul Weber <stephen.weber@shopify.com>
Date: Mon, 15 Oct 2018 10:22:09 -0400
Subject: [PATCH 5/6] Add test for the full array structure

---
 test/integration/parse_tree_visitor_test.rb | 78 ++++++++++++---------
 1 file changed, 46 insertions(+), 32 deletions(-)

diff --git a/test/integration/parse_tree_visitor_test.rb b/test/integration/parse_tree_visitor_test.rb
index 4ffef08..6ad6a2d 100644
--- a/test/integration/parse_tree_visitor_test.rb
+++ b/test/integration/parse_tree_visitor_test.rb
@@ -8,217 +8,228 @@ class ParseTreeVisitorTest < Minitest::Test
   def test_variable
     assert_equal(
       ["test"],
-      traversal(%({{ test }}))
+      visit(%({{ test }}))
     )
   end
 
   def test_varible_with_filter
     assert_equal(
       ["test", "infilter"],
-      traversal(%({{ test | split: infilter }}))
+      visit(%({{ test | split: infilter }}))
     )
   end
 
   def test_dynamic_variable
     assert_equal(
       ["test", "inlookup"],
-      traversal(%({{ test[inlookup] }}))
+      visit(%({{ test[inlookup] }}))
     )
   end
 
   def test_if_condition
     assert_equal(
       ["test"],
-      traversal(%({% if test %}{% endif %}))
+      visit(%({% if test %}{% endif %}))
     )
   end
 
   def test_complex_if_condition
     assert_equal(
       ["test"],
-      traversal(%({% if 1 == 1 and 2 == test %}{% endif %}))
+      visit(%({% if 1 == 1 and 2 == test %}{% endif %}))
     )
   end
 
   def test_if_body
     assert_equal(
       ["test"],
-      traversal(%({% if 1 == 1 %}{{ test }}{% endif %}))
+      visit(%({% if 1 == 1 %}{{ test }}{% endif %}))
     )
   end
 
   def test_unless_condition
     assert_equal(
       ["test"],
-      traversal(%({% unless test %}{% endunless %}))
+      visit(%({% unless test %}{% endunless %}))
     )
   end
 
   def test_complex_unless_condition
     assert_equal(
       ["test"],
-      traversal(%({% unless 1 == 1 and 2 == test %}{% endunless %}))
+      visit(%({% unless 1 == 1 and 2 == test %}{% endunless %}))
     )
   end
 
   def test_unless_body
     assert_equal(
       ["test"],
-      traversal(%({% unless 1 == 1 %}{{ test }}{% endunless %}))
+      visit(%({% unless 1 == 1 %}{{ test }}{% endunless %}))
     )
   end
 
   def test_elsif_condition
     assert_equal(
       ["test"],
-      traversal(%({% if 1 == 1 %}{% elsif test %}{% endif %}))
+      visit(%({% if 1 == 1 %}{% elsif test %}{% endif %}))
     )
   end
 
   def test_complex_elsif_condition
     assert_equal(
       ["test"],
-      traversal(%({% if 1 == 1 %}{% elsif 1 == 1 and 2 == test %}{% endif %}))
+      visit(%({% if 1 == 1 %}{% elsif 1 == 1 and 2 == test %}{% endif %}))
     )
   end
 
   def test_elsif_body
     assert_equal(
       ["test"],
-      traversal(%({% if 1 == 1 %}{% elsif 2 == 2 %}{{ test }}{% endif %}))
+      visit(%({% if 1 == 1 %}{% elsif 2 == 2 %}{{ test }}{% endif %}))
     )
   end
 
   def test_else_body
     assert_equal(
       ["test"],
-      traversal(%({% if 1 == 1 %}{% else %}{{ test }}{% endif %}))
+      visit(%({% if 1 == 1 %}{% else %}{{ test }}{% endif %}))
     )
   end
 
   def test_case_left
     assert_equal(
       ["test"],
-      traversal(%({% case test %}{% endcase %}))
+      visit(%({% case test %}{% endcase %}))
     )
   end
 
   def test_case_condition
     assert_equal(
       ["test"],
-      traversal(%({% case 1 %}{% when test %}{% endcase %}))
+      visit(%({% case 1 %}{% when test %}{% endcase %}))
     )
   end
 
   def test_case_when_body
     assert_equal(
       ["test"],
-      traversal(%({% case 1 %}{% when 2 %}{{ test }}{% endcase %}))
+      visit(%({% case 1 %}{% when 2 %}{{ test }}{% endcase %}))
     )
   end
 
   def test_case_else_body
     assert_equal(
       ["test"],
-      traversal(%({% case 1 %}{% else %}{{ test }}{% endcase %}))
+      visit(%({% case 1 %}{% else %}{{ test }}{% endcase %}))
     )
   end
 
   def test_for_in
     assert_equal(
       ["test"],
-      traversal(%({% for x in test %}{% endfor %}))
+      visit(%({% for x in test %}{% endfor %}))
     )
   end
 
   def test_for_limit
     assert_equal(
       ["test"],
-      traversal(%({% for x in (1..5) limit: test %}{% endfor %}))
+      visit(%({% for x in (1..5) limit: test %}{% endfor %}))
     )
   end
 
   def test_for_offset
     assert_equal(
       ["test"],
-      traversal(%({% for x in (1..5) offset: test %}{% endfor %}))
+      visit(%({% for x in (1..5) offset: test %}{% endfor %}))
     )
   end
 
   def test_for_body
     assert_equal(
       ["test"],
-      traversal(%({% for x in (1..5) %}{{ test }}{% endfor %}))
+      visit(%({% for x in (1..5) %}{{ test }}{% endfor %}))
     )
   end
 
   def test_tablerow_in
     assert_equal(
       ["test"],
-      traversal(%({% tablerow x in test %}{% endtablerow %}))
+      visit(%({% tablerow x in test %}{% endtablerow %}))
     )
   end
 
   def test_tablerow_limit
     assert_equal(
       ["test"],
-      traversal(%({% tablerow x in (1..5) limit: test %}{% endtablerow %}))
+      visit(%({% tablerow x in (1..5) limit: test %}{% endtablerow %}))
     )
   end
 
   def test_tablerow_offset
     assert_equal(
       ["test"],
-      traversal(%({% tablerow x in (1..5) offset: test %}{% endtablerow %}))
+      visit(%({% tablerow x in (1..5) offset: test %}{% endtablerow %}))
     )
   end
 
   def test_tablerow_body
     assert_equal(
       ["test"],
-      traversal(%({% tablerow x in (1..5) %}{{ test }}{% endtablerow %}))
+      visit(%({% tablerow x in (1..5) %}{{ test }}{% endtablerow %}))
     )
   end
 
   def test_cycle
     assert_equal(
       ["test"],
-      traversal(%({% cycle test %}))
+      visit(%({% cycle test %}))
     )
   end
 
   def test_assign
     assert_equal(
       ["test"],
-      traversal(%({% assign x = test %}))
+      visit(%({% assign x = test %}))
     )
   end
 
   def test_capture
     assert_equal(
       ["test"],
-      traversal(%({% capture x %}{{ test }}{% endcapture %}))
+      visit(%({% capture x %}{{ test }}{% endcapture %}))
     )
   end
 
   def test_include
     assert_equal(
       ["test"],
-      traversal(%({% include test %}))
+      visit(%({% include test %}))
     )
   end
 
   def test_include_with
     assert_equal(
       ["test"],
-      traversal(%({% include "hai" with test %}))
+      visit(%({% include "hai" with test %}))
     )
   end
 
   def test_include_for
     assert_equal(
       ["test"],
-      traversal(%({% include "hai" for test %}))
+      visit(%({% include "hai" for test %}))
+    )
+  end
+
+  def test_preserve_tree_structure
+    assert_equal(
+      [[nil, [
+        [nil, [[nil, [["other", []]]]]],
+        ["test", []],
+        ["xs", []]
+      ]]],
+      traversal(%({% for x in xs offset: test %}{{ other }}{% endfor %})).visit
     )
   end
 
@@ -228,6 +239,9 @@ class ParseTreeVisitorTest < Minitest::Test
     ParseTreeVisitor
       .for(Template.parse(template).root)
       .add_callback_for(VariableLookup, &:name)
-      .visit.flatten.compact
+  end
+
+  def visit(template)
+    traversal(template).visit.flatten.compact
   end
 end

From 52ee303a3656e798f8a5a47a97a80640c9eb15ba Mon Sep 17 00:00:00 2001
From: Stephen Paul Weber <stephen.weber@shopify.com>
Date: Thu, 18 Oct 2018 09:41:53 -0400
Subject: [PATCH 6/6] s/block.call/yield

---
 lib/liquid/parse_tree_visitor.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/liquid/parse_tree_visitor.rb b/lib/liquid/parse_tree_visitor.rb
index 62f6c7b..74f5563 100644
--- a/lib/liquid/parse_tree_visitor.rb
+++ b/lib/liquid/parse_tree_visitor.rb
@@ -17,8 +17,8 @@ module Liquid
 
     def add_callback_for(*classes, &block)
       callback = block
-      callback = ->(node, _) { block.call(node) } if block.arity.abs == 1
-      callback = ->(_, _) { block.call } if block.arity.zero?
+      callback = ->(node, _) { yield node } if block.arity.abs == 1
+      callback = ->(_, _) { yield } if block.arity.zero?
       classes.each { |klass| @callbacks[klass] = callback }
       self
     end